From 2b0d9bc964f9d9f7b86eac4eb0c4dbfb43c25c2c Mon Sep 17 00:00:00 2001 From: Ian Dunn Date: Thu, 7 Sep 2023 14:30:08 -0700 Subject: [PATCH] Add method to disable an individual provider (#587) --- class-two-factor-core.php | 37 +++++++++++++++++++++- tests/class-two-factor-core.php | 55 +++++++++++++++++++++++++++++++++ 2 files changed, 91 insertions(+), 1 deletion(-) diff --git a/class-two-factor-core.php b/class-two-factor-core.php index 7bf3c52f..4aeaa509 100644 --- 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 index a1075b23..29c1336b 100644 --- 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. *