From 143c61f28f2ae1cf3f0686c1027a9d52b4a9fefa Mon Sep 17 00:00:00 2001 From: Dan Knauss Date: Sat, 28 Mar 2026 16:43:30 -0600 Subject: [PATCH 1/5] Move rate-limit check before provider pre-processing and invalidate tokens MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Moves the rate-limit gate in process_provider() ahead of pre_process_authentication() so all providers — not just email — are blocked from resending or rotating state while rate-limited. Also invalidates provider tokens (e.g. email OTP) when the rate limit fires, preventing a captured token from remaining valid for its full TTL after brute-force detection. Closes #847 --- class-two-factor-core.php | 17 +++++++---- tests/class-two-factor-core.php | 50 +++++++++++++++++++++++++++++++++ 2 files changed, 61 insertions(+), 6 deletions(-) diff --git a/class-two-factor-core.php b/class-two-factor-core.php index 1f039866..6d4e7209 100644 --- a/class-two-factor-core.php +++ b/class-two-factor-core.php @@ -1841,18 +1841,18 @@ public static function process_provider( $provider, $user, $is_post_request ) { ); } - // Allow the provider to re-send codes, etc. - if ( true === $provider->pre_process_authentication( $user ) ) { - return false; - } - // If it's not a POST request, there's no processing to perform. if ( ! $is_post_request ) { return false; } - // Rate limit two factor authentication attempts. + // Rate limit two factor authentication attempts, including pre-processing (e.g. resend). if ( true === self::is_user_rate_limited( $user ) ) { + // Invalidate any provider token to prevent reuse after rate limiting. + if ( method_exists( $provider, 'delete_token' ) ) { + $provider->delete_token( $user->ID ); + } + $time_delay = self::get_user_time_delay( $user ); $last_login = get_user_meta( $user->ID, self::USER_RATE_LIMIT_KEY, true ); @@ -1866,6 +1866,11 @@ public static function process_provider( $provider, $user, $is_post_request ) { ); } + // Allow the provider to re-send codes, etc. + if ( true === $provider->pre_process_authentication( $user ) ) { + return false; + } + // Ask the provider to verify the second factor. if ( true !== $provider->validate_authentication( $user ) ) { // Store the last time a failed login occurred. diff --git a/tests/class-two-factor-core.php b/tests/class-two-factor-core.php index 38052106..3ad4e413 100644 --- a/tests/class-two-factor-core.php +++ b/tests/class-two-factor-core.php @@ -772,6 +772,56 @@ public function test_is_user_rate_limited() { $this->assertFalse( Two_Factor_Core::is_user_rate_limited( $user ) ); } + /** + * Test that email resend requests are blocked while rate limited. + * + * @covers Two_Factor_Core::process_provider() + */ + public function test_process_provider_blocks_email_resend_while_rate_limited() { + $user = $this->get_dummy_user( array( 'Two_Factor_Email' => 'Two_Factor_Email' ) ); + $provider = Two_Factor_Email::get_instance(); + + $provider->generate_token( $user->ID ); + $original_token = $provider->get_user_token( $user->ID ); + + update_user_meta( $user->ID, Two_Factor_Core::USER_FAILED_LOGIN_ATTEMPTS_KEY, 1 ); + update_user_meta( $user->ID, Two_Factor_Core::USER_RATE_LIMIT_KEY, time() ); + + $_REQUEST[ Two_Factor_Email::INPUT_NAME_RESEND_CODE ] = 1; + + $result = Two_Factor_Core::process_provider( $provider, $user, true ); + + unset( $_REQUEST[ Two_Factor_Email::INPUT_NAME_RESEND_CODE ] ); + + $this->assertWPError( $result ); + $this->assertSame( 'two_factor_too_fast', $result->get_error_code() ); + $this->assertFalse( $provider->get_user_token( $user->ID ), 'Token is invalidated when rate limited' ); + } + + /** + * Test that rate limiting invalidates the email token on validation attempts. + * + * @covers Two_Factor_Core::process_provider() + */ + public function test_process_provider_invalidates_email_token_when_rate_limited() { + $user = $this->get_dummy_user( array( 'Two_Factor_Email' => 'Two_Factor_Email' ) ); + $provider = Two_Factor_Email::get_instance(); + + $provider->generate_token( $user->ID ); + + $this->assertTrue( $provider->user_has_token( $user->ID ), 'Token exists before rate limiting' ); + + // Simulate a rate-limited state. + update_user_meta( $user->ID, Two_Factor_Core::USER_FAILED_LOGIN_ATTEMPTS_KEY, 3 ); + update_user_meta( $user->ID, Two_Factor_Core::USER_RATE_LIMIT_KEY, time() ); + + $result = Two_Factor_Core::process_provider( $provider, $user, true ); + + $this->assertWPError( $result ); + $this->assertSame( 'two_factor_too_fast', $result->get_error_code() ); + $this->assertFalse( $provider->user_has_token( $user->ID ), 'Token is invalidated when rate limited' ); + } + /** * Test that the "invalid login attempts have occurred" login notice works as expected. * From 72c7b354f2b07e0a10e8d012e125c2faade9ccf6 Mon Sep 17 00:00:00 2001 From: Dan Knauss Date: Sat, 28 Mar 2026 17:43:59 -0600 Subject: [PATCH 2/5] Add tests for provider switch, GET bypass, and revalidation rate limiting - Provider switch: email token is invalidated when rate-limited via failures on another provider (e.g. TOTP) - GET bypass: page reloads while rate-limited return false without deleting the token or showing errors - Revalidation: rate limiting and token invalidation apply equally to the revalidation flow --- tests/class-two-factor-core.php | 80 +++++++++++++++++++++++++++++++++ 1 file changed, 80 insertions(+) diff --git a/tests/class-two-factor-core.php b/tests/class-two-factor-core.php index 3ad4e413..533104d0 100644 --- a/tests/class-two-factor-core.php +++ b/tests/class-two-factor-core.php @@ -822,6 +822,86 @@ public function test_process_provider_invalidates_email_token_when_rate_limited( $this->assertFalse( $provider->user_has_token( $user->ID ), 'Token is invalidated when rate limited' ); } + /** + * Test that switching providers while rate-limited invalidates email token. + * + * If a user fails on TOTP triggering rate limiting, then switches back + * to email, the rate-limit gate should invalidate the email token. + * + * @covers Two_Factor_Core::process_provider() + */ + public function test_process_provider_invalidates_email_token_on_provider_switch_while_rate_limited() { + $user = $this->get_dummy_user( array( 'Two_Factor_Email' => 'Two_Factor_Email' ) ); + $email_provider = Two_Factor_Email::get_instance(); + + // Generate an email token. + $email_provider->generate_token( $user->ID ); + $this->assertTrue( $email_provider->user_has_token( $user->ID ), 'Email token exists before TOTP failures' ); + + // Simulate rate-limited state from TOTP failures. + update_user_meta( $user->ID, Two_Factor_Core::USER_FAILED_LOGIN_ATTEMPTS_KEY, 5 ); + update_user_meta( $user->ID, Two_Factor_Core::USER_RATE_LIMIT_KEY, time() ); + + // User switches back to email provider while rate-limited. + $result = Two_Factor_Core::process_provider( $email_provider, $user, true ); + + $this->assertWPError( $result ); + $this->assertSame( 'two_factor_too_fast', $result->get_error_code() ); + $this->assertFalse( $email_provider->user_has_token( $user->ID ), 'Email token is invalidated when rate-limited via another provider' ); + } + + /** + * Test that GET requests pass through without rate-limit side effects. + * + * Page reloads should not trigger rate limiting, token deletion, or + * error messages — even when the user is rate-limited. + * + * @covers Two_Factor_Core::process_provider() + */ + public function test_process_provider_get_request_bypasses_rate_limit() { + $user = $this->get_dummy_user( array( 'Two_Factor_Email' => 'Two_Factor_Email' ) ); + $provider = Two_Factor_Email::get_instance(); + + $provider->generate_token( $user->ID ); + + $this->assertTrue( $provider->user_has_token( $user->ID ), 'Token exists before GET request' ); + + // Simulate a rate-limited state. + update_user_meta( $user->ID, Two_Factor_Core::USER_FAILED_LOGIN_ATTEMPTS_KEY, 3 ); + update_user_meta( $user->ID, Two_Factor_Core::USER_RATE_LIMIT_KEY, time() ); + + // GET request (is_post_request = false). + $result = Two_Factor_Core::process_provider( $provider, $user, false ); + + $this->assertFalse( $result, 'GET request returns false, not WP_Error' ); + $this->assertTrue( $provider->user_has_token( $user->ID ), 'Token survives GET request while rate-limited' ); + } + + /** + * Test that rate limiting applies during the revalidation flow. + * + * @covers Two_Factor_Core::process_provider() + */ + public function test_process_provider_rate_limits_revalidation() { + $user = $this->get_dummy_user( array( 'Two_Factor_Email' => 'Two_Factor_Email' ) ); + $provider = Two_Factor_Email::get_instance(); + + $provider->generate_token( $user->ID ); + + $this->assertTrue( $provider->user_has_token( $user->ID ), 'Token exists before revalidation' ); + + // Simulate rate-limited state from prior failures. + update_user_meta( $user->ID, Two_Factor_Core::USER_FAILED_LOGIN_ATTEMPTS_KEY, 3 ); + update_user_meta( $user->ID, Two_Factor_Core::USER_RATE_LIMIT_KEY, time() ); + + // Revalidation is also a POST through process_provider. + $result = Two_Factor_Core::process_provider( $provider, $user, true ); + + $this->assertWPError( $result ); + $this->assertSame( 'two_factor_too_fast', $result->get_error_code() ); + $this->assertFalse( $provider->user_has_token( $user->ID ), 'Token is invalidated during rate-limited revalidation' ); + } + /** * Test that the "invalid login attempts have occurred" login notice works as expected. * From 11dad744745cb049c49b951d9989c1270f0c7a78 Mon Sep 17 00:00:00 2001 From: Dan Knauss Date: Sun, 29 Mar 2026 09:34:24 -0600 Subject: [PATCH 3/5] Remove unused variable in resend rate-limit test --- tests/class-two-factor-core.php | 1 - 1 file changed, 1 deletion(-) diff --git a/tests/class-two-factor-core.php b/tests/class-two-factor-core.php index 533104d0..ab372ef5 100644 --- a/tests/class-two-factor-core.php +++ b/tests/class-two-factor-core.php @@ -782,7 +782,6 @@ public function test_process_provider_blocks_email_resend_while_rate_limited() { $provider = Two_Factor_Email::get_instance(); $provider->generate_token( $user->ID ); - $original_token = $provider->get_user_token( $user->ID ); update_user_meta( $user->ID, Two_Factor_Core::USER_FAILED_LOGIN_ATTEMPTS_KEY, 1 ); update_user_meta( $user->ID, Two_Factor_Core::USER_RATE_LIMIT_KEY, time() ); From 7ffeb84ff905a4f9c0ef7b68b6b2f33fd70dfbaa Mon Sep 17 00:00:00 2001 From: Dan Knauss Date: Thu, 2 Jul 2026 22:12:05 -0600 Subject: [PATCH 4/5] fix: preserve email token on rate-limit to prevent re-send on form re-render Removing the delete_token() call that fired when the rate-limit gate blocked a POST. Deleting the token caused authentication_page() to immediately regenerate and email a new code on every blocked submission (called via login_html() after each WP_Error return), recreating the flooding vector the rate-limit was meant to prevent. The existing token TTL (default 15 minutes) already closes the replay window: the max rate-limit duration is also 15 minutes, so a captured token cannot outlive the lockout period without the deletion. Updates three core assertions and adds a regression test in the email provider suite that exercises the full process_provider() -> authentication_page() sequence. --- class-two-factor-core.php | 5 --- tests/class-two-factor-core.php | 8 ++--- tests/providers/class-two-factor-email.php | 42 ++++++++++++++++++++++ 3 files changed, 46 insertions(+), 9 deletions(-) diff --git a/class-two-factor-core.php b/class-two-factor-core.php index 6d4e7209..8f0ee2f4 100644 --- a/class-two-factor-core.php +++ b/class-two-factor-core.php @@ -1848,11 +1848,6 @@ public static function process_provider( $provider, $user, $is_post_request ) { // Rate limit two factor authentication attempts, including pre-processing (e.g. resend). if ( true === self::is_user_rate_limited( $user ) ) { - // Invalidate any provider token to prevent reuse after rate limiting. - if ( method_exists( $provider, 'delete_token' ) ) { - $provider->delete_token( $user->ID ); - } - $time_delay = self::get_user_time_delay( $user ); $last_login = get_user_meta( $user->ID, self::USER_RATE_LIMIT_KEY, true ); diff --git a/tests/class-two-factor-core.php b/tests/class-two-factor-core.php index ab372ef5..442310d8 100644 --- a/tests/class-two-factor-core.php +++ b/tests/class-two-factor-core.php @@ -818,11 +818,11 @@ public function test_process_provider_invalidates_email_token_when_rate_limited( $this->assertWPError( $result ); $this->assertSame( 'two_factor_too_fast', $result->get_error_code() ); - $this->assertFalse( $provider->user_has_token( $user->ID ), 'Token is invalidated when rate limited' ); + $this->assertTrue( $provider->user_has_token( $user->ID ), 'Token is preserved when rate limited to avoid triggering re-send on form re-render' ); } /** - * Test that switching providers while rate-limited invalidates email token. + * Test that switching providers while rate-limited preserves the email token. * * If a user fails on TOTP triggering rate limiting, then switches back * to email, the rate-limit gate should invalidate the email token. @@ -846,7 +846,7 @@ public function test_process_provider_invalidates_email_token_on_provider_switch $this->assertWPError( $result ); $this->assertSame( 'two_factor_too_fast', $result->get_error_code() ); - $this->assertFalse( $email_provider->user_has_token( $user->ID ), 'Email token is invalidated when rate-limited via another provider' ); + $this->assertTrue( $email_provider->user_has_token( $user->ID ), 'Email token is preserved when rate-limited via another provider' ); } /** @@ -898,7 +898,7 @@ public function test_process_provider_rate_limits_revalidation() { $this->assertWPError( $result ); $this->assertSame( 'two_factor_too_fast', $result->get_error_code() ); - $this->assertFalse( $provider->user_has_token( $user->ID ), 'Token is invalidated during rate-limited revalidation' ); + $this->assertTrue( $provider->user_has_token( $user->ID ), 'Token is preserved during rate-limited revalidation' ); } /** diff --git a/tests/providers/class-two-factor-email.php b/tests/providers/class-two-factor-email.php index 8b37a983..daaa493f 100644 --- a/tests/providers/class-two-factor-email.php +++ b/tests/providers/class-two-factor-email.php @@ -495,6 +495,48 @@ public function test_authentication_page_with_existing_token() { $this->assertStringContainsString( 'two-factor-email-code', $output ); } + /** + * Verify that a rate-limited POST does not trigger a new email when login_html() + * re-renders the form via authentication_page(). + * + * process_provider() blocks the attempt and returns WP_Error; login_html() then + * calls authentication_page() to redisplay the form. If the token were deleted by + * the rate-limit gate, authentication_page() would see no token and send a new email + * on every blocked submission — recreating the flooding vector the rate-limit prevents. + * + * @covers Two_Factor_Email::authentication_page + * @covers Two_Factor_Core::process_provider + */ + public function test_authentication_page_does_not_send_email_when_rate_limited() { + $user = self::factory()->user->create_and_get(); + Two_Factor_Core::enable_provider_for_user( $user->ID, 'Two_Factor_Email' ); + + // Generate an initial token (simulates the user already having a code). + $this->provider->generate_token( $user->ID ); + $this->assertTrue( $this->provider->user_has_token( $user->ID ) ); + + // Force the rate-limited state. + update_user_meta( $user->ID, Two_Factor_Core::USER_FAILED_LOGIN_ATTEMPTS_KEY, 3 ); + update_user_meta( $user->ID, Two_Factor_Core::USER_RATE_LIMIT_KEY, time() ); + + // Step 1: process_provider() blocks the POST (as in login_form_validate_2fa). + $result = Two_Factor_Core::process_provider( $this->provider, $user, true ); + $this->assertWPError( $result ); + $this->assertSame( 'two_factor_too_fast', $result->get_error_code() ); + + // Step 2: login_html() calls authentication_page() to re-render the form. + $emails_before = count( self::$mockmailer->mock_sent ); + ob_start(); + $this->provider->authentication_page( $user ); + ob_get_clean(); + + $this->assertCount( + $emails_before, + self::$mockmailer->mock_sent, + 'No new email must be sent when re-rendering the form after a rate-limited POST' + ); + } + /** * Verify user_options outputs the user's email address. * From 6e80092d2b4e13671af6da56b20ea99c4863414b Mon Sep 17 00:00:00 2001 From: Dan Knauss Date: Fri, 3 Jul 2026 01:18:36 -0600 Subject: [PATCH 5/5] fix(tests): update missed assertion in resend-while-rate-limited test Commit 7bbb96e updated three `user_has_token` assertions to `assertTrue` when the token-preservation behaviour was introduced, but missed one that used `get_user_token` instead. Same fix: the token is preserved when a rate-limited resend is blocked, to prevent auto-send on form re-render. --- tests/class-two-factor-core.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/class-two-factor-core.php b/tests/class-two-factor-core.php index 442310d8..70a53d7b 100644 --- a/tests/class-two-factor-core.php +++ b/tests/class-two-factor-core.php @@ -794,7 +794,7 @@ public function test_process_provider_blocks_email_resend_while_rate_limited() { $this->assertWPError( $result ); $this->assertSame( 'two_factor_too_fast', $result->get_error_code() ); - $this->assertFalse( $provider->get_user_token( $user->ID ), 'Token is invalidated when rate limited' ); + $this->assertTrue( $provider->user_has_token( $user->ID ), 'Token is preserved when resend is blocked, to prevent auto-send on form re-render' ); } /**