Skip to content

Let providers declare their secrets; move secrets logic behind an injectable store #3

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
ericmann merged 3 commits into from
Oct 9, 2026
Merged

Let providers declare their secrets; move secrets logic behind an injectable store #3

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
ae4c114
Put the secrets logic in an instance class behind the static facade
ericmann Oct 6, 2026
ce48499
Let providers declare their secrets and have core look after them
ericmann Oct 6, 2026
fe12797
Document the secrets store, manager and declared provider secrets
ericmann Oct 6, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 8 additions & 4 deletions AGENTS.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,9 +50,14 @@ The plugin follows a provider pattern. `Two_Factor_Core` owns the login intercep
- **`two-factor.php`** — Entry point. Defines `TWO_FACTOR_DIR` and `TWO_FACTOR_VERSION`, loads all core files (including the settings class), instantiates `Two_Factor_Compat`, and calls `Two_Factor_Core::add_hooks()`. Also registers the wp-admin settings page (Settings → Two-Factor) and the filter callbacks that enforce the site-wide enabled-providers option (`two_factor_filter_enabled_providers`, `two_factor_filter_enabled_providers_for_user`).
- **`class-two-factor-core.php`** — Central class. Owns the login flow, user meta, nonce management, rate limiting, session tracking, REST API endpoints, and the user profile settings UI.
- **`class-two-factor-compat.php`** — Compatibility shims for third-party plugins (currently: Jetpack SSO). New integrations go here; the goal is to avoid any plugin-specific logic outside this file.
- **`class-two-factor-secrets.php`** — `Two_Factor_Secrets`, a static adapter and the only place Two-Factor calls the WordPress Secrets API (`wp_get_network_secret()` and friends). Secrets are always network scope, on single site and multisite alike. A per-user marker in user meta records which network holds the secret. Storage is opt-in: new writes and migration happen only once an administrator has turned it on (`is_opted_in()`, the `two_factor_secrets_api_enabled` network option), as adjusted by the `two_factor_use_secrets_api` filter (`is_enabled()`); reads of already-migrated users are not gated.
- **`class-two-factor-secrets.php`** — `Two_Factor_Secrets`, the static entry point the rest of the plugin calls to store a secret. It only forwards to a shared `Two_Factor_Secrets_Manager`; `set_instance()` and `reset()` exist for tests.
- **`secrets/`** — Where the secrets code lives.
- `interface-two-factor-secrets-store.php` — `Two_Factor_Secrets_Store`, the contract for where secrets are physically kept.
- `class-two-factor-secrets-api-store.php` — `Two_Factor_Secrets_Api_Store`, the only place Two-Factor calls the WordPress Secrets API (`wp_get_network_secret()` and friends). Secrets are always network scope, on single site and multisite alike.
- `class-two-factor-secrets-manager.php` — `Two_Factor_Secrets_Manager`, the logic, constructed with a store. A per-user marker in user meta records which network holds the secret. Storage is opt-in: new writes and migration happen only once an administrator has turned it on (`is_opted_in()`, the `two_factor_secrets_api_enabled` network option), as adjusted by the `two_factor_use_secrets_api` filter (`is_enabled()`); reads of already-migrated users are not gated. The `*_stored_secret()` methods implement "plaintext user meta or the Secrets API" for any secret a provider declares: verified writes, lazy one-way migration, deletion and storage state.
- `class-two-factor-secrets-lifecycle.php` — `Two_Factor_Secrets_Lifecycle`, static. For every secret a provider declares: deletion with the user, deletion on uninstall, the unreachable-secrets admin notice, the Site Health test and the per-secret user counts. Hooks are registered from `Two_Factor_Core::add_hooks()`.
- **`settings/class-two-factor-settings.php`** — `Two_Factor_Settings`, renders the site-wide settings screen (Settings → Two-Factor, requires `manage_options`) where admins choose which providers are available on the site. Saving writes the `two_factor_enabled_providers` option; enforcement happens via the filters registered in `two-factor.php`, not in this class. The same screen has the Secrets API storage opt-in, which needs an acknowledgement checkbox to turn on and, on multisite, `manage_network_options`.
- **`providers/class-two-factor-provider.php`** — Abstract base class all providers extend. Defines the required interface: `get_label()`, `is_available_for_user()`, `authentication_page()`, `validate_authentication()`, and optional hooks for REST routes, settings UI, and uninstall cleanup. It also defines `is_enrolled_but_unavailable_for_user()` (a provider the user enrolled whose credential cannot currently be used; core then forces the fallback provider instead of dropping to single factor) and `uninstall_user_data()` (delete per-user data held outside user meta during uninstall).
- **`providers/class-two-factor-provider.php`** — Abstract base class all providers extend. Defines the required interface: `get_label()`, `is_available_for_user()`, `authentication_page()`, `validate_authentication()`, and optional hooks for REST routes, settings UI, and uninstall cleanup. It also defines `user_secret_meta_keys()` (static; secret slug => user meta key for each value the provider must read back in full, which core then stores, migrates, cleans up and reports on), `is_enrolled_but_unavailable_for_user()` (a provider the user enrolled whose credential cannot currently be used; core then forces the fallback provider instead of dropping to single factor; by default true when a declared secret is out of reach) and `uninstall_user_data()` (delete per-user data held outside user meta during uninstall).
- **`providers/`** — Concrete providers: `class-two-factor-totp.php`, `class-two-factor-email.php`, `class-two-factor-backup-codes.php`, `class-two-factor-dummy.php`.
- **`includes/`** — Custom `login_header()` and `login_footer()` template functions that replace the WordPress core versions with additional filter hooks. Excluded from PHPCS because they intentionally deviate from core function signatures. Do not modify files in includes/ directly. They are intentionally kept close to WordPress core function signatures to ease future merging into Core. Any functional changes should go through the filter hooks they expose instead.
- **`tests/`** — PHPUnit tests. See [TESTS.md](TESTS.md).
Expand Down Expand Up @@ -88,7 +93,7 @@ Each concrete provider registers its own hooks in its constructor:

