Add method to disable an individual provider (#587)

iandunn · web-flow · commit 2b0d9bc964f9 · 2023-09-07T14:30:08.000-07:00
diff --git a/class-two-factor-core.php b/class-two-factor-core.php
@@ -125,7 +125,7 @@ public static function add_hooks( $compat ) {
 		add_filter( 'authenticate', array( __CLASS__, 'filter_authenticate_block_cookies' ), PHP_INT_MAX );
 
 		add_filter( 'attach_session_information', array( __CLASS__, 'filter_session_information' ), 10, 2 );
-		
+
 		add_action( 'admin_init', array( __CLASS__, 'trigger_user_settings_action' ) );
 		add_filter( 'two_factor_providers', array( __CLASS__, 'enable_dummy_method_for_debug' ) );
 
@@ -1788,6 +1788,8 @@ public static function user_two_factor_options( $user ) {
 	/**
 	 * Enable a provider for a user.
 	 *
+	 * The caller is responsible for checking the user has permission to do this.
+	 *
 	 * @param int    $user_id      The ID of the user.
 	 * @param string $new_provider The name of the provider class.
 	 *
@@ -1820,6 +1822,39 @@ public static function enable_provider_for_user( $user_id, $new_provider ) {
 		return $enabled && $has_primary;
 	}
 
+	/**
+	 * Disable a provider for a user.
+	 *
+	 * This intentionally doesn't set a new primary provider when disabling the current primary provider, because
+	 * `get_primary_provider_for_user()` will pick a new one automatically.
+	 *
+	 * The caller is responsible for checking the user has permission to do this.
+	 *
+	 * @param int    $user_id  The ID of the user.
+	 * @param string $provider The name of the provider class.
+	 *
+	 * @return bool True if the provider was disabled, false otherwise.
+	 */
+	public static function disable_provider_for_user( $user_id, $provider_to_delete ) {
+		$is_registered = array_key_exists( $provider_to_delete, self::get_providers() );
+
+		if ( ! $is_registered ) {
+			return false;
+		}
+
+		$old_enabled_providers = self::get_enabled_providers_for_user( $user_id );
+		$is_enabled            = in_array( $provider_to_delete, $old_enabled_providers );
+
+		if ( ! $is_enabled ) {
+			return true;
+		}
+
+		$new_enabled_providers     = array_diff( $old_enabled_providers, array( $provider_to_delete ) );
+		$was_disabled              = update_user_meta( $user_id, self::ENABLED_PROVIDERS_USER_META_KEY, $new_enabled_providers );
+
+		return (bool) $was_disabled;
+	}
+
 	/**
 	 * Update the user meta value.
 	 *
diff --git a/tests/class-two-factor-core.php b/tests/class-two-factor-core.php
@@ -811,6 +811,61 @@ public function test_show_password_reset_error() {
 		$this->assertStringContainsString( 'check your email for instructions on regaining access', $contents );
 	}
 
+	/**
+	 * @covers Two_Factor_Core::enable_provider_for_user()
+	 * @covers Two_Factor_Core::disable_provider_for_user()
+	 */
+	public function test_enable_disable_provider_for_user() {
+		$user              = self::factory()->user->create_and_get();
+		$enabled_providers = Two_Factor_Core::get_enabled_providers_for_user( $user->ID );
+		$this->assertEmpty( $enabled_providers );
+
+		// Disabling one that's already disabled should succeed.
+		$totp_disabled = Two_Factor_Core::disable_provider_for_user( $user->ID, 'Two_Factor_Totp' );
+		$this->assertTrue( $totp_disabled );
+
+		// Disabling one that doesn't exist should fail.
+		$nonexistent_enabled = Two_Factor_Core::enable_provider_for_user( $user->ID, 'Nonexistent_Provider' );
+		$enabled_providers = Two_Factor_Core::get_enabled_providers_for_user( $user->ID );
+		$this->assertFalse( $nonexistent_enabled );
+		$this->assertEmpty( $enabled_providers );
+		$this->assertNull( Two_Factor_Core::get_primary_provider_for_user( $user->ID ) );
+
+		// Enabling a valid one should succeed. The first one that's enabled and configured should be the default primary.
+		$totp = Two_Factor_Totp::get_instance();
+		$totp->set_user_totp_key( $user->ID, 'foo' );
+		$totp_enabled = Two_Factor_Core::enable_provider_for_user( $user->ID, 'Two_Factor_Totp' );
+		$enabled_providers = Two_Factor_Core::get_enabled_providers_for_user( $user->ID );
+		$this->assertTrue( $totp_enabled );
+		$this->assertSame( array( 'Two_Factor_Totp' ), $enabled_providers );
+		$this->assertSame( 'Two_Factor_Totp', Two_Factor_Core::get_primary_provider_for_user( $user->ID )->get_key() );
+
+		// Enabling one that's already enabled should succeed.
+		$totp_enabled = Two_Factor_Core::enable_provider_for_user( $user->ID, 'Two_Factor_Totp' );
+		$this->assertTrue( $totp_enabled );
+
+		// Enabling another should succeed, and not change the primary.
+		$dummy_enabled = Two_Factor_Core::enable_provider_for_user( $user->ID, 'Two_Factor_Dummy' );
+		$enabled_providers = Two_Factor_Core::get_enabled_providers_for_user( $user->ID );
+		$this->assertTrue( $dummy_enabled );
+		$this->assertSame( array( 'Two_Factor_Totp', 'Two_Factor_Dummy' ), $enabled_providers );
+		$this->assertSame( 'Two_Factor_Totp', Two_Factor_Core::get_primary_provider_for_user( $user->ID )->get_key() );
+
+		// Disabling one that doesn't exist should fail.
+		$nonexistent_disabled = Two_Factor_Core::disable_provider_for_user( $user->ID, 'Nonexistent_Provider' );
+		$enabled_providers = Two_Factor_Core::get_enabled_providers_for_user( $user->ID );
+		$this->assertFalse( $nonexistent_disabled );
+		$this->assertSame( array( 'Two_Factor_Totp', 'Two_Factor_Dummy' ), $enabled_providers );
+		$this->assertSame( 'Two_Factor_Totp', Two_Factor_Core::get_primary_provider_for_user( $user->ID )->get_key() );
+
+		// Disabling one that's enabled should succeed, and change the primary to the next available one.
+		$totp_disabled = Two_Factor_Core::disable_provider_for_user( $user->ID, 'Two_Factor_Totp' );
+		$enabled_providers = Two_Factor_Core::get_enabled_providers_for_user( $user->ID );
+		$this->assertTrue( $totp_disabled ); //todo enable and fix
+		$this->assertSame( array( 1 => 'Two_Factor_Dummy' ), $enabled_providers );
+		$this->assertSame( 'Two_Factor_Dummy', Two_Factor_Core::get_primary_provider_for_user( $user->ID )->get_key() );
+	}
+
 	/**
 	 * Ensure that when a user enables two factor, that they are able to continue to change settings.
 	 *
