diff --git a/class-two-factor-core.php b/class-two-factor-core.php index 1f039866..bc6fa310 100644 --- a/class-two-factor-core.php +++ b/class-two-factor-core.php @@ -1859,8 +1859,8 @@ public static function process_provider( $provider, $user, $is_post_request ) { return new WP_Error( 'two_factor_too_fast', sprintf( - /* translators: %s: human-readable time delay until another attempt can be made. */ - __( 'ERROR: Too many invalid verification codes, you can try again in %s. This limit protects your account against automated attacks.', 'two-factor' ), + /* translators: %s: human-readable time until another attempt is allowed, e.g. "2 minutes". */ + __( 'Too many incorrect verification codes. Please wait %s and reload this page to try again.', 'two-factor' ), human_time_diff( $last_login + $time_delay ) ) ); diff --git a/package-lock.json b/package-lock.json index 5a7b2a11..1fbfd1f9 100644 --- a/package-lock.json +++ b/package-lock.json @@ -8351,9 +8351,9 @@ } }, "node_modules/basic-ftp": { - "version": "5.2.2", - "resolved": "https://registry.npmjs.org/basic-ftp/-/basic-ftp-5.2.2.tgz", - "integrity": "sha512-1tDrzKsdCg70WGvbFss/ulVAxupNauGnOlgpyjKzeQxzyllBLS0CGLV7tjIXTK3ZQA9/FBEm9qyFFN1bciA6pw==", + "version": "5.3.0", + "resolved": "https://registry.npmjs.org/basic-ftp/-/basic-ftp-5.3.0.tgz", + "integrity": "sha512-5K9eNNn7ywHPsYnFwjKgYH8Hf8B5emh7JKcPaVjjrMJFQQwGpwowEnZNEtHs7DfR7hCZsmaK3VA4HUK0YarT+w==", "dev": true, "license": "MIT", "engines": { diff --git a/playground/blueprint.json b/playground/blueprint.json new file mode 100644 index 00000000..92741026 --- /dev/null +++ b/playground/blueprint.json @@ -0,0 +1,20 @@ +{ + "$schema": "https://playground.wordpress.net/blueprint-schema.json", + "preferredVersions": { "php": "8.2", "wp": "latest" }, + "landingPage": "/wp-login.php?action=two_factor_demo", + "steps": [ + { + "step": "installPlugin", + "pluginData": { + "resource": "url", + "url": "https://wordpress-playground-cors-proxy.net/?https://github.com/dknauss/two-factor/archive/refs/heads/ux/rate-limit-messaging.zip" + }, + "options": { "activate": true, "targetFolderName": "two-factor" } + }, + { + "step": "writeFile", + "path": "/wordpress/wp-content/mu-plugins/two-factor-demo.php", + "data": "ID, '_two_factor_provider', 'Two_Factor_Email' );\n\tupdate_user_meta( $user->ID, '_two_factor_enabled_providers', array( 'Two_Factor_Email' ) );\n\tupdate_user_meta( $user->ID, Two_Factor_Core::USER_FAILED_LOGIN_ATTEMPTS_KEY, 4 );\n\tupdate_user_meta( $user->ID, Two_Factor_Core::USER_RATE_LIMIT_KEY, time() - 1 );\n\t$login_nonce = Two_Factor_Core::create_login_nonce( $user->ID );\n\tif ( ! $login_nonce ) { wp_die( 'Could not create login nonce.' ); }\n\t$time_delay = Two_Factor_Core::get_user_time_delay( $user );\n\t$last_login = get_user_meta( $user->ID, Two_Factor_Core::USER_RATE_LIMIT_KEY, true );\n\t$error_msg = sprintf(\n\t\t__( 'Too many incorrect verification codes. Please wait %s and reload this page to try again.', 'two-factor' ),\n\t\thuman_time_diff( $last_login + $time_delay )\n\t);\n\tTwo_Factor_Core::login_html( $user, $login_nonce['key'], admin_url(), $error_msg );\n\texit;\n} );" + } + ] +} diff --git a/providers/class-two-factor-email.php b/providers/class-two-factor-email.php index e6ca9bf7..d57b865f 100644 --- a/providers/class-two-factor-email.php +++ b/providers/class-two-factor-email.php @@ -344,8 +344,12 @@ public function authentication_page( $user ) { return; } - if ( ! $this->user_has_token( $user->ID ) || $this->user_token_has_expired( $user->ID ) ) { - $this->generate_and_email_token( $user ); + $is_rate_limited = Two_Factor_Core::is_user_rate_limited( $user ); + + if ( ! $is_rate_limited ) { + if ( ! $this->user_has_token( $user->ID ) || $this->user_token_has_expired( $user->ID ) ) { + $this->generate_and_email_token( $user ); + } } $token_length = $this->get_token_length(); @@ -357,7 +361,9 @@ public function authentication_page( $user ) { /** This action is documented in providers/class-two-factor-backup-codes.php */ do_action( 'two_factor_before_authentication_prompt', $this ); ?> +
+ ++ assertSame( 'two_factor_too_fast', $result->get_error_code() ); } + /** + * Verify the rate-limit error message does not use alarming language. + * + * "ERROR:" and "automated attacks" are alarming to a legitimate user who + * simply mistyped their code. The message should be calm and actionable. + * + * @covers Two_Factor_Core::process_provider + */ + public function test_rate_limit_error_message_is_calm_and_actionable() { + $user = self::factory()->user->create_and_get(); + $provider = Two_Factor_Dummy::get_instance(); + + 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() ); + + $result = Two_Factor_Core::process_provider( $provider, $user, true ); + $message = $result->get_error_message(); + + $this->assertStringNotContainsString( 'ERROR:', $message, 'Rate-limit message must not use the ERROR: prefix' ); + $this->assertStringNotContainsString( 'automated attacks', $message, 'Rate-limit message must not mention automated attacks' ); + } + /** * Verify process_provider() returns WP_Error and increments the failed-attempts * counter when authentication fails. diff --git a/tests/providers/class-two-factor-email.php b/tests/providers/class-two-factor-email.php index 8b37a983..3a8c56e5 100644 --- a/tests/providers/class-two-factor-email.php +++ b/tests/providers/class-two-factor-email.php @@ -495,6 +495,56 @@ public function test_authentication_page_with_existing_token() { $this->assertStringContainsString( 'two-factor-email-code', $output ); } + /** + * Verify the Resend Code button is hidden when the user is rate-limited. + * + * Showing an interactive resend button during a lockout misleads the user + * into thinking it will work. + * + * @covers Two_Factor_Email::authentication_page + */ + public function test_authentication_page_hides_resend_button_when_rate_limited() { + $user = self::factory()->user->create_and_get(); + $this->provider->generate_token( $user->ID ); + + 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() ); + + ob_start(); + $this->provider->authentication_page( $user ); + $output = ob_get_clean(); + + $this->assertStringNotContainsString( + Two_Factor_Email::INPUT_NAME_RESEND_CODE, + $output, + 'Resend Code button must not be rendered while the user is rate-limited' + ); + } + + /** + * Verify no email is sent when authentication_page() is called while rate-limited and no token exists. + * + * @covers Two_Factor_Email::authentication_page + */ + public function test_authentication_page_does_not_send_email_when_rate_limited_and_no_token() { + $user = self::factory()->user->create_and_get(); + + 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() ); + $this->assertFalse( $this->provider->user_has_token( $user->ID ) ); + + $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 email must be sent when authentication_page() is called while rate-limited' + ); + } + /** * Verify user_options outputs the user's email address. *