New providers should follow this pattern rather than registering hooks from outside the class.

The one exception is data that outlives the provider being enabled. `Two_Factor_Totp::register_secret_lifecycle_hooks()` registers user-deletion cleanup, the unreachable-secrets admin notice and the Site Health test statically, and `two-factor.php` always loads the TOTP class and calls it, so these keep working when an admin turns TOTP off in Settings → Two-Factor.
The one exception is data that outlives the provider being enabled. A provider that keeps a secret declares it in the static `user_secret_meta_keys()` instead of registering hooks for it. `Two_Factor_Core::get_provider_secrets()` builds the list from every registered provider class without instantiating any, and includes providers an admin has turned off in Settings → Two-Factor (the enforcement filter stands aside while `Two_Factor_Core::is_listing_all_providers()` is true). `Two_Factor_Secrets_Lifecycle` works from that list, so cleanup, the admin notice and Site Health keep working for a disabled provider. Secret slugs share one namespace and are part of the stored secret's name: a duplicate is refused with `_doing_it_wrong()`, and a slug must never change once secrets exist.

### Key User Meta (constants on `Two_Factor_Core`)

Expand All @@ -112,7 +117,6 @@ The one exception is data that outlives the provider being enabled. `Two_Factor_
### Secrets API hooks

- `two_factor_use_secrets_api` filter (`$enabled`, `$user_id`) receives the administrator's opt-in as its default, so it can turn storage off or force it on. It controls writes and migration only; already-migrated users are still read from the Secrets API while it is present. `$user_id` may be `0`.
- `two_factor_secrets_api_present` filter (`$present`, `$resolved`) is an internal test seam. It receives whether the API is actually loaded as `$resolved`, can only force "absent", and is not a public extension point.
- `two_factor_secrets_migrated` action (`$user_id`, `$slug`) after a secret was moved into the Secrets API.
- `two_factor_secrets_migration_failed` action (`$user_id`, `$slug`, `$error`) when a migration failed and the plaintext copy was kept.
- `two_factor_secret_unavailable` action (`$user_id`, `$slug`, `$error`) when a stored secret cannot be read.
Expand Down
94 changes: 52 additions & 42 deletions CLI/class-two-factor-cli-command.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -805,10 +805,14 @@ public function secrets( $args, $assoc_args ) {
* @param array $assoc_args Associative arguments.
*/
private function secrets_status( $assoc_args ) {
$counts = Two_Factor_Totp::count_users_by_storage();
$items = array();

$items = array(
array(
// One row per secret a provider declares; the storage columns are the same on every row.
foreach ( Two_Factor_Core::get_provider_secrets() as $slug => $secret ) {
$counts = Two_Factor_Secrets_Lifecycle::count_users_by_storage( $slug, $secret['meta_key'] );

$items[] = array(
'secret' => $slug,
'api_present' => Two_Factor_Secrets::is_api_present() ? 'true' : 'false',
'provider' => Two_Factor_Secrets::provider_label(),
'writable' => Two_Factor_Secrets::is_provider_writable() ? 'true' : 'false',
Expand All @@ -817,14 +821,14 @@ private function secrets_status( $assoc_args ) {
'plaintext_users' => $counts['plaintext'],
'migrated_users' => $counts['migrated'],
'affected_users' => $counts['affected'],
),
);
);
}

$format = WP_CLI\Utils\get_flag_value( $assoc_args, 'format', 'table' );
WP_CLI\Utils\format_items(
$format,
$items,
array( 'api_present', 'provider', 'writable', 'opted_in', 'enabled', 'plaintext_users', 'migrated_users', 'affected_users' )
array( 'secret', 'api_present', 'provider', 'writable', 'opted_in', 'enabled', 'plaintext_users', 'migrated_users', 'affected_users' )
);
}

Expand Down Expand Up @@ -853,7 +857,7 @@ private function get_secrets_batch_size( $assoc_args ) {
}

/**
* Migrate plaintext TOTP secrets into the Secrets API.
* Migrate the plaintext secrets providers keep for users into the Secrets API.
*
* @since 0.18.0
*
Expand All @@ -872,25 +876,27 @@ private function secrets_migrate( $user_identifier, $assoc_args ) {
WP_CLI::error( __( 'The Secrets API is not available for writing (missing, read-only, or disabled by the two_factor_use_secrets_api filter). Nothing was migrated.', 'two-factor' ) );
}

$totp = Two_Factor_Totp::get_instance();
$secrets = Two_Factor_Core::get_provider_secrets();
$migrated = 0;
$would = 0;
$failed = 0;
$skipped = 0;

$migrate_one = function ( $user_id ) use ( $totp, $dry_run, &$migrated, &$would, &$failed, &$skipped ) {
$migrate_one = function ( $user_id, $slug ) use ( $secrets, $dry_run, &$migrated, &$would, &$failed, &$skipped ) {
$meta_key = $secrets[ $slug ]['meta_key'];

if ( $dry_run ) {
if ( '' === (string) get_user_meta( $user_id, Two_Factor_Totp::SECRET_META_KEY, true ) ) {
if ( '' === (string) get_user_meta( $user_id, $meta_key, true ) ) {
++$skipped;
return 'skipped';
}

WP_CLI::log( sprintf( 'Would migrate user %d', $user_id ) );
WP_CLI::log( sprintf( 'Would migrate user %1$d (%2$s)', $user_id, $slug ) );
++$would;
return 'would-migrate';
}

$result = $totp->migrate_user_totp_key( $user_id );
$result = Two_Factor_Secrets::migrate_stored_secret( (int) $user_id, $slug, $meta_key );

if ( true === $result ) {
++$migrated;
Expand All @@ -899,7 +905,7 @@ private function secrets_migrate( $user_identifier, $assoc_args ) {

if ( is_wp_error( $result ) ) {
++$failed;
WP_CLI::warning( sprintf( 'User %d: %s', $user_id, $result->get_error_message() ) );
WP_CLI::warning( sprintf( 'User %1$d (%2$s): %3$s', $user_id, $slug, $result->get_error_message() ) );
return 'failed';
}

Expand All @@ -919,40 +925,44 @@ private function secrets_migrate( $user_identifier, $assoc_args ) {
);
}

$migrate_one( $user->ID );
foreach ( array_keys( $secrets ) as $slug ) {
$migrate_one( $user->ID, $slug );
}
} else {
$offset = 0;

do {
$query = new WP_User_Query(
array(
'blog_id' => 0,
'fields' => 'ID',
'number' => $batch,
'offset' => $offset,
'orderby' => 'ID',
'order' => 'ASC',
'meta_key' => Two_Factor_Totp::SECRET_META_KEY, // phpcs:ignore WordPress.DB.SlowDBQuery.slow_db_query_meta_key -- One-off CLI migration.
'meta_value' => '', // phpcs:ignore WordPress.DB.SlowDBQuery.slow_db_query_meta_value -- One-off CLI migration.
'meta_compare' => '!=',
'count_total' => false,
)
);
$ids = array_map( 'intval', $query->get_results() );

// Migrated users drop out of the result set; skipped and failed ones stay, so step past them.
$stayed = 0;
foreach ( $ids as $user_id ) {
if ( 'migrated' !== $migrate_one( $user_id ) ) {
++$stayed;
foreach ( $secrets as $slug => $secret ) {
$offset = 0;

do {
$query = new WP_User_Query(
array(
'blog_id' => 0,
'fields' => 'ID',
'number' => $batch,
'offset' => $offset,
'orderby' => 'ID',
'order' => 'ASC',
'meta_key' => $secret['meta_key'], // phpcs:ignore WordPress.DB.SlowDBQuery.slow_db_query_meta_key -- One-off CLI migration.
'meta_value' => '', // phpcs:ignore WordPress.DB.SlowDBQuery.slow_db_query_meta_value -- One-off CLI migration.
'meta_compare' => '!=',
'count_total' => false,
)
);
$ids = array_map( 'intval', $query->get_results() );

// Migrated users drop out of the result set; skipped and failed ones stay, so step past them.
$stayed = 0;
foreach ( $ids as $user_id ) {
if ( 'migrated' !== $migrate_one( $user_id, $slug ) ) {
++$stayed;
}
}
}

$offset += $stayed;
} while ( ! empty( $ids ) );
$offset += $stayed;
} while ( ! empty( $ids ) );
}
}

Two_Factor_Totp::clear_affected_users_cache();
Two_Factor_Secrets::clear_affected_users_cache();

if ( $dry_run ) {
WP_CLI::success(
Expand Down
Loading