diff --git a/class-two-factor-core.php b/class-two-factor-core.php index d2ae83f4..03e696d4 100644 --- a/class-two-factor-core.php +++ b/class-two-factor-core.php @@ -547,7 +547,7 @@ public static function get_enabled_providers_for_user( $user = null ) { * @see Two_Factor_Core::get_enabled_providers_for_user() * * @param int|WP_User $user Optional. User ID, or WP_User object of the the user. Defaults to current user. - * @return array List of provider instances. + * @return array|WP_Error List of provider instances, or a WP_Error if all configured providers are unavailable. */ public static function get_available_providers_for_user( $user = null ) { $user = self::fetch_user( $user ); @@ -558,6 +558,31 @@ public static function get_available_providers_for_user( $user = null ) { $providers = self::get_supported_providers_for_user( $user ); // Returns full objects. $enabled_providers = self::get_enabled_providers_for_user( $user ); // Returns just the keys. $configured_providers = array(); + $user_providers_raw = get_user_meta( $user->ID, self::ENABLED_PROVIDERS_USER_META_KEY, true ); + + /** + * If the user had enabled providers, but none of them exist currently, + * if emailed codes is available force it to be on, so that deprecated + * or removed providers don't result in the two-factor requirement being + * removed and 'failing open'. + * + * Possible enhancement: add a filter to change the fallback method? + */ + if ( empty( $enabled_providers ) && $user_providers_raw ) { + if ( isset( $providers['Two_Factor_Email'] ) ) { + // Force Emailed codes to 'on'. + $enabled_providers[] = 'Two_Factor_Email'; + } else { + return new WP_Error( + 'no_available_2fa_methods', + __( 'Error: You have Two Factor method(s) enabled, but the provider(s) no longer exist. Please contact a site administrator for assistance.', 'two-factor' ), + array( + 'user_providers_raw' => $user_providers_raw, + 'available_providers' => array_keys( $providers ), + ) + ); + } + } foreach ( $providers as $provider_key => $provider ) { if ( in_array( $provider_key, $enabled_providers, true ) && $provider->is_available_for_user( $user ) ) { @@ -596,7 +621,7 @@ public static function get_provider_for_user( $user = null, $preferred_provider if ( is_string( $preferred_provider ) ) { $providers = self::get_available_providers_for_user( $user ); - if ( isset( $providers[ $preferred_provider ] ) ) { + if ( ! is_wp_error( $providers ) && isset( $providers[ $preferred_provider ] ) ) { return $providers[ $preferred_provider ]; } } @@ -643,6 +668,9 @@ public static function get_primary_provider_for_user( $user = null ) { // If there's only one available provider, force that to be the primary. if ( 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 ) ) { $provider = key( $available_providers ); } else { @@ -935,6 +963,11 @@ public static function login_html( $user, $login_nonce, $redirect_to, $error_msg $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 ); + } + 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'; diff --git a/tests/class-two-factor-core.php b/tests/class-two-factor-core.php index b1c187a3..9899bf67 100644 --- a/tests/class-two-factor-core.php +++ b/tests/class-two-factor-core.php @@ -301,6 +301,52 @@ public function test_get_available_providers_for_user_logged_in() { wp_set_current_user( $old_user_id ); } + /** + * Verify that if a user has a non-existent provider set, that it + * swaps to email instead, rather than treating the user as having + * no methods enabled. + */ + public function test_deprecated_provider_for_user() { + $user = $this->get_dummy_user(); + + // Set the dummy user with a non-existent provider. + update_user_meta( + $user->ID, + Two_Factor_Core::ENABLED_PROVIDERS_USER_META_KEY, + array( + 'Two_Factor_Deprecated', + ) + ); + + // This should fail back to `Two_Factor_Email` then. + $this->assertEquals( + array( + 'Two_Factor_Email', + ), + array_keys( Two_Factor_Core::get_available_providers_for_user( $user ) ) + ); + + // Set the dummy user with a non-existent provider and a valid one. + update_user_meta( + $user->ID, + Two_Factor_Core::ENABLED_PROVIDERS_USER_META_KEY, + array( + 'Two_Factor_Deprecated', + 'Two_Factor_Dummy', + ) + ); + + // This time it should just strip out the invalid one, and not inject a new one. + $this->assertEquals( + array( + 'Two_Factor_Dummy', + ), + array_keys( Two_Factor_Core::get_available_providers_for_user( $user ) ) + ); + + $this->clean_dummy_user(); + } + /** * Verify primary provider for not-logged-in user. *