From 8aef3df2aed851759ad75d92250584de2e819830 Mon Sep 17 00:00:00 2001 From: Kaspars Dambis Date: Fri, 10 Jan 2025 10:22:24 +0200 Subject: [PATCH 01/16] Move to auth trigger --- class-two-factor-core.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/class-two-factor-core.php b/class-two-factor-core.php index 61179cd9..a411e128 100644 --- a/class-two-factor-core.php +++ b/class-two-factor-core.php @@ -94,7 +94,6 @@ class Two_Factor_Core { */ public static function add_hooks( $compat ) { add_action( 'init', array( __CLASS__, 'get_providers' ) ); // @phpstan-ignore return.void - add_action( 'wp_login', array( __CLASS__, 'wp_login' ), 10, 2 ); add_filter( 'wp_login_errors', array( __CLASS__, 'maybe_show_reset_password_notice' ) ); add_action( 'after_password_reset', array( __CLASS__, 'clear_password_reset_notice' ) ); add_action( 'login_form_validate_2fa', array( __CLASS__, 'login_form_validate_2fa' ) ); @@ -710,6 +709,7 @@ public static function filter_authenticate_block_cookies( $user ) { * rather than through an unsupported 3rd-party login process which this plugin doesn't support. */ if ( $user instanceof WP_User && self::is_user_using_two_factor( $user->ID ) && did_action( 'login_init' ) ) { + add_action( 'wp_login', array( __CLASS__, 'wp_login' ), 10, 2 ); add_filter( 'send_auth_cookies', '__return_false', PHP_INT_MAX ); } From 0e1b2446f44bba2b932b3379cfc5bd131d388b69 Mon Sep 17 00:00:00 2001 From: Kaspars Dambis Date: Fri, 10 Jan 2025 10:28:24 +0200 Subject: [PATCH 02/16] Move to the generic authenticate filter since there we reliably know the user state --- class-two-factor-core.php | 17 +++++++++++------ 1 file changed, 11 insertions(+), 6 deletions(-) diff --git a/class-two-factor-core.php b/class-two-factor-core.php index a411e128..eed2daeb 100644 --- a/class-two-factor-core.php +++ b/class-two-factor-core.php @@ -682,11 +682,17 @@ public static function destroy_current_session_for_user( $user ) { * @return WP_User|WP_Error */ public static function filter_authenticate( $user ) { - if ( $user instanceof WP_User && self::is_api_request() && self::is_user_using_two_factor( $user->ID ) && ! self::is_user_api_login_enabled( $user->ID ) ) { - return new WP_Error( - 'invalid_application_credentials', - __( 'Error: API login for user disabled.', 'two-factor' ) - ); + if ( $user instanceof WP_User && self::is_user_using_two_factor( $user->ID ) ) { + // Trigger the second-factor flow if the password was correct. + add_action( 'wp_login', array( __CLASS__, 'wp_login' ), 10, 2 ); + + // Disable the XML-RPC and REST API for users with two-factor enabled. + if ( self::is_api_request() && ! self::is_user_api_login_enabled( $user->ID ) ) { + return new WP_Error( + 'invalid_application_credentials', + __( 'Error: API login for user disabled.', 'two-factor' ) + ); + } } return $user; @@ -709,7 +715,6 @@ public static function filter_authenticate_block_cookies( $user ) { * rather than through an unsupported 3rd-party login process which this plugin doesn't support. */ if ( $user instanceof WP_User && self::is_user_using_two_factor( $user->ID ) && did_action( 'login_init' ) ) { - add_action( 'wp_login', array( __CLASS__, 'wp_login' ), 10, 2 ); add_filter( 'send_auth_cookies', '__return_false', PHP_INT_MAX ); } From e7f1ad68c7f4e14db436774df2872f027c6bf1f5 Mon Sep 17 00:00:00 2001 From: Kaspars Dambis Date: Fri, 10 Jan 2025 10:34:20 +0200 Subject: [PATCH 03/16] Revert "Move to the generic authenticate filter since there we reliably know the user state" This reverts commit 0e1b2446f44bba2b932b3379cfc5bd131d388b69. --- class-two-factor-core.php | 17 ++++++----------- 1 file changed, 6 insertions(+), 11 deletions(-) diff --git a/class-two-factor-core.php b/class-two-factor-core.php index eed2daeb..a411e128 100644 --- a/class-two-factor-core.php +++ b/class-two-factor-core.php @@ -682,17 +682,11 @@ public static function destroy_current_session_for_user( $user ) { * @return WP_User|WP_Error */ public static function filter_authenticate( $user ) { - if ( $user instanceof WP_User && self::is_user_using_two_factor( $user->ID ) ) { - // Trigger the second-factor flow if the password was correct. - add_action( 'wp_login', array( __CLASS__, 'wp_login' ), 10, 2 ); - - // Disable the XML-RPC and REST API for users with two-factor enabled. - if ( self::is_api_request() && ! self::is_user_api_login_enabled( $user->ID ) ) { - return new WP_Error( - 'invalid_application_credentials', - __( 'Error: API login for user disabled.', 'two-factor' ) - ); - } + if ( $user instanceof WP_User && self::is_api_request() && self::is_user_using_two_factor( $user->ID ) && ! self::is_user_api_login_enabled( $user->ID ) ) { + return new WP_Error( + 'invalid_application_credentials', + __( 'Error: API login for user disabled.', 'two-factor' ) + ); } return $user; @@ -715,6 +709,7 @@ public static function filter_authenticate_block_cookies( $user ) { * rather than through an unsupported 3rd-party login process which this plugin doesn't support. */ if ( $user instanceof WP_User && self::is_user_using_two_factor( $user->ID ) && did_action( 'login_init' ) ) { + add_action( 'wp_login', array( __CLASS__, 'wp_login' ), 10, 2 ); add_filter( 'send_auth_cookies', '__return_false', PHP_INT_MAX ); } From 7a3d5edd36b89dc051baa8fff4c80e7da910f647 Mon Sep 17 00:00:00 2001 From: Kaspars Dambis Date: Fri, 10 Jan 2025 10:52:24 +0200 Subject: [PATCH 04/16] Move earlier right after the cookie check --- class-two-factor-core.php | 32 ++++++++++++++++++++++---------- 1 file changed, 22 insertions(+), 10 deletions(-) diff --git a/class-two-factor-core.php b/class-two-factor-core.php index a411e128..846cff49 100644 --- a/class-two-factor-core.php +++ b/class-two-factor-core.php @@ -116,8 +116,13 @@ public static function add_hooks( $compat ) { add_action( 'set_auth_cookie', array( __CLASS__, 'collect_auth_cookie_tokens' ) ); add_action( 'set_logged_in_cookie', array( __CLASS__, 'collect_auth_cookie_tokens' ) ); - // Run only after the core wp_authenticate_username_password() check. - add_filter( 'authenticate', array( __CLASS__, 'filter_authenticate' ), 50 ); + /** + * Enable the two-factor workflow only for login requests with username + * and without an existing user cookie. + * + * Run after core username/password and cookie checks. + */ + add_filter( 'authenticate', array( __CLASS__, 'filter_authenticate' ), 31, 3 ); // Run as late as possible to prevent other plugins from unintentionally bypassing. add_filter( 'authenticate', array( __CLASS__, 'filter_authenticate_block_cookies' ), PHP_INT_MAX ); @@ -675,18 +680,26 @@ public static function destroy_current_session_for_user( $user ) { * Prevent login through XML-RPC and REST API for users with at least one * two-factor method enabled. * - * @param WP_User|WP_Error $user Valid WP_User only if the previous filters + * @param WP_User|WP_Error $user Valid WP_User only if the previous filters * have verified and confirmed the * authentication credentials. + * @param string $username The username. + * @param string $password The password. * * @return WP_User|WP_Error */ - public static function filter_authenticate( $user ) { - if ( $user instanceof WP_User && self::is_api_request() && self::is_user_using_two_factor( $user->ID ) && ! self::is_user_api_login_enabled( $user->ID ) ) { - return new WP_Error( - 'invalid_application_credentials', - __( 'Error: API login for user disabled.', 'two-factor' ) - ); + public static function filter_authenticate( $user, $username, $password ) { + if ( ! empty( $username ) && $user instanceof WP_User && self::is_user_using_two_factor( $user->ID ) ) { + // Disable authentication requests for API requests for users with two-factor enabled. + if ( self::is_api_request() && ! self::is_user_api_login_enabled( $user->ID ) ) { + return new WP_Error( + 'invalid_application_credentials', + __( 'Error: API login for user disabled.', 'two-factor' ) + ); + } + + // Trigger the second-factor flow for valid users. + add_action( 'wp_login', array( __CLASS__, 'wp_login' ), 10, 2 ); } return $user; @@ -709,7 +722,6 @@ public static function filter_authenticate_block_cookies( $user ) { * rather than through an unsupported 3rd-party login process which this plugin doesn't support. */ if ( $user instanceof WP_User && self::is_user_using_two_factor( $user->ID ) && did_action( 'login_init' ) ) { - add_action( 'wp_login', array( __CLASS__, 'wp_login' ), 10, 2 ); add_filter( 'send_auth_cookies', '__return_false', PHP_INT_MAX ); } From 0de50dfaa642f8c4a77542e228e881d7fe6974dc Mon Sep 17 00:00:00 2001 From: Kaspars Dambis Date: Fri, 10 Jan 2025 10:53:58 +0200 Subject: [PATCH 05/16] =?UTF-8?q?Fire=20super=20late=20since=20we=E2=80=99?= =?UTF-8?q?re=20replacing=20the=20rest=20of=20login=20flow?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- class-two-factor-core.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/class-two-factor-core.php b/class-two-factor-core.php index 846cff49..3b01913c 100644 --- a/class-two-factor-core.php +++ b/class-two-factor-core.php @@ -699,7 +699,7 @@ public static function filter_authenticate( $user, $username, $password ) { } // Trigger the second-factor flow for valid users. - add_action( 'wp_login', array( __CLASS__, 'wp_login' ), 10, 2 ); + add_action( 'wp_login', array( __CLASS__, 'wp_login' ), PHP_INT_MAX, 2 ); } return $user; From 76192e6703e0e6f9112ded20d131491a63f313e8 Mon Sep 17 00:00:00 2001 From: Kaspars Dambis Date: Fri, 10 Jan 2025 11:03:31 +0200 Subject: [PATCH 06/16] Pass the missing args --- tests/class-two-factor-core.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/class-two-factor-core.php b/tests/class-two-factor-core.php index 992ddc60..e6086aff 100644 --- a/tests/class-two-factor-core.php +++ b/tests/class-two-factor-core.php @@ -397,12 +397,12 @@ public function test_filter_authenticate() { $this->assertInstanceOf( 'WP_User', - Two_Factor_Core::filter_authenticate( $user_default ) + Two_Factor_Core::filter_authenticate( $user_default, 'username', 'password' ) ); $this->assertInstanceOf( 'WP_Error', - Two_Factor_Core::filter_authenticate( $user_2fa_enabled ) + Two_Factor_Core::filter_authenticate( $user_2fa_enabled, 'username', 'password' ) ); } From 4eafeed21840240cbd4c8305f3549017b994106e Mon Sep 17 00:00:00 2001 From: Kaspars Dambis Date: Fri, 10 Jan 2025 11:03:41 +0200 Subject: [PATCH 07/16] Add a test case for existing user session --- tests/class-two-factor-core.php | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/tests/class-two-factor-core.php b/tests/class-two-factor-core.php index e6086aff..d4ed0d68 100644 --- a/tests/class-two-factor-core.php +++ b/tests/class-two-factor-core.php @@ -404,6 +404,12 @@ public function test_filter_authenticate() { 'WP_Error', Two_Factor_Core::filter_authenticate( $user_2fa_enabled, 'username', 'password' ) ); + + $this->assertInstanceOf( + 'WP_User', + Two_Factor_Core::filter_authenticate( $user_2fa_enabled, '', null ), + 'Existing user session without a username should not trigger 2FA' + ); } /** From 315cf267c6d56fd69ac673b6d26b3610be548aeb Mon Sep 17 00:00:00 2001 From: Kaspars Dambis Date: Fri, 10 Jan 2025 11:05:57 +0200 Subject: [PATCH 08/16] Break out for readability --- class-two-factor-core.php | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/class-two-factor-core.php b/class-two-factor-core.php index 3b01913c..ec7cab22 100644 --- a/class-two-factor-core.php +++ b/class-two-factor-core.php @@ -689,16 +689,16 @@ public static function destroy_current_session_for_user( $user ) { * @return WP_User|WP_Error */ public static function filter_authenticate( $user, $username, $password ) { - if ( ! empty( $username ) && $user instanceof WP_User && self::is_user_using_two_factor( $user->ID ) ) { - // Disable authentication requests for API requests for users with two-factor enabled. - if ( self::is_api_request() && ! self::is_user_api_login_enabled( $user->ID ) ) { - return new WP_Error( - 'invalid_application_credentials', - __( 'Error: API login for user disabled.', 'two-factor' ) - ); - } + // Disable authentication requests for API requests for users with two-factor enabled. + if ( $user instanceof WP_User && self::is_user_using_two_factor( $user->ID ) && self::is_api_request() && ! self::is_user_api_login_enabled( $user->ID ) ) { + return new WP_Error( + 'invalid_application_credentials', + __( 'Error: API login for user disabled.', 'two-factor' ) + ); + } - // Trigger the second-factor flow for valid users. + // Trigger the second-factor flow only for login attempts. + if ( ! empty( $username ) && $user instanceof WP_User ) { add_action( 'wp_login', array( __CLASS__, 'wp_login' ), PHP_INT_MAX, 2 ); } From 4c2345be8e13a708d707981f61ccd2af1a7ea61a Mon Sep 17 00:00:00 2001 From: Kaspars Dambis Date: Fri, 10 Jan 2025 11:06:24 +0200 Subject: [PATCH 09/16] =?UTF-8?q?Account=20for=20username=20=E2=80=9C0?= =?UTF-8?q?=E2=80=9D?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- class-two-factor-core.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/class-two-factor-core.php b/class-two-factor-core.php index ec7cab22..5a9bf38f 100644 --- a/class-two-factor-core.php +++ b/class-two-factor-core.php @@ -698,7 +698,7 @@ public static function filter_authenticate( $user, $username, $password ) { } // Trigger the second-factor flow only for login attempts. - if ( ! empty( $username ) && $user instanceof WP_User ) { + if ( strlen( $username ) && $user instanceof WP_User ) { add_action( 'wp_login', array( __CLASS__, 'wp_login' ), PHP_INT_MAX, 2 ); } From 2a713ea15ff6cc538c6baaca198a831b0977dc1b Mon Sep 17 00:00:00 2001 From: Kaspars Dambis Date: Fri, 10 Jan 2025 11:13:16 +0200 Subject: [PATCH 10/16] Use positive lookup for readability --- class-two-factor-core.php | 21 +++++++++++---------- 1 file changed, 11 insertions(+), 10 deletions(-) diff --git a/class-two-factor-core.php b/class-two-factor-core.php index 5a9bf38f..c24f1320 100644 --- a/class-two-factor-core.php +++ b/class-two-factor-core.php @@ -688,17 +688,18 @@ public static function destroy_current_session_for_user( $user ) { * * @return WP_User|WP_Error */ - public static function filter_authenticate( $user, $username, $password ) { - // Disable authentication requests for API requests for users with two-factor enabled. - if ( $user instanceof WP_User && self::is_user_using_two_factor( $user->ID ) && self::is_api_request() && ! self::is_user_api_login_enabled( $user->ID ) ) { - return new WP_Error( - 'invalid_application_credentials', - __( 'Error: API login for user disabled.', 'two-factor' ) - ); - } - - // Trigger the second-factor flow only for login attempts. + public static function filter_authenticate( $user, $username ) { + // Trigger the two-factor workflow only for login attempts and non-existent user sessions. if ( strlen( $username ) && $user instanceof WP_User ) { + // Disable authentication requests for API requests for users with two-factor enabled. + if ( self::is_api_request() && self::is_user_using_two_factor( $user->ID ) && ! self::is_user_api_login_enabled( $user->ID ) ) { + return new WP_Error( + 'invalid_application_credentials', + __( 'Error: API login for user disabled.', 'two-factor' ) + ); + } + + // Trigger the second-factor flow only for login attempts. add_action( 'wp_login', array( __CLASS__, 'wp_login' ), PHP_INT_MAX, 2 ); } From b9c200dfb67e55018f3dc394e49dc0ace3abe6e7 Mon Sep 17 00:00:00 2001 From: Kaspars Dambis Date: Fri, 10 Jan 2025 11:13:38 +0200 Subject: [PATCH 11/16] Describe tests for easier lookup during fails --- tests/class-two-factor-core.php | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/tests/class-two-factor-core.php b/tests/class-two-factor-core.php index d4ed0d68..0de19857 100644 --- a/tests/class-two-factor-core.php +++ b/tests/class-two-factor-core.php @@ -397,12 +397,14 @@ public function test_filter_authenticate() { $this->assertInstanceOf( 'WP_User', - Two_Factor_Core::filter_authenticate( $user_default, 'username', 'password' ) + Two_Factor_Core::filter_authenticate( $user_default, 'username', 'password' ), + 'Non-2FA user should be able to authenticate during API requests' ); $this->assertInstanceOf( 'WP_Error', - Two_Factor_Core::filter_authenticate( $user_2fa_enabled, 'username', 'password' ) + Two_Factor_Core::filter_authenticate( $user_2fa_enabled, 'username', 'password' ), + '2FA user should not be able to authenticate during API requests' ); $this->assertInstanceOf( From 7cb84f5a739e4a32d827f324ed5f328ebbe396e7 Mon Sep 17 00:00:00 2001 From: Kaspars Dambis Date: Fri, 10 Jan 2025 11:14:39 +0200 Subject: [PATCH 12/16] Test during API requests --- tests/class-two-factor-core.php | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/tests/class-two-factor-core.php b/tests/class-two-factor-core.php index 0de19857..4fd11d96 100644 --- a/tests/class-two-factor-core.php +++ b/tests/class-two-factor-core.php @@ -388,13 +388,15 @@ public function test_is_api_request() { * * @covers Two_Factor_Core::filter_authenticate */ - public function test_filter_authenticate() { + public function test_filter_authenticate_api() { $user_default = new WP_User( self::factory()->user->create() ); $user_2fa_enabled = $this->get_dummy_user(); // User with a dummy two-factor method enabled. // TODO: Get Two_Factor_Core away from static methods to allow mocking this. define( 'XMLRPC_REQUEST', true ); + $this->assertTrue( Two_Factor_Core::is_api_request(), 'Can detect an API request' ); + $this->assertInstanceOf( 'WP_User', Two_Factor_Core::filter_authenticate( $user_default, 'username', 'password' ), From cf0852bb2e741c56990025456e628de7d2c49791 Mon Sep 17 00:00:00 2001 From: Kaspars Dambis Date: Fri, 10 Jan 2025 11:24:23 +0200 Subject: [PATCH 13/16] Move the description to the main docblock --- class-two-factor-core.php | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/class-two-factor-core.php b/class-two-factor-core.php index c24f1320..cacf9f9f 100644 --- a/class-two-factor-core.php +++ b/class-two-factor-core.php @@ -677,8 +677,9 @@ public static function destroy_current_session_for_user( $user ) { } /** - * Prevent login through XML-RPC and REST API for users with at least one - * two-factor method enabled. + * Trigget the two-factor workflow only for valid login attempts + * with username present. Prevent authentication during API requests + * unless explicitly enabled for the user (disabled by default). * * @param WP_User|WP_Error $user Valid WP_User only if the previous filters * have verified and confirmed the @@ -689,17 +690,16 @@ public static function destroy_current_session_for_user( $user ) { * @return WP_User|WP_Error */ public static function filter_authenticate( $user, $username ) { - // Trigger the two-factor workflow only for login attempts and non-existent user sessions. - if ( strlen( $username ) && $user instanceof WP_User ) { + if ( strlen( $username ) && $user instanceof WP_User && self::is_user_using_two_factor( $user->ID ) ) { // Disable authentication requests for API requests for users with two-factor enabled. - if ( self::is_api_request() && self::is_user_using_two_factor( $user->ID ) && ! self::is_user_api_login_enabled( $user->ID ) ) { + if ( self::is_api_request() && ! self::is_user_api_login_enabled( $user->ID ) ) { return new WP_Error( 'invalid_application_credentials', __( 'Error: API login for user disabled.', 'two-factor' ) ); } - // Trigger the second-factor flow only for login attempts. + // Trigger the two-factor flow only for login attempts. add_action( 'wp_login', array( __CLASS__, 'wp_login' ), PHP_INT_MAX, 2 ); } From 2dd1c2f12235a2191a71b43431bec48e59d18434 Mon Sep 17 00:00:00 2001 From: Kaspars Dambis Date: Fri, 10 Jan 2025 11:29:54 +0200 Subject: [PATCH 14/16] Test the base flow --- tests/class-two-factor-core.php | 47 +++++++++++++++++++++++++++++++++ 1 file changed, 47 insertions(+) diff --git a/tests/class-two-factor-core.php b/tests/class-two-factor-core.php index 4fd11d96..5c02c689 100644 --- a/tests/class-two-factor-core.php +++ b/tests/class-two-factor-core.php @@ -388,6 +388,53 @@ public function test_is_api_request() { * * @covers Two_Factor_Core::filter_authenticate */ + public function test_filter_authenticate() { + $user_default = new WP_User( self::factory()->user->create() ); + $user_2fa_enabled = $this->get_dummy_user(); // User with a dummy two-factor method enabled. + + $this->assertFalse( Two_Factor_Core::is_api_request(), 'Is not an API request by default' ); + + $this->assertInstanceOf( + 'WP_User', + Two_Factor_Core::filter_authenticate( $user_default, '', '' ), + 'Existing non-2FA user session should not trigger 2FA' + ); + + $this->assertInstanceOf( + 'WP_User', + Two_Factor_Core::filter_authenticate( $user_default, 'username', '' ), + 'Existing non-2FA user login attempts should not trigger 2FA' + ); + + $this->assertInstanceOf( + 'WP_User', + Two_Factor_Core::filter_authenticate( $user_2fa_enabled, '', '' ), + 'Existing 2FA user sessions should not trigger 2FA' + ); + + $this->assertFalse( + has_action( 'wp_login', array( 'Two_Factor_Core', 'wp_login' ) ), + 'Requests with existing user sessions should not trigger the two-factor flow' + ); + + $this->assertInstanceOf( + 'WP_User', + Two_Factor_Core::filter_authenticate( $user_2fa_enabled, 'user-name', 'password' ), + 'Existing 2FA user session with username present should forward the user' + ); + + $this->assertNotFalse( + has_action( 'wp_login', array( 'Two_Factor_Core', 'wp_login' ) ), + 'Existing 2FA user session with username present should trigger two-factor flow' + ); + } + + /** + * Verify authentication filters. + * + * @covers Two_Factor_Core::filter_authenticate + * @covers Two_Factor_Core::is_api_request + */ public function test_filter_authenticate_api() { $user_default = new WP_User( self::factory()->user->create() ); $user_2fa_enabled = $this->get_dummy_user(); // User with a dummy two-factor method enabled. From 89b682901a7a9ca657c22636ef467f70cc82b662 Mon Sep 17 00:00:00 2001 From: Kaspars Dambis Date: Fri, 10 Jan 2025 11:34:26 +0200 Subject: [PATCH 15/16] Add back the password as the hook will provide it --- class-two-factor-core.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/class-two-factor-core.php b/class-two-factor-core.php index cacf9f9f..8febda05 100644 --- a/class-two-factor-core.php +++ b/class-two-factor-core.php @@ -689,7 +689,7 @@ public static function destroy_current_session_for_user( $user ) { * * @return WP_User|WP_Error */ - public static function filter_authenticate( $user, $username ) { + public static function filter_authenticate( $user, $username, $password ) { if ( strlen( $username ) && $user instanceof WP_User && self::is_user_using_two_factor( $user->ID ) ) { // Disable authentication requests for API requests for users with two-factor enabled. if ( self::is_api_request() && ! self::is_user_api_login_enabled( $user->ID ) ) { From a096066ef15d4582283651599ee279cdf44cd8a9 Mon Sep 17 00:00:00 2001 From: Kaspars Dambis Date: Fri, 10 Jan 2025 18:11:51 +0200 Subject: [PATCH 16/16] =?UTF-8?q?Skip=20the=20cookies=20when=20we=20know?= =?UTF-8?q?=20we=E2=80=99re=20doing=20the=20two-factor=20dance?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- class-two-factor-core.php | 31 ++++--------------------------- 1 file changed, 4 insertions(+), 27 deletions(-) diff --git a/class-two-factor-core.php b/class-two-factor-core.php index 8febda05..2f140a97 100644 --- a/class-two-factor-core.php +++ b/class-two-factor-core.php @@ -124,9 +124,6 @@ public static function add_hooks( $compat ) { */ add_filter( 'authenticate', array( __CLASS__, 'filter_authenticate' ), 31, 3 ); - // Run as late as possible to prevent other plugins from unintentionally bypassing. - 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' ) ); @@ -699,6 +696,9 @@ public static function filter_authenticate( $user, $username, $password ) { ); } + // Disable core auth cookies because we must send them manually once the 2nd factor has been verified. + add_filter( 'send_auth_cookies', '__return_false', PHP_INT_MAX ); + // Trigger the two-factor flow only for login attempts. add_action( 'wp_login', array( __CLASS__, 'wp_login' ), PHP_INT_MAX, 2 ); } @@ -706,29 +706,6 @@ public static function filter_authenticate( $user, $username, $password ) { return $user; } - /** - * Prevent login cookies being set on login for Two Factor users. - * - * This makes it so that Core never sends the auth cookies. `login_form_validate_2fa()` will send them manually once the 2nd factor has been verified. - * - * @param WP_User|WP_Error $user Valid WP_User only if the previous filters - * have verified and confirmed the - * authentication credentials. - * - * @return WP_User|WP_Error - */ - public static function filter_authenticate_block_cookies( $user ) { - /* - * NOTE: The `login_init` action is checked for here to ensure we're within the regular login flow, - * rather than through an unsupported 3rd-party login process which this plugin doesn't support. - */ - if ( $user instanceof WP_User && self::is_user_using_two_factor( $user->ID ) && did_action( 'login_init' ) ) { - add_filter( 'send_auth_cookies', '__return_false', PHP_INT_MAX ); - } - - return $user; - } - /** * If the current user can login via API requests such as XML-RPC and REST. * @@ -1403,7 +1380,7 @@ public static function _login_form_validate_2fa( $user, $nonce = '', $provider = /* * NOTE: This filter removal is not normally required, this is included for protection against * a plugin/two factor provider which runs the `authenticate` filter during it's validation. - * Such a plugin would cause self::filter_authenticate_block_cookies() to run and add this filter. + * Such a plugin would cause self::filter_authenticate() to run and add this filter. */ remove_filter( 'send_auth_cookies', '__return_false', PHP_INT_MAX );