Skip to content

Fix: Users list table fatals (wp_die) when a user's 2FA provider is deregistered #933

New issue

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

Sign up for GitHub

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

Already on GitHub? Sign in to your account

Open
masteradhoc wants to merge 3 commits into
base: master
Choose a base branch
from
+72 −18
Open

Fix: Users list table fatals (wp_die) when a user's 2FA provider is deregistered #933

Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
3242ee9
update wp_die feature
masteradhoc Jul 22, 2026
fd85c8a
Update class-two-factor-core.php
masteradhoc Jul 22, 2026
4fde20d
Fix WP_Error guard order and linter issues
masteradhoc Jul 23, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
83 changes: 70 additions & 13 deletions class-two-factor-core.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -744,7 +744,9 @@
*
* @param int|WP_User $user Optional. User ID, or WP_User object of the the user. Defaults to current user.
* @param null|string|object $preferred_provider Optional. The name of the provider, the provider, or empty.
* @return null|object The provider
* @return null|object|WP_Error The provider, null if none is available, or a WP_Error if the user has
* provider(s) enabled that are no longer registered (see
* Two_Factor_Core::get_primary_provider_for_user()).
*/
public static function get_provider_for_user( $user = null, $preferred_provider = null ) {
$user = self::fetch_user( $user );
Expand Down Expand Up @@ -789,6 +791,10 @@
$primary_provider = get_user_meta( $user->ID, self::PROVIDER_USER_META_KEY, true );
$available_providers = self::get_available_providers_for_user( $user );

if ( is_wp_error( $available_providers ) ) {
return null;
}

if ( ! empty( $primary_provider ) && ! empty( $available_providers[ $primary_provider ] ) ) {
return $primary_provider;
}
Expand All @@ -802,7 +808,10 @@
* @since 0.2.0
*
* @param int|WP_User $user Optional. User ID, or WP_User object of the the user. Defaults to current user.
* @return object|null
* @return object|null|WP_Error Provider instance, null if the user has none configured, or a WP_Error if the
* user has provider(s) enabled that are no longer registered. Callers that render
* shared admin UI (e.g. list tables) must not `wp_die()` on the WP_Error case, since
* that would break the page for everyone, not just the affected user.
*/
public static function get_primary_provider_for_user( $user = null ) {
$user = self::fetch_user( $user );
Expand All @@ -813,13 +822,15 @@
$providers = self::get_supported_providers_for_user( $user );
$available_providers = self::get_available_providers_for_user( $user );

// If there's only one available provider, force that to be the primary.
if ( empty( $available_providers ) ) {
if ( is_wp_error( $available_providers ) ) {
// The user's configured methods don't exist, and there was no replacement to swap in. Bubble the
// error up instead of dying here — this can run from contexts (like the Users list table) where
// killing the whole request would break the page for an admin who isn't even the affected user.
return $available_providers;
} elseif ( empty( $available_providers ) ) {
return null;
} elseif ( is_wp_error( $available_providers ) ) {
// If it returned an error, the configured methods don't exist, and it couldn't swap in a replacement.
wp_die( $available_providers );
} elseif ( 1 === count( $available_providers ) ) {
// If there's only one available provider, force that to be the primary.
$provider = key( $available_providers );
} else {
$provider = self::get_primary_provider_key_selected_for_user( $user );
Expand Down Expand Up @@ -857,7 +868,11 @@
*/
public static function is_user_using_two_factor( $user = null ) {
$provider = self::get_primary_provider_for_user( $user );
return ! empty( $provider );

// A WP_Error means the user has a provider enabled that's no longer registered. Still treat them as
// "using" two-factor so the login requirement isn't dropped (failing open) just because their specific
// method disappeared.
return ! empty( $provider ) || is_wp_error( $provider );

Check failure on line 875 in class-two-factor-core.php

View workflow job for this annotation

GitHub Actions / Lint PHP & PHP Compatibility checks. 

is_wp_error(null) will always evaluate to false.
}

/**
Expand Down Expand Up @@ -1116,6 +1131,11 @@
*/
public static function login_html( $user, $login_nonce, $redirect_to, $error_msg = '', $provider = null, $action = 'validate_2fa' ) {
$provider = self::get_provider_for_user( $user, $provider );
if ( is_wp_error( $provider ) ) {
// The user's configured methods don't exist, and there was no replacement to swap in. This is the
// user's own login screen, so it's appropriate to stop here with a specific, actionable message.
wp_die( $provider );
}
if ( ! $provider ) {
wp_die( esc_html__( 'Two-factor provider not available for this user.', 'two-factor' ) );
}
Expand Down Expand Up @@ -1620,6 +1640,11 @@
}

$provider = self::get_provider_for_user( $user, $provider );
if ( is_wp_error( $provider ) ) {
// The user's configured methods don't exist, and there was no replacement to swap in. This is the
// user's own login attempt, so it's appropriate to stop here with a specific, actionable message.
wp_die( $provider );
}
if ( ! $provider ) {
wp_die( esc_html__( 'Two-factor provider not available for this user.', 'two-factor' ) );
}
Expand Down Expand Up @@ -1761,6 +1786,11 @@
}

$provider = self::get_provider_for_user( $user, $provider );
if ( is_wp_error( $provider ) ) {
// The user's configured methods don't exist, and there was no replacement to swap in. This is the
// user's own session revalidation, so it's appropriate to stop here with a specific, actionable message.
wp_die( $provider );
}
if ( ! $provider ) {
wp_die( esc_html__( 'Two-factor provider not available for this user.', 'two-factor' ) );
}
Expand Down Expand Up @@ -2088,10 +2118,22 @@

if ( ! self::is_user_using_two_factor( $user_id ) ) {
return sprintf( '<span class="dashicons-before dashicons-no-alt">%s</span>', esc_html__( 'Disabled', 'two-factor' ) );
} else {
$provider = self::get_primary_provider_for_user( $user_id );
return esc_html( $provider->get_label() );
}

$provider = self::get_primary_provider_for_user( $user_id );
Comment thread
masteradhoc marked this conversation as resolved.
Outdated

if ( is_wp_error( $provider ) ) {
// The user has a provider enabled that's no longer registered on the site. Show a clear,
// non-fatal indicator instead of erroring out — this must never wp_die(), since that would
// truncate the Users list table for every admin viewing the page, not just this one user's row.
return sprintf( '<span class="dashicons-before dashicons-warning">%s</span>', esc_html__( 'Error: legacy 2FA method', 'two-factor' ) );
}
Comment on lines +2122 to +2127

if ( ! $provider ) {
return sprintf( '<span class="dashicons-before dashicons-no-alt">%s</span>', esc_html__( 'Disabled', 'two-factor' ) );
}

return esc_html( $provider->get_label() );
}

/**
Expand All @@ -2108,7 +2150,16 @@

wp_enqueue_style( 'user-edit-2fa', plugins_url( 'user-edit.css', __FILE__ ), array(), TWO_FACTOR_VERSION );

$enabled_providers = array_keys( self::get_available_providers_for_user( $user ) );
$available_providers_or_error = self::get_available_providers_for_user( $user );

if ( is_wp_error( $available_providers_or_error ) ) {
// The user has provider(s) enabled that are no longer registered on the site. Surface the existing
// admin-contact message on their own profile screen (where they can act on it) rather than crashing.
self::add_error( $available_providers_or_error );
$enabled_providers = array();
} else {
$enabled_providers = array_keys( $available_providers_or_error );
}

// This is specific to the current session, not the displayed user.
$show_2fa_options = self::current_user_can_update_two_factor_options();
Expand Down Expand Up @@ -2258,6 +2309,12 @@
$available_providers = self::get_available_providers_for_user( $user );
$recommended_provider_keys = self::get_recommended_providers( $user );

if ( is_wp_error( $available_providers ) ) {
// Already surfaced via self::add_error() in user_two_factor_options(); avoid treating the WP_Error
// as an array of providers here.
$available_providers = array();
}

// Move the recommended providers first.
$recommended_providers = array_intersect_key( $providers, array_flip( $recommended_provider_keys ) );
$providers = array_merge( $recommended_providers, $providers );
Expand Down Expand Up @@ -2417,7 +2474,7 @@

// Remove this from being a primary provider, if set.
$primary_provider = self::get_primary_provider_for_user( $user_id );
if ( $primary_provider && $primary_provider->get_key() === $provider_to_delete ) {
if ( $primary_provider && ! is_wp_error( $primary_provider ) && $primary_provider->get_key() === $provider_to_delete ) {
delete_user_meta( $user_id, self::PROVIDER_USER_META_KEY );
}

Expand Down
Loading