diff --git a/class-two-factor-core.php b/class-two-factor-core.php index 51c3cee3..d7b5ef1b 100644 --- a/class-two-factor-core.php +++ b/class-two-factor-core.php @@ -418,7 +418,7 @@ public static function get_enabled_providers_for_user( $user = null ) { * Get all Two-Factor Auth providers that are both enabled and configured for the specified|current user. * * @param int|WP_User $user Optional. User ID, or WP_User object of the the user. Defaults to current user. - * @return array + * @return array|WP_Error */ public static function get_available_providers_for_user( $user = null ) { $user = self::fetch_user( $user ); @@ -429,6 +429,31 @@ public static function get_available_providers_for_user( $user = null ) { $providers = self::get_providers(); $enabled_providers = self::get_enabled_providers_for_user( $user ); $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 ) ) { @@ -467,7 +492,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 ]; } } @@ -495,6 +520,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 { @@ -773,6 +801,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'; @@ -1689,7 +1722,14 @@ public static function manage_users_custom_column( $output, $column_name, $user_ 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 = self::get_available_providers_for_user( $user ); + + 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 ); + } + + $enabled_providers = array_keys( $available_providers ); $primary_provider = self::get_primary_provider_for_user( $user->ID ); if ( ! empty( $primary_provider ) && is_object( $primary_provider ) ) { diff --git a/tests/class-two-factor-core.php b/tests/class-two-factor-core.php index 29c1336b..63c7150b 100644 --- a/tests/class-two-factor-core.php +++ b/tests/class-two-factor-core.php @@ -295,6 +295,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. *