From 423bb2df0d18c4e41baf490d8a3e5d189fd7adb3 Mon Sep 17 00:00:00 2001 From: Kaspars Dambis Date: Wed, 18 Sep 2024 11:31:34 +0300 Subject: [PATCH 01/14] Introduce a helper to return all available two-factor providers (even if disabled) for uninstall purposes --- class-two-factor-core.php | 28 ++++++++++++++++++++++------ 1 file changed, 22 insertions(+), 6 deletions(-) diff --git a/class-two-factor-core.php b/class-two-factor-core.php index 82b42d96..88eee556 100644 --- a/class-two-factor-core.php +++ b/class-two-factor-core.php @@ -132,13 +132,11 @@ public static function add_hooks( $compat ) { } /** - * For each provider, include it and then instantiate it. - * - * @since 0.1-dev + * Get the registered providers of which some might not be enabled. * - * @return array + * @return array List of provider keys and paths to class files. */ - public static function get_providers() { + public static function get_providers_registered() { $providers = array( 'Two_Factor_Email' => TWO_FACTOR_DIR . 'providers/class-two-factor-email.php', 'Two_Factor_Totp' => TWO_FACTOR_DIR . 'providers/class-two-factor-totp.php', @@ -147,6 +145,24 @@ public static function get_providers() { 'Two_Factor_Dummy' => TWO_FACTOR_DIR . 'providers/class-two-factor-dummy.php', ); + // Get providers registered by other plugins. + $additional_providers = apply_filters( 'two_factor_providers', array() ); + + if ( ! empty( $additional_providers ) ) + return array_merge( $providers, $additional_providers ); + } + + return $providers; + } + + /** + * For each provider, include it and then instantiate it. + * + * @since 0.1-dev + * + * @return array + */ + public static function get_providers() { /** * Filter the supplied providers. * @@ -156,7 +172,7 @@ public static function get_providers() { * @param array $providers A key-value array where the key is the class name, and * the value is the path to the file containing the class. */ - $providers = apply_filters( 'two_factor_providers', $providers ); + $providers = apply_filters( 'two_factor_providers', self::get_providers_registered() ); // FIDO U2F is PHP 5.3+ only. if ( isset( $providers['Two_Factor_FIDO_U2F'] ) && version_compare( PHP_VERSION, '5.3.0', '<' ) ) { From 9119613dba91ccec2d4e452c088370d9f10a89be Mon Sep 17 00:00:00 2001 From: Kaspars Dambis Date: Wed, 18 Sep 2024 11:32:49 +0300 Subject: [PATCH 02/14] Let each provider announce the meta keys and options used --- providers/class-two-factor-provider.php | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/providers/class-two-factor-provider.php b/providers/class-two-factor-provider.php index 2e9f2a8f..68610c69 100644 --- a/providers/class-two-factor-provider.php +++ b/providers/class-two-factor-provider.php @@ -165,4 +165,22 @@ public static function sanitize_code_from_request( $field, $length = 0 ) { return (string) $code; } + + /** + * Return the user meta keys that need to be deletated on plugin uninstall. + * + * @return array + */ + public static function uninstall_user_meta_keys() { + return array(); + } + + /** + * Return the option keys that need to be deleted on plugin uninstall. + * + * @return array + */ + public static function uninstall_options() { + return array(); + } } From 8ea9cd3095fbaef03685dfedab976a5ffd5ccb50 Mon Sep 17 00:00:00 2001 From: Kaspars Dambis Date: Wed, 18 Sep 2024 11:40:35 +0300 Subject: [PATCH 03/14] Keep the same format --- class-two-factor-core.php | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/class-two-factor-core.php b/class-two-factor-core.php index 88eee556..c429366f 100644 --- a/class-two-factor-core.php +++ b/class-two-factor-core.php @@ -163,6 +163,8 @@ public static function get_providers_registered() { * @return array */ public static function get_providers() { + $providers = self::get_providers_registered(); + /** * Filter the supplied providers. * @@ -172,7 +174,7 @@ public static function get_providers() { * @param array $providers A key-value array where the key is the class name, and * the value is the path to the file containing the class. */ - $providers = apply_filters( 'two_factor_providers', self::get_providers_registered() ); + $providers = apply_filters( 'two_factor_providers', $providers ); // FIDO U2F is PHP 5.3+ only. if ( isset( $providers['Two_Factor_FIDO_U2F'] ) && version_compare( PHP_VERSION, '5.3.0', '<' ) ) { From 129cbfb04b1d51539a7c3bbda29f7d40ffdbf66b Mon Sep 17 00:00:00 2001 From: Kaspars Dambis Date: Wed, 18 Sep 2024 11:59:54 +0300 Subject: [PATCH 04/14] Split into dedicated helpers to allow re-use for uninstall and setup --- class-two-factor-core.php | 97 ++++++++++++++++++++++++++++++--------- 1 file changed, 75 insertions(+), 22 deletions(-) diff --git a/class-two-factor-core.php b/class-two-factor-core.php index c429366f..d9de5261 100644 --- a/class-two-factor-core.php +++ b/class-two-factor-core.php @@ -145,16 +145,84 @@ public static function get_providers_registered() { 'Two_Factor_Dummy' => TWO_FACTOR_DIR . 'providers/class-two-factor-dummy.php', ); - // Get providers registered by other plugins. + /** + * Filter the supplied providers. + * + * @param array $providers A key-value array where the key is the class name, and + * the value is the path to the file containing the class. + */ $additional_providers = apply_filters( 'two_factor_providers', array() ); - if ( ! empty( $additional_providers ) ) + if ( ! empty( $additional_providers ) ) { return array_merge( $providers, $additional_providers ); } return $providers; } + /** + * Get the user meta keys to delete when uninstalling from each provider. + * + * This attempts to get keys also from methods that are not enabled to + * ensure that all meta keys ever used are removed. + * + * @return array List of user meta keys to delete. + */ + private static function get_providers_uninstall_user_meta_keys() { + $user_meta_keys = array(); + + foreach ( self::get_providers_classes() as $provider_class ) { + if ( method_exists( $provider_class, 'uninstall_user_meta_keys' ) ) { + try { + $user_meta_keys = array_merge( + $user_meta_keys, + call_user_func( array( $provider_class, 'uninstall_user_meta_keys' ) ) + ); + } catch ( Exception $e ) { + // Do nothing. + } + } + } + + return $user_meta_keys; + } + + /** + * Get the classnames for all registered providers. + * + * Note some of these providers might not be enabled. + * + * @return array List of provider keys and classnames. + */ + private static function get_providers_classes() { + $providers = self::get_providers_registered(); + + foreach ( $providers as $provider_key => $path ) { + require_once $path; + + $class = $provider_key; + + /** + * Filters the classname for a provider. The dynamic portion of the filter is the defined providers key. + * + * @param string $class The PHP Classname of the provider. + * @param string $path The provided provider path to be included. + */ + $class = apply_filters( "two_factor_provider_classname_{$provider_key}", $class, $path ); + + /** + * Confirm that it's been successfully included before instantiating. + */ + if ( method_exists( $class, 'get_instance' ) ) { + $providers[ $provider_key ] = $class; + } else { + unset( $providers[ $provider_key ] ); + } + } + + return $providers; + } + /** * For each provider, include it and then instantiate it. * @@ -188,28 +256,13 @@ public static function get_providers() { ); } - /** - * For each filtered provider, - */ - foreach ( $providers as $provider_key => $path ) { - require_once $path; - - $class = $provider_key; - - /** - * Filters the classname for a provider. The dynamic portion of the filter is the defined providers key. - * - * @param string $class The PHP Classname of the provider. - * @param string $path The provided provider path to be included. - */ - $class = apply_filters( "two_factor_provider_classname_{$provider_key}", $class, $path ); + // Get the classes for the enabled providers. + $providers = array_intersect_key( self::get_providers_classes(), $providers ); - /** - * Confirm that it's been successfully included before instantiating. - */ - if ( class_exists( $class ) ) { + foreach ( $providers as $provider_key => $provider_class ) { + if ( method_exists( $provider_class, 'get_instance' ) ) { try { - $providers[ $provider_key ] = call_user_func( array( $class, 'get_instance' ) ); + $providers[ $provider_key ] = call_user_func( array( $provider_class, 'get_instance' ) ); } catch ( Exception $e ) { unset( $providers[ $provider_key ] ); } From 369ead9f63bf8d665c2fc3262143f2a9003e4718 Mon Sep 17 00:00:00 2001 From: Kaspars Dambis Date: Wed, 18 Sep 2024 12:00:54 +0300 Subject: [PATCH 05/14] Add the actual uninstall hook --- class-two-factor-core.php | 37 +++++++++++++++++++++++++++++++++++++ two-factor.php | 3 +++ 2 files changed, 40 insertions(+) diff --git a/class-two-factor-core.php b/class-two-factor-core.php index d9de5261..5a7a4b20 100644 --- a/class-two-factor-core.php +++ b/class-two-factor-core.php @@ -131,6 +131,43 @@ public static function add_hooks( $compat ) { $compat->init(); } + /** + * Delete all plugin data on uninstall. + * + * @return void + */ + public static function uninstall() { + // Keep this updated as user meta keys are added or removed. + $user_meta_keys = array( + self::PROVIDER_USER_META_KEY, + self::ENABLED_PROVIDERS_USER_META_KEY, + self::USER_META_NONCE_KEY, + self::USER_RATE_LIMIT_KEY, + self::USER_FAILED_LOGIN_ATTEMPTS_KEY, + self::USER_PASSWORD_WAS_RESET_KEY, + ); + + // Merge with any provider-specific user meta keys. + $user_meta_keys = array_merge( + $user_meta_keys, + self::get_providers_uninstall_user_meta_keys() + ); + + $user_ids = get_users( + array( + 'blog_id' => 0, // Return all users. + 'fields' => 'ID', + 'number' => -1, // This might take a while on larger sites but we have only one uninstall hook to run this. + ) + ); + + foreach ( $user_ids as $user_id ) { + foreach ( $user_meta_keys as $meta_key ) { + delete_user_meta( $user_id, $meta_key ); + } + } + } + /** * Get the registered providers of which some might not be enabled. * diff --git a/two-factor.php b/two-factor.php index 380ad4c2..20996b54 100644 --- a/two-factor.php +++ b/two-factor.php @@ -50,3 +50,6 @@ $two_factor_compat = new Two_Factor_Compat(); Two_Factor_Core::add_hooks( $two_factor_compat ); + +// Delete our options and user meta during uninstall. +register_uninstall_hook( __FILE__, array( Two_Factor_Core::class, 'uninstall' ) ); From 638f6d1898e9dcb0bae25dda7bfcfbe8491ee777 Mon Sep 17 00:00:00 2001 From: Kaspars Dambis Date: Wed, 18 Sep 2024 12:03:15 +0300 Subject: [PATCH 06/14] =?UTF-8?q?Move=20inline=20because=20it=20isn?= =?UTF-8?q?=E2=80=99t=20really=20used=20anywhere=20else?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- class-two-factor-core.php | 43 +++++++++++---------------------------- 1 file changed, 12 insertions(+), 31 deletions(-) diff --git a/class-two-factor-core.php b/class-two-factor-core.php index 5a7a4b20..caa18eff 100644 --- a/class-two-factor-core.php +++ b/class-two-factor-core.php @@ -148,10 +148,18 @@ public static function uninstall() { ); // Merge with any provider-specific user meta keys. - $user_meta_keys = array_merge( - $user_meta_keys, - self::get_providers_uninstall_user_meta_keys() - ); + foreach ( self::get_providers_classes() as $provider_class ) { + if ( method_exists( $provider_class, 'uninstall_user_meta_keys' ) ) { + try { + $user_meta_keys = array_merge( + $user_meta_keys, + call_user_func( array( $provider_class, 'uninstall_user_meta_keys' ) ) + ); + } catch ( Exception $e ) { + // Do nothing. + } + } + } $user_ids = get_users( array( @@ -197,33 +205,6 @@ public static function get_providers_registered() { return $providers; } - /** - * Get the user meta keys to delete when uninstalling from each provider. - * - * This attempts to get keys also from methods that are not enabled to - * ensure that all meta keys ever used are removed. - * - * @return array List of user meta keys to delete. - */ - private static function get_providers_uninstall_user_meta_keys() { - $user_meta_keys = array(); - - foreach ( self::get_providers_classes() as $provider_class ) { - if ( method_exists( $provider_class, 'uninstall_user_meta_keys' ) ) { - try { - $user_meta_keys = array_merge( - $user_meta_keys, - call_user_func( array( $provider_class, 'uninstall_user_meta_keys' ) ) - ); - } catch ( Exception $e ) { - // Do nothing. - } - } - } - - return $user_meta_keys; - } - /** * Get the classnames for all registered providers. * From f5f75d2965953fac15192a9aabe3883edb1e5ada Mon Sep 17 00:00:00 2001 From: Kaspars Dambis Date: Wed, 18 Sep 2024 12:06:25 +0300 Subject: [PATCH 07/14] Account for classes being replaced for core providers --- class-two-factor-core.php | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/class-two-factor-core.php b/class-two-factor-core.php index caa18eff..34615267 100644 --- a/class-two-factor-core.php +++ b/class-two-factor-core.php @@ -196,8 +196,9 @@ public static function get_providers_registered() { * @param array $providers A key-value array where the key is the class name, and * the value is the path to the file containing the class. */ - $additional_providers = apply_filters( 'two_factor_providers', array() ); + $additional_providers = apply_filters( 'two_factor_providers', $providers ); + // Merge them with the default providers. if ( ! empty( $additional_providers ) ) { return array_merge( $providers, $additional_providers ); } @@ -242,7 +243,7 @@ private static function get_providers_classes() { } /** - * For each provider, include it and then instantiate it. + * Get all enabled two-factor providers. * * @since 0.1-dev * From 6f50ece0c6ee39f9636dad6b02bbdcd3e411e5f1 Mon Sep 17 00:00:00 2001 From: Kaspars Dambis Date: Wed, 18 Sep 2024 12:06:43 +0300 Subject: [PATCH 08/14] Describe the mapping logic --- class-two-factor-core.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/class-two-factor-core.php b/class-two-factor-core.php index 34615267..c96a689f 100644 --- a/class-two-factor-core.php +++ b/class-two-factor-core.php @@ -275,7 +275,7 @@ public static function get_providers() { ); } - // Get the classes for the enabled providers. + // Map provider keys to classes so that we can instantiate them. $providers = array_intersect_key( self::get_providers_classes(), $providers ); foreach ( $providers as $provider_key => $provider_class ) { From 5d3b5299212cde6203bbf96178d5b5b0e860e8d0 Mon Sep 17 00:00:00 2001 From: Kaspars Dambis Date: Wed, 18 Sep 2024 12:12:31 +0300 Subject: [PATCH 09/14] Account for options too --- class-two-factor-core.php | 28 +++++++++++++++++++++++++++- 1 file changed, 27 insertions(+), 1 deletion(-) diff --git a/class-two-factor-core.php b/class-two-factor-core.php index c96a689f..575ed43b 100644 --- a/class-two-factor-core.php +++ b/class-two-factor-core.php @@ -147,8 +147,10 @@ public static function uninstall() { self::USER_PASSWORD_WAS_RESET_KEY, ); - // Merge with any provider-specific user meta keys. + $option_keys = array(); + foreach ( self::get_providers_classes() as $provider_class ) { + // Merge with provider-specific user meta keys. if ( method_exists( $provider_class, 'uninstall_user_meta_keys' ) ) { try { $user_meta_keys = array_merge( @@ -159,8 +161,32 @@ public static function uninstall() { // Do nothing. } } + + // Merge with provider-specific option keys. + if ( method_exists( $provider_class, 'uninstall_options' ) ) { + try { + $option_keys = array_merge( + $option_keys, + call_user_func( array( $provider_class, 'uninstall_options' ) ) + ); + } catch ( Exception $e ) { + // Do nothing. + } + } + } + + // Delete options first since that is faster. + if ( ! empty( $option_keys ) ) { + foreach ( $option_keys as $option_key ) { + delete_option( $option_key ); + } } + /** + * Get all user IDs to delete their user meta. + * + * Consider replacing this with a direct SQL query to speed up the process. + */ $user_ids = get_users( array( 'blog_id' => 0, // Return all users. From 9297b60a8785eee0651413da8a2c9508bca71d66 Mon Sep 17 00:00:00 2001 From: Kaspars Dambis Date: Wed, 18 Sep 2024 12:51:12 +0300 Subject: [PATCH 10/14] Report the meta keys to delete --- providers/class-two-factor-backup-codes.php | 11 +++++++++++ providers/class-two-factor-email.php | 12 ++++++++++++ providers/class-two-factor-fido-u2f.php | 12 ++++++++++++ 3 files changed, 35 insertions(+) diff --git a/providers/class-two-factor-backup-codes.php b/providers/class-two-factor-backup-codes.php index c53a8448..4044b265 100644 --- a/providers/class-two-factor-backup-codes.php +++ b/providers/class-two-factor-backup-codes.php @@ -399,4 +399,15 @@ public function delete_code( $user, $code_hashed ) { // Update the backup code master list. update_user_meta( $user->ID, self::BACKUP_CODES_META_KEY, $backup_codes ); } + + /** + * Return user meta keys to delete during plugin uninstall. + * + * @return array + */ + public static function uninstall_user_meta_keys() { + return array( + self::BACKUP_CODES_META_KEY, + ); + } } diff --git a/providers/class-two-factor-email.php b/providers/class-two-factor-email.php index ac3955ce..64ccb1dd 100644 --- a/providers/class-two-factor-email.php +++ b/providers/class-two-factor-email.php @@ -351,4 +351,16 @@ public function user_options( $user ) { Date: Wed, 18 Sep 2024 12:56:40 +0300 Subject: [PATCH 11/14] Add the missing keys --- providers/class-two-factor-fido-u2f.php | 1 + providers/class-two-factor-totp.php | 12 ++++++++++++ 2 files changed, 13 insertions(+) diff --git a/providers/class-two-factor-fido-u2f.php b/providers/class-two-factor-fido-u2f.php index d631ee64..d5ea9a8b 100644 --- a/providers/class-two-factor-fido-u2f.php +++ b/providers/class-two-factor-fido-u2f.php @@ -398,6 +398,7 @@ public static function uninstall_user_meta_keys() { return array( self::REGISTERED_KEY_USER_META_KEY, self::AUTH_DATA_USER_META_KEY, + Two_Factor_FIDO_U2F_Admin::REGISTER_DATA_USER_META_KEY, ); } } diff --git a/providers/class-two-factor-totp.php b/providers/class-two-factor-totp.php index 88f743db..7bbcf856 100644 --- a/providers/class-two-factor-totp.php +++ b/providers/class-two-factor-totp.php @@ -771,4 +771,16 @@ private static function abssort( $a, $b ) { } return ( $a < $b ) ? -1 : 1; } + + /** + * Return user meta keys to delete during plugin uninstall. + * + * @return array + */ + public static function uninstall_user_meta_keys() { + return array( + self::SECRET_META_KEY, + self::LAST_SUCCESSFUL_LOGIN_META_KEY, + ); + } } From f4d463dcfcad1f3d27133d515ccc8f4800ef2cd2 Mon Sep 17 00:00:00 2001 From: Kaspars Dambis Date: Wed, 18 Sep 2024 13:04:49 +0300 Subject: [PATCH 12/14] Pull in key name copies when no access otherwise --- providers/class-two-factor-fido-u2f.php | 2 +- providers/class-two-factor-provider.php | 2 ++ 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/providers/class-two-factor-fido-u2f.php b/providers/class-two-factor-fido-u2f.php index d5ea9a8b..f24e55e2 100644 --- a/providers/class-two-factor-fido-u2f.php +++ b/providers/class-two-factor-fido-u2f.php @@ -398,7 +398,7 @@ public static function uninstall_user_meta_keys() { return array( self::REGISTERED_KEY_USER_META_KEY, self::AUTH_DATA_USER_META_KEY, - Two_Factor_FIDO_U2F_Admin::REGISTER_DATA_USER_META_KEY, + '_two_factor_fido_u2f_register_request', // From Two_Factor_FIDO_U2F_Admin which is not loaded during uninstall. ); } } diff --git a/providers/class-two-factor-provider.php b/providers/class-two-factor-provider.php index 68610c69..0e82dd67 100644 --- a/providers/class-two-factor-provider.php +++ b/providers/class-two-factor-provider.php @@ -178,6 +178,8 @@ public static function uninstall_user_meta_keys() { /** * Return the option keys that need to be deleted on plugin uninstall. * + * Note: this method doesn't have access to the instantiated provider object. + * * @return array */ public static function uninstall_options() { From 72ee3e0f9941debef15d2854187dacda52e1ce3d Mon Sep 17 00:00:00 2001 From: Kaspars Dambis Date: Wed, 18 Sep 2024 13:57:40 +0300 Subject: [PATCH 13/14] Test the uninstall --- tests/class-two-factor-core.php | 26 ++++++++++++++++++++++++++ 1 file changed, 26 insertions(+) diff --git a/tests/class-two-factor-core.php b/tests/class-two-factor-core.php index 29c1336b..5f6a15a2 100644 --- a/tests/class-two-factor-core.php +++ b/tests/class-two-factor-core.php @@ -1555,4 +1555,30 @@ public function test_all_sessions_destroyed_when_enabling_2fa_by_admin() { // Validate that the Admin still has a session. $this->assertCount( 1, $admin_session_manager->get_all(), 'No admin sessions are present first' ); } + + /** + * Plugin uninstall removes all user meta. + * + * @covers Two_Factor_Core::uninstall + */ + public function test_uninstall_removes_user_meta() { + $user = self::factory()->user->create_and_get(); + + // Enable a provider for the user. + Two_Factor_Core::enable_provider_for_user( $user->ID, 'Two_Factor_Totp' ); + + $this->assertContains( + 'Two_Factor_Totp', + Two_Factor_Core::get_enabled_providers_for_user( $user->ID ), + 'Sample provider was enabled' + ); + + Two_Factor_Core::uninstall(); + + $this->assertNotContains( + 'Two_Factor_Totp', + Two_Factor_Core::get_enabled_providers_for_user( $user->ID ), + 'Provider was disabled due to uninstall' + ); + } } From 86bc0072b152858de46d28e43cf0138e3af4541b Mon Sep 17 00:00:00 2001 From: Kaspars Dambis Date: Wed, 18 Sep 2024 14:06:10 +0300 Subject: [PATCH 14/14] Switch to a core helper --- class-two-factor-core.php | 19 ++----------------- 1 file changed, 2 insertions(+), 17 deletions(-) diff --git a/class-two-factor-core.php b/class-two-factor-core.php index 575ed43b..fa87135e 100644 --- a/class-two-factor-core.php +++ b/class-two-factor-core.php @@ -182,23 +182,8 @@ public static function uninstall() { } } - /** - * Get all user IDs to delete their user meta. - * - * Consider replacing this with a direct SQL query to speed up the process. - */ - $user_ids = get_users( - array( - 'blog_id' => 0, // Return all users. - 'fields' => 'ID', - 'number' => -1, // This might take a while on larger sites but we have only one uninstall hook to run this. - ) - ); - - foreach ( $user_ids as $user_id ) { - foreach ( $user_meta_keys as $meta_key ) { - delete_user_meta( $user_id, $meta_key ); - } + foreach ( $user_meta_keys as $meta_key ) { + delete_metadata( 'user', null, $meta_key, null, true ); } }