Skip to content

Commit b6aafe6

Browse files
committed
fix: improve rate-limit UX — calm error message and hide resend button during lockout
Addresses #918 and #920. The rate-limit error message previously read "ERROR: Too many invalid verification codes... This limit protects your account against automated attacks." For a legitimate user who mistyped their code, this is alarming and inaccurate. Replaced with a calm, actionable message that tells the user to wait and reload. The email provider's authentication_page() now checks is_user_rate_limited() before rendering the Resend Code button or the "A verification code has been sent" prompt. Showing an interactive resend button during a lockout misled users into thinking it would work; clicking it only returned the rate-limit error again. The button is hidden until the lockout expires. The rate-limit check in authentication_page() also prevents token regeneration while locked out, which complements the existing test added in the rate-limit gate PR.
1 parent 3a97559 commit b6aafe6

4 files changed

Lines changed: 84 additions & 4 deletions

File tree

class-two-factor-core.php

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1859,8 +1859,8 @@ public static function process_provider( $provider, $user, $is_post_request ) {
18591859
return new WP_Error(
18601860
'two_factor_too_fast',
18611861
sprintf(
1862-
/* translators: %s: human-readable time delay until another attempt can be made. */
1863-
__( 'ERROR: Too many invalid verification codes, you can try again in %s. This limit protects your account against automated attacks.', 'two-factor' ),
1862+
/* translators: %s: human-readable time until another attempt is allowed, e.g. "2 minutes". */
1863+
__( 'Too many incorrect verification codes. Please wait %s and reload this page to try again.', 'two-factor' ),
18641864
human_time_diff( $last_login + $time_delay )
18651865
)
18661866
);

providers/class-two-factor-email.php

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -344,8 +344,12 @@ public function authentication_page( $user ) {
344344
return;
345345
}
346346

347-
if ( ! $this->user_has_token( $user->ID ) || $this->user_token_has_expired( $user->ID ) ) {
348-
$this->generate_and_email_token( $user );
347+
$is_rate_limited = Two_Factor_Core::is_user_rate_limited( $user );
348+
349+
if ( ! $is_rate_limited ) {
350+
if ( ! $this->user_has_token( $user->ID ) || $this->user_token_has_expired( $user->ID ) ) {
351+
$this->generate_and_email_token( $user );
352+
}
349353
}
350354

351355
$token_length = $this->get_token_length();
@@ -357,7 +361,9 @@ public function authentication_page( $user ) {
357361
/** This action is documented in providers/class-two-factor-backup-codes.php */
358362
do_action( 'two_factor_before_authentication_prompt', $this );
359363
?>
364+
<?php if ( ! $is_rate_limited ) : ?>
360365
<p class="two-factor-prompt"><?php esc_html_e( 'A verification code has been sent to the email address associated with your account.', 'two-factor' ); ?></p>
366+
<?php endif; ?>
361367
<?php
362368
/** This action is documented in providers/class-two-factor-backup-codes.php */
363369
do_action( 'two_factor_after_authentication_prompt', $this );
@@ -371,9 +377,11 @@ public function authentication_page( $user ) {
371377
do_action( 'two_factor_after_authentication_input', $this );
372378
?>
373379
<?php submit_button( __( 'Verify', 'two-factor' ) ); ?>
380+
<?php if ( ! $is_rate_limited ) : ?>
374381
<p class="two-factor-email-resend">
375382
<input type="submit" class="button" name="<?php echo esc_attr( self::INPUT_NAME_RESEND_CODE ); ?>" value="<?php esc_attr_e( 'Resend Code', 'two-factor' ); ?>" />
376383
</p>
384+
<?php endif; ?>
377385
<?php wp_enqueue_script( 'two-factor-login' ); ?>
378386
<?php
379387
}

tests/class-two-factor-core.php

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2476,6 +2476,28 @@ public function test_process_provider_rate_limited() {
24762476
$this->assertSame( 'two_factor_too_fast', $result->get_error_code() );
24772477
}
24782478

2479+
/**
2480+
* Verify the rate-limit error message does not use alarming language.
2481+
*
2482+
* "ERROR:" and "automated attacks" are alarming to a legitimate user who
2483+
* simply mistyped their code. The message should be calm and actionable.
2484+
*
2485+
* @covers Two_Factor_Core::process_provider
2486+
*/
2487+
public function test_rate_limit_error_message_is_calm_and_actionable() {
2488+
$user = self::factory()->user->create_and_get();
2489+
$provider = Two_Factor_Dummy::get_instance();
2490+
2491+
update_user_meta( $user->ID, Two_Factor_Core::USER_FAILED_LOGIN_ATTEMPTS_KEY, 1 );
2492+
update_user_meta( $user->ID, Two_Factor_Core::USER_RATE_LIMIT_KEY, time() );
2493+
2494+
$result = Two_Factor_Core::process_provider( $provider, $user, true );
2495+
$message = $result->get_error_message();
2496+
2497+
$this->assertStringNotContainsString( 'ERROR:', $message, 'Rate-limit message must not use the ERROR: prefix' );
2498+
$this->assertStringNotContainsString( 'automated attacks', $message, 'Rate-limit message must not mention automated attacks' );
2499+
}
2500+
24792501
/**
24802502
* Verify process_provider() returns WP_Error and increments the failed-attempts
24812503
* counter when authentication fails.

tests/providers/class-two-factor-email.php

Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -495,6 +495,56 @@ public function test_authentication_page_with_existing_token() {
495495
$this->assertStringContainsString( 'two-factor-email-code', $output );
496496
}
497497

498+
/**
499+
* Verify the Resend Code button is hidden when the user is rate-limited.
500+
*
501+
* Showing an interactive resend button during a lockout misleads the user
502+
* into thinking it will work.
503+
*
504+
* @covers Two_Factor_Email::authentication_page
505+
*/
506+
public function test_authentication_page_hides_resend_button_when_rate_limited() {
507+
$user = self::factory()->user->create_and_get();
508+
$this->provider->generate_token( $user->ID );
509+
510+
update_user_meta( $user->ID, Two_Factor_Core::USER_FAILED_LOGIN_ATTEMPTS_KEY, 3 );
511+
update_user_meta( $user->ID, Two_Factor_Core::USER_RATE_LIMIT_KEY, time() );
512+
513+
ob_start();
514+
$this->provider->authentication_page( $user );
515+
$output = ob_get_clean();
516+
517+
$this->assertStringNotContainsString(
518+
Two_Factor_Email::INPUT_NAME_RESEND_CODE,
519+
$output,
520+
'Resend Code button must not be rendered while the user is rate-limited'
521+
);
522+
}
523+
524+
/**
525+
* Verify no email is sent when authentication_page() is called while rate-limited and no token exists.
526+
*
527+
* @covers Two_Factor_Email::authentication_page
528+
*/
529+
public function test_authentication_page_does_not_send_email_when_rate_limited_and_no_token() {
530+
$user = self::factory()->user->create_and_get();
531+
532+
update_user_meta( $user->ID, Two_Factor_Core::USER_FAILED_LOGIN_ATTEMPTS_KEY, 3 );
533+
update_user_meta( $user->ID, Two_Factor_Core::USER_RATE_LIMIT_KEY, time() );
534+
$this->assertFalse( $this->provider->user_has_token( $user->ID ) );
535+
536+
$emails_before = count( self::$mockmailer->mock_sent );
537+
ob_start();
538+
$this->provider->authentication_page( $user );
539+
ob_get_clean();
540+
541+
$this->assertCount(
542+
$emails_before,
543+
self::$mockmailer->mock_sent,
544+
'No email must be sent when authentication_page() is called while rate-limited'
545+
);
546+
}
547+
498548
/**
499549
* Verify user_options outputs the user's email address.
500550
*

0 commit comments

Comments
 (0)