From ae4c114458b5e309a1d6c96d8b2ccc2b1dc7a112 Mon Sep 17 00:00:00 2001 From: Eric Mann Date: Tue, 6 Oct 2026 12:18:16 -0700 Subject: [PATCH 1/3] Put the secrets logic in an instance class behind the static facade Two_Factor_Secrets was all static, so tests reached into it through a public $test_overrides array and the two_factor_secrets_api_present filter, and most of them needed the real Secrets API loaded. The logic now lives in Two_Factor_Secrets_Manager, which is constructed with a Two_Factor_Secrets_Store. Two_Factor_Secrets_Api_Store is the only code that calls the Secrets API. Two_Factor_Secrets keeps every public static method, forwarding to a shared manager, so callers are unchanged. Tests install an in-memory store instead, which removes $test_overrides and the filter and lets the secrets tests run without the feature plugin. A small set of tests still exercises the real store. --- class-two-factor-secrets.php | 319 +++++------------- phpstan.dist.neon | 1 + phpunit-multisite.xml | 1 + phpunit.xml.dist | 1 + .../class-two-factor-secrets-api-store.php | 102 ++++++ secrets/class-two-factor-secrets-manager.php | 313 +++++++++++++++++ .../interface-two-factor-secrets-store.php | 76 +++++ tests/bootstrap.php | 1 + .../class-two-factor-secrets-memory-store.php | 121 +++++++ tests/class-two-factor-secrets-test-case.php | 20 +- tests/class-two-factor-secrets-tests.php | 67 ++-- tests/cli/class-two-factor-cli-command.php | 49 +-- .../class-two-factor-totp-secrets-tests.php | 102 ++---- ...ass-two-factor-secrets-api-store-tests.php | 106 ++++++ .../class-tests-two-factor-settings.php | 5 - two-factor.php | 5 +- 16 files changed, 893 insertions(+), 396 deletions(-) create mode 100644 secrets/class-two-factor-secrets-api-store.php create mode 100644 secrets/class-two-factor-secrets-manager.php create mode 100644 secrets/interface-two-factor-secrets-store.php create mode 100644 tests/class-two-factor-secrets-memory-store.php create mode 100644 tests/secrets/class-two-factor-secrets-api-store-tests.php diff --git a/class-two-factor-secrets.php b/class-two-factor-secrets.php index 516ce661..6c3d6d9f 100644 --- a/class-two-factor-secrets.php +++ b/class-two-factor-secrets.php @@ -1,119 +1,116 @@ + * @var string */ - public static $test_overrides = array(); + const OPT_IN_OPTION_KEY = 'two_factor_secrets_api_enabled'; /** - * Network option recording that an administrator turned Secrets API storage on. - * - * A network option because secrets are network scoped: one site of a network - * cannot opt in on its own. + * The shared manager. * * @since 0.18.0 * - * @var string + * @var Two_Factor_Secrets_Manager|null */ - const OPT_IN_OPTION_KEY = 'two_factor_secrets_api_enabled'; + private static $instance = null; /** - * Whether the Secrets API is available right now. + * Get the shared manager, building one over the Secrets API on first use. * * @since 0.18.0 * - * @return bool + * @return Two_Factor_Secrets_Manager */ - public static function is_api_present(): bool { - $present = function_exists( 'wp_get_network_secret' ) - && function_exists( 'wp_set_network_secret' ) - && function_exists( 'wp_delete_network_secret' ) - && function_exists( 'wp_secrets_provider_is_writable' ) - && function_exists( 'wp_secrets_provider_label' ) - && function_exists( 'wp_secrets_memzero' ) - && class_exists( 'WP_Secret' ); + public static function instance(): Two_Factor_Secrets_Manager { + if ( null === self::$instance ) { + self::$instance = new Two_Factor_Secrets_Manager( new Two_Factor_Secrets_Api_Store() ); + } - /** - * Filters whether the Secrets API is treated as present. - * - * @internal Test seam only; not a public extension point. Can only force false. - * - * @param bool $present Whether to treat the API as present. Default true. Returning - * true cannot make a missing API count as present. - * @param bool $resolved Whether the API's functions and classes are actually loaded. - */ - $filtered = (bool) apply_filters( 'two_factor_secrets_api_present', true, $present ); + return self::$instance; + } - return $present && $filtered; + /** + * Replace the shared manager. + * + * @internal For tests, which pass a manager built over an in-memory store. + * + * @since 0.18.0 + * + * @param Two_Factor_Secrets_Manager $instance The manager to use. + * @return void + */ + public static function set_instance( Two_Factor_Secrets_Manager $instance ): void { + self::$instance = $instance; } /** - * Whether an error means the Secrets API cannot use its encryption key. + * Forget the shared manager, so the next call builds one over the Secrets API. * - * The Secrets API wraps a single root key with the site key (WP_SECRETS_KEY, or one - * derived from the salts). When the site key changes without the API's own rotation, - * the root key cannot be unwrapped and every read and write fails, including writes - * of brand-new secrets, until the original key is restored. + * @internal For tests. * * @since 0.18.0 * - * @param mixed $error Value to check. + * @return void + */ + public static function reset(): void { + self::$instance = null; + } + + /** + * Whether the store is available right now. + * + * @since 0.18.0 * * @return bool */ - public static function is_key_unavailable_error( $error ): bool { - $code = defined( 'WP_SECRETS_ERROR_KEY_UNAVAILABLE' ) ? WP_SECRETS_ERROR_KEY_UNAVAILABLE : 'secret_key_unavailable'; - - return is_wp_error( $error ) && $code === $error->get_error_code(); + public static function is_api_present(): bool { + return self::instance()->is_api_present(); } /** - * Whether the active secrets provider accepts writes. + * Whether the store accepts writes. * * @since 0.18.0 * * @return bool */ public static function is_provider_writable(): bool { - if ( ! self::is_api_present() ) { - return false; - } - - if ( isset( self::$test_overrides['writable'] ) ) { - return (bool) call_user_func( self::$test_overrides['writable'] ); - } + return self::instance()->is_provider_writable(); + } - return (bool) wp_secrets_provider_is_writable(); + /** + * Label of whatever backs the store, or an empty string when it is unavailable. + * + * @since 0.18.0 + * + * @return string + */ + public static function provider_label(): string { + return self::instance()->provider_label(); } /** @@ -128,7 +125,7 @@ public static function is_provider_writable(): bool { * @return bool */ public static function is_opted_in(): bool { - return '1' === (string) get_site_option( self::OPT_IN_OPTION_KEY, '' ); + return self::instance()->is_opted_in(); } /** @@ -143,12 +140,7 @@ public static function is_opted_in(): bool { * @return void */ public static function set_opted_in( bool $opted_in ): void { - if ( $opted_in ) { - update_site_option( self::OPT_IN_OPTION_KEY, '1' ); - return; - } - - delete_site_option( self::OPT_IN_OPTION_KEY ); + self::instance()->set_opted_in( $opted_in ); } /** @@ -163,23 +155,7 @@ public static function set_opted_in( bool $opted_in ): void { * @return bool */ public static function is_enabled( int $user_id = 0 ): bool { - /** - * Filters whether Two-Factor writes secrets to the Secrets API. - * - * This controls writes and migration only. Users whose secret was already - * migrated are still read from the Secrets API while it is present. - * - * Returning true turns storage on without the administrator opting in on the - * settings screen, so only do that where the encryption key is managed for - * the site. - * - * @since 0.18.0 - * - * @param bool $enabled Whether to use the Secrets API. Defaults to the administrator's - * opt-in on the settings screen, which is off until it is saved. - * @param int $user_id User ID, which may be 0 when no user is in context. - */ - return (bool) apply_filters( 'two_factor_use_secrets_api', self::is_opted_in(), $user_id ); + return self::instance()->is_enabled( $user_id ); } /** @@ -191,15 +167,7 @@ public static function is_enabled( int $user_id = 0 ): bool { * @return bool */ public static function can_write( int $user_id = 0 ): bool { - if ( ! self::is_api_present() ) { - return false; - } - - if ( ! self::is_enabled( $user_id ) ) { - return false; - } - - return self::is_provider_writable(); + return self::instance()->can_write( $user_id ); } /** @@ -214,9 +182,7 @@ public static function can_write( int $user_id = 0 ): bool { * @throws InvalidArgumentException When the slug is invalid. */ public static function get_secret_name( int $user_id, string $slug ): string { - self::assert_slug( $slug ); - - return "two-factor/{$slug}-{$user_id}"; + return self::instance()->get_secret_name( $user_id, $slug ); } /** @@ -230,9 +196,7 @@ public static function get_secret_name( int $user_id, string $slug ): string { * @throws InvalidArgumentException When the slug is invalid. */ public static function get_marker_meta_key( string $slug ): string { - self::assert_slug( $slug ); - - return "_two_factor_{$slug}_key_network"; + return self::instance()->get_marker_meta_key( $slug ); } /** @@ -245,36 +209,7 @@ public static function get_marker_meta_key( string $slug ): string { * @return string|null|WP_Error Null when no secret has been migrated for the user. */ public static function get_user_secret( int $user_id, string $slug ) { - $marker = get_user_meta( $user_id, self::get_marker_meta_key( $slug ), true ); - - if ( '' === $marker || false === $marker || null === $marker ) { - return null; - } - - if ( ! self::is_api_present() ) { - return new WP_Error( - 'two_factor_secrets_api_missing', - __( 'The Secrets API is not available, so this secret cannot be read.', 'two-factor' ) - ); - } - - if ( (int) get_current_network_id() !== (int) $marker ) { - return new WP_Error( - 'two_factor_secret_wrong_network', - __( 'This secret is stored for a different network.', 'two-factor' ) - ); - } - - $value = self::api_get( self::get_secret_name( $user_id, $slug ) ); - - if ( null === $value ) { - return new WP_Error( - 'two_factor_secret_missing', - __( 'The stored secret could not be found.', 'two-factor' ) - ); - } - - return $value; + return self::instance()->get_user_secret( $user_id, $slug ); } /** @@ -288,22 +223,7 @@ public static function get_user_secret( int $user_id, string $slug ) { * @return true|WP_Error */ public static function set_user_secret( int $user_id, string $slug, string $value ) { - if ( ! self::is_api_present() ) { - return new WP_Error( - 'two_factor_secrets_api_missing', - __( 'The Secrets API is not available, so this secret cannot be stored.', 'two-factor' ) - ); - } - - $result = self::api_set( self::get_secret_name( $user_id, $slug ), $value ); - - if ( is_wp_error( $result ) ) { - return $result; - } - - update_user_meta( $user_id, self::get_marker_meta_key( $slug ), (string) get_current_network_id() ); - - return true; + return self::instance()->set_user_secret( $user_id, $slug, $value ); } /** @@ -318,33 +238,27 @@ public static function set_user_secret( int $user_id, string $slug, string $valu * @return true|WP_Error */ public static function delete_user_secret( int $user_id, string $slug ) { - $error = null; - - if ( self::is_api_present() ) { - $result = self::api_delete( self::get_secret_name( $user_id, $slug ) ); - if ( is_wp_error( $result ) ) { - $error = $result; - } - } - - delete_user_meta( $user_id, self::get_marker_meta_key( $slug ) ); - - return $error ? $error : true; + return self::instance()->delete_user_secret( $user_id, $slug ); } /** - * Label of the active secrets provider. + * Whether an error means the Secrets API cannot use its encryption key. + * + * The Secrets API wraps a single root key with the site key (WP_SECRETS_KEY, or one + * derived from the salts). When the site key changes without the API's own rotation, + * the root key cannot be unwrapped and every read and write fails, including writes + * of brand-new secrets, until the original key is restored. * * @since 0.18.0 * - * @return string + * @param mixed $error Value to check. + * + * @return bool */ - public static function provider_label(): string { - if ( self::is_api_present() ) { - return (string) wp_secrets_provider_label(); - } + public static function is_key_unavailable_error( $error ): bool { + $code = defined( 'WP_SECRETS_ERROR_KEY_UNAVAILABLE' ) ? WP_SECRETS_ERROR_KEY_UNAVAILABLE : 'secret_key_unavailable'; - return ''; + return is_wp_error( $error ) && $code === $error->get_error_code(); } /** @@ -363,75 +277,4 @@ public static function memzero( &$value ): void { $value = ''; } - - /** - * Validate a slug. - * - * @since 0.18.0 - * - * @param string $slug Secret slug. - * @return void - * - * @throws InvalidArgumentException When the slug is invalid. - */ - private static function assert_slug( string $slug ): void { - if ( 1 !== preg_match( '/^[a-z0-9]([a-z0-9_-]*[a-z0-9])?$/', $slug ) ) { - throw new InvalidArgumentException( 'Invalid secret slug.' ); - } - } - - /** - * Write to the Secrets API. - * - * @since 0.18.0 - * - * @param string $name Secret name. - * @param string $value Secret value. - * @return true|WP_Error - */ - private static function api_set( string $name, string $value ) { - if ( isset( self::$test_overrides['set'] ) ) { - return call_user_func( self::$test_overrides['set'], $name, $value ); - } - - return wp_set_network_secret( $name, $value ); - } - - /** - * Read and reveal from the Secrets API. - * - * @since 0.18.0 - * - * @param string $name Secret name. - * @return string|null|WP_Error - */ - private static function api_get( string $name ) { - if ( isset( self::$test_overrides['get'] ) ) { - return call_user_func( self::$test_overrides['get'], $name ); - } - - $secret = wp_get_network_secret( $name ); - - if ( is_wp_error( $secret ) || null === $secret ) { - return $secret; - } - - return $secret->reveal(); - } - - /** - * Delete from the Secrets API. - * - * @since 0.18.0 - * - * @param string $name Secret name. - * @return true|WP_Error - */ - private static function api_delete( string $name ) { - if ( isset( self::$test_overrides['delete'] ) ) { - return call_user_func( self::$test_overrides['delete'], $name ); - } - - return wp_delete_network_secret( $name ); - } } diff --git a/phpstan.dist.neon b/phpstan.dist.neon index ef80279c..73cf7afd 100644 --- a/phpstan.dist.neon +++ b/phpstan.dist.neon @@ -7,6 +7,7 @@ parameters: paths: - CLI - providers + - secrets - settings - class-two-factor-compat.php - class-two-factor-secrets.php diff --git a/phpunit-multisite.xml b/phpunit-multisite.xml index 668e2784..e5b47096 100644 --- a/phpunit-multisite.xml +++ b/phpunit-multisite.xml @@ -24,6 +24,7 @@ includes providers + secrets settings class-two-factor-compat.php class-two-factor-core.php diff --git a/phpunit.xml.dist b/phpunit.xml.dist index aac5ac86..05314b9f 100644 --- a/phpunit.xml.dist +++ b/phpunit.xml.dist @@ -19,6 +19,7 @@ includes providers + secrets settings class-two-factor-compat.php class-two-factor-core.php diff --git a/secrets/class-two-factor-secrets-api-store.php b/secrets/class-two-factor-secrets-api-store.php new file mode 100644 index 00000000..34bb92ac --- /dev/null +++ b/secrets/class-two-factor-secrets-api-store.php @@ -0,0 +1,102 @@ +reveal(); + } + + /** + * Write a secret. + * + * @since 0.18.0 + * + * @param string $name Secret name. + * @param string $value Secret value. + * @return true|WP_Error + */ + public function set( string $name, string $value ) { + return wp_set_network_secret( $name, $value ); + } + + /** + * Delete a secret. + * + * @since 0.18.0 + * + * @param string $name Secret name. + * @return true|WP_Error + */ + public function delete( string $name ) { + return wp_delete_network_secret( $name ); + } +} diff --git a/secrets/class-two-factor-secrets-manager.php b/secrets/class-two-factor-secrets-manager.php new file mode 100644 index 00000000..630789c8 --- /dev/null +++ b/secrets/class-two-factor-secrets-manager.php @@ -0,0 +1,313 @@ +store = $store; + } + + /** + * Whether the store is available right now. + * + * @since 0.18.0 + * + * @return bool + */ + public function is_api_present(): bool { + return $this->store->is_available(); + } + + /** + * Whether the store accepts writes. + * + * @since 0.18.0 + * + * @return bool + */ + public function is_provider_writable(): bool { + return $this->store->is_available() && $this->store->is_writable(); + } + + /** + * Label of whatever backs the store, or an empty string when it is unavailable. + * + * @since 0.18.0 + * + * @return string + */ + public function provider_label(): string { + return $this->store->is_available() ? $this->store->get_label() : ''; + } + + /** + * Whether an administrator has turned Secrets API storage on. + * + * Storage is opt-in: moving a secret into the Secrets API is one-directional, and + * a changed encryption key makes every stored secret unreadable, so an administrator + * has to acknowledge that before anything is written. + * + * @since 0.18.0 + * + * @return bool + */ + public function is_opted_in(): bool { + return '1' === (string) get_site_option( Two_Factor_Secrets::OPT_IN_OPTION_KEY, '' ); + } + + /** + * Record whether an administrator has turned Secrets API storage on. + * + * Turning it off stops new writes and migration only. Secrets that were already + * stored stay in the Secrets API and are still read from it. + * + * @since 0.18.0 + * + * @param bool $opted_in Whether Secrets API storage is turned on. + * @return void + */ + public function set_opted_in( bool $opted_in ): void { + if ( $opted_in ) { + update_site_option( Two_Factor_Secrets::OPT_IN_OPTION_KEY, '1' ); + return; + } + + delete_site_option( Two_Factor_Secrets::OPT_IN_OPTION_KEY ); + } + + /** + * Whether Two-Factor should write secrets to the Secrets API. + * + * This is the administrator's opt-in, as adjusted by the filter. It says nothing + * about whether the API is present or writable; see can_write() for that. + * + * @since 0.18.0 + * + * @param int $user_id User ID; may be 0 when no user is in context. + * @return bool + */ + public function is_enabled( int $user_id = 0 ): bool { + /** + * Filters whether Two-Factor writes secrets to the Secrets API. + * + * This controls writes and migration only. Users whose secret was already + * migrated are still read from the Secrets API while it is present. + * + * Returning true turns storage on without the administrator opting in on the + * settings screen, so only do that where the encryption key is managed for + * the site. + * + * @since 0.18.0 + * + * @param bool $enabled Whether to use the Secrets API. Defaults to the administrator's + * opt-in on the settings screen, which is off until it is saved. + * @param int $user_id User ID, which may be 0 when no user is in context. + */ + return (bool) apply_filters( 'two_factor_use_secrets_api', $this->is_opted_in(), $user_id ); + } + + /** + * Whether new secrets may be written to the Secrets API. + * + * @since 0.18.0 + * + * @param int $user_id User ID; may be 0 when no user is in context. + * @return bool + */ + public function can_write( int $user_id = 0 ): bool { + if ( ! $this->is_api_present() ) { + return false; + } + + if ( ! $this->is_enabled( $user_id ) ) { + return false; + } + + return $this->is_provider_writable(); + } + + /** + * Build the secret name for a user. + * + * @since 0.18.0 + * + * @param int $user_id User ID. + * @param string $slug Secret slug, such as "totp". + * @return string + * + * @throws InvalidArgumentException When the slug is invalid. + */ + public function get_secret_name( int $user_id, string $slug ): string { + $this->assert_slug( $slug ); + + return "two-factor/{$slug}-{$user_id}"; + } + + /** + * Get the user meta key holding the network marker. + * + * @since 0.18.0 + * + * @param string $slug Secret slug. + * @return string + * + * @throws InvalidArgumentException When the slug is invalid. + */ + public function get_marker_meta_key( string $slug ): string { + $this->assert_slug( $slug ); + + return "_two_factor_{$slug}_key_network"; + } + + /** + * Read a user's secret. + * + * @since 0.18.0 + * + * @param int $user_id User ID. + * @param string $slug Secret slug. + * @return string|null|WP_Error Null when no secret has been migrated for the user. + */ + public function get_user_secret( int $user_id, string $slug ) { + $marker = get_user_meta( $user_id, $this->get_marker_meta_key( $slug ), true ); + + if ( '' === $marker || false === $marker || null === $marker ) { + return null; + } + + if ( ! $this->is_api_present() ) { + return new WP_Error( + 'two_factor_secrets_api_missing', + __( 'The Secrets API is not available, so this secret cannot be read.', 'two-factor' ) + ); + } + + if ( (int) get_current_network_id() !== (int) $marker ) { + return new WP_Error( + 'two_factor_secret_wrong_network', + __( 'This secret is stored for a different network.', 'two-factor' ) + ); + } + + $value = $this->store->get( $this->get_secret_name( $user_id, $slug ) ); + + if ( null === $value ) { + return new WP_Error( + 'two_factor_secret_missing', + __( 'The stored secret could not be found.', 'two-factor' ) + ); + } + + return $value; + } + + /** + * Store a user's secret. + * + * @since 0.18.0 + * + * @param int $user_id User ID. + * @param string $slug Secret slug. + * @param string $value Secret value. + * @return true|WP_Error + */ + public function set_user_secret( int $user_id, string $slug, string $value ) { + if ( ! $this->is_api_present() ) { + return new WP_Error( + 'two_factor_secrets_api_missing', + __( 'The Secrets API is not available, so this secret cannot be stored.', 'two-factor' ) + ); + } + + $result = $this->store->set( $this->get_secret_name( $user_id, $slug ), $value ); + + if ( is_wp_error( $result ) ) { + return $result; + } + + update_user_meta( $user_id, $this->get_marker_meta_key( $slug ), (string) get_current_network_id() ); + + return true; + } + + /** + * Delete a user's secret and its marker. + * + * A missing secret counts as success. + * + * @since 0.18.0 + * + * @param int $user_id User ID. + * @param string $slug Secret slug. + * @return true|WP_Error + */ + public function delete_user_secret( int $user_id, string $slug ) { + $error = null; + + if ( $this->is_api_present() ) { + $result = $this->store->delete( $this->get_secret_name( $user_id, $slug ) ); + if ( is_wp_error( $result ) ) { + $error = $result; + } + } + + delete_user_meta( $user_id, $this->get_marker_meta_key( $slug ) ); + + return $error ? $error : true; + } + + /** + * Validate a slug. + * + * @since 0.18.0 + * + * @param string $slug Secret slug. + * @return void + * + * @throws InvalidArgumentException When the slug is invalid. + */ + private function assert_slug( string $slug ): void { + if ( 1 !== preg_match( '/^[a-z0-9]([a-z0-9_-]*[a-z0-9])?$/', $slug ) ) { + throw new InvalidArgumentException( 'Invalid secret slug.' ); + } + } +} diff --git a/secrets/interface-two-factor-secrets-store.php b/secrets/interface-two-factor-secrets-store.php new file mode 100644 index 00000000..808e5889 --- /dev/null +++ b/secrets/interface-two-factor-secrets-store.php @@ -0,0 +1,76 @@ + + */ + public $values = array(); + + /** + * When set, called instead of reading; receives the name and returns what get() should. + * + * @var callable|null + */ + public $on_get = null; + + /** + * When set, called instead of writing; receives the name and value and returns what set() should. + * + * @var callable|null + */ + public $on_set = null; + + /** + * Whether the store is available. + * + * @return bool + */ + public function is_available(): bool { + return $this->available; + } + + /** + * Whether the store is writable. + * + * @return bool + */ + public function is_writable(): bool { + return $this->writable; + } + + /** + * Label of the store. + * + * @return string + */ + public function get_label(): string { + return 'Memory'; + } + + /** + * Read a secret. + * + * @param string $name Secret name. + * @return string|null|WP_Error + */ + public function get( string $name ) { + if ( null !== $this->on_get ) { + return call_user_func( $this->on_get, $name ); + } + + return isset( $this->values[ $name ] ) ? $this->values[ $name ] : null; + } + + /** + * Write a secret. + * + * @param string $name Secret name. + * @param string $value Secret value. + * @return true|WP_Error + */ + public function set( string $name, string $value ) { + if ( null !== $this->on_set ) { + return call_user_func( $this->on_set, $name, $value ); + } + + $this->values[ $name ] = $value; + + return true; + } + + /** + * Delete a secret. + * + * @param string $name Secret name. + * @return true + */ + public function delete( string $name ) { + unset( $this->values[ $name ] ); + + return true; + } +} diff --git a/tests/class-two-factor-secrets-test-case.php b/tests/class-two-factor-secrets-test-case.php index cd7439eb..c456f9f4 100644 --- a/tests/class-two-factor-secrets-test-case.php +++ b/tests/class-two-factor-secrets-test-case.php @@ -11,7 +11,14 @@ abstract class Two_Factor_Secrets_UnitTestCase extends WP_UnitTestCase { /** - * Opt in to Secrets API storage, as an administrator would on the settings screen. + * The in-memory store the plugin writes secrets to during a test. + * + * @var Two_Factor_Secrets_Memory_Store + */ + protected $secrets_store; + + /** + * Store secrets in memory and opt in, as an administrator would on the settings screen. * * Storage is off until an administrator turns it on. Most tests here exercise the * storage itself, so they start opted in; tests of the default call opt_out(). @@ -20,6 +27,9 @@ abstract class Two_Factor_Secrets_UnitTestCase extends WP_UnitTestCase { */ public function set_up() { parent::set_up(); + + $this->secrets_store = new Two_Factor_Secrets_Memory_Store(); + Two_Factor_Secrets::set_instance( new Two_Factor_Secrets_Manager( $this->secrets_store ) ); Two_Factor_Secrets::set_opted_in( true ); } @@ -33,7 +43,7 @@ protected function opt_out() { } /** - * Skip the test unless the Secrets API feature plugin is loaded. + * Skip the test unless the Secrets API feature plugin is loaded. Only tests of the real store need it. * * @return void */ @@ -44,12 +54,12 @@ protected function require_secrets_api() { } /** - * Make the adapter behave as though the Secrets API were absent. + * Make the store report itself as unavailable, as when the Secrets API is absent. * * @return void */ protected function simulate_api_absent() { - add_filter( 'two_factor_secrets_api_present', '__return_false' ); + $this->secrets_store->available = false; } /** @@ -58,7 +68,7 @@ protected function simulate_api_absent() { * @return void */ public function tear_down() { - Two_Factor_Secrets::$test_overrides = array(); + Two_Factor_Secrets::reset(); delete_site_transient( 'two_factor_totp_affected_users' ); parent::tear_down(); } diff --git a/tests/class-two-factor-secrets-tests.php b/tests/class-two-factor-secrets-tests.php index 601143c9..67de2cba 100644 --- a/tests/class-two-factor-secrets-tests.php +++ b/tests/class-two-factor-secrets-tests.php @@ -33,36 +33,32 @@ public function test_get_secret_name_rejects_invalid_slug() { } /** - * Presence reflects loaded functions. + * Presence reflects whether the store is available. */ - public function test_is_api_present_reflects_functions() { - $this->assertSame( function_exists( 'wp_get_network_secret' ) && class_exists( 'WP_Secret' ), Two_Factor_Secrets::is_api_present() ); + public function test_is_api_present_reflects_store() { + $this->assertTrue( Two_Factor_Secrets::is_api_present() ); + $this->assertSame( 'Memory', Two_Factor_Secrets::provider_label() ); + + $this->secrets_store->available = false; + + $this->assertFalse( Two_Factor_Secrets::is_api_present() ); + $this->assertFalse( Two_Factor_Secrets::is_provider_writable() ); + $this->assertSame( '', Two_Factor_Secrets::provider_label() ); } /** - * The internal filter receives the resolved presence, and cannot override a missing API. + * Without a manager set, the facade builds one over the Secrets API. */ - public function test_is_api_present_filter_receives_resolved_value() { - $expected = function_exists( 'wp_get_network_secret' ) && class_exists( 'WP_Secret' ); - $seen = array(); - add_filter( - 'two_factor_secrets_api_present', - function ( $present, $resolved ) use ( &$seen ) { - $seen = array( $present, $resolved ); - return true; - }, - 10, - 2 - ); + public function test_facade_defaults_to_the_secrets_api_store() { + Two_Factor_Secrets::reset(); - $this->assertSame( $expected, Two_Factor_Secrets::is_api_present() ); - $this->assertSame( array( true, $expected ), $seen ); + $this->assertSame( ( new Two_Factor_Secrets_Api_Store() )->is_available(), Two_Factor_Secrets::is_api_present() ); } /** - * The internal filter can force absence. + * An unavailable store means the API is absent. */ - public function test_is_api_present_internal_filter_forces_absent() { + public function test_is_api_present_false_when_store_unavailable() { $this->simulate_api_absent(); $this->assertFalse( Two_Factor_Secrets::is_api_present() ); } @@ -71,7 +67,6 @@ public function test_is_api_present_internal_filter_forces_absent() { * Writes are allowed with the API present once an administrator has opted in. */ public function test_can_write_true_with_api_present() { - $this->require_secrets_api(); $this->assertTrue( Two_Factor_Secrets::can_write( 1 ) ); } @@ -79,7 +74,6 @@ public function test_can_write_true_with_api_present() { * Storage is off until an administrator opts in. */ public function test_can_write_false_until_opted_in() { - $this->require_secrets_api(); $this->opt_out(); $this->assertFalse( Two_Factor_Secrets::is_opted_in() ); @@ -106,7 +100,6 @@ public function test_set_opted_in_round_trip() { * The filter receives the opt-in as its default and can turn storage on without it. */ public function test_filter_receives_opt_in_and_can_force_on() { - $this->require_secrets_api(); $this->opt_out(); $seen = null; add_filter( @@ -134,7 +127,6 @@ public function test_can_write_false_when_api_absent() { * The opt-out filter disables writes and receives the user ID. */ public function test_can_write_false_when_filter_opts_out() { - $this->require_secrets_api(); $seen = null; add_filter( 'two_factor_use_secrets_api', @@ -154,8 +146,7 @@ function ( $enabled, $user_id ) use ( &$seen ) { * A read-only provider disables writes. */ public function test_can_write_false_when_provider_read_only() { - $this->require_secrets_api(); - Two_Factor_Secrets::$test_overrides['writable'] = '__return_false'; + $this->secrets_store->writable = false; $this->assertTrue( Two_Factor_Secrets::is_api_present() ); $this->assertFalse( Two_Factor_Secrets::can_write( 1 ) ); @@ -170,16 +161,14 @@ public function test_get_user_secret_null_without_marker() { } /** - * Secrets are stored in network scope. + * A secret round trips through the store and records the network. */ - public function test_get_user_secret_round_trip_uses_network_scope() { - $this->require_secrets_api(); + public function test_get_user_secret_round_trip() { $user_id = self::factory()->user->create(); $this->assertTrue( Two_Factor_Secrets::set_user_secret( $user_id, 'totp', 'ABCDEF' ) ); $this->assertSame( 'ABCDEF', Two_Factor_Secrets::get_user_secret( $user_id, 'totp' ) ); - $this->assertNotNull( wp_get_network_secret( "two-factor/totp-{$user_id}" ) ); - $this->assertNull( wp_get_secret( "two-factor/totp-{$user_id}" ) ); + $this->assertNotNull( $this->secrets_store->get( "two-factor/totp-{$user_id}" ) ); $this->assertSame( (string) get_current_network_id(), get_user_meta( $user_id, Two_Factor_Secrets::get_marker_meta_key( 'totp' ), true ) @@ -205,7 +194,6 @@ public function test_get_user_secret_error_when_api_absent_with_marker() { * Marker naming another network is an error. */ public function test_get_user_secret_error_when_marker_names_other_network() { - $this->require_secrets_api(); $user_id = self::factory()->user->create(); update_user_meta( $user_id, Two_Factor_Secrets::get_marker_meta_key( 'totp' ), (string) ( get_current_network_id() + 1 ) ); @@ -218,10 +206,9 @@ public function test_get_user_secret_error_when_marker_names_other_network() { * Marker with no stored secret is an error. */ public function test_get_user_secret_error_when_marker_present_but_secret_gone() { - $this->require_secrets_api(); $user_id = self::factory()->user->create(); Two_Factor_Secrets::set_user_secret( $user_id, 'totp', 'ABCDEF' ); - wp_delete_network_secret( "two-factor/totp-{$user_id}" ); + $this->secrets_store->delete( "two-factor/totp-{$user_id}" ); $result = Two_Factor_Secrets::get_user_secret( $user_id, 'totp' ); $this->assertWPError( $result ); @@ -232,10 +219,9 @@ public function test_get_user_secret_error_when_marker_present_but_secret_gone() * API errors pass through. */ public function test_get_user_secret_passes_through_api_error() { - $this->require_secrets_api(); $user_id = self::factory()->user->create(); update_user_meta( $user_id, Two_Factor_Secrets::get_marker_meta_key( 'totp' ), (string) get_current_network_id() ); - Two_Factor_Secrets::$test_overrides['get'] = function () { + $this->secrets_store->on_get = function () { return new WP_Error( 'secret_decryption_failed' ); }; @@ -248,9 +234,8 @@ public function test_get_user_secret_passes_through_api_error() { * A failed write leaves no marker. */ public function test_set_user_secret_error_leaves_marker_absent() { - $this->require_secrets_api(); - $user_id = self::factory()->user->create(); - Two_Factor_Secrets::$test_overrides['set'] = function () { + $user_id = self::factory()->user->create(); + $this->secrets_store->on_set = function () { return new WP_Error( 'write_failed' ); }; @@ -262,12 +247,11 @@ public function test_set_user_secret_error_leaves_marker_absent() { * Delete removes secret and marker. */ public function test_delete_user_secret_removes_secret_and_marker() { - $this->require_secrets_api(); $user_id = self::factory()->user->create(); Two_Factor_Secrets::set_user_secret( $user_id, 'totp', 'ABCDEF' ); $this->assertTrue( Two_Factor_Secrets::delete_user_secret( $user_id, 'totp' ) ); - $this->assertNull( wp_get_network_secret( "two-factor/totp-{$user_id}" ) ); + $this->assertNull( $this->secrets_store->get( "two-factor/totp-{$user_id}" ) ); $this->assertSame( '', get_user_meta( $user_id, Two_Factor_Secrets::get_marker_meta_key( 'totp' ), true ) ); } @@ -287,7 +271,6 @@ public function test_delete_user_secret_removes_marker_when_api_absent() { * Deleting a missing secret succeeds. */ public function test_delete_user_secret_missing_is_success() { - $this->require_secrets_api(); $user_id = self::factory()->user->create(); $this->assertTrue( Two_Factor_Secrets::delete_user_secret( $user_id, 'totp' ) ); diff --git a/tests/cli/class-two-factor-cli-command.php b/tests/cli/class-two-factor-cli-command.php index 2ad8e601..6a019ed9 100644 --- a/tests/cli/class-two-factor-cli-command.php +++ b/tests/cli/class-two-factor-cli-command.php @@ -20,6 +20,13 @@ class Tests_Two_Factor_CLI_Command extends WP_UnitTestCase { */ protected $command; + /** + * The in-memory store the plugin writes secrets to during a test. + * + * @var Two_Factor_Secrets_Memory_Store + */ + protected $secrets_store; + /** * Load the WP-CLI test doubles and the command under test. * @@ -54,7 +61,9 @@ public function set_up() { WP_CLI::reset(); - // Secrets API storage is opt-in; the secrets tests below start opted in. + // Store secrets in memory. Storage is opt-in; the secrets tests below start opted in. + $this->secrets_store = new Two_Factor_Secrets_Memory_Store(); + Two_Factor_Secrets::set_instance( new Two_Factor_Secrets_Manager( $this->secrets_store ) ); Two_Factor_Secrets::set_opted_in( true ); $this->command = new Two_Factor_CLI_Command(); @@ -304,9 +313,6 @@ public function test_status_reports_totp_storage_plaintext() { * @covers Two_Factor_CLI_Command::status */ public function test_status_reports_totp_storage_secrets_api() { - if ( ! function_exists( 'wp_get_network_secret' ) ) { - $this->markTestSkipped( 'Secrets API feature plugin is not loaded.' ); - } Two_Factor_Totp::get_instance()->set_user_totp_key( $this->user->ID, 'ABCDEFGH' ); $this->command->status( array( 'cli_test_user' ), array() ); @@ -321,7 +327,7 @@ public function test_status_reports_totp_storage_secrets_api() { */ public function test_status_reports_totp_storage_unavailable() { update_user_meta( $this->user->ID, Two_Factor_Totp::SECRET_NETWORK_META_KEY, (string) get_current_network_id() ); - add_filter( 'two_factor_secrets_api_present', '__return_false' ); + $this->secrets_store->available = false; $this->command->status( array( 'cli_test_user' ), array() ); @@ -334,9 +340,6 @@ public function test_status_reports_totp_storage_unavailable() { * @covers Two_Factor_CLI_Command::secrets */ public function test_secrets_status_reports_present_api_and_counts() { - if ( ! function_exists( 'wp_get_network_secret' ) ) { - $this->markTestSkipped( 'Secrets API feature plugin is not loaded.' ); - } $plaintext_user = self::factory()->user->create(); $migrated_user = self::factory()->user->create(); $totp = Two_Factor_Totp::get_instance(); @@ -379,7 +382,7 @@ public function test_secrets_status_works_when_totp_disabled_site_wide() { * @covers Two_Factor_CLI_Command::secrets */ public function test_secrets_status_reports_absent_api() { - add_filter( 'two_factor_secrets_api_present', '__return_false' ); + $this->secrets_store->available = false; update_user_meta( $this->user->ID, Two_Factor_Totp::SECRET_NETWORK_META_KEY, (string) get_current_network_id() ); $this->command->secrets( array( 'status' ), array() ); @@ -441,19 +444,10 @@ protected function seed_plaintext_key( $user_id ) { } /** - * Skip unless the Secrets API is loaded. - */ - protected function require_secrets_api() { - if ( ! function_exists( 'wp_get_network_secret' ) ) { - $this->markTestSkipped( 'Secrets API feature plugin is not loaded.' ); - } - } - - /** - * Reset adapter test seams. + * Go back to storing secrets with the Secrets API. */ public function tear_down() { - Two_Factor_Secrets::$test_overrides = array(); + Two_Factor_Secrets::reset(); parent::tear_down(); } @@ -463,7 +457,6 @@ public function tear_down() { * @covers Two_Factor_CLI_Command::secrets */ public function test_secrets_migrate_single_user() { - $this->require_secrets_api(); $this->seed_plaintext_key( $this->user->ID ); $this->command->secrets( array( 'migrate', 'cli_test_user' ), array() ); @@ -479,7 +472,6 @@ public function test_secrets_migrate_single_user() { * @covers Two_Factor_CLI_Command::secrets */ public function test_secrets_migrate_dry_run_changes_nothing() { - $this->require_secrets_api(); $this->seed_plaintext_key( $this->user->ID ); $this->command->secrets( array( 'migrate' ), array( 'dry-run' => true ) ); @@ -495,7 +487,6 @@ public function test_secrets_migrate_dry_run_changes_nothing() { * @covers Two_Factor_CLI_Command::secrets */ public function test_secrets_migrate_dry_run_skips_user_without_plaintext() { - $this->require_secrets_api(); $this->command->secrets( array( 'migrate', 'cli_test_user' ), array( 'dry-run' => true ) ); @@ -508,7 +499,6 @@ public function test_secrets_migrate_dry_run_skips_user_without_plaintext() { * @covers Two_Factor_CLI_Command::secrets */ public function test_secrets_migrate_pages_through_users_in_batches() { - $this->require_secrets_api(); $ids = array( $this->user->ID, self::factory()->user->create(), self::factory()->user->create() ); foreach ( $ids as $id ) { $this->seed_plaintext_key( $id ); @@ -528,17 +518,17 @@ public function test_secrets_migrate_pages_through_users_in_batches() { * @covers Two_Factor_CLI_Command::secrets */ public function test_secrets_migrate_reports_failed_users_and_continues() { - $this->require_secrets_api(); $bad = self::factory()->user->create(); $ids = array( $this->user->ID, $bad, self::factory()->user->create() ); foreach ( $ids as $id ) { $this->seed_plaintext_key( $id ); } - Two_Factor_Secrets::$test_overrides['set'] = function ( $name, $value ) use ( $bad ) { + $this->secrets_store->on_set = function ( $name, $value ) use ( $bad ) { if ( "two-factor/totp-{$bad}" === $name ) { return new WP_Error( 'write_failed', 'write failed' ); } - return wp_set_network_secret( $name, $value ); + $this->secrets_store->values[ $name ] = $value; + return true; }; $this->command->secrets( array( 'migrate' ), array( 'batch-size' => '2' ) ); @@ -554,7 +544,7 @@ public function test_secrets_migrate_reports_failed_users_and_continues() { * @covers Two_Factor_CLI_Command::secrets */ public function test_secrets_migrate_errors_when_api_absent() { - add_filter( 'two_factor_secrets_api_present', '__return_false' ); + $this->secrets_store->available = false; $message = $this->assert_command_aborts( function () { @@ -588,7 +578,6 @@ function () { * @covers Two_Factor_CLI_Command::secrets */ public function test_secrets_migrate_errors_when_not_opted_in() { - $this->require_secrets_api(); Two_Factor_Secrets::set_opted_in( false ); update_user_meta( $this->user->ID, Two_Factor_Totp::SECRET_META_KEY, 'ABCDEFGH' ); @@ -639,7 +628,6 @@ function () { * @covers Two_Factor_CLI_Command::secrets */ public function test_secrets_migrate_user_not_found() { - $this->require_secrets_api(); $message = $this->assert_command_aborts( function () { @@ -656,7 +644,6 @@ function () { * @covers Two_Factor_CLI_Command::secrets */ public function test_secrets_export_is_not_an_action() { - $this->require_secrets_api(); Two_Factor_Totp::get_instance()->set_user_totp_key( $this->user->ID, 'ABCDEFGH' ); $message = $this->assert_command_aborts( diff --git a/tests/providers/class-two-factor-totp-secrets-tests.php b/tests/providers/class-two-factor-totp-secrets-tests.php index 511a0d9a..86f2d3e6 100644 --- a/tests/providers/class-two-factor-totp-secrets-tests.php +++ b/tests/providers/class-two-factor-totp-secrets-tests.php @@ -105,14 +105,13 @@ public function test_marker_key_matches_adapter() { * Keys are stored in the Secrets API. */ public function test_set_key_with_api_stores_in_secrets_api() { - $this->require_secrets_api(); $user_id = $this->user(); $this->assertTrue( $this->provider->set_user_totp_key( $user_id, 'ABCDEFGH' ) ); $this->assertSame( '', $this->plaintext( $user_id ) ); $this->assertSame( (string) get_current_network_id(), $this->marker( $user_id ) ); $this->assertSame( 'ABCDEFGH', $this->provider->get_user_totp_key( $user_id ) ); - $this->assertNotNull( wp_get_network_secret( "two-factor/totp-{$user_id}" ) ); + $this->assertNotNull( $this->secrets_store->get( "two-factor/totp-{$user_id}" ) ); } /** @@ -133,9 +132,8 @@ public function test_set_key_without_api_stores_plaintext() { * A failed Secrets API write never falls back to plaintext. */ public function test_set_key_returns_false_when_secrets_write_fails() { - $this->require_secrets_api(); - $user_id = $this->user(); - Two_Factor_Secrets::$test_overrides['set'] = function () { + $user_id = $this->user(); + $this->secrets_store->on_set = function () { return new WP_Error( 'write_failed' ); }; @@ -148,14 +146,13 @@ public function test_set_key_returns_false_when_secrets_write_fails() { * A read-back mismatch fails the write and cleans up. */ public function test_set_key_returns_false_when_readback_mismatches() { - $this->require_secrets_api(); - $user_id = $this->user(); - Two_Factor_Secrets::$test_overrides['get'] = function () { + $user_id = $this->user(); + $this->secrets_store->on_get = function () { return 'SOMETHINGELSE'; }; $this->assertFalse( $this->provider->set_user_totp_key( $user_id, 'ABCDEFGH' ) ); - $this->assertNull( wp_get_network_secret( "two-factor/totp-{$user_id}" ) ); + $this->assertArrayNotHasKey( "two-factor/totp-{$user_id}", $this->secrets_store->values ); $this->assertSame( '', $this->marker( $user_id ) ); } @@ -177,7 +174,6 @@ public function test_set_empty_key_deletes() { * Delete clears everything. */ public function test_delete_key_removes_secret_marker_and_plaintext() { - $this->require_secrets_api(); $user_id = $this->user(); $this->provider->set_user_totp_key( $user_id, 'ABCDEFGH' ); update_user_meta( $user_id, Two_Factor_Totp::SECRET_META_KEY, 'LEFTOVER' ); @@ -185,7 +181,7 @@ public function test_delete_key_removes_secret_marker_and_plaintext() { $this->assertTrue( $this->provider->delete_user_totp_key( $user_id ) ); $this->assertSame( '', $this->plaintext( $user_id ) ); $this->assertSame( '', $this->marker( $user_id ) ); - $this->assertNull( wp_get_network_secret( "two-factor/totp-{$user_id}" ) ); + $this->assertNull( $this->secrets_store->get( "two-factor/totp-{$user_id}" ) ); } /** @@ -204,7 +200,6 @@ public function test_delete_key_removes_marker_when_api_absent() { * Reading a plaintext key migrates it. */ public function test_lazy_migration_moves_plaintext_to_secrets_api() { - $this->require_secrets_api(); $user_id = $this->user(); update_user_meta( $user_id, Two_Factor_Totp::SECRET_META_KEY, 'ABCDEFGH' ); @@ -219,7 +214,6 @@ public function test_lazy_migration_moves_plaintext_to_secrets_api() { * Migration runs once. */ public function test_lazy_migration_is_idempotent() { - $this->require_secrets_api(); $user_id = $this->user(); update_user_meta( $user_id, Two_Factor_Totp::SECRET_META_KEY, 'ABCDEFGH' ); @@ -234,10 +228,9 @@ public function test_lazy_migration_is_idempotent() { * A failed write keeps plaintext. */ public function test_lazy_migration_write_failure_keeps_plaintext_and_fires_failed_action() { - $this->require_secrets_api(); $user_id = $this->user(); update_user_meta( $user_id, Two_Factor_Totp::SECRET_META_KEY, 'ABCDEFGH' ); - Two_Factor_Secrets::$test_overrides['set'] = function () { + $this->secrets_store->on_set = function () { return new WP_Error( 'write_failed' ); }; @@ -252,10 +245,9 @@ public function test_lazy_migration_write_failure_keeps_plaintext_and_fires_fail * A read-back mismatch keeps plaintext and cleans up. */ public function test_lazy_migration_readback_mismatch_keeps_plaintext_and_cleans_up() { - $this->require_secrets_api(); $user_id = $this->user(); update_user_meta( $user_id, Two_Factor_Totp::SECRET_META_KEY, 'ABCDEFGH' ); - Two_Factor_Secrets::$test_overrides['get'] = function () { + $this->secrets_store->on_get = function () { return 'SOMETHINGELSE'; }; @@ -265,7 +257,7 @@ public function test_lazy_migration_readback_mismatch_keeps_plaintext_and_cleans $this->assertSame( 'two_factor_secrets_migration_mismatch', $result->get_error_code() ); $this->assertSame( 'ABCDEFGH', $this->plaintext( $user_id ) ); $this->assertSame( '', $this->marker( $user_id ) ); - $this->assertNull( wp_get_network_secret( "two-factor/totp-{$user_id}" ) ); + $this->assertArrayNotHasKey( "two-factor/totp-{$user_id}", $this->secrets_store->values ); $this->assertSame( 1, $this->count_calls( 'failed' ) ); } @@ -273,7 +265,6 @@ public function test_lazy_migration_readback_mismatch_keeps_plaintext_and_cleans * Opting out skips migration. */ public function test_lazy_migration_skipped_when_filter_opts_out() { - $this->require_secrets_api(); add_filter( 'two_factor_use_secrets_api', '__return_false' ); $user_id = $this->user(); update_user_meta( $user_id, Two_Factor_Totp::SECRET_META_KEY, 'ABCDEFGH' ); @@ -287,7 +278,6 @@ public function test_lazy_migration_skipped_when_filter_opts_out() { * An unreadable secret with the API present keeps TOTP enrolled and does not force the fallback. */ public function test_unreadable_secret_with_api_present_keeps_totp_and_does_not_force_fallback() { - $this->require_secrets_api(); $user_id = $this->user(); $key = Two_Factor_Totp::generate_key(); $this->provider->set_user_totp_key( $user_id, $key ); @@ -306,7 +296,6 @@ public function test_unreadable_secret_with_api_present_keeps_totp_and_does_not_ * Migration is one-directional: opting out never moves a migrated secret back to plaintext. */ public function test_opt_out_never_moves_migrated_secret_back_to_plaintext() { - $this->require_secrets_api(); $user_id = $this->user(); $this->provider->set_user_totp_key( $user_id, 'ABCDEFGH' ); @@ -323,7 +312,6 @@ public function test_opt_out_never_moves_migrated_secret_back_to_plaintext() { * Plaintext wins over a marker. */ public function test_plaintext_beats_marker() { - $this->require_secrets_api(); add_filter( 'two_factor_use_secrets_api', '__return_false' ); $user_id = $this->user(); update_user_meta( $user_id, Two_Factor_Totp::SECRET_META_KEY, 'PLAINTEXT' ); @@ -358,7 +346,6 @@ public function test_key_state_error_when_api_absent_with_marker() { * A marker for another network is an error. */ public function test_key_state_error_when_marker_names_other_network() { - $this->require_secrets_api(); $user_id = $this->user(); update_user_meta( $user_id, Two_Factor_Totp::SECRET_NETWORK_META_KEY, (string) ( get_current_network_id() + 1 ) ); @@ -372,7 +359,6 @@ public function test_key_state_error_when_marker_names_other_network() { * Opting out does not stop reads of migrated users. */ public function test_migrated_user_still_readable_when_filter_opts_out() { - $this->require_secrets_api(); $user_id = $this->user(); $this->provider->set_user_totp_key( $user_id, 'ABCDEFGH' ); add_filter( 'two_factor_use_secrets_api', '__return_false' ); @@ -384,7 +370,6 @@ public function test_migrated_user_still_readable_when_filter_opts_out() { * Available with marker and API. */ public function test_is_available_for_user_true_with_marker_and_api() { - $this->require_secrets_api(); $user_id = $this->user(); $this->provider->set_user_totp_key( $user_id, 'ABCDEFGH' ); @@ -406,7 +391,6 @@ public function test_is_available_for_user_false_with_marker_and_api_absent() { * Unavailable with a marker for another network. */ public function test_is_available_for_user_false_with_marker_for_other_network() { - $this->require_secrets_api(); $user_id = $this->user(); update_user_meta( $user_id, Two_Factor_Totp::SECRET_NETWORK_META_KEY, (string) ( get_current_network_id() + 1 ) ); @@ -417,7 +401,6 @@ public function test_is_available_for_user_false_with_marker_for_other_network() * Availability checks never migrate. */ public function test_is_available_for_user_does_not_migrate() { - $this->require_secrets_api(); $user_id = $this->user(); update_user_meta( $user_id, Two_Factor_Totp::SECRET_META_KEY, 'ABCDEFGH' ); @@ -443,14 +426,12 @@ public function test_is_enrolled_but_unavailable_for_user_matrix() { // Marker for this network, API present. update_user_meta( $user_id, Two_Factor_Totp::SECRET_NETWORK_META_KEY, (string) get_current_network_id() ); - if ( function_exists( 'wp_get_network_secret' ) ) { - $this->assertFalse( $this->provider->is_enrolled_but_unavailable_for_user( $user ) ); + $this->assertFalse( $this->provider->is_enrolled_but_unavailable_for_user( $user ) ); - // Marker for another network. - update_user_meta( $user_id, Two_Factor_Totp::SECRET_NETWORK_META_KEY, (string) ( get_current_network_id() + 1 ) ); - $this->assertTrue( $this->provider->is_enrolled_but_unavailable_for_user( $user ) ); - update_user_meta( $user_id, Two_Factor_Totp::SECRET_NETWORK_META_KEY, (string) get_current_network_id() ); - } + // Marker for another network. + update_user_meta( $user_id, Two_Factor_Totp::SECRET_NETWORK_META_KEY, (string) ( get_current_network_id() + 1 ) ); + $this->assertTrue( $this->provider->is_enrolled_but_unavailable_for_user( $user ) ); + update_user_meta( $user_id, Two_Factor_Totp::SECRET_NETWORK_META_KEY, (string) get_current_network_id() ); // Marker with the API absent. $this->simulate_api_absent(); @@ -461,7 +442,6 @@ public function test_is_enrolled_but_unavailable_for_user_matrix() { * Storage descriptions. */ public function test_get_user_totp_key_storage_values() { - $this->require_secrets_api(); $user_id = $this->user(); $this->assertSame( 'none', $this->provider->get_user_totp_key_storage( $user_id ) ); @@ -492,7 +472,6 @@ public function test_secret_set_on_blog_one_validates_on_blog_two() { if ( ! is_multisite() ) { $this->markTestSkipped( 'Multisite only.' ); } - $this->require_secrets_api(); $user_id = $this->user(); $blog_id = self::factory()->blog->create(); @@ -531,7 +510,7 @@ function () { * @return void */ private function make_unreadable() { - Two_Factor_Secrets::$test_overrides['get'] = function () { + $this->secrets_store->on_get = function () { return new WP_Error( 'secret_decryption_failed', 'sensitive detail' ); }; } @@ -552,7 +531,6 @@ private function capture( $callback ) { * Unreadable secrets fail validation. */ public function test_validate_code_fails_when_secret_unreadable() { - $this->require_secrets_api(); $user_id = $this->user(); $key = Two_Factor_Totp::generate_key(); $this->provider->set_user_totp_key( $user_id, $key ); @@ -579,7 +557,6 @@ public function test_validate_code_fails_when_secret_absent() { * A Secrets API key validates. */ public function test_validate_code_succeeds_from_secrets_api() { - $this->require_secrets_api(); $user_id = $this->user(); $key = Two_Factor_Totp::generate_key(); $this->provider->set_user_totp_key( $user_id, $key ); @@ -591,7 +568,6 @@ public function test_validate_code_succeeds_from_secrets_api() { * A marker without the API fails validation. */ public function test_validate_authentication_fails_when_api_absent_with_marker() { - $this->require_secrets_api(); $user_id = $this->user(); $key = Two_Factor_Totp::generate_key(); $this->provider->set_user_totp_key( $user_id, $key ); @@ -609,7 +585,6 @@ public function test_validate_authentication_fails_when_api_absent_with_marker() * The login prompt explains the problem. */ public function test_authentication_page_shows_unavailable_message() { - $this->require_secrets_api(); $user_id = $this->user(); $this->provider->set_user_totp_key( $user_id, Two_Factor_Totp::generate_key() ); $this->make_unreadable(); @@ -653,7 +628,6 @@ function () use ( $user ) { * The profile offers a reset, not setup, for an unreadable secret. */ public function test_user_options_shows_reset_not_setup_when_unreadable() { - $this->require_secrets_api(); $user_id = $this->user(); $this->provider->set_user_totp_key( $user_id, Two_Factor_Totp::generate_key() ); $this->make_unreadable(); @@ -693,7 +667,6 @@ function () use ( $user ) { * An affected TOTP-only user is forced onto the fallback, never single factor. */ public function test_login_fails_closed_for_affected_totp_only_user() { - $this->require_secrets_api(); $user_id = $this->user(); $this->provider->set_user_totp_key( $user_id, Two_Factor_Totp::generate_key() ); update_user_meta( $user_id, Two_Factor_Core::ENABLED_PROVIDERS_USER_META_KEY, array( 'Two_Factor_Totp' ) ); @@ -719,7 +692,6 @@ public function test_login_fails_closed_for_affected_totp_only_user() { * Without a valid fallback the user is locked out, not let in. */ public function test_login_fails_closed_for_affected_totp_only_user_without_fallback() { - $this->require_secrets_api(); $user_id = $this->user(); $this->provider->set_user_totp_key( $user_id, Two_Factor_Totp::generate_key() ); update_user_meta( $user_id, Two_Factor_Core::ENABLED_PROVIDERS_USER_META_KEY, array( 'Two_Factor_Totp' ) ); @@ -739,7 +711,6 @@ function () { * Backup codes remain usable for an affected user. */ public function test_affected_user_keeps_backup_codes() { - $this->require_secrets_api(); $user_id = $this->user(); $user = get_userdata( $user_id ); $this->provider->set_user_totp_key( $user_id, Two_Factor_Totp::generate_key() ); @@ -757,13 +728,12 @@ public function test_delete_user_removes_secret_on_single_site() { if ( is_multisite() ) { $this->markTestSkipped( 'Single site only.' ); } - $this->require_secrets_api(); $user_id = $this->user(); $this->provider->set_user_totp_key( $user_id, 'ABCDEFGH' ); wp_delete_user( $user_id ); - $this->assertNull( wp_get_network_secret( "two-factor/totp-{$user_id}" ) ); + $this->assertNull( $this->secrets_store->get( "two-factor/totp-{$user_id}" ) ); } /** @@ -773,7 +743,6 @@ public function test_delete_user_keeps_secret_on_multisite() { if ( ! is_multisite() ) { $this->markTestSkipped( 'Multisite only.' ); } - $this->require_secrets_api(); $user_id = $this->user(); $blog_id = self::factory()->blog->create(); add_user_to_blog( $blog_id, $user_id, 'subscriber' ); @@ -786,7 +755,7 @@ public function test_delete_user_keeps_secret_on_multisite() { restore_current_blog(); } - $this->assertNotNull( wp_get_network_secret( "two-factor/totp-{$user_id}" ) ); + $this->assertNotNull( $this->secrets_store->get( "two-factor/totp-{$user_id}" ) ); $this->assertSame( (string) get_current_network_id(), $this->marker( $user_id ) ); } @@ -797,14 +766,13 @@ public function test_wpmu_delete_user_removes_secret_on_multisite() { if ( ! is_multisite() ) { $this->markTestSkipped( 'Multisite only.' ); } - $this->require_secrets_api(); require_once ABSPATH . 'wp-admin/includes/ms.php'; $user_id = $this->user(); $this->provider->set_user_totp_key( $user_id, 'ABCDEFGH' ); wpmu_delete_user( $user_id ); - $this->assertNull( wp_get_network_secret( "two-factor/totp-{$user_id}" ) ); + $this->assertNull( $this->secrets_store->get( "two-factor/totp-{$user_id}" ) ); $this->assertSame( '', $this->marker( $user_id ) ); } @@ -822,7 +790,6 @@ public function test_registers_correct_deletion_hook() { * Lifecycle hooks stay registered, and still delete secrets, when TOTP is disabled site-wide. */ public function test_lifecycle_hooks_work_when_totp_disabled_site_wide() { - $this->require_secrets_api(); $user_id = $this->user(); $this->provider->set_user_totp_key( $user_id, 'ABCDEFGH' ); @@ -843,7 +810,7 @@ public function test_lifecycle_hooks_work_when_totp_disabled_site_wide() { wp_delete_user( $user_id ); } - $this->assertNull( wp_get_network_secret( "two-factor/totp-{$user_id}" ) ); + $this->assertNull( $this->secrets_store->get( "two-factor/totp-{$user_id}" ) ); } finally { delete_option( Two_Factor_Core::ENABLED_PROVIDERS_OPTION_KEY ); } @@ -853,7 +820,6 @@ public function test_lifecycle_hooks_work_when_totp_disabled_site_wide() { * Uninstall removes all secrets. */ public function test_uninstall_user_data_removes_secrets_for_all_marked_users() { - $this->require_secrets_api(); $user_ids = array( $this->user(), $this->user(), $this->user() ); foreach ( $user_ids as $user_id ) { $this->provider->set_user_totp_key( $user_id, 'ABCDEFGH' ); @@ -862,7 +828,7 @@ public function test_uninstall_user_data_removes_secrets_for_all_marked_users() Two_Factor_Totp::uninstall_user_data(); foreach ( $user_ids as $user_id ) { - $this->assertNull( wp_get_network_secret( "two-factor/totp-{$user_id}" ) ); + $this->assertNull( $this->secrets_store->get( "two-factor/totp-{$user_id}" ) ); $this->assertSame( '', $this->marker( $user_id ) ); } } @@ -918,7 +884,6 @@ public function test_has_affected_users_true_for_other_network_marker() { if ( ! is_multisite() ) { $this->markTestSkipped( 'Multisite only.' ); } - $this->require_secrets_api(); $this->seed_marker( (string) ( get_current_network_id() + 1 ) ); $this->assertTrue( Two_Factor_Totp::has_affected_users() ); @@ -1068,7 +1033,6 @@ public function test_site_health_critical_when_affected_users() { * Remaining plaintext is recommended to migrate. */ public function test_site_health_recommended_when_plaintext_remains() { - $this->require_secrets_api(); $user_id = $this->user(); update_user_meta( $user_id, Two_Factor_Totp::SECRET_META_KEY, 'ABCDEFGH' ); @@ -1083,7 +1047,6 @@ public function test_site_health_recommended_when_plaintext_remains() { * Fully migrated is good and names the provider. */ public function test_site_health_good_when_fully_migrated() { - $this->require_secrets_api(); $result = $this->provider->site_health_secret_storage(); @@ -1095,7 +1058,6 @@ public function test_site_health_good_when_fully_migrated() { * Opting out with plaintext remaining is good. */ public function test_site_health_good_when_filter_opts_out_with_plaintext() { - $this->require_secrets_api(); add_filter( 'two_factor_use_secrets_api', '__return_false' ); update_user_meta( $this->user(), Two_Factor_Totp::SECRET_META_KEY, 'ABCDEFGH' ); @@ -1124,7 +1086,6 @@ public function test_site_health_good_neutral_when_api_absent() { * Until an administrator opts in, Site Health recommends it and links to the settings screen. */ public function test_site_health_recommended_when_not_opted_in() { - $this->require_secrets_api(); $this->opt_out(); $result = $this->provider->site_health_secret_storage(); @@ -1139,8 +1100,7 @@ public function test_site_health_recommended_when_not_opted_in() { * A read-only provider is reported as such rather than blamed on the filter. */ public function test_site_health_good_when_provider_read_only() { - $this->require_secrets_api(); - Two_Factor_Secrets::$test_overrides['writable'] = '__return_false'; + $this->secrets_store->writable = false; $result = $this->provider->site_health_secret_storage(); @@ -1152,7 +1112,6 @@ public function test_site_health_good_when_provider_read_only() { * Without the opt-in a new key stays in user meta, even with the API present. */ public function test_set_key_without_opt_in_stores_plaintext() { - $this->require_secrets_api(); $this->opt_out(); $user_id = $this->user(); @@ -1160,14 +1119,13 @@ public function test_set_key_without_opt_in_stores_plaintext() { $this->assertSame( 'ABCDEFGH', get_user_meta( $user_id, Two_Factor_Totp::SECRET_META_KEY, true ) ); $this->assertSame( '', (string) get_user_meta( $user_id, Two_Factor_Totp::SECRET_NETWORK_META_KEY, true ) ); - $this->assertNull( wp_get_network_secret( "two-factor/totp-{$user_id}" ) ); + $this->assertNull( $this->secrets_store->get( "two-factor/totp-{$user_id}" ) ); } /** * Without the opt-in a plaintext key is read but not migrated. */ public function test_lazy_migration_skipped_without_opt_in() { - $this->require_secrets_api(); $this->opt_out(); $user_id = $this->user(); update_user_meta( $user_id, Two_Factor_Totp::SECRET_META_KEY, 'ABCDEFGH' ); @@ -1181,7 +1139,6 @@ public function test_lazy_migration_skipped_without_opt_in() { * Opting back out keeps already-migrated users working. */ public function test_migrated_user_still_readable_after_opting_out() { - $this->require_secrets_api(); $user_id = $this->user(); $this->provider->set_user_totp_key( $user_id, 'ABCDEFGH' ); $this->opt_out(); @@ -1199,8 +1156,8 @@ private function make_key_unavailable() { return new WP_Error( 'secret_key_unavailable', 'The wrapped key material could not be decrypted with the configured site key.' ); }; - Two_Factor_Secrets::$test_overrides['get'] = $error; - Two_Factor_Secrets::$test_overrides['set'] = $error; + $this->secrets_store->on_get = $error; + $this->secrets_store->on_set = $error; } /** @@ -1244,7 +1201,6 @@ private function admin_user() { * since a reset could not be followed by re-enrollment. */ public function test_user_options_offers_no_reset_when_key_unavailable() { - $this->require_secrets_api(); $user_id = $this->user(); $this->provider->set_user_totp_key( $user_id, Two_Factor_Totp::generate_key() ); $this->make_key_unavailable(); @@ -1269,7 +1225,6 @@ function () use ( $user ) { * @covers Two_Factor_Totp::rest_delete_totp */ public function test_rest_reset_refused_when_key_unavailable() { - $this->require_secrets_api(); $user_id = $this->user(); $this->provider->set_user_totp_key( $user_id, $this->provider->generate_key() ); Two_Factor_Core::enable_provider_for_user( $user_id, 'Two_Factor_Totp' ); @@ -1298,7 +1253,6 @@ public function test_rest_reset_refused_when_key_unavailable() { * @covers Two_Factor_Totp::set_user_totp_key */ public function test_reenroll_after_reset_with_unavailable_key_fails_closed_with_clear_error() { - $this->require_secrets_api(); $user_id = $this->admin_user(); $this->assertTrue( $this->provider->set_user_totp_key( $user_id, $this->provider->generate_key() ) ); Two_Factor_Core::enable_provider_for_user( $user_id, 'Two_Factor_Totp' ); @@ -1326,7 +1280,8 @@ public function test_reenroll_after_reset_with_unavailable_key_fails_closed_with $this->assertFalse( $this->provider->set_user_totp_key( $user_id, $new_key ) ); // Once the key is usable again, the same enrollment succeeds. - Two_Factor_Secrets::$test_overrides = array(); + $this->secrets_store->on_get = null; + $this->secrets_store->on_set = null; $response = $this->rest_setup( $user_id, $new_key ); @@ -1342,10 +1297,9 @@ public function test_reenroll_after_reset_with_unavailable_key_fails_closed_with * @covers Two_Factor_Totp::rest_setup_totp */ public function test_rest_setup_reports_secrets_write_failure() { - $this->require_secrets_api(); $user_id = $this->admin_user(); - Two_Factor_Secrets::$test_overrides['set'] = function () { + $this->secrets_store->on_set = function () { return new WP_Error( 'secret_store_unavailable', 'sensitive detail' ); }; diff --git a/tests/secrets/class-two-factor-secrets-api-store-tests.php b/tests/secrets/class-two-factor-secrets-api-store-tests.php new file mode 100644 index 00000000..c0285f33 --- /dev/null +++ b/tests/secrets/class-two-factor-secrets-api-store-tests.php @@ -0,0 +1,106 @@ +store = new Two_Factor_Secrets_Api_Store(); + } + + /** + * Availability reflects whether the Secrets API is loaded. + * + * @covers Two_Factor_Secrets_Api_Store::is_available + */ + public function test_is_available_reflects_loaded_functions() { + $this->assertSame( function_exists( 'wp_get_network_secret' ) && class_exists( 'WP_Secret' ), $this->store->is_available() ); + } + + /** + * The default provider is writable and has a label. + * + * @covers Two_Factor_Secrets_Api_Store::is_writable + * @covers Two_Factor_Secrets_Api_Store::get_label + */ + public function test_default_provider_is_writable_and_labelled() { + $this->require_secrets_api(); + + $this->assertTrue( $this->store->is_writable() ); + $this->assertNotSame( '', $this->store->get_label() ); + } + + /** + * A secret round trips in network scope, as a revealed string. + * + * @covers Two_Factor_Secrets_Api_Store::set + * @covers Two_Factor_Secrets_Api_Store::get + * @covers Two_Factor_Secrets_Api_Store::delete + */ + public function test_round_trip_uses_network_scope() { + $this->require_secrets_api(); + $name = 'two-factor/totp-424242'; + + $this->assertNotWPError( wp_secrets_validate_name( $name ) ); + $this->assertTrue( $this->store->set( $name, 'ABCDEF' ) ); + $this->assertSame( 'ABCDEF', $this->store->get( $name ) ); + $this->assertNotNull( wp_get_network_secret( $name ) ); + $this->assertNull( wp_get_secret( $name ) ); + + $this->assertTrue( $this->store->delete( $name ) ); + $this->assertNull( $this->store->get( $name ) ); + } + + /** + * A missing secret reads as null and deletes without error, as the in-memory store does. + * + * @covers Two_Factor_Secrets_Api_Store::get + * @covers Two_Factor_Secrets_Api_Store::delete + */ + public function test_missing_secret_reads_null_and_deletes_cleanly() { + $this->require_secrets_api(); + + $this->assertNull( $this->store->get( 'two-factor/totp-434343' ) ); + $this->assertTrue( $this->store->delete( 'two-factor/totp-434343' ) ); + } + + /** + * The manager works end to end over the real store. + * + * @covers Two_Factor_Secrets_Manager::set_user_secret + * @covers Two_Factor_Secrets_Manager::get_user_secret + */ + public function test_manager_round_trip_over_the_real_store() { + $this->require_secrets_api(); + $manager = new Two_Factor_Secrets_Manager( $this->store ); + $user_id = self::factory()->user->create(); + + $this->assertTrue( $manager->set_user_secret( $user_id, 'totp', 'ABCDEF' ) ); + $this->assertSame( 'ABCDEF', $manager->get_user_secret( $user_id, 'totp' ) ); + $this->assertTrue( $manager->delete_user_secret( $user_id, 'totp' ) ); + $this->assertNull( wp_get_network_secret( "two-factor/totp-{$user_id}" ) ); + } +} diff --git a/tests/settings/class-tests-two-factor-settings.php b/tests/settings/class-tests-two-factor-settings.php index 47c12a7e..484947f8 100644 --- a/tests/settings/class-tests-two-factor-settings.php +++ b/tests/settings/class-tests-two-factor-settings.php @@ -83,7 +83,6 @@ public function test_section_explains_absent_api_without_a_checkbox() { * @covers Two_Factor_Settings::render_settings_page */ public function test_section_offers_opt_in_with_acknowledgement() { - $this->require_secrets_api(); $output = $this->render(); @@ -99,7 +98,6 @@ public function test_section_offers_opt_in_with_acknowledgement() { * @covers Two_Factor_Settings::render_settings_page */ public function test_opt_in_refused_without_acknowledgement() { - $this->require_secrets_api(); $output = $this->render( array( @@ -118,7 +116,6 @@ public function test_opt_in_refused_without_acknowledgement() { * @covers Two_Factor_Settings::render_settings_page */ public function test_opt_in_saved_with_acknowledgement() { - $this->require_secrets_api(); $output = $this->render( array( @@ -139,7 +136,6 @@ public function test_opt_in_saved_with_acknowledgement() { * @covers Two_Factor_Settings::render_settings_page */ public function test_opt_in_kept_then_turned_off() { - $this->require_secrets_api(); Two_Factor_Secrets::set_opted_in( true ); $this->render( @@ -174,7 +170,6 @@ public function test_save_without_storage_section_keeps_opt_in() { * @covers Two_Factor_Settings::render_settings_page */ public function test_site_admin_cannot_change_network_setting_on_multisite() { - $this->require_secrets_api(); if ( ! is_multisite() ) { $this->markTestSkipped( 'Only applies on multisite.' ); diff --git a/two-factor.php b/two-factor.php index f83d3410..af3d66cf 100644 --- a/two-factor.php +++ b/two-factor.php @@ -50,8 +50,11 @@ require_once TWO_FACTOR_DIR . 'class-two-factor-compat.php'; /** - * Storage adapter for the WordPress Secrets API. + * Storage of secrets with the WordPress Secrets API. */ +require_once TWO_FACTOR_DIR . 'secrets/interface-two-factor-secrets-store.php'; +require_once TWO_FACTOR_DIR . 'secrets/class-two-factor-secrets-api-store.php'; +require_once TWO_FACTOR_DIR . 'secrets/class-two-factor-secrets-manager.php'; require_once TWO_FACTOR_DIR . 'class-two-factor-secrets.php'; /** From ce48499515e25cbaf05627bd4f46971418383996 Mon Sep 17 00:00:00 2001 From: Eric Mann Date: Tue, 6 Oct 2026 12:18:27 -0700 Subject: [PATCH 2/3] Let providers declare their secrets and have core look after them All Secrets API handling for TOTP lived in Two_Factor_Totp, and the CLI called that class by name, so a custom provider got none of it. Providers now declare the user meta keys that hold secrets through the static Two_Factor_Provider::user_secret_meta_keys(). The storage logic (verified writes, lazy one-way migration, deletion, storage state) moves into Two_Factor_Secrets_Manager, generalised over a slug and a meta key. Two_Factor_Secrets_Lifecycle handles deletion with the user and on uninstall, the unreachable-secrets notice, Site Health and the counts for every declared secret, registered from Two_Factor_Core::add_hooks(). Two_Factor_Totp's public methods keep their signatures and become wrappers. Its slug, marker key and secret name are unchanged, so nothing already stored moves. Two_Factor_Totp::register_secret_lifecycle_hooks() is removed. Two_Factor_Core::get_provider_secrets() lists secrets from every provider class, including ones turned off site-wide, without instantiating any. A duplicate slug is refused. wp two-factor secrets migrate covers every declared secret, and secrets status prints one row per secret. --- CLI/class-two-factor-cli-command.php | 94 +-- class-two-factor-core.php | 104 ++++ class-two-factor-secrets.php | 139 +++++ providers/class-two-factor-provider.php | 32 +- providers/class-two-factor-totp.php | 552 +----------------- .../class-two-factor-secrets-lifecycle.php | 333 +++++++++++ secrets/class-two-factor-secrets-manager.php | 353 +++++++++++ tests/class-two-factor-dummy-secret.php | 37 ++ tests/class-two-factor-secrets-test-case.php | 2 +- tests/cli/class-two-factor-cli-command.php | 41 +- .../class-two-factor-totp-secrets-tests.php | 60 +- ...ass-two-factor-secrets-lifecycle-tests.php | 290 +++++++++ two-factor.php | 13 +- 13 files changed, 1426 insertions(+), 624 deletions(-) create mode 100644 secrets/class-two-factor-secrets-lifecycle.php create mode 100644 tests/class-two-factor-dummy-secret.php create mode 100644 tests/secrets/class-two-factor-secrets-lifecycle-tests.php diff --git a/CLI/class-two-factor-cli-command.php b/CLI/class-two-factor-cli-command.php index 82a18156..88611348 100644 --- a/CLI/class-two-factor-cli-command.php +++ b/CLI/class-two-factor-cli-command.php @@ -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', @@ -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' ) ); } @@ -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 * @@ -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; @@ -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'; } @@ -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( diff --git a/class-two-factor-core.php b/class-two-factor-core.php index 2f8fab74..43f84dfa 100644 --- a/class-two-factor-core.php +++ b/class-two-factor-core.php @@ -37,6 +37,15 @@ class Two_Factor_Core { */ const ENABLED_PROVIDERS_OPTION_KEY = 'two_factor_enabled_providers'; + /** + * Whether get_provider_secrets() is currently listing providers. + * + * @since 0.18.0 + * + * @var bool + */ + private static $listing_all_providers = false; + /** * The user meta nonce key. * @@ -120,6 +129,9 @@ public static function add_hooks( $compat ) { // Allow providers to register their hooks. add_action( 'init', array( __CLASS__, 'get_providers' ) ); // @phpstan-ignore return.void + // Look after the secrets providers declare, whether or not those providers are enabled. + Two_Factor_Secrets_Lifecycle::add_hooks(); + add_filter( 'wp_login_errors', array( __CLASS__, 'maybe_show_reset_password_notice' ) ); add_action( 'after_password_reset', array( __CLASS__, 'clear_password_reset_notice' ) ); add_action( 'login_form_validate_2fa', array( __CLASS__, 'login_form_validate_2fa' ) ); @@ -260,6 +272,13 @@ public static function uninstall() { } } + // Delete secrets held in the Secrets API while their markers still exist to find them by. + Two_Factor_Secrets_Lifecycle::delete_all_secrets(); + + foreach ( array_keys( self::get_provider_secrets() ) as $slug ) { + $user_meta_keys[] = Two_Factor_Secrets::get_marker_meta_key( $slug ); + } + // Delete options first since that is faster. foreach ( $option_keys as $option_key ) { delete_option( $option_key ); @@ -289,6 +308,91 @@ private static function get_default_providers() { ); } + /** + * Whether providers are being listed regardless of the site-wide enabled-providers setting. + * + * The filter that enforces that setting checks this and leaves the list alone. + * + * @since 0.18.0 + * + * @return bool + */ + public static function is_listing_all_providers() { + return self::$listing_all_providers; + } + + /** + * Get the secrets that providers keep for users. + * + * Built from every registered provider class, including ones the site has turned off + * in Settings, because a provider that was enabled once may still hold users' secrets. + * No provider is instantiated. + * + * @since 0.18.0 + * + * @see Two_Factor_Provider::user_secret_meta_keys() + * + * @return array Secret slug => the user meta key holding the plaintext value and the provider key that declared it. + */ + public static function get_provider_secrets() { + $providers = self::get_default_providers(); + + // The site-wide setting is enforced by removing providers in this filter, so ask it to stand aside. + self::$listing_all_providers = true; + + try { + /** This filter is documented in the get_providers() method */ + $additional_providers = apply_filters( 'two_factor_providers', $providers ); + } finally { + self::$listing_all_providers = false; + } + + if ( ! empty( $additional_providers ) && is_array( $additional_providers ) ) { + $providers = array_merge( $providers, $additional_providers ); + } + + $secrets = array(); + + foreach ( self::get_providers_classes( $providers ) as $provider_key => $provider_class ) { + if ( ! method_exists( $provider_class, 'user_secret_meta_keys' ) ) { + continue; + } + + foreach ( (array) call_user_func( array( $provider_class, 'user_secret_meta_keys' ) ) as $slug => $meta_key ) { + if ( ! is_string( $slug ) || ! is_string( $meta_key ) || '' === $meta_key ) { + continue; + } + + try { + Two_Factor_Secrets::get_marker_meta_key( $slug ); + } catch ( InvalidArgumentException $e ) { + continue; + } + + if ( isset( $secrets[ $slug ] ) ) { + _doing_it_wrong( + __METHOD__, + sprintf( + /* translators: 1: secret slug, 2: provider key that declared it first */ + esc_html__( 'The secret slug "%1$s" is already declared by %2$s. Secret slugs must be unique across providers.', 'two-factor' ), + esc_html( $slug ), + esc_html( $secrets[ $slug ]['provider'] ) + ), + '0.18.0' + ); + continue; + } + + $secrets[ $slug ] = array( + 'meta_key' => $meta_key, + 'provider' => (string) $provider_key, + ); + } + } + + return $secrets; + } + /** * Get the classnames for specific providers. * diff --git a/class-two-factor-secrets.php b/class-two-factor-secrets.php index 6c3d6d9f..195e6368 100644 --- a/class-two-factor-secrets.php +++ b/class-two-factor-secrets.php @@ -241,6 +241,145 @@ public static function delete_user_secret( int $user_id, string $slug ) { return self::instance()->delete_user_secret( $user_id, $slug ); } + /** + * Read a secret a provider keeps for a user, wherever it is stored. + * + * A plaintext value in the provider's own user meta wins, and is moved into the + * Secrets API on the way past when that is allowed. Otherwise the Secrets API is + * consulted when the user has a marker. + * + * @since 0.18.0 + * + * @param int $user_id User ID. + * @param string $slug Secret slug. + * @param string $meta_key User meta key that holds the plaintext value. + * @param bool $migrate Optional. Whether to migrate a plaintext value. Default true. + * @return string|null|WP_Error The value, null when the user has none, or a WP_Error when one exists but cannot be read. + */ + public static function get_stored_secret_state( int $user_id, string $slug, string $meta_key, bool $migrate = true ) { + return self::instance()->get_stored_secret_state( $user_id, $slug, $meta_key, $migrate ); + } + + /** + * Move a plaintext secret from user meta into the Secrets API. + * + * The plaintext is removed only after the stored secret has been read back and + * matches. Nothing ever moves a secret the other way. + * + * @since 0.18.0 + * + * @param int $user_id User ID. + * @param string $slug Secret slug. + * @param string $meta_key User meta key that holds the plaintext value. + * @return true|null|WP_Error True when migrated, null when there is nothing to migrate, WP_Error on failure. + */ + public static function migrate_stored_secret( int $user_id, string $slug, string $meta_key ) { + return self::instance()->migrate_stored_secret( $user_id, $slug, $meta_key ); + } + + /** + * Store a secret a provider keeps for a user. + * + * Writes to the Secrets API when that is allowed, verifying by read-back, and to + * the provider's user meta otherwise. A Secrets API failure never results in a + * plaintext write. The value must be a single non-empty string. + * + * @since 0.18.0 + * + * @param int $user_id User ID. + * @param string $slug Secret slug. + * @param string $meta_key User meta key that holds the plaintext value. + * @param string $value Secret value. + * @return int|bool|WP_Error True, or what update_user_meta() returned for a plaintext write, or a WP_Error when the Secrets API write or its read-back failed. + */ + public static function save_stored_secret( int $user_id, string $slug, string $meta_key, string $value ) { + return self::instance()->save_stored_secret( $user_id, $slug, $meta_key, $value ); + } + + /** + * Delete a secret a provider keeps for a user, from user meta and the Secrets API. + * + * @since 0.18.0 + * + * @param int $user_id User ID. + * @param string $slug Secret slug. + * @param string $meta_key User meta key that holds the plaintext value. + * @return bool Whether nothing of the secret is left behind. + */ + public static function delete_stored_secret( int $user_id, string $slug, string $meta_key ): bool { + return self::instance()->delete_stored_secret( $user_id, $slug, $meta_key ); + } + + /** + * Get where a secret a provider keeps for a user is stored. Never migrates. + * + * @since 0.18.0 + * + * @param int $user_id User ID. + * @param string $slug Secret slug. + * @param string $meta_key User meta key that holds the plaintext value. + * @return string One of 'plaintext', 'secrets-api', 'unavailable' or 'none'. + */ + public static function get_stored_secret_storage( int $user_id, string $slug, string $meta_key ): string { + return self::instance()->get_stored_secret_storage( $user_id, $slug, $meta_key ); + } + + /** + * Whether a user has a secret that can be reached from here. Does not decrypt anything. + * + * @since 0.18.0 + * + * @param int $user_id User ID. + * @param string $slug Secret slug. + * @param string $meta_key User meta key that holds the plaintext value. + * @return bool + */ + public static function is_stored_secret_reachable( int $user_id, string $slug, string $meta_key ): bool { + return self::instance()->is_stored_secret_reachable( $user_id, $slug, $meta_key ); + } + + /** + * Whether a user has a secret in the Secrets API that cannot be reached from here. + * + * True when the Secrets API is missing or the secret belongs to another network. + * Does not decrypt anything. + * + * @since 0.18.0 + * + * @param int $user_id User ID. + * @param string $slug Secret slug. + * @param string $meta_key User meta key that holds the plaintext value. + * @return bool + */ + public static function is_stored_secret_unreachable( int $user_id, string $slug, string $meta_key ): bool { + return self::instance()->is_stored_secret_unreachable( $user_id, $slug, $meta_key ); + } + + /** + * Whether any user has a secret in the Secrets API that this site cannot currently reach. + * + * The result is cached briefly in a site transient. + * + * @since 0.18.0 + * + * @param string[] $slugs Secret slugs to look for. + * @return bool + */ + public static function has_affected_users( array $slugs ): bool { + return self::instance()->has_affected_users( $slugs ); + } + + /** + * Forget the cached unreachable-secrets result. + * + * @since 0.18.0 + * + * @return void + */ + public static function clear_affected_users_cache(): void { + self::instance()->clear_affected_users_cache(); + } + /** * Whether an error means the Secrets API cannot use its encryption key. * diff --git a/providers/class-two-factor-provider.php b/providers/class-two-factor-provider.php index 70627001..e5b91570 100644 --- a/providers/class-two-factor-provider.php +++ b/providers/class-two-factor-provider.php @@ -224,10 +224,40 @@ public static function uninstall_options() { * @param WP_User $user WP_User object of the user. * @return bool */ - public function is_enrolled_but_unavailable_for_user( $user ) { // phpcs:ignore Generic.CodeAnalysis.UnusedFunctionParameter.Found -- Base implementation keeps the provider interface signature but does not use the user. + public function is_enrolled_but_unavailable_for_user( $user ) { + // A secret the provider declared that sits in the Secrets API out of reach is exactly this case. + foreach ( static::user_secret_meta_keys() as $slug => $meta_key ) { + if ( Two_Factor_Secrets::is_stored_secret_unreachable( (int) $user->ID, (string) $slug, (string) $meta_key ) ) { + return true; + } + } + return false; } + /** + * Declare the user meta keys in which this provider keeps secrets. + * + * A secret is a value the provider has to read back in full, such as a TOTP key. Hashed + * values are not secrets in this sense. For each one declared, core stores the value with + * the WordPress Secrets API once an administrator has turned that on, migrates existing + * plaintext values, deletes the secret with the user and on uninstall, and reports on it in + * Site Health and WP-CLI. The provider reads and writes the value through + * Two_Factor_Secrets::get_stored_secret_state(), save_stored_secret() and + * delete_stored_secret() rather than user meta directly. + * + * Each value must be a single string. The slug is part of the stored secret's name, so it + * has to be unique across providers and must not change once secrets exist: lowercase + * letters, digits, hyphens and underscores. + * + * @since 0.18.0 + * + * @return array Secret slug => user meta key holding the plaintext value. + */ + public static function user_secret_meta_keys() { + return array(); + } + /** * Delete per-user data held outside user meta. * diff --git a/providers/class-two-factor-totp.php b/providers/class-two-factor-totp.php index ec5e4460..8604de24 100644 --- a/providers/class-two-factor-totp.php +++ b/providers/class-two-factor-totp.php @@ -37,24 +37,6 @@ class Two_Factor_Totp extends Two_Factor_Provider { */ const SECRET_SLUG = 'totp'; - /** - * The site transient caching whether any user has a secret that is currently unreachable. - * - * @since 0.18.0 - * - * @var string - */ - const AFFECTED_USERS_TRANSIENT = 'two_factor_totp_affected_users'; - - /** - * How long, in seconds, the affected-users result is cached. - * - * @since 0.18.0 - * - * @var int - */ - const AFFECTED_USERS_CACHE_TTL = 300; - /** * The user meta key for the last successful TOTP token timestamp logged in with. * @@ -91,32 +73,6 @@ protected function __construct() { parent::__construct(); } - /** - * Register the hooks that manage stored secrets for the lifetime of a user. - * - * These run whether or not TOTP is enabled on the site: a site that turns TOTP off - * after users enrolled still holds their secrets, and must still delete them with - * the user, warn about unreachable ones and report their storage in Site Health. - * They are static so that registering them does not instantiate the provider, - * which would expose its REST routes while it is disabled. - * - * @since 0.18.0 - * - * @return void - */ - public static function register_secret_lifecycle_hooks() { - add_filter( 'site_status_tests', array( __CLASS__, 'register_site_health_test' ) ); - add_action( 'admin_notices', array( __CLASS__, 'admin_notice_secrets_api_missing' ) ); - add_action( 'network_admin_notices', array( __CLASS__, 'admin_notice_secrets_api_missing' ) ); - - // On multisite, `delete_user` also fires when a user is only removed from one site, so wait for the network-level deletion. - if ( is_multisite() ) { - add_action( 'wpmu_delete_user', array( __CLASS__, 'delete_user_secrets_on_user_deletion' ) ); - } else { - add_action( 'delete_user', array( __CLASS__, 'delete_user_secrets_on_user_deletion' ) ); - } - } - /** * Timestamp returned by time() * @@ -602,21 +558,7 @@ public function get_user_totp_key( $user_id ) { * @return string|null|WP_Error The key, null when the user has none, or a WP_Error when a key exists but cannot be read. */ public function get_user_totp_key_state( $user_id, $migrate = true ) { - $plaintext = (string) get_user_meta( $user_id, self::SECRET_META_KEY, true ); - - if ( '' !== $plaintext ) { - if ( $migrate && Two_Factor_Secrets::can_write( (int) $user_id ) ) { - $this->migrate_user_totp_key( $user_id ); - } - - return $plaintext; - } - - if ( '' === (string) get_user_meta( $user_id, self::SECRET_NETWORK_META_KEY, true ) ) { - return null; - } - - return Two_Factor_Secrets::get_user_secret( (int) $user_id, self::SECRET_SLUG ); + return Two_Factor_Secrets::get_stored_secret_state( (int) $user_id, self::SECRET_SLUG, self::SECRET_META_KEY, (bool) $migrate ); } /** @@ -631,82 +573,7 @@ public function get_user_totp_key_state( $user_id, $migrate = true ) { * @return true|null|WP_Error True when migrated, null when there is nothing to migrate, WP_Error on failure. */ public function migrate_user_totp_key( $user_id ) { - $plaintext = (string) get_user_meta( $user_id, self::SECRET_META_KEY, true ); - - if ( '' === $plaintext ) { - return null; - } - - if ( ! Two_Factor_Secrets::can_write( (int) $user_id ) ) { - return new WP_Error( - 'two_factor_secrets_not_writable', - __( 'The Secrets API cannot be written to, so the secret was not migrated.', 'two-factor' ) - ); - } - - $result = Two_Factor_Secrets::set_user_secret( (int) $user_id, self::SECRET_SLUG, $plaintext ); - - if ( is_wp_error( $result ) ) { - $this->fire_migration_failed( $user_id, $result ); - return $result; - } - - $readback = Two_Factor_Secrets::get_user_secret( (int) $user_id, self::SECRET_SLUG ); - - if ( ! is_string( $readback ) || ! hash_equals( $plaintext, $readback ) ) { - $error = is_wp_error( $readback ) - ? $readback - : new WP_Error( - 'two_factor_secrets_migration_mismatch', - __( 'The stored secret did not match the original, so the migration was rolled back.', 'two-factor' ) - ); - - Two_Factor_Secrets::delete_user_secret( (int) $user_id, self::SECRET_SLUG ); - $this->fire_migration_failed( $user_id, $error ); - - return $error; - } - - delete_user_meta( $user_id, self::SECRET_META_KEY ); - Two_Factor_Secrets::memzero( $readback ); - self::clear_affected_users_cache(); - - /** - * Fires after a user's TOTP secret was moved into the Secrets API. - * - * @since 0.18.0 - * - * @param int $user_id User ID. - * @param string $slug Secret slug, "totp". - */ - do_action( 'two_factor_secrets_migrated', $user_id, self::SECRET_SLUG ); - - return true; - } - - /** - * Fire the migration failure action. - * - * @since 0.18.0 - * - * @param int $user_id User ID. - * @param WP_Error $error The failure. - * - * @return void - */ - private function fire_migration_failed( $user_id, $error ) { - /** - * Fires when moving a user's TOTP secret into the Secrets API failed. - * - * The plaintext copy is kept, so the user can still log in. The error never contains the secret. - * - * @since 0.18.0 - * - * @param int $user_id User ID. - * @param string $slug Secret slug, "totp". - * @param WP_Error $error The failure. - */ - do_action( 'two_factor_secrets_migration_failed', $user_id, self::SECRET_SLUG, $error ); + return Two_Factor_Secrets::migrate_stored_secret( (int) $user_id, self::SECRET_SLUG, self::SECRET_META_KEY ); } /** @@ -746,40 +613,7 @@ private function save_user_totp_key( $user_id, $key ) { return $this->delete_user_totp_key( $user_id ); } - if ( Two_Factor_Secrets::can_write( (int) $user_id ) ) { - $result = Two_Factor_Secrets::set_user_secret( (int) $user_id, self::SECRET_SLUG, (string) $key ); - - if ( is_wp_error( $result ) ) { - return $result; - } - - $readback = Two_Factor_Secrets::get_user_secret( (int) $user_id, self::SECRET_SLUG ); - - if ( ! is_string( $readback ) || ! hash_equals( (string) $key, $readback ) ) { - Two_Factor_Secrets::delete_user_secret( (int) $user_id, self::SECRET_SLUG ); - - return is_wp_error( $readback ) - ? $readback - : new WP_Error( - 'two_factor_secrets_write_mismatch', - __( 'The stored secret did not match the original, so it was removed.', 'two-factor' ) - ); - } - - Two_Factor_Secrets::memzero( $readback ); - delete_user_meta( $user_id, self::SECRET_META_KEY ); - self::clear_affected_users_cache(); - - return true; - } - - $result = update_user_meta( $user_id, self::SECRET_META_KEY, $key ); - - // Clear any stale marker or secret so the plaintext value is unambiguous. - Two_Factor_Secrets::delete_user_secret( (int) $user_id, self::SECRET_SLUG ); - self::clear_affected_users_cache(); - - return $result; + return Two_Factor_Secrets::save_stored_secret( (int) $user_id, self::SECRET_SLUG, self::SECRET_META_KEY, (string) $key ); } /** @@ -793,295 +627,8 @@ private function save_user_totp_key( $user_id, $key ) { */ public function delete_user_totp_key( $user_id ) { delete_user_meta( $user_id, self::LAST_SUCCESSFUL_LOGIN_META_KEY ); - delete_user_meta( $user_id, self::SECRET_META_KEY ); - - $secret = Two_Factor_Secrets::delete_user_secret( (int) $user_id, self::SECRET_SLUG ); - self::clear_affected_users_cache(); - - return true === $secret - && '' === (string) get_user_meta( $user_id, self::SECRET_META_KEY, true ) - && '' === (string) get_user_meta( $user_id, self::SECRET_NETWORK_META_KEY, true ); - } - - /** - * Delete a user's TOTP data when the user account is deleted. - * - * Wrapper around delete_user_totp_key() so the deletion actions get a callback that returns nothing. - * - * @since 0.18.0 - * - * @param int $user_id ID of the user being deleted. - * - * @return void - */ - public static function delete_user_secrets_on_user_deletion( $user_id ) { - self::get_instance()->delete_user_totp_key( $user_id ); - } - - /** - * Whether any user has a secret in the Secrets API that this site cannot currently reach. - * - * The result is cached briefly in a site transient. - * - * @since 0.18.0 - * - * @return bool - */ - public static function has_affected_users() { - $cached = get_site_transient( self::AFFECTED_USERS_TRANSIENT ); - - if ( 'yes' === $cached || 'no' === $cached ) { - return 'yes' === $cached; - } - - $args = array( - 'blog_id' => 0, - 'meta_key' => self::SECRET_NETWORK_META_KEY, // phpcs:ignore WordPress.DB.SlowDBQuery.slow_db_query_meta_key -- Single-row lookup, result cached. - 'meta_compare' => 'EXISTS', - 'number' => 1, - 'fields' => 'ID', - 'count_total' => false, - ); - - if ( Two_Factor_Secrets::is_api_present() ) { - if ( is_multisite() ) { - $args['meta_value'] = (string) get_current_network_id(); // phpcs:ignore WordPress.DB.SlowDBQuery.slow_db_query_meta_value -- Single-row lookup, result cached. - $args['meta_compare'] = '!='; - $affected = ! empty( get_users( $args ) ); - } else { - $affected = false; - } - } else { - $affected = ! empty( get_users( $args ) ); - } - - set_site_transient( self::AFFECTED_USERS_TRANSIENT, $affected ? 'yes' : 'no', self::AFFECTED_USERS_CACHE_TTL ); - - return $affected; - } - - /** - * Forget the cached affected-users result. - * - * @since 0.18.0 - * - * @return void - */ - public static function clear_affected_users_cache() { - delete_site_transient( self::AFFECTED_USERS_TRANSIENT ); - } - - /** - * Warn administrators that some users' authenticator secrets are unreachable. - * - * @since 0.18.0 - * - * @return void - */ - public static function admin_notice_secrets_api_missing() { - $capability = is_network_admin() ? 'manage_network_options' : 'manage_options'; - - if ( ! current_user_can( $capability ) || ! self::has_affected_users() ) { - return; - } - - wp_admin_notice( - esc_html__( 'Authenticator app secrets for one or more users were stored with the WordPress Secrets API, which is no longer available on this site. Those users cannot use their authenticator app until it is restored. Re-activate the Secrets API, or reset those users\' authenticator app so they can set it up again.', 'two-factor' ), - array( - 'type' => 'error', - 'dismissible' => false, - ) - ); - } - - /** - * Register the Site Health test for TOTP secret storage. - * - * @since 0.18.0 - * - * @param array $tests Site Health tests. - * - * @return array - */ - public static function register_site_health_test( $tests ) { - $tests['direct']['two_factor_totp_secret_storage'] = array( - 'label' => __( 'Authenticator app secret storage', 'two-factor' ), - 'test' => array( __CLASS__, 'site_health_secret_storage' ), - ); - - return $tests; - } - - /** - * Whether any user still has a plaintext TOTP secret in user meta. - * - * Not cached. - * - * @since 0.18.0 - * - * @return bool - */ - public static function has_plaintext_users() { - $users = get_users( - array( - 'blog_id' => 0, - 'meta_key' => self::SECRET_META_KEY, // phpcs:ignore WordPress.DB.SlowDBQuery.slow_db_query_meta_key -- Single-row lookup for Site Health. - 'meta_value' => '', // phpcs:ignore WordPress.DB.SlowDBQuery.slow_db_query_meta_value -- Single-row lookup for Site Health. - 'meta_compare' => '!=', - 'number' => 1, - 'fields' => 'ID', - 'count_total' => false, - ) - ); - - return ! empty( $users ); - } - - /** - * Site Health test result for TOTP secret storage. - * - * @since 0.18.0 - * - * @return array - */ - public static function site_health_secret_storage() { - $result = array( - 'label' => '', - 'status' => 'good', - 'badge' => array( - 'label' => __( 'Security', 'two-factor' ), - 'color' => 'blue', - ), - 'description' => '', - 'actions' => '', - 'test' => 'two_factor_totp_secret_storage', - ); - - if ( self::has_affected_users() ) { - $result['label'] = __( 'Some authenticator app secrets are unavailable', 'two-factor' ); - $result['status'] = 'critical'; - $result['badge']['color'] = 'red'; - $result['description'] = sprintf( - '

%s

', - esc_html__( 'Some users\' authenticator app secrets were stored with the WordPress Secrets API, which this site cannot currently reach. Those users cannot use their authenticator app. Re-activate the Secrets API, or reset those users\' authenticator app so they can set it up again. Secrets are never moved back out of the Secrets API.', 'two-factor' ) - ); - - return $result; - } - - if ( ! Two_Factor_Secrets::is_api_present() ) { - $result['label'] = __( 'Authenticator app secrets are stored in user meta', 'two-factor' ); - $result['description'] = sprintf( - '

%s

', - esc_html__( 'TOTP secrets are stored in user meta. When the WordPress Secrets API is available, an administrator can choose to store them encrypted instead.', 'two-factor' ) - ); - - return $result; - } - - if ( ! Two_Factor_Secrets::is_enabled() ) { - // Opted in on the settings screen, but a filter turned storage back off: a deliberate choice, not a to-do. - if ( Two_Factor_Secrets::is_opted_in() ) { - $result['label'] = __( 'Authenticator app secrets remain in user meta', 'two-factor' ); - $result['description'] = sprintf( - '

%s

', - esc_html__( 'Storing authenticator app secrets with the WordPress Secrets API is disabled by the two_factor_use_secrets_api filter, so new secrets are stored in user meta.', 'two-factor' ) - ); - - return $result; - } - - $result['label'] = __( 'Authenticator app secrets can be stored encrypted', 'two-factor' ); - $result['status'] = 'recommended'; - $result['badge']['color'] = 'orange'; - $result['description'] = sprintf( - '

%s

', - esc_html__( 'The WordPress Secrets API is available, but authenticator app secrets are still stored in user meta, where anyone with a copy of the database can read them. Turning on encrypted storage is a one-way change that depends on this site\'s secrets key, so an administrator has to turn it on.', 'two-factor' ) - ); - $result['actions'] = sprintf( - '

%s

', - esc_url( admin_url( 'options-general.php?page=two-factor-settings' ) ), - esc_html__( 'Review Two-Factor settings', 'two-factor' ) - ); - - return $result; - } - - if ( ! Two_Factor_Secrets::is_provider_writable() ) { - $result['label'] = __( 'Authenticator app secrets remain in user meta', 'two-factor' ); - $result['description'] = sprintf( - '

%s

', - esc_html__( 'The active secrets provider is read-only, so new authenticator app secrets are stored in user meta and existing ones are not migrated.', 'two-factor' ) - ); - - return $result; - } - if ( ! self::has_plaintext_users() ) { - $result['label'] = __( 'Authenticator app secrets are stored securely', 'two-factor' ); - $result['description'] = sprintf( - '

%s

', - sprintf( - /* translators: %s: name of the secrets storage provider. */ - esc_html__( 'Authenticator app secrets are stored with the WordPress Secrets API (%s).', 'two-factor' ), - esc_html( Two_Factor_Secrets::provider_label() ) - ) - ); - - return $result; - } - - $result['label'] = __( 'Some authenticator app secrets are not yet migrated', 'two-factor' ); - $result['status'] = 'recommended'; - $result['badge']['color'] = 'orange'; - $result['description'] = sprintf( - '

%s

', - sprintf( - /* translators: %s: WP-CLI migrate command. */ - esc_html__( 'Some authenticator app secrets are still stored in user meta. They move to the WordPress Secrets API when those users next log in, or you can run %s to migrate them all now.', 'two-factor' ), - 'wp two-factor secrets migrate' - ) - ); - - return $result; - } - - /** - * Count users by where their TOTP key is stored. - * - * @since 0.18.0 - * - * @return array{plaintext: int, migrated: int, affected: int} - */ - public static function count_users_by_storage() { - $count = function ( $meta_key, $compare, $value = null ) { - $args = array( - 'blog_id' => 0, - 'fields' => 'ID', - 'number' => 1, - 'count_total' => true, - 'meta_key' => $meta_key, // phpcs:ignore WordPress.DB.SlowDBQuery.slow_db_query_meta_key -- CLI reporting. - 'meta_compare' => $compare, - ); - - if ( null !== $value ) { - $args['meta_value'] = $value; // phpcs:ignore WordPress.DB.SlowDBQuery.slow_db_query_meta_value -- CLI reporting. - } - - $query = new WP_User_Query( $args ); - - return (int) $query->get_total(); - }; - - $affected = Two_Factor_Secrets::is_api_present() - ? $count( self::SECRET_NETWORK_META_KEY, '!=', (string) get_current_network_id() ) - : $count( self::SECRET_NETWORK_META_KEY, 'EXISTS' ); - - return array( - 'plaintext' => $count( self::SECRET_META_KEY, '!=', '' ), - 'migrated' => $count( self::SECRET_NETWORK_META_KEY, 'EXISTS' ), - 'affected' => $affected, - ); + return Two_Factor_Secrets::delete_stored_secret( (int) $user_id, self::SECRET_SLUG, self::SECRET_META_KEY ); } /** @@ -1096,22 +643,7 @@ public static function count_users_by_storage() { * @return string One of 'plaintext', 'secrets-api', 'unavailable' or 'none'. */ public function get_user_totp_key_storage( $user_id ) { - if ( '' !== (string) get_user_meta( $user_id, self::SECRET_META_KEY, true ) ) { - return 'plaintext'; - } - - if ( '' === (string) get_user_meta( $user_id, self::SECRET_NETWORK_META_KEY, true ) ) { - return 'none'; - } - - $secret = Two_Factor_Secrets::get_user_secret( (int) $user_id, self::SECRET_SLUG ); - - if ( is_string( $secret ) ) { - Two_Factor_Secrets::memzero( $secret ); - return 'secrets-api'; - } - - return 'unavailable'; + return Two_Factor_Secrets::get_stored_secret_storage( (int) $user_id, self::SECRET_SLUG, self::SECRET_META_KEY ); } /** @@ -1394,41 +926,7 @@ public static function calc_totp( $key, $step_count = false, $digits = self::DEF */ public function is_available_for_user( $user ) { // Available if a plaintext key is saved, or the key lives in a Secrets API network we can reach. - if ( '' !== (string) get_user_meta( $user->ID, self::SECRET_META_KEY, true ) ) { - return true; - } - - $marker = (string) get_user_meta( $user->ID, self::SECRET_NETWORK_META_KEY, true ); - - return '' !== $marker - && Two_Factor_Secrets::is_api_present() - && get_current_network_id() === (int) $marker; - } - - /** - * Whether the user enrolled this provider but the stored key cannot currently be used. - * - * True when the key lives in the Secrets API and the API is missing, or the key belongs to a - * different network. Does not decrypt anything. - * - * @since 0.18.0 - * - * @param WP_User $user WP_User object of the user. - * - * @return boolean - */ - public function is_enrolled_but_unavailable_for_user( $user ) { - if ( '' !== (string) get_user_meta( $user->ID, self::SECRET_META_KEY, true ) ) { - return false; - } - - $marker = (string) get_user_meta( $user->ID, self::SECRET_NETWORK_META_KEY, true ); - - if ( '' === $marker ) { - return false; - } - - return ! Two_Factor_Secrets::is_api_present() || get_current_network_id() !== (int) $marker; + return Two_Factor_Secrets::is_stored_secret_reachable( (int) $user->ID, self::SECRET_SLUG, self::SECRET_META_KEY ); } /** @@ -1603,44 +1101,14 @@ private static function abssort( $a, $b ) { } /** - * Delete every user's TOTP secret held in the Secrets API during plugin uninstall. - * - * When the Secrets API is absent at uninstall time the secrets are orphaned in the store, which - * is acceptable because they cannot be reached from here. + * Declare the TOTP key as a secret, so core stores it with the Secrets API when it can. * * @since 0.18.0 * - * @return void + * @return array Secret slug => user meta key holding the plaintext value. */ - public static function uninstall_user_data() { - if ( ! Two_Factor_Secrets::is_api_present() ) { - return; - } - - $seen = array(); - - do { - // The marker is removed with each secret, so the same offset always yields the next page. - $user_ids = ( new WP_User_Query( - array( - 'blog_id' => 0, - 'fields' => 'ID', - 'number' => 100, - 'offset' => 0, - 'orderby' => 'ID', - 'meta_key' => self::SECRET_NETWORK_META_KEY, // phpcs:ignore WordPress.DB.SlowDBQuery.slow_db_query_meta_key -- Uninstall only. - 'meta_compare' => 'EXISTS', - 'count_total' => false, - ) - ) )->get_results(); - - $new_ids = array_diff( array_map( 'intval', $user_ids ), $seen ); - - foreach ( $new_ids as $user_id ) { - $seen[] = $user_id; - Two_Factor_Secrets::delete_user_secret( (int) $user_id, self::SECRET_SLUG ); - } - } while ( ! empty( $new_ids ) ); + public static function user_secret_meta_keys() { + return array( self::SECRET_SLUG => self::SECRET_META_KEY ); } /** diff --git a/secrets/class-two-factor-secrets-lifecycle.php b/secrets/class-two-factor-secrets-lifecycle.php new file mode 100644 index 00000000..9d183d75 --- /dev/null +++ b/secrets/class-two-factor-secrets-lifecycle.php @@ -0,0 +1,333 @@ + $secret ) { + Two_Factor_Secrets::delete_stored_secret( (int) $user_id, $slug, $secret['meta_key'] ); + } + } + + /** + * Delete every user's secrets from the Secrets API, for plugin uninstall. + * + * When the Secrets API is absent the secrets are left behind in the store, which is + * acceptable because they cannot be reached from here. + * + * @since 0.18.0 + * + * @return void + */ + public static function delete_all_secrets() { + if ( ! Two_Factor_Secrets::is_api_present() ) { + return; + } + + foreach ( array_keys( Two_Factor_Core::get_provider_secrets() ) as $slug ) { + $seen = array(); + + do { + // The marker is removed with each secret, so the same offset always yields the next page. + $user_ids = ( new WP_User_Query( + array( + 'blog_id' => 0, + 'fields' => 'ID', + 'number' => 100, + 'offset' => 0, + 'orderby' => 'ID', + 'meta_key' => Two_Factor_Secrets::get_marker_meta_key( $slug ), // phpcs:ignore WordPress.DB.SlowDBQuery.slow_db_query_meta_key -- Uninstall only. + 'meta_compare' => 'EXISTS', + 'count_total' => false, + ) + ) )->get_results(); + + $new_ids = array_diff( array_map( 'intval', $user_ids ), $seen ); + + foreach ( $new_ids as $user_id ) { + $seen[] = $user_id; + Two_Factor_Secrets::delete_user_secret( (int) $user_id, $slug ); + } + } while ( ! empty( $new_ids ) ); + } + + Two_Factor_Secrets::clear_affected_users_cache(); + } + + /** + * Whether any user has a secret in the Secrets API that this site cannot currently reach. + * + * @since 0.18.0 + * + * @return bool + */ + public static function has_affected_users() { + return Two_Factor_Secrets::has_affected_users( array_keys( Two_Factor_Core::get_provider_secrets() ) ); + } + + /** + * Warn administrators that some users' authenticator secrets are unreachable. + * + * @since 0.18.0 + * + * @return void + */ + public static function admin_notice_secrets_api_missing() { + $capability = is_network_admin() ? 'manage_network_options' : 'manage_options'; + + if ( ! current_user_can( $capability ) || ! self::has_affected_users() ) { + return; + } + + wp_admin_notice( + esc_html__( 'Authenticator app secrets for one or more users were stored with the WordPress Secrets API, which is no longer available on this site. Those users cannot use their authenticator app until it is restored. Re-activate the Secrets API, or reset those users\' authenticator app so they can set it up again.', 'two-factor' ), + array( + 'type' => 'error', + 'dismissible' => false, + ) + ); + } + + /** + * Register the Site Health test for secret storage. + * + * @since 0.18.0 + * + * @param array $tests Site Health tests. + * + * @return array + */ + public static function register_site_health_test( $tests ) { + $tests['direct']['two_factor_secret_storage'] = array( + 'label' => __( 'Authenticator app secret storage', 'two-factor' ), + 'test' => array( __CLASS__, 'site_health_secret_storage' ), + ); + + return $tests; + } + + /** + * Whether any user still has a plaintext secret in user meta. + * + * Not cached. + * + * @since 0.18.0 + * + * @return bool + */ + public static function has_plaintext_users() { + foreach ( Two_Factor_Core::get_provider_secrets() as $secret ) { + $users = get_users( + array( + 'blog_id' => 0, + 'meta_key' => $secret['meta_key'], // phpcs:ignore WordPress.DB.SlowDBQuery.slow_db_query_meta_key -- Single-row lookup for Site Health. + 'meta_value' => '', // phpcs:ignore WordPress.DB.SlowDBQuery.slow_db_query_meta_value -- Single-row lookup for Site Health. + 'meta_compare' => '!=', + 'number' => 1, + 'fields' => 'ID', + 'count_total' => false, + ) + ); + + if ( ! empty( $users ) ) { + return true; + } + } + + return false; + } + + /** + * Site Health test result for secret storage. + * + * @since 0.18.0 + * + * @return array + */ + public static function site_health_secret_storage() { + $result = array( + 'label' => '', + 'status' => 'good', + 'badge' => array( + 'label' => __( 'Security', 'two-factor' ), + 'color' => 'blue', + ), + 'description' => '', + 'actions' => '', + 'test' => 'two_factor_secret_storage', + ); + + if ( self::has_affected_users() ) { + $result['label'] = __( 'Some authenticator app secrets are unavailable', 'two-factor' ); + $result['status'] = 'critical'; + $result['badge']['color'] = 'red'; + $result['description'] = sprintf( + '

%s

', + esc_html__( 'Some users\' authenticator app secrets were stored with the WordPress Secrets API, which this site cannot currently reach. Those users cannot use their authenticator app. Re-activate the Secrets API, or reset those users\' authenticator app so they can set it up again. Secrets are never moved back out of the Secrets API.', 'two-factor' ) + ); + + return $result; + } + + if ( ! Two_Factor_Secrets::is_api_present() ) { + $result['label'] = __( 'Authenticator app secrets are stored in user meta', 'two-factor' ); + $result['description'] = sprintf( + '

%s

', + esc_html__( 'TOTP secrets are stored in user meta. When the WordPress Secrets API is available, an administrator can choose to store them encrypted instead.', 'two-factor' ) + ); + + return $result; + } + + if ( ! Two_Factor_Secrets::is_enabled() ) { + // Opted in on the settings screen, but a filter turned storage back off: a deliberate choice, not a to-do. + if ( Two_Factor_Secrets::is_opted_in() ) { + $result['label'] = __( 'Authenticator app secrets remain in user meta', 'two-factor' ); + $result['description'] = sprintf( + '

%s

', + esc_html__( 'Storing authenticator app secrets with the WordPress Secrets API is disabled by the two_factor_use_secrets_api filter, so new secrets are stored in user meta.', 'two-factor' ) + ); + + return $result; + } + + $result['label'] = __( 'Authenticator app secrets can be stored encrypted', 'two-factor' ); + $result['status'] = 'recommended'; + $result['badge']['color'] = 'orange'; + $result['description'] = sprintf( + '

%s

', + esc_html__( 'The WordPress Secrets API is available, but authenticator app secrets are still stored in user meta, where anyone with a copy of the database can read them. Turning on encrypted storage is a one-way change that depends on this site\'s secrets key, so an administrator has to turn it on.', 'two-factor' ) + ); + $result['actions'] = sprintf( + '

%s

', + esc_url( admin_url( 'options-general.php?page=two-factor-settings' ) ), + esc_html__( 'Review Two-Factor settings', 'two-factor' ) + ); + + return $result; + } + + if ( ! Two_Factor_Secrets::is_provider_writable() ) { + $result['label'] = __( 'Authenticator app secrets remain in user meta', 'two-factor' ); + $result['description'] = sprintf( + '

%s

', + esc_html__( 'The active secrets provider is read-only, so new authenticator app secrets are stored in user meta and existing ones are not migrated.', 'two-factor' ) + ); + + return $result; + } + + if ( ! self::has_plaintext_users() ) { + $result['label'] = __( 'Authenticator app secrets are stored securely', 'two-factor' ); + $result['description'] = sprintf( + '

%s

', + sprintf( + /* translators: %s: name of the secrets storage provider. */ + esc_html__( 'Authenticator app secrets are stored with the WordPress Secrets API (%s).', 'two-factor' ), + esc_html( Two_Factor_Secrets::provider_label() ) + ) + ); + + return $result; + } + + $result['label'] = __( 'Some authenticator app secrets are not yet migrated', 'two-factor' ); + $result['status'] = 'recommended'; + $result['badge']['color'] = 'orange'; + $result['description'] = sprintf( + '

%s

', + sprintf( + /* translators: %s: WP-CLI migrate command. */ + esc_html__( 'Some authenticator app secrets are still stored in user meta. They move to the WordPress Secrets API when those users next log in, or you can run %s to migrate them all now.', 'two-factor' ), + 'wp two-factor secrets migrate' + ) + ); + + return $result; + } + + /** + * Count users by where one secret is stored. + * + * @since 0.18.0 + * + * @param string $slug Secret slug. + * @param string $meta_key User meta key that holds the plaintext value. + * @return array{plaintext: int, migrated: int, affected: int} + */ + public static function count_users_by_storage( $slug, $meta_key ) { + $marker_key = Two_Factor_Secrets::get_marker_meta_key( $slug ); + + $count = function ( $key, $compare, $value = null ) { + $args = array( + 'blog_id' => 0, + 'fields' => 'ID', + 'number' => 1, + 'count_total' => true, + 'meta_key' => $key, // phpcs:ignore WordPress.DB.SlowDBQuery.slow_db_query_meta_key -- CLI reporting. + 'meta_compare' => $compare, + ); + + if ( null !== $value ) { + $args['meta_value'] = $value; // phpcs:ignore WordPress.DB.SlowDBQuery.slow_db_query_meta_value -- CLI reporting. + } + + $query = new WP_User_Query( $args ); + + return (int) $query->get_total(); + }; + + $affected = Two_Factor_Secrets::is_api_present() + ? $count( $marker_key, '!=', (string) get_current_network_id() ) + : $count( $marker_key, 'EXISTS' ); + + return array( + 'plaintext' => $count( $meta_key, '!=', '' ), + 'migrated' => $count( $marker_key, 'EXISTS' ), + 'affected' => $affected, + ); + } +} diff --git a/secrets/class-two-factor-secrets-manager.php b/secrets/class-two-factor-secrets-manager.php index 630789c8..fe5f701b 100644 --- a/secrets/class-two-factor-secrets-manager.php +++ b/secrets/class-two-factor-secrets-manager.php @@ -26,6 +26,24 @@ */ class Two_Factor_Secrets_Manager { + /** + * Site transient caching whether any user's stored secret is unreachable. + * + * @since 0.18.0 + * + * @var string + */ + const AFFECTED_USERS_TRANSIENT = 'two_factor_secrets_affected_users'; + + /** + * How long, in seconds, the unreachable-secrets result is cached. + * + * @since 0.18.0 + * + * @var int + */ + const AFFECTED_USERS_CACHE_TTL = 300; + /** * Where secrets are kept. * @@ -295,6 +313,341 @@ public function delete_user_secret( int $user_id, string $slug ) { return $error ? $error : true; } + /** + * Read a secret a provider keeps for a user, wherever it is stored. + * + * A plaintext value in the provider's own user meta wins, and is moved into the + * Secrets API on the way past when that is allowed. Otherwise the Secrets API is + * consulted when the user has a marker. + * + * @since 0.18.0 + * + * @param int $user_id User ID. + * @param string $slug Secret slug. + * @param string $meta_key User meta key that holds the plaintext value. + * @param bool $migrate Optional. Whether to migrate a plaintext value. Default true. + * @return string|null|WP_Error The value, null when the user has none, or a WP_Error when one exists but cannot be read. + */ + public function get_stored_secret_state( int $user_id, string $slug, string $meta_key, bool $migrate = true ) { + $plaintext = (string) get_user_meta( $user_id, $meta_key, true ); + + if ( '' !== $plaintext ) { + if ( $migrate && $this->can_write( $user_id ) ) { + $this->migrate_stored_secret( $user_id, $slug, $meta_key ); + } + + return $plaintext; + } + + if ( '' === (string) get_user_meta( $user_id, $this->get_marker_meta_key( $slug ), true ) ) { + return null; + } + + return $this->get_user_secret( $user_id, $slug ); + } + + /** + * Move a plaintext secret from user meta into the Secrets API. + * + * The plaintext is removed only after the stored secret has been read back and + * matches. Nothing ever moves a secret the other way. + * + * @since 0.18.0 + * + * @param int $user_id User ID. + * @param string $slug Secret slug. + * @param string $meta_key User meta key that holds the plaintext value. + * @return true|null|WP_Error True when migrated, null when there is nothing to migrate, WP_Error on failure. + */ + public function migrate_stored_secret( int $user_id, string $slug, string $meta_key ) { + $plaintext = (string) get_user_meta( $user_id, $meta_key, true ); + + if ( '' === $plaintext ) { + return null; + } + + if ( ! $this->can_write( $user_id ) ) { + return new WP_Error( + 'two_factor_secrets_not_writable', + __( 'The Secrets API cannot be written to, so the secret was not migrated.', 'two-factor' ) + ); + } + + $result = $this->set_user_secret( $user_id, $slug, $plaintext ); + + if ( is_wp_error( $result ) ) { + $this->fire_migration_failed( $user_id, $slug, $result ); + return $result; + } + + $readback = $this->get_user_secret( $user_id, $slug ); + + if ( ! is_string( $readback ) || ! hash_equals( $plaintext, $readback ) ) { + $error = is_wp_error( $readback ) + ? $readback + : new WP_Error( + 'two_factor_secrets_migration_mismatch', + __( 'The stored secret did not match the original, so the migration was rolled back.', 'two-factor' ) + ); + + $this->delete_user_secret( $user_id, $slug ); + $this->fire_migration_failed( $user_id, $slug, $error ); + + return $error; + } + + delete_user_meta( $user_id, $meta_key ); + Two_Factor_Secrets::memzero( $readback ); + $this->clear_affected_users_cache(); + + /** + * Fires after a user's secret was moved into the Secrets API. + * + * @since 0.18.0 + * + * @param int $user_id User ID. + * @param string $slug Secret slug, such as "totp". + */ + do_action( 'two_factor_secrets_migrated', $user_id, $slug ); + + return true; + } + + /** + * Fire the migration failure action. + * + * @since 0.18.0 + * + * @param int $user_id User ID. + * @param string $slug Secret slug. + * @param WP_Error $error The failure. + * @return void + */ + private function fire_migration_failed( int $user_id, string $slug, WP_Error $error ): void { + /** + * Fires when moving a user's secret into the Secrets API failed. + * + * The plaintext copy is kept, so the user can still log in. The error never contains the secret. + * + * @since 0.18.0 + * + * @param int $user_id User ID. + * @param string $slug Secret slug, such as "totp". + * @param WP_Error $error The failure. + */ + do_action( 'two_factor_secrets_migration_failed', $user_id, $slug, $error ); + } + + /** + * Store a secret a provider keeps for a user. + * + * Writes to the Secrets API when that is allowed, verifying by read-back, and to + * the provider's user meta otherwise. A Secrets API failure never results in a + * plaintext write. The value must be a single non-empty string. + * + * @since 0.18.0 + * + * @param int $user_id User ID. + * @param string $slug Secret slug. + * @param string $meta_key User meta key that holds the plaintext value. + * @param string $value Secret value. + * @return int|bool|WP_Error True, or what update_user_meta() returned for a plaintext write, or a WP_Error when the Secrets API write or its read-back failed. + */ + public function save_stored_secret( int $user_id, string $slug, string $meta_key, string $value ) { + if ( $this->can_write( $user_id ) ) { + $result = $this->set_user_secret( $user_id, $slug, $value ); + + if ( is_wp_error( $result ) ) { + return $result; + } + + $readback = $this->get_user_secret( $user_id, $slug ); + + if ( ! is_string( $readback ) || ! hash_equals( $value, $readback ) ) { + $this->delete_user_secret( $user_id, $slug ); + + return is_wp_error( $readback ) + ? $readback + : new WP_Error( + 'two_factor_secrets_write_mismatch', + __( 'The stored secret did not match the original, so it was removed.', 'two-factor' ) + ); + } + + Two_Factor_Secrets::memzero( $readback ); + delete_user_meta( $user_id, $meta_key ); + $this->clear_affected_users_cache(); + + return true; + } + + $result = update_user_meta( $user_id, $meta_key, $value ); + + // Clear any stale marker or secret so the plaintext value is unambiguous. + $this->delete_user_secret( $user_id, $slug ); + $this->clear_affected_users_cache(); + + return $result; + } + + /** + * Delete a secret a provider keeps for a user, from user meta and the Secrets API. + * + * @since 0.18.0 + * + * @param int $user_id User ID. + * @param string $slug Secret slug. + * @param string $meta_key User meta key that holds the plaintext value. + * @return bool Whether nothing of the secret is left behind. + */ + public function delete_stored_secret( int $user_id, string $slug, string $meta_key ): bool { + delete_user_meta( $user_id, $meta_key ); + + $deleted = $this->delete_user_secret( $user_id, $slug ); + $this->clear_affected_users_cache(); + + return true === $deleted + && '' === (string) get_user_meta( $user_id, $meta_key, true ) + && '' === (string) get_user_meta( $user_id, $this->get_marker_meta_key( $slug ), true ); + } + + /** + * Get where a secret a provider keeps for a user is stored. Never migrates. + * + * @since 0.18.0 + * + * @param int $user_id User ID. + * @param string $slug Secret slug. + * @param string $meta_key User meta key that holds the plaintext value. + * @return string One of 'plaintext', 'secrets-api', 'unavailable' or 'none'. + */ + public function get_stored_secret_storage( int $user_id, string $slug, string $meta_key ): string { + if ( '' !== (string) get_user_meta( $user_id, $meta_key, true ) ) { + return 'plaintext'; + } + + if ( '' === (string) get_user_meta( $user_id, $this->get_marker_meta_key( $slug ), true ) ) { + return 'none'; + } + + $secret = $this->get_user_secret( $user_id, $slug ); + + if ( is_string( $secret ) ) { + Two_Factor_Secrets::memzero( $secret ); + return 'secrets-api'; + } + + return 'unavailable'; + } + + /** + * Whether a user has a secret that can be reached from here. Does not decrypt anything. + * + * @since 0.18.0 + * + * @param int $user_id User ID. + * @param string $slug Secret slug. + * @param string $meta_key User meta key that holds the plaintext value. + * @return bool + */ + public function is_stored_secret_reachable( int $user_id, string $slug, string $meta_key ): bool { + if ( '' !== (string) get_user_meta( $user_id, $meta_key, true ) ) { + return true; + } + + $marker = (string) get_user_meta( $user_id, $this->get_marker_meta_key( $slug ), true ); + + return '' !== $marker + && $this->is_api_present() + && get_current_network_id() === (int) $marker; + } + + /** + * Whether a user has a secret in the Secrets API that cannot be reached from here. + * + * True when the Secrets API is missing or the secret belongs to another network. + * Does not decrypt anything. + * + * @since 0.18.0 + * + * @param int $user_id User ID. + * @param string $slug Secret slug. + * @param string $meta_key User meta key that holds the plaintext value. + * @return bool + */ + public function is_stored_secret_unreachable( int $user_id, string $slug, string $meta_key ): bool { + if ( '' !== (string) get_user_meta( $user_id, $meta_key, true ) ) { + return false; + } + + $marker = (string) get_user_meta( $user_id, $this->get_marker_meta_key( $slug ), true ); + + if ( '' === $marker ) { + return false; + } + + return ! $this->is_api_present() || get_current_network_id() !== (int) $marker; + } + + /** + * Whether any user has a secret in the Secrets API that this site cannot currently reach. + * + * The result is cached briefly in a site transient. + * + * @since 0.18.0 + * + * @param string[] $slugs Secret slugs to look for. + * @return bool + */ + public function has_affected_users( array $slugs ): bool { + $cached = get_site_transient( self::AFFECTED_USERS_TRANSIENT ); + + if ( 'yes' === $cached || 'no' === $cached ) { + return 'yes' === $cached; + } + + $affected = false; + + // With the API present on a single site every marker names the only network, so nothing can be out of reach. + if ( ! $this->is_api_present() || is_multisite() ) { + foreach ( $slugs as $slug ) { + $args = array( + 'blog_id' => 0, + 'meta_key' => $this->get_marker_meta_key( $slug ), // phpcs:ignore WordPress.DB.SlowDBQuery.slow_db_query_meta_key -- Single-row lookup, result cached. + 'meta_compare' => 'EXISTS', + 'number' => 1, + 'fields' => 'ID', + 'count_total' => false, + ); + + if ( $this->is_api_present() ) { + $args['meta_value'] = (string) get_current_network_id(); // phpcs:ignore WordPress.DB.SlowDBQuery.slow_db_query_meta_value -- Single-row lookup, result cached. + $args['meta_compare'] = '!='; + } + + if ( ! empty( get_users( $args ) ) ) { + $affected = true; + break; + } + } + } + + set_site_transient( self::AFFECTED_USERS_TRANSIENT, $affected ? 'yes' : 'no', self::AFFECTED_USERS_CACHE_TTL ); + + return $affected; + } + + /** + * Forget the cached unreachable-secrets result. + * + * @since 0.18.0 + * + * @return void + */ + public function clear_affected_users_cache(): void { + delete_site_transient( self::AFFECTED_USERS_TRANSIENT ); + } + /** * Validate a slug. * diff --git a/tests/class-two-factor-dummy-secret.php b/tests/class-two-factor-dummy-secret.php new file mode 100644 index 00000000..60dd6a7d --- /dev/null +++ b/tests/class-two-factor-dummy-secret.php @@ -0,0 +1,37 @@ + self::SECRET_META_KEY ); + + /** + * Declare the secret. + * + * @return array + */ + public static function user_secret_meta_keys() { + return self::$secrets; + } +} diff --git a/tests/class-two-factor-secrets-test-case.php b/tests/class-two-factor-secrets-test-case.php index c456f9f4..caecdfe8 100644 --- a/tests/class-two-factor-secrets-test-case.php +++ b/tests/class-two-factor-secrets-test-case.php @@ -69,7 +69,7 @@ protected function simulate_api_absent() { */ public function tear_down() { Two_Factor_Secrets::reset(); - delete_site_transient( 'two_factor_totp_affected_users' ); + delete_site_transient( Two_Factor_Secrets_Manager::AFFECTED_USERS_TRANSIENT ); parent::tear_down(); } } diff --git a/tests/cli/class-two-factor-cli-command.php b/tests/cli/class-two-factor-cli-command.php index 6a019ed9..dff1bd9e 100644 --- a/tests/cli/class-two-factor-cli-command.php +++ b/tests/cli/class-two-factor-cli-command.php @@ -356,7 +356,8 @@ public function test_secrets_status_reports_present_api_and_counts() { $this->assertSame( 1, $item['plaintext_users'] ); $this->assertSame( 1, $item['migrated_users'] ); $this->assertSame( 0, $item['affected_users'] ); - $this->assertSame( array( 'api_present', 'provider', 'writable', 'opted_in', 'enabled', 'plaintext_users', 'migrated_users', 'affected_users' ), $this->last_format()['fields'] ); + $this->assertSame( array( 'secret', 'api_present', 'provider', 'writable', 'opted_in', 'enabled', 'plaintext_users', 'migrated_users', 'affected_users' ), $this->last_format()['fields'] ); + $this->assertSame( 'totp', $item['secret'] ); $this->assertSame( 'true', $item['opted_in'] ); $this->assertSame( 'true', $item['enabled'] ); } @@ -534,7 +535,7 @@ public function test_secrets_migrate_reports_failed_users_and_continues() { $this->command->secrets( array( 'migrate' ), array( 'batch-size' => '2' ) ); $this->assertSame( 'Migrated 2, failed 1, skipped 0.', $this->last_message( 'success' ) ); - $this->assertStringContainsString( "User {$bad}:", $this->last_message( 'warning' ) ); + $this->assertStringContainsString( "User {$bad} (totp):", $this->last_message( 'warning' ) ); $this->assertSame( 'ABCDEFGH', get_user_meta( $bad, Two_Factor_Totp::SECRET_META_KEY, true ) ); } @@ -607,6 +608,42 @@ public function test_secrets_status_reports_not_opted_in() { $this->assertSame( 'false', $item['enabled'] ); } + /** + * Migration and status cover a secret declared by a provider other than TOTP. + * + * @covers Two_Factor_CLI_Command::secrets + */ + public function test_secrets_commands_cover_other_providers_secrets() { + require_once dirname( __DIR__ ) . '/class-two-factor-dummy-secret.php'; + $register = function ( $providers ) { + $providers['Two_Factor_Dummy_Secret'] = dirname( __DIR__ ) . '/class-two-factor-dummy-secret.php'; + return $providers; + }; + add_filter( 'two_factor_providers', $register ); + + $this->seed_plaintext_key( $this->user->ID ); + update_user_meta( $this->user->ID, Two_Factor_Dummy_Secret::SECRET_META_KEY, 'DUMMYSECRET' ); + + $this->command->secrets( array( 'status' ), array() ); + $rows = wp_list_pluck( $this->last_format()['items'], 'plaintext_users', 'secret' ); + $this->assertSame( + array( + 'totp' => 1, + 'dummy' => 1, + ), + $rows + ); + + $this->command->secrets( array( 'migrate' ), array() ); + + $this->assertSame( 'DUMMYSECRET', $this->secrets_store->get( "two-factor/dummy-{$this->user->ID}" ) ); + $this->assertSame( '', (string) get_user_meta( $this->user->ID, Two_Factor_Dummy_Secret::SECRET_META_KEY, true ) ); + $this->assertSame( '', (string) get_user_meta( $this->user->ID, Two_Factor_Totp::SECRET_META_KEY, true ) ); + $this->assertStringContainsString( 'Migrated 2, failed 0', $this->last_message( 'success' ) ); + + remove_filter( 'two_factor_providers', $register ); + } + /** * An invalid batch size errors. * diff --git a/tests/providers/class-two-factor-totp-secrets-tests.php b/tests/providers/class-two-factor-totp-secrets-tests.php index 86f2d3e6..db08eded 100644 --- a/tests/providers/class-two-factor-totp-secrets-tests.php +++ b/tests/providers/class-two-factor-totp-secrets-tests.php @@ -780,7 +780,7 @@ public function test_wpmu_delete_user_removes_secret_on_multisite() { * The right deletion hook is registered. */ public function test_registers_correct_deletion_hook() { - $callback = array( 'Two_Factor_Totp', 'delete_user_secrets_on_user_deletion' ); + $callback = array( 'Two_Factor_Secrets_Lifecycle', 'delete_user_secrets' ); $this->assertNotFalse( has_action( is_multisite() ? 'wpmu_delete_user' : 'delete_user', $callback ) ); $this->assertFalse( has_action( is_multisite() ? 'delete_user' : 'wpmu_delete_user', $callback ) ); @@ -798,9 +798,11 @@ public function test_lifecycle_hooks_work_when_totp_disabled_site_wide() { try { $this->assertArrayNotHasKey( 'Two_Factor_Totp', Two_Factor_Core::get_providers() ); - $this->assertNotFalse( has_filter( 'site_status_tests', array( 'Two_Factor_Totp', 'register_site_health_test' ) ) ); - $this->assertNotFalse( has_action( 'admin_notices', array( 'Two_Factor_Totp', 'admin_notice_secrets_api_missing' ) ) ); - $this->assertNotFalse( has_action( 'network_admin_notices', array( 'Two_Factor_Totp', 'admin_notice_secrets_api_missing' ) ) ); + $this->assertArrayHasKey( 'totp', Two_Factor_Core::get_provider_secrets() ); + + $this->assertNotFalse( has_filter( 'site_status_tests', array( 'Two_Factor_Secrets_Lifecycle', 'register_site_health_test' ) ) ); + $this->assertNotFalse( has_action( 'admin_notices', array( 'Two_Factor_Secrets_Lifecycle', 'admin_notice_secrets_api_missing' ) ) ); + $this->assertNotFalse( has_action( 'network_admin_notices', array( 'Two_Factor_Secrets_Lifecycle', 'admin_notice_secrets_api_missing' ) ) ); if ( is_multisite() ) { require_once ABSPATH . 'wp-admin/includes/ms.php'; @@ -825,7 +827,7 @@ public function test_uninstall_user_data_removes_secrets_for_all_marked_users() $this->provider->set_user_totp_key( $user_id, 'ABCDEFGH' ); } - Two_Factor_Totp::uninstall_user_data(); + Two_Factor_Secrets_Lifecycle::delete_all_secrets(); foreach ( $user_ids as $user_id ) { $this->assertNull( $this->secrets_store->get( "two-factor/totp-{$user_id}" ) ); @@ -841,7 +843,7 @@ public function test_uninstall_user_data_noop_when_api_absent() { update_user_meta( $user_id, Two_Factor_Totp::SECRET_NETWORK_META_KEY, (string) get_current_network_id() ); $this->simulate_api_absent(); - Two_Factor_Totp::uninstall_user_data(); + Two_Factor_Secrets_Lifecycle::delete_all_secrets(); $this->assertSame( (string) get_current_network_id(), $this->marker( $user_id ) ); } @@ -855,7 +857,7 @@ public function test_uninstall_user_data_noop_when_api_absent() { private function seed_marker( $network = null ) { $user_id = $this->user(); update_user_meta( $user_id, Two_Factor_Totp::SECRET_NETWORK_META_KEY, null === $network ? (string) get_current_network_id() : $network ); - Two_Factor_Totp::clear_affected_users_cache(); + Two_Factor_Secrets::clear_affected_users_cache(); return $user_id; } @@ -864,7 +866,7 @@ private function seed_marker( $network = null ) { * No markers means no affected users. */ public function test_has_affected_users_false_without_markers() { - $this->assertFalse( Two_Factor_Totp::has_affected_users() ); + $this->assertFalse( Two_Factor_Secrets_Lifecycle::has_affected_users() ); } /** @@ -874,7 +876,7 @@ public function test_has_affected_users_true_when_api_absent_with_marker() { $this->seed_marker(); $this->simulate_api_absent(); - $this->assertTrue( Two_Factor_Totp::has_affected_users() ); + $this->assertTrue( Two_Factor_Secrets_Lifecycle::has_affected_users() ); } /** @@ -886,7 +888,7 @@ public function test_has_affected_users_true_for_other_network_marker() { } $this->seed_marker( (string) ( get_current_network_id() + 1 ) ); - $this->assertTrue( Two_Factor_Totp::has_affected_users() ); + $this->assertTrue( Two_Factor_Secrets_Lifecycle::has_affected_users() ); } /** @@ -894,18 +896,18 @@ public function test_has_affected_users_true_for_other_network_marker() { */ public function test_has_affected_users_is_cached() { $this->simulate_api_absent(); - Two_Factor_Totp::clear_affected_users_cache(); + Two_Factor_Secrets::clear_affected_users_cache(); - $this->assertFalse( Two_Factor_Totp::has_affected_users() ); + $this->assertFalse( Two_Factor_Secrets_Lifecycle::has_affected_users() ); $user_id = $this->user(); update_user_meta( $user_id, Two_Factor_Totp::SECRET_NETWORK_META_KEY, (string) get_current_network_id() ); - $this->assertFalse( Two_Factor_Totp::has_affected_users() ); + $this->assertFalse( Two_Factor_Secrets_Lifecycle::has_affected_users() ); - Two_Factor_Totp::clear_affected_users_cache(); + Two_Factor_Secrets::clear_affected_users_cache(); - $this->assertTrue( Two_Factor_Totp::has_affected_users() ); + $this->assertTrue( Two_Factor_Secrets_Lifecycle::has_affected_users() ); } /** @@ -921,7 +923,7 @@ public function test_admin_notice_shown_to_manage_options_user() { $html = $this->capture( function () { - $this->provider->admin_notice_secrets_api_missing(); + Two_Factor_Secrets_Lifecycle::admin_notice_secrets_api_missing(); } ); @@ -943,7 +945,7 @@ public function test_admin_notice_hidden_without_capability() { '', $this->capture( function () { - $this->provider->admin_notice_secrets_api_missing(); + Two_Factor_Secrets_Lifecycle::admin_notice_secrets_api_missing(); } ) ); @@ -962,7 +964,7 @@ public function test_admin_notice_hidden_when_no_affected_users() { '', $this->capture( function () { - $this->provider->admin_notice_secrets_api_missing(); + Two_Factor_Secrets_Lifecycle::admin_notice_secrets_api_missing(); } ) ); @@ -985,7 +987,7 @@ public function test_admin_notice_network_admin_requires_manage_network_options( '', $this->capture( function () { - $this->provider->admin_notice_secrets_api_missing(); + Two_Factor_Secrets_Lifecycle::admin_notice_secrets_api_missing(); } ) ); @@ -995,7 +997,7 @@ function () { 'reset those users', $this->capture( function () { - $this->provider->admin_notice_secrets_api_missing(); + Two_Factor_Secrets_Lifecycle::admin_notice_secrets_api_missing(); } ) ); @@ -1010,7 +1012,7 @@ function () { public function test_site_health_test_is_registered() { $tests = apply_filters( 'site_status_tests', array( 'direct' => array() ) ); - $this->assertArrayHasKey( 'two_factor_totp_secret_storage', $tests['direct'] ); + $this->assertArrayHasKey( 'two_factor_secret_storage', $tests['direct'] ); } /** @@ -1020,13 +1022,13 @@ public function test_site_health_critical_when_affected_users() { $this->seed_marker(); $this->simulate_api_absent(); - $result = $this->provider->site_health_secret_storage(); + $result = Two_Factor_Secrets_Lifecycle::site_health_secret_storage(); $this->assertSame( 'critical', $result['status'] ); $this->assertSame( 'red', $result['badge']['color'] ); $this->assertStringContainsString( 'reset those users', $result['description'] ); $this->assertStringNotContainsString( 'export', $result['description'] ); - $this->assertSame( 'two_factor_totp_secret_storage', $result['test'] ); + $this->assertSame( 'two_factor_secret_storage', $result['test'] ); } /** @@ -1036,7 +1038,7 @@ public function test_site_health_recommended_when_plaintext_remains() { $user_id = $this->user(); update_user_meta( $user_id, Two_Factor_Totp::SECRET_META_KEY, 'ABCDEFGH' ); - $result = $this->provider->site_health_secret_storage(); + $result = Two_Factor_Secrets_Lifecycle::site_health_secret_storage(); $this->assertSame( 'recommended', $result['status'] ); $this->assertSame( 'orange', $result['badge']['color'] ); @@ -1048,7 +1050,7 @@ public function test_site_health_recommended_when_plaintext_remains() { */ public function test_site_health_good_when_fully_migrated() { - $result = $this->provider->site_health_secret_storage(); + $result = Two_Factor_Secrets_Lifecycle::site_health_secret_storage(); $this->assertSame( 'good', $result['status'] ); $this->assertStringContainsString( esc_html( Two_Factor_Secrets::provider_label() ), $result['description'] ); @@ -1061,7 +1063,7 @@ public function test_site_health_good_when_filter_opts_out_with_plaintext() { add_filter( 'two_factor_use_secrets_api', '__return_false' ); update_user_meta( $this->user(), Two_Factor_Totp::SECRET_META_KEY, 'ABCDEFGH' ); - $result = $this->provider->site_health_secret_storage(); + $result = Two_Factor_Secrets_Lifecycle::site_health_secret_storage(); $this->assertSame( 'good', $result['status'] ); $this->assertStringContainsString( 'two_factor_use_secrets_api', $result['description'] ); @@ -1073,7 +1075,7 @@ public function test_site_health_good_when_filter_opts_out_with_plaintext() { public function test_site_health_good_neutral_when_api_absent() { $this->simulate_api_absent(); - $result = $this->provider->site_health_secret_storage(); + $result = Two_Factor_Secrets_Lifecycle::site_health_secret_storage(); $this->assertSame( 'good', $result['status'] ); $this->assertSame( @@ -1088,7 +1090,7 @@ public function test_site_health_good_neutral_when_api_absent() { public function test_site_health_recommended_when_not_opted_in() { $this->opt_out(); - $result = $this->provider->site_health_secret_storage(); + $result = Two_Factor_Secrets_Lifecycle::site_health_secret_storage(); $this->assertSame( 'recommended', $result['status'] ); $this->assertSame( 'orange', $result['badge']['color'] ); @@ -1102,7 +1104,7 @@ public function test_site_health_recommended_when_not_opted_in() { public function test_site_health_good_when_provider_read_only() { $this->secrets_store->writable = false; - $result = $this->provider->site_health_secret_storage(); + $result = Two_Factor_Secrets_Lifecycle::site_health_secret_storage(); $this->assertSame( 'good', $result['status'] ); $this->assertStringContainsString( 'read-only', $result['description'] ); diff --git a/tests/secrets/class-two-factor-secrets-lifecycle-tests.php b/tests/secrets/class-two-factor-secrets-lifecycle-tests.php new file mode 100644 index 00000000..14417c7b --- /dev/null +++ b/tests/secrets/class-two-factor-secrets-lifecycle-tests.php @@ -0,0 +1,290 @@ + Two_Factor_Dummy_Secret::SECRET_META_KEY ); + + // Before the site-wide setting is enforced at the default priority, as a plugin loaded earlier would be. + add_filter( 'two_factor_providers', array( $this, 'register_fixture' ), 5 ); + } + + /** + * Unregister the fixture provider. + */ + public function tear_down() { + remove_filter( 'two_factor_providers', array( $this, 'register_fixture' ), 5 ); + Two_Factor_Dummy_Secret::$secrets = array( 'dummy' => Two_Factor_Dummy_Secret::SECRET_META_KEY ); + delete_option( Two_Factor_Core::ENABLED_PROVIDERS_OPTION_KEY ); + + parent::tear_down(); + } + + /** + * Add the fixture provider to the registered providers. + * + * @param array $providers Registered providers. + * @return array + */ + public function register_fixture( $providers ) { + $providers['Two_Factor_Dummy_Secret'] = dirname( __DIR__ ) . '/class-two-factor-dummy-secret.php'; + + return $providers; + } + + /** + * Store the fixture's secret for a new user. + * + * @return int User ID. + */ + private function user_with_stored_secret() { + $user_id = self::factory()->user->create(); + Two_Factor_Secrets::save_stored_secret( $user_id, 'dummy', Two_Factor_Dummy_Secret::SECRET_META_KEY, 'DUMMYSECRET' ); + + return $user_id; + } + + /** + * A provider that declares nothing keeps no secrets. + * + * @covers Two_Factor_Provider::user_secret_meta_keys + */ + public function test_base_provider_declares_no_secrets() { + $this->assertSame( array(), Two_Factor_Dummy::user_secret_meta_keys() ); + $this->assertSame( array(), Two_Factor_Email::user_secret_meta_keys() ); + } + + /** + * The registry lists TOTP and the fixture, with the provider that declared each. + * + * @covers Two_Factor_Core::get_provider_secrets + */ + public function test_registry_lists_declared_secrets() { + $secrets = Two_Factor_Core::get_provider_secrets(); + + $this->assertSame( + array( + 'meta_key' => Two_Factor_Totp::SECRET_META_KEY, + 'provider' => 'Two_Factor_Totp', + ), + $secrets['totp'] + ); + $this->assertSame( + array( + 'meta_key' => Two_Factor_Dummy_Secret::SECRET_META_KEY, + 'provider' => 'Two_Factor_Dummy_Secret', + ), + $secrets['dummy'] + ); + } + + /** + * A provider the site has turned off is still in the registry, and the setting is still enforced afterwards. + * + * @covers Two_Factor_Core::get_provider_secrets + */ + public function test_registry_includes_providers_disabled_site_wide() { + $this->assertNotFalse( has_filter( 'two_factor_providers', 'two_factor_filter_enabled_providers' ) ); + update_option( Two_Factor_Core::ENABLED_PROVIDERS_OPTION_KEY, array( 'Two_Factor_Email' ) ); + + $this->assertArrayNotHasKey( 'Two_Factor_Dummy_Secret', Two_Factor_Core::get_providers() ); + + $secrets = Two_Factor_Core::get_provider_secrets(); + + $this->assertArrayHasKey( 'dummy', $secrets ); + $this->assertArrayHasKey( 'totp', $secrets ); + $this->assertFalse( Two_Factor_Core::is_listing_all_providers() ); + $this->assertArrayNotHasKey( 'Two_Factor_Dummy_Secret', Two_Factor_Core::get_providers() ); + } + + /** + * Invalid declarations are ignored. + * + * @covers Two_Factor_Core::get_provider_secrets + */ + public function test_registry_ignores_invalid_declarations() { + Two_Factor_Dummy_Secret::$secrets = array( + 'Bad Slug' => '_two_factor_bad', + 'empty' => '', + 0 => '_two_factor_numeric', + 'good' => '_two_factor_good', + ); + + $secrets = Two_Factor_Core::get_provider_secrets(); + + $this->assertSame( array( 'totp', 'good' ), array_keys( $secrets ) ); + } + + /** + * A slug another provider already declared is refused, and the first declaration wins. + * + * @covers Two_Factor_Core::get_provider_secrets + */ + public function test_registry_refuses_duplicate_slug() { + Two_Factor_Dummy_Secret::$secrets = array( 'totp' => Two_Factor_Dummy_Secret::SECRET_META_KEY ); + $this->setExpectedIncorrectUsage( 'Two_Factor_Core::get_provider_secrets' ); + + $secrets = Two_Factor_Core::get_provider_secrets(); + + $this->assertSame( 'Two_Factor_Totp', $secrets['totp']['provider'] ); + $this->assertSame( Two_Factor_Totp::SECRET_META_KEY, $secrets['totp']['meta_key'] ); + } + + /** + * A declared secret is written to the store, read back and reported, under its own name. + * + * @covers Two_Factor_Secrets_Manager::save_stored_secret + * @covers Two_Factor_Secrets_Manager::get_stored_secret_state + * @covers Two_Factor_Secrets_Manager::get_stored_secret_storage + */ + public function test_declared_secret_round_trips_through_the_store() { + $user_id = $this->user_with_stored_secret(); + + $this->assertSame( 'DUMMYSECRET', $this->secrets_store->get( "two-factor/dummy-{$user_id}" ) ); + $this->assertSame( '', (string) get_user_meta( $user_id, Two_Factor_Dummy_Secret::SECRET_META_KEY, true ) ); + $this->assertSame( 'DUMMYSECRET', Two_Factor_Secrets::get_stored_secret_state( $user_id, 'dummy', Two_Factor_Dummy_Secret::SECRET_META_KEY ) ); + $this->assertSame( 'secrets-api', Two_Factor_Secrets::get_stored_secret_storage( $user_id, 'dummy', Two_Factor_Dummy_Secret::SECRET_META_KEY ) ); + } + + /** + * A plaintext value is migrated when it is read, and the action names the secret. + * + * @covers Two_Factor_Secrets_Manager::migrate_stored_secret + */ + public function test_declared_secret_migrates_lazily() { + $user_id = self::factory()->user->create(); + update_user_meta( $user_id, Two_Factor_Dummy_Secret::SECRET_META_KEY, 'DUMMYSECRET' ); + $seen = array(); + add_action( + 'two_factor_secrets_migrated', + function ( $migrated_user_id, $slug ) use ( &$seen ) { + $seen[] = array( $migrated_user_id, $slug ); + }, + 10, + 2 + ); + + $this->assertSame( 'DUMMYSECRET', Two_Factor_Secrets::get_stored_secret_state( $user_id, 'dummy', Two_Factor_Dummy_Secret::SECRET_META_KEY ) ); + + $this->assertSame( array( array( $user_id, 'dummy' ) ), $seen ); + $this->assertSame( 'DUMMYSECRET', $this->secrets_store->get( "two-factor/dummy-{$user_id}" ) ); + $this->assertSame( '', (string) get_user_meta( $user_id, Two_Factor_Dummy_Secret::SECRET_META_KEY, true ) ); + } + + /** + * The base provider reports an unreachable declared secret as enrolled but unavailable. + * + * @covers Two_Factor_Provider::is_enrolled_but_unavailable_for_user + */ + public function test_base_provider_reports_unreachable_declared_secret() { + $user = get_user_by( 'id', $this->user_with_stored_secret() ); + $provider = Two_Factor_Dummy_Secret::get_instance(); + + $this->assertFalse( $provider->is_enrolled_but_unavailable_for_user( $user ) ); + + $this->simulate_api_absent(); + + $this->assertTrue( $provider->is_enrolled_but_unavailable_for_user( $user ) ); + $this->assertFalse( Two_Factor_Dummy::get_instance()->is_enrolled_but_unavailable_for_user( $user ) ); + } + + /** + * Deleting a user deletes every declared secret, including for a provider turned off site-wide. + * + * @covers Two_Factor_Secrets_Lifecycle::delete_user_secrets + */ + public function test_user_deletion_removes_declared_secrets() { + $user_id = $this->user_with_stored_secret(); + Two_Factor_Totp::get_instance()->set_user_totp_key( $user_id, 'ABCDEFGH' ); + update_option( Two_Factor_Core::ENABLED_PROVIDERS_OPTION_KEY, array( 'Two_Factor_Email' ) ); + + if ( is_multisite() ) { + require_once ABSPATH . 'wp-admin/includes/ms.php'; + wpmu_delete_user( $user_id ); + } else { + require_once ABSPATH . 'wp-admin/includes/user.php'; + wp_delete_user( $user_id ); + } + + $this->assertSame( array(), $this->secrets_store->values ); + } + + /** + * Uninstall deletes every declared secret and its marker. + * + * @covers Two_Factor_Core::uninstall + * @covers Two_Factor_Secrets_Lifecycle::delete_all_secrets + */ + public function test_uninstall_removes_declared_secrets_and_markers() { + $user_id = $this->user_with_stored_secret(); + Two_Factor_Totp::get_instance()->set_user_totp_key( $user_id, 'ABCDEFGH' ); + + Two_Factor_Core::uninstall(); + + $this->assertSame( array(), $this->secrets_store->values ); + $this->assertSame( '', (string) get_user_meta( $user_id, Two_Factor_Secrets::get_marker_meta_key( 'dummy' ), true ) ); + $this->assertSame( '', (string) get_user_meta( $user_id, Two_Factor_Totp::SECRET_NETWORK_META_KEY, true ) ); + } + + /** + * An unreachable secret of any provider counts as an affected user and turns Site Health critical. + * + * @covers Two_Factor_Secrets_Lifecycle::has_affected_users + * @covers Two_Factor_Secrets_Lifecycle::site_health_secret_storage + */ + public function test_unreachable_declared_secret_is_reported() { + $this->user_with_stored_secret(); + $this->assertFalse( Two_Factor_Secrets_Lifecycle::has_affected_users() ); + + $this->simulate_api_absent(); + Two_Factor_Secrets::clear_affected_users_cache(); + + $this->assertTrue( Two_Factor_Secrets_Lifecycle::has_affected_users() ); + $this->assertSame( 'critical', Two_Factor_Secrets_Lifecycle::site_health_secret_storage()['status'] ); + } + + /** + * A plaintext secret of any provider makes Site Health recommend migrating. + * + * @covers Two_Factor_Secrets_Lifecycle::has_plaintext_users + * @covers Two_Factor_Secrets_Lifecycle::count_users_by_storage + */ + public function test_plaintext_declared_secret_is_reported() { + $this->assertFalse( Two_Factor_Secrets_Lifecycle::has_plaintext_users() ); + + $user_id = self::factory()->user->create(); + update_user_meta( $user_id, Two_Factor_Dummy_Secret::SECRET_META_KEY, 'DUMMYSECRET' ); + $this->user_with_stored_secret(); + + $this->assertTrue( Two_Factor_Secrets_Lifecycle::has_plaintext_users() ); + $this->assertSame( 'recommended', Two_Factor_Secrets_Lifecycle::site_health_secret_storage()['status'] ); + $this->assertSame( + array( + 'plaintext' => 1, + 'migrated' => 1, + 'affected' => 0, + ), + Two_Factor_Secrets_Lifecycle::count_users_by_storage( 'dummy', Two_Factor_Dummy_Secret::SECRET_META_KEY ) + ); + } +} diff --git a/two-factor.php b/two-factor.php index af3d66cf..7f84b75e 100644 --- a/two-factor.php +++ b/two-factor.php @@ -56,13 +56,7 @@ require_once TWO_FACTOR_DIR . 'secrets/class-two-factor-secrets-api-store.php'; require_once TWO_FACTOR_DIR . 'secrets/class-two-factor-secrets-manager.php'; require_once TWO_FACTOR_DIR . 'class-two-factor-secrets.php'; - -/** - * The TOTP provider owns secrets that outlive its being enabled, so its lifecycle - * hooks are registered even when the site has turned TOTP off. - */ -require_once TWO_FACTOR_DIR . 'providers/class-two-factor-totp.php'; -Two_Factor_Totp::register_secret_lifecycle_hooks(); +require_once TWO_FACTOR_DIR . 'secrets/class-two-factor-secrets-lifecycle.php'; // Load settings UI class so the settings page can be rendered. require_once TWO_FACTOR_DIR . 'settings/class-two-factor-settings.php'; @@ -172,6 +166,11 @@ function two_factor_filter_enabled_providers( $providers ) { return $providers; } + // Core is looking up every provider's stored secrets, which outlive a provider being turned off. + if ( Two_Factor_Core::is_listing_all_providers() ) { + return $providers; + } + // On the settings page itself, show all providers so admins can change the selection. $page = isset( $_GET['page'] ) ? sanitize_key( wp_unslash( $_GET['page'] ) ) : ''; // phpcs:ignore WordPress.Security.NonceVerification.Recommended -- Reading the current admin page slug only; no state change occurs here. if ( is_admin() && 'two-factor-settings' === $page ) { From fe127971cf3f3b0ac63c04369ad150ca818bd79f Mon Sep 17 00:00:00 2001 From: Eric Mann Date: Tue, 6 Oct 2026 12:18:27 -0700 Subject: [PATCH 3/3] Document the secrets store, manager and declared provider secrets --- AGENTS.md | 12 ++++++++---- TESTS.md | 30 ++++++++++++++++++++++-------- readme.txt | 4 ++-- 3 files changed, 32 insertions(+), 14 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index f0fcf3cf..2ea2e56f 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -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). @@ -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`) @@ -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. diff --git a/TESTS.md b/TESTS.md index 333c0c67..c500a6d6 100644 --- a/TESTS.md +++ b/TESTS.md @@ -176,17 +176,29 @@ Extends `WP_Test_REST_TestCase`. Tests the backup codes REST endpoints: - User cannot generate codes for a different user - Admin can generate codes for other users -### Secrets API Adapter — `tests/class-two-factor-secrets-tests.php` +### Secrets Storage — `tests/class-two-factor-secrets-tests.php` **Class:** `Two_Factor_Secrets_Tests` · **Group:** `secrets` -Tests `Two_Factor_Secrets`: secret names, marker meta, API presence, the administrator opt-in, `can_write()` and its filter, the read/write/delete round trip in network scope, and the error codes for a missing API, another network's marker, a vanished secret and API errors. The file is named after the class, and sorts after `class-two-factor-core.php` on purpose: the core tests register hooks in `set_up_before_class()`, which an earlier test class would wipe. +Tests `Two_Factor_Secrets` and the `Two_Factor_Secrets_Manager` behind it, against the in-memory store: secret names, marker meta, store availability, the administrator opt-in, `can_write()` and its filter, the read/write/delete round trip, and the error codes for a missing API, another network's marker, a vanished secret and store errors. The file is named after the class, and sorts after `class-two-factor-core.php` on purpose: the core tests register hooks in `set_up_before_class()`, which an earlier test class would wipe. + +### Secrets API Store — `tests/secrets/class-two-factor-secrets-api-store-tests.php` + +**Class:** `Two_Factor_Secrets_Api_Store_Tests` · **Group:** `secrets` + +The only tests that need the real Secrets API. They check that `Two_Factor_Secrets_Api_Store` behaves the way the in-memory store assumes: availability, a writable labelled provider, a network-scope round trip, a missing secret reading as null, and the manager working end to end over the real store. They skip when the feature plugin is not loaded. + +### Declared Secrets — `tests/secrets/class-two-factor-secrets-lifecycle-tests.php` + +**Class:** `Two_Factor_Secrets_Lifecycle_Tests` · **Group:** `secrets` + +Tests what core does for any secret a provider declares, using the `Two_Factor_Dummy_Secret` fixture so nothing depends on TOTP: the registry (`Two_Factor_Core::get_provider_secrets()`), providers turned off site-wide, invalid and duplicate declarations, storage and lazy migration, the base provider's `is_enrolled_but_unavailable_for_user()`, deletion with the user and on uninstall, and the affected-users, plaintext-users and per-secret counts behind the admin notice, Site Health and WP-CLI. ### TOTP Secrets Storage — `tests/providers/class-two-factor-totp-secrets-tests.php` **Class:** `Two_Factor_Totp_Secrets_Tests` · **Groups:** `providers`, `totp`, `secrets` -Tests how `Two_Factor_Totp` uses the adapter: plaintext storage until an administrator opts in, storage precedence, verified writes, lazy one-directional migration, fail-closed validation, the login prompt and profile UI when a secret is unreadable, setup and reset when the Secrets API key is unusable (no plaintext fallback, a clear 503, no reset), the affected-user login regression, user deletion, uninstall, the administrator notice, the Site Health test and multisite cross-site reads. +Tests how `Two_Factor_Totp` stores its key through `Two_Factor_Secrets`: plaintext storage until an administrator opts in, storage precedence, verified writes, lazy one-directional migration, fail-closed validation, the login prompt and profile UI when a secret is unreadable, setup and reset when the Secrets API key is unusable (no plaintext fallback, a clear 503, no reset), the affected-user login regression, user deletion, uninstall, the administrator notice, the Site Health test and multisite cross-site reads. ### Settings Screen — `tests/settings/class-tests-two-factor-settings.php` @@ -228,19 +240,21 @@ Tests the `Two_Factor_CLI_Command` WP-CLI command class. The WP-CLI runtime is n - `backup-codes generate` — default and `--count` code counts, regeneration replaces the set, enables the provider so codes are usable at login, session destruction when first enabled, unknown-action and missing-argument errors - `unlock` — clears the login throttle for a rate-limited user; no-op message otherwise - `status` `totp_storage` field for each storage state -- `secrets status` — API availability, provider label, the `opted_in` and `enabled` fields and per-state user counts, `--format` passthrough, unknown/missing action errors -- `secrets migrate` — single user, `--dry-run`, `--batch-size` paging, per-user failures, error when the API is absent, not yet turned on by an administrator, or opted out +- `secrets status` — one row per declared secret, with API availability, provider label, the `opted_in` and `enabled` fields and per-state user counts, `--format` passthrough, unknown/missing action errors +- `secrets migrate` — every declared secret, including one from a provider other than TOTP; single user, `--dry-run`, `--batch-size` paging, per-user failures, error when the API is absent, not yet turned on by an administrator, or opted out - `secrets export` — not an action; the secret stays in the Secrets API ## Test Helpers -- **`tests/class-two-factor-secrets-test-case.php`** — `Two_Factor_Secrets_UnitTestCase`, the base class for secrets tests. `set_up()` opts in to Secrets API storage, since it is off by default, and `opt_out()` returns to that default. `require_secrets_api()` skips the test when the feature plugin is not loaded, `simulate_api_absent()` makes the adapter report the API as missing, and `tear_down()` resets `Two_Factor_Secrets::$test_overrides` and the affected-users transient. `tests/bootstrap.php` loads it after the WordPress test library. +- **`tests/class-two-factor-secrets-test-case.php`** — `Two_Factor_Secrets_UnitTestCase`, the base class for secrets tests. `set_up()` installs a `Two_Factor_Secrets_Memory_Store` (available as `$this->secrets_store`) and opts in to Secrets API storage, since it is off by default; `opt_out()` returns to that default. `simulate_api_absent()` makes the store report itself unavailable. `require_secrets_api()` skips the test when the feature plugin is not loaded and is only needed by tests of the real store. `tear_down()` calls `Two_Factor_Secrets::reset()` and clears the affected-users transient. `tests/bootstrap.php` loads it after the WordPress test library. +- **`tests/class-two-factor-secrets-memory-store.php`** — `Two_Factor_Secrets_Memory_Store`, a `Two_Factor_Secrets_Store` that keeps secrets in a public `$values` array. Set `$available` or `$writable` to false, or `$on_get` / `$on_set` to a callable, to simulate a missing, read-only or failing store. +- **`tests/class-two-factor-dummy-secret.php`** — `Two_Factor_Dummy_Secret`, a provider that declares a `dummy` secret through `user_secret_meta_keys()`. Its static `$secrets` can be replaced to try invalid and clashing declarations. - **`tests/class-two-factor-dummy-unavailable.php`** — `Two_Factor_Dummy_Unavailable`, a provider that is enrolled but never available, and counts `uninstall_user_data()` calls. -- **`tests/phpstan/`** — Stubs for the Secrets API functions and classes so PHPStan can analyse the adapter (`scanFiles` in `phpstan.dist.neon`). PHPUnit excludes the directory in both configs because the stubs would redeclare the real functions once the feature plugin is loaded. +- **`tests/phpstan/`** — Stubs for the Secrets API functions and classes so PHPStan can analyse the Secrets API store (`scanFiles` in `phpstan.dist.neon`). PHPUnit excludes the directory in both configs because the stubs would redeclare the real functions once the feature plugin is loaded. ### Secrets API feature plugin -The tests need the Secrets API feature plugin, which is a Composer dev dependency: `wordpress/secrets-api`, pinned to `0.2.1`. It is not on Packagist, so `composer.json` declares it as a `package` repository that downloads the tagged zip from `github.com/ericmann/secrets-api`. A `vcs` repository would not work here: the wp-env container has no `git`, and the GitHub API it would use instead is rate limited for unauthenticated CI runs. To move to a new release, change the version and the zip URL together and run `npm run composer -- update wordpress/secrets-api`. +Most tests store secrets in memory and do not need the Secrets API. The tests of the real store, and a few integration tests, need the feature plugin, which is a Composer dev dependency: `wordpress/secrets-api`, pinned to `0.2.1`. It is not on Packagist, so `composer.json` declares it as a `package` repository that downloads the tagged zip from `github.com/ericmann/secrets-api`. A `vcs` repository would not work here: the wp-env container has no `git`, and the GitHub API it would use instead is rate limited for unauthenticated CI runs. To move to a new release, change the version and the zip URL together and run `npm run composer -- update wordpress/secrets-api`. `npm run composer install` puts it in `vendor/wordpress/secrets-api/`, and `tests/bootstrap.php` loads `secrets-api.php` from there, or the file named by the `TWO_FACTOR_SECRETS_API_FILE` environment variable. `.wp-env.json` sets a deterministic `WP_SECRETS_KEY` for the tests environment. When the file is absent the suite still runs and tests that need the API are skipped; tests that simulate an absent API run either way. diff --git a/readme.txt b/readme.txt index cf8900a4..a9664f8d 100644 --- a/readme.txt +++ b/readme.txt @@ -121,8 +121,8 @@ The plugin includes a `wp two-factor` WP-CLI namespace for managing two-factor a * `wp two-factor enable ` — Enables a provider for a user. Providers that require a shared secret (like TOTP) can't be enabled this way and will point you to the profile page instead. * `wp two-factor disable []` — Disables a single provider, or performs a full reset of all 2FA for the user when no provider is given. Both forms prompt for confirmation unless `--yes` is passed. * `wp two-factor backup-codes generate [--count=]` — Generates a fresh set of backup codes for a user, replacing any existing ones. Defaults to 10 codes. -* `wp two-factor secrets status` — Shows whether the WordPress Secrets API is available, whether an administrator has turned on storing secrets with it, and how many users have authenticator app secrets in user meta, in the Secrets API, or unreachable. Supports `--format=json`. -* `wp two-factor secrets migrate [] [--batch-size=] [--dry-run]` — Moves plaintext authenticator app secrets into the Secrets API. +* `wp two-factor secrets status` — Shows whether the WordPress Secrets API is available, whether an administrator has turned on storing secrets with it, and how many users have secrets in user meta, in the Secrets API, or unreachable, with one row per secret a provider keeps. Supports `--format=json`. +* `wp two-factor secrets migrate [] [--batch-size=] [--dry-run]` — Moves plaintext secrets, such as authenticator app keys, into the Secrets API. * `wp two-factor unlock ` — Clears a user's login rate-limit/throttle without changing their 2FA configuration. Run `wp help two-factor` for the full list, or `wp help two-factor ` for options and examples for a specific command.