Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

Delete user meta on plugin uninstall #637

Merged
merged 16 commits into from
Sep 19, 2024
Merged
Show file tree
Hide file tree
Changes from 12 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
162 changes: 139 additions & 23 deletions class-two-factor-core.php
Original file line number Diff line number Diff line change
Expand Up @@ -132,13 +132,82 @@ public static function add_hooks( $compat ) {
}

/**
* For each provider, include it and then instantiate it.
* Delete all plugin data on uninstall.
*
* @since 0.1-dev
* @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,
);

$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(
$user_meta_keys,
call_user_func( array( $provider_class, 'uninstall_user_meta_keys' ) )
);
} catch ( Exception $e ) {
// 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.
'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.
*
* @return array
* @return array List of provider keys and paths to class files.
*/
public static function get_providers() {
public static function get_providers_registered() {
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Introducing dedicated methods for retrieving:

  1. all registered two-factor providers and their files.
  2. all registered two-factor providers and their classes without instantiating them (re-used by uninstall logic).
  3. all enabled providers (a subset of registered) and their classes instantiated (used as before).

$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',
Expand All @@ -150,29 +219,29 @@ public static function get_providers() {
/**
* Filter the supplied providers.
*
* This lets third-parties either remove providers (such as Email), or
* add their own providers (such as text message or Clef).
*
* @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 );
$additional_providers = apply_filters( 'two_factor_providers', $providers );
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@kasparsd Using the same existing filter seems wrong here. Shouldn't this be a new filter so you only get the additional providers?
Right now the filter is called twice in get_providers(), see

$providers = self::get_providers_registered();
/**
* Filter the supplied providers.
*
* This lets third-parties either remove providers (such as Email), or
* add their own providers (such as text message or Clef).
*
* @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 );
.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@kasparsd This still seems wrong to me and is now released. Why running the filter twice?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ocean90 Sorry for missing your comment here. I'll create a patch to address this.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here is a pull request that removes the duplicate #651.


// FIDO U2F is PHP 5.3+ only.
if ( isset( $providers['Two_Factor_FIDO_U2F'] ) && version_compare( PHP_VERSION, '5.3.0', '<' ) ) {
unset( $providers['Two_Factor_FIDO_U2F'] );
trigger_error( // phpcs:ignore WordPress.PHP.DevelopmentFunctions.error_log_trigger_error
sprintf(
/* translators: %s: version number */
__( 'FIDO U2F is not available because you are using PHP %s. (Requires 5.3 or greater)', 'two-factor' ), // phpcs:ignore WordPress.Security.EscapeOutput.OutputNotEscaped
PHP_VERSION
)
);
// Merge them with the default providers.
if ( ! empty( $additional_providers ) ) {
return array_merge( $providers, $additional_providers );
}

/**
* For each filtered provider,
*/
return $providers;
}

/**
* 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;

Expand All @@ -189,9 +258,56 @@ public static function get_providers() {
/**
* Confirm that it's been successfully included before instantiating.
*/
if ( class_exists( $class ) ) {
if ( method_exists( $class, 'get_instance' ) ) {
$providers[ $provider_key ] = $class;
} else {
unset( $providers[ $provider_key ] );
}
}

return $providers;
}

/**
* Get all enabled two-factor providers.
*
* @since 0.1-dev
*
* @return array
*/
public static function get_providers() {
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is now much shorter as it can now get the class names for all enabled providers from the helpers.

$providers = self::get_providers_registered();

/**
* Filter the supplied providers.
*
* This lets third-parties either remove providers (such as Email), or
* add their own providers (such as text message or Clef).
*
* @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 );

// FIDO U2F is PHP 5.3+ only.
if ( isset( $providers['Two_Factor_FIDO_U2F'] ) && version_compare( PHP_VERSION, '5.3.0', '<' ) ) {
unset( $providers['Two_Factor_FIDO_U2F'] );
trigger_error( // phpcs:ignore WordPress.PHP.DevelopmentFunctions.error_log_trigger_error
sprintf(
/* translators: %s: version number */
__( 'FIDO U2F is not available because you are using PHP %s. (Requires 5.3 or greater)', 'two-factor' ), // phpcs:ignore WordPress.Security.EscapeOutput.OutputNotEscaped
PHP_VERSION
)
);
}

// 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 ) {
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 ] );
}
Expand Down
11 changes: 11 additions & 0 deletions providers/class-two-factor-backup-codes.php
Original file line number Diff line number Diff line change
Expand Up @@ -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,
);
}
}
12 changes: 12 additions & 0 deletions providers/class-two-factor-email.php
Original file line number Diff line number Diff line change
Expand Up @@ -351,4 +351,16 @@ public function user_options( $user ) {
</div>
<?php
}

/**
* Return user meta keys to delete during plugin uninstall.
*
* @return array
*/
public static function uninstall_user_meta_keys() {
return array(
self::TOKEN_META_KEY,
self::TOKEN_META_KEY_TIMESTAMP,
);
}
}
13 changes: 13 additions & 0 deletions providers/class-two-factor-fido-u2f.php
Original file line number Diff line number Diff line change
Expand Up @@ -388,4 +388,17 @@ public static function delete_security_key( $user_id, $keyHandle = null ) { // p

return true;
}

/**
* Return user meta keys to delete during plugin uninstall.
*
* @return array
*/
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_register_request', // From Two_Factor_FIDO_U2F_Admin which is not loaded during uninstall.
);
}
}
20 changes: 20 additions & 0 deletions providers/class-two-factor-provider.php
Original file line number Diff line number Diff line change
Expand Up @@ -165,4 +165,24 @@ 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() {
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These are the two methods that providers can report their user meta key usage.

return array();
}

/**
* 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() {
return array();
}
}
12 changes: 12 additions & 0 deletions providers/class-two-factor-totp.php
Original file line number Diff line number Diff line change
Expand Up @@ -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,
);
}
}
3 changes: 3 additions & 0 deletions two-factor.php
Original file line number Diff line number Diff line change
Expand Up @@ -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' ) );
Loading