Skip to content
Open
Changes from all 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
90 changes: 72 additions & 18 deletions class-two-factor-core.php
Original file line number Diff line number Diff line change
Expand Up @@ -744,7 +744,9 @@ public static function get_available_providers_for_user( $user = null ) {
*
* @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 @@ private static function get_primary_provider_key_selected_for_user( $user ) {
$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 @@ private static function get_primary_provider_key_selected_for_user( $user ) {
* @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 @@ public static function get_primary_provider_for_user( $user = null ) {
$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,6 +868,10 @@ public static function get_primary_provider_for_user( $user = null ) {
*/
public static function is_user_using_two_factor( $user = null ) {
$provider = self::get_primary_provider_for_user( $user );

// 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. WP_Error is a non-null object, so !empty() already covers it.
return ! empty( $provider );
}

Expand Down Expand Up @@ -1116,22 +1131,28 @@ public static function clear_password_reset_notice( $user ) {
*/
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( esc_html( $provider->get_error_message() ) );
}
if ( ! $provider ) {
wp_die( esc_html__( 'Two-factor provider not available for this user.', 'two-factor' ) );
}

$provider_key = $provider->get_key();
$available_providers = self::get_available_providers_for_user( $user );
$backup_providers = array_diff_key( $available_providers, array( $provider_key => null ) );
$interim_login = isset( $_REQUEST['interim-login'] ); // phpcs:ignore WordPress.Security.NonceVerification.Recommended

$rememberme = intval( self::rememberme() );

if ( 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 );
wp_die( esc_html( $available_providers->get_error_message() ) );
}

$backup_providers = array_diff_key( $available_providers, array( $provider_key => null ) );
$interim_login = isset( $_REQUEST['interim-login'] ); // phpcs:ignore WordPress.Security.NonceVerification.Recommended

$rememberme = intval( self::rememberme() );

if ( ! function_exists( 'login_header' ) ) {
// We really should migrate login_header() out of `wp-login.php` so it can be called from an includes file.
require_once TWO_FACTOR_DIR . 'includes/function.login-header.php';
Expand Down Expand Up @@ -1620,6 +1641,11 @@ public static function _login_form_validate_2fa( $user, $nonce = '', $provider =
}

$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( esc_html( $provider->get_error_message() ) );
}
if ( ! $provider ) {
wp_die( esc_html__( 'Two-factor provider not available for this user.', 'two-factor' ) );
}
Expand Down Expand Up @@ -1761,6 +1787,11 @@ public static function _login_form_revalidate_2fa( $nonce = '', $provider = '',
}

$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( esc_html( $provider->get_error_message() ) );
}
if ( ! $provider ) {
wp_die( esc_html__( 'Two-factor provider not available for this user.', 'two-factor' ) );
}
Expand Down Expand Up @@ -2086,12 +2117,20 @@ public static function manage_users_custom_column( $output, $column_name, $user_
return $output;
}

if ( ! self::is_user_using_two_factor( $user_id ) ) {
$provider = self::get_primary_provider_for_user( $user_id );

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' ) );
} else {
$provider = self::get_primary_provider_for_user( $user_id );
return esc_html( $provider->get_label() );
}

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

/**
Expand All @@ -2108,7 +2147,16 @@ public static function user_two_factor_options( $user ) {

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 +2306,12 @@ private static function render_user_providers_form( $user, $providers ) {
$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 +2471,7 @@ public static function disable_provider_for_user( $user_id, $provider_to_delete

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