From 547328af154827eb58cba620b2e7b84a99784f85 Mon Sep 17 00:00:00 2001 From: dkmyta <43220201+dkmyta@users.noreply.github.com> Date: Sat, 15 Feb 2025 10:46:04 -0800 Subject: [PATCH] Account Protection: Improve auth success flow (#41727) --- .../src/class-password-detection.php | 181 +++++++++++++++--- .../src/css/password-detection.css | 39 +++- .../tests/php/test-password-detection.php | 48 +++-- 3 files changed, 218 insertions(+), 50 deletions(-) diff --git a/projects/packages/account-protection/src/class-password-detection.php b/projects/packages/account-protection/src/class-password-detection.php index 9f7a91249c781..8b07789b61b79 100644 --- a/projects/packages/account-protection/src/class-password-detection.php +++ b/projects/packages/account-protection/src/class-password-detection.php @@ -60,7 +60,13 @@ public function login_form_password_detection( $user, string $password ) { $email_sent = $this->email_service->api_send_auth_email( $user, $transient['auth_code'] ); if ( ! $email_sent ) { - $this->set_transient_error( $user->ID, __( 'Failed to send authentication email. Please try again.', 'jetpack-account-protection' ) ); + $this->set_transient_error( + $user->ID, + array( + 'code' => 'email_send_error', + 'message' => __( 'Failed to send authentication email. Please try again.', 'jetpack-account-protection' ), + ) + ); } return new \WP_Error( @@ -152,21 +158,41 @@ public function render_page() { && wp_verify_nonce( sanitize_text_field( wp_unslash( $_GET['_wpnonce'] ) ), 'resend_email_nonce' ) ) { $email_resent = $this->email_service->resend_auth_email( $user, $transient_data, $token ); - if ( ! $email_resent ) { - $message = __( 'Failed to resend authentication email. Please try again.', 'jetpack-account-protection' ); + if ( $email_resent ) { + $this->set_transient_success( + $user->ID, + array( + 'code' => 'email_resend_success', + 'message' => __( 'Authentication email resent successfully.', 'jetpack-account-protection' ), + ) + ); + } else { + $error = array( + 'code' => 'email_resend_error', + 'message' => __( 'Failed to resend authentication email. Please try again.', 'jetpack-account-protection' ), + ); if ( $transient_data['resend_attempts'] >= Config::PASSWORD_DETECTION_MAX_RESEND_ATTEMPTS ) { - $message = __( 'Resend limit exceeded. Please try again later.', 'jetpack-account-protection' ); + $error = array( + 'code' => 'email_resend_limit_error', + 'message' => __( 'Resend limit exceeded. Please try again later.', 'jetpack-account-protection' ), + ); } - $this->set_transient_error( $user->ID, $message ); + $this->set_transient_error( $user->ID, $error ); } $this->redirect_and_exit( $this->get_redirect_url( $token ) ); // @phan-suppress-next-line PhanPluginUnreachableCode This would fall through in unit tests otherwise. return; } else { - $this->set_transient_error( $user->ID, __( 'Resend nonce verification failed. Please try again.', 'jetpack-account-protection' ) ); + $this->set_transient_error( + $user->ID, + array( + 'code' => 'email_resend_nonce_error', + 'message' => __( 'Resend nonce verification failed. Please try again.', 'jetpack-account-protection' ), + ) + ); } } @@ -177,7 +203,13 @@ public function render_page() { $this->handle_auth_form_submission( $user, $token, $transient_data['auth_code'] ?? null, $user_input ); } else { - $this->set_transient_error( $user->ID, __( 'Verify nonce verification failed. Please try again.', 'jetpack-account-protection' ) ); + $this->set_transient_error( + $user->ID, + array( + 'code' => 'verify_nonce_error', + 'message' => __( 'Verify nonce verification failed. Please try again.', 'jetpack-account-protection' ), + ) + ); } } @@ -193,9 +225,36 @@ public function render_page() { * @return void */ public function render_content( \WP_User $user, string $token ): void { - $transient_key = Config::PASSWORD_DETECTION_TRANSIENT_PREFIX . "_error_{$user->ID}"; - $error_message = get_transient( $transient_key ); - delete_transient( $transient_key ); + $error_transient_key = Config::PASSWORD_DETECTION_TRANSIENT_PREFIX . "_error_{$user->ID}"; + $success_transient_key = Config::PASSWORD_DETECTION_TRANSIENT_PREFIX . "_success_{$user->ID}"; + + $error_data = get_transient( $error_transient_key ); + $success_data = get_transient( $success_transient_key ); + + delete_transient( $error_transient_key ); + delete_transient( $success_transient_key ); + + $error_message = null; + $error_code = null; + if ( is_array( $error_data ) ) { + if ( isset( $error_data['message'] ) ) { + $error_message = $error_data['message']; + } + if ( isset( $error_data['code'] ) ) { + $error_code = $error_data['code']; + } + } + + $success_message = null; + $success_code = null; + if ( is_array( $success_data ) ) { + if ( isset( $success_data['message'] ) ) { + $success_message = $success_data['message']; + } + if ( isset( $success_data['code'] ) ) { + $success_code = $success_data['code']; + } + } defined( 'ABSPATH' ) || exit; ?> @@ -210,12 +269,39 @@ public function render_content( \WP_User $user, string $token ): void {
+ ' . esc_html__( 'risks of using weak passwords', 'jetpack-account-protection' ) . '' // TODO: Update this redirect URL once document exists + ); + ?> +
+
- - - - -
+ ++ + + + +
+ + @@ -293,7 +382,13 @@ private function generate_and_store_transient_data( int $user_id ): array { $transient_set = set_transient( Config::PASSWORD_DETECTION_TRANSIENT_PREFIX . "_{$token}", $data, Config::PASSWORD_DETECTION_EMAIL_SENT_EXPIRATION ); if ( ! $transient_set ) { - $this->set_transient_error( $user_id, __( 'Failed to set transient data. Please try again.', 'jetpack-account-protection' ) ); + $this->set_transient_error( + $user_id, + array( + 'code' => 'transient_error', + 'message' => __( 'Failed to set transient data. Please try again.', 'jetpack-account-protection' ), + ) + ); } return array( @@ -334,27 +429,51 @@ private function get_redirect_url( string $token ): string { */ private function handle_auth_form_submission( \WP_User $user, string $token, string $auth_code, string $user_input ): void { if ( $auth_code && $auth_code === $user_input ) { + $this->set_transient_success( + $user->ID, + array( + 'code' => 'auth_code_success', + 'message' => __( 'Authentication code verified successfully.', 'jetpack-account-protection' ), + ) + ); // TODO: Ensure all transient are also removed on module and/or plugin deactivation delete_transient( Config::PASSWORD_DETECTION_TRANSIENT_PREFIX . "_{$token}" ); wp_set_auth_cookie( $user->ID, true ); - // TODO: Notify user to update their password/redirect to password update page - $this->redirect_and_exit( admin_url() ); } else { - $this->set_transient_error( $user->ID, __( 'Authentication code verification failed. Please try again.', 'jetpack-account-protection' ) ); + $this->set_transient_error( + $user->ID, + array( + 'code' => 'auth_code_error', + 'message' => __( 'Authentication code verification failed. Please try again.', 'jetpack-account-protection' ), + ) + ); } } + /** + * Set a transient success message. + * + * @param int $user_id The user ID. + * @param array $success An array of the success code and message. + * @param int $expiration The expiration time in seconds. + * + * @return void + */ + private function set_transient_success( int $user_id, array $success, int $expiration = 60 ): void { + set_transient( Config::PASSWORD_DETECTION_TRANSIENT_PREFIX . "_success_{$user_id}", $success, $expiration ); + } + /** * Set a transient error message. * - * @param int $user_id The user ID. - * @param string $message The error message. - * @param int $expiration The expiration time in seconds. + * @param int $user_id The user ID. + * @param array $error An array of the error code and message. + * @param int $expiration The expiration time in seconds. * * @return void */ - private function set_transient_error( int $user_id, string $message, int $expiration = 60 ): void { - set_transient( Config::PASSWORD_DETECTION_TRANSIENT_PREFIX . "_error_{$user_id}", $message, $expiration ); + private function set_transient_error( int $user_id, array $error, int $expiration = 60 ): void { + set_transient( Config::PASSWORD_DETECTION_TRANSIENT_PREFIX . "_error_{$user_id}", $error, $expiration ); } /** diff --git a/projects/packages/account-protection/src/css/password-detection.css b/projects/packages/account-protection/src/css/password-detection.css index ebce1017e3083..7cfdfd85dff42 100644 --- a/projects/packages/account-protection/src/css/password-detection.css +++ b/projects/packages/account-protection/src/css/password-detection.css @@ -31,10 +31,15 @@ } .action { + display: flex; + justify-content: center; + align-items: center; height: 36px; cursor: pointer; width: 100%; border-radius: 2px; + font-size: 13px; + text-decoration: none; } .action-input { @@ -49,30 +54,52 @@ } } -.action-verify { +.action-verify, +.action-update-password { margin-top: 10px; background-color: #0000EE; - border: 1px solid #0000EE; - color: #fff; - font-size: 13px; + border: 2px solid #0000EE; + color: #FFF; +} + +.action-proceed { + background-color: #FFF; + border: 2px solid #0000EE; + color: #0000EE; +} + +a.risks-link, +a.resend-email-link { + color: #0000EE; } .email-status { text-align: center; } -.notice-wrapper { +.notice { + width: 100%; display: flex; - height: 36px; align-items: center; font-weight: 500; box-sizing: border-box; justify-content: center; } +.notice-message { + text-align: center; + margin: 7px 0; +} + .error { background: #FACFD2; border: 2px solid #E65054; color: #E65054; } +.success { + background: #D0E6b8; + border: 2px solid #069E08; + color: #069E08; +} + diff --git a/projects/packages/account-protection/tests/php/test-password-detection.php b/projects/packages/account-protection/tests/php/test-password-detection.php index 0417a6f6f0b29..79562b2b59120 100644 --- a/projects/packages/account-protection/tests/php/test-password-detection.php +++ b/projects/packages/account-protection/tests/php/test-password-detection.php @@ -130,7 +130,14 @@ public function test_login_form_password_detection_sets_transient_error_if_unabl $sut->login_form_password_detection( $user, 'pw' ); $transient_data = get_transient( Config::PASSWORD_DETECTION_TRANSIENT_PREFIX . "_error_{$user->ID}" ); - $this->assertSame( 'Failed to send authentication email. Please try again.', $transient_data, 'Should have set the correct error message.' ); + $this->assertSame( + array( + 'code' => 'email_send_error', + 'message' => 'Failed to send authentication email. Please try again.', + ), + $transient_data, + 'Should have set the correct error message.' + ); remove_filter( 'check_password', '__return_true' ); } @@ -201,14 +208,11 @@ public function test_render_page_checks_2fa_code_successfully(): void { $user->user_pass = 'pw'; $user->add_cap( 'publish_posts' ); - $sut = $this->createPartialMock( Password_Detection::class, array( 'load_user', 'redirect_and_exit', 'render_content' ) ); + $sut = $this->createPartialMock( Password_Detection::class, array( 'load_user', 'render_content' ) ); $sut->expects( $this->once() ) ->method( 'load_user' ) ->with( 123 ) ->willReturn( $user ); - $sut->expects( $this->once() ) - ->method( 'redirect_and_exit' ) - ->with( 'http://example.org/wp-admin/' ); $sut->expects( $this->once() ) ->method( 'render_content' ) ->with( $user, 'my_cool_token' ); @@ -265,7 +269,14 @@ public function test_render_page_sets_transient_error_if_2fa_code_is_wrong(): vo $error = get_transient( Config::PASSWORD_DETECTION_TRANSIENT_PREFIX . '_error_123' ); - $this->assertSame( 'Authentication code verification failed. Please try again.', $error, 'Error message is not as expected.' ); + $this->assertSame( + array( + 'code' => 'auth_code_error', + 'message' => 'Authentication code verification failed. Please try again.', + ), + $error, + 'Error message is not as expected.' + ); unset( $_GET['token'] ); unset( $_POST['verify'] ); @@ -286,9 +297,10 @@ public function test_render_page_sets_transient_error_if_2fa_nonce_is_wrong(): v ) ); - $user = new \WP_User(); - $user->ID = 123; - $user->user_pass = 'pw'; + $user = new \WP_User(); + $user->ID = 123; + $user->user_email = 'email@example.com'; + $user->user_pass = 'pw'; $user->add_cap( 'publish_posts' ); $sut = $this->createPartialMock( Password_Detection::class, array( 'load_user', 'render_content' ) ); @@ -304,7 +316,14 @@ public function test_render_page_sets_transient_error_if_2fa_nonce_is_wrong(): v $error = get_transient( Config::PASSWORD_DETECTION_TRANSIENT_PREFIX . '_error_123' ); - $this->assertSame( 'Verify nonce verification failed. Please try again.', $error, 'Error message is not as expected.' ); + $this->assertSame( + array( + 'code' => 'verify_nonce_error', + 'message' => 'Verify nonce verification failed. Please try again.', + ), + $error, + 'Error message is not as expected.' + ); unset( $_GET['token'] ); unset( $_POST['verify'] ); @@ -383,9 +402,12 @@ public function test_render_content_explains_the_2fa_form(): void { } public function test_render_content_shows_transient_error_if_set(): void { - $error_message = 'This is a error message to test things with.'; + $error = array( + 'code' => 'error', + 'message' => 'This is a error message to test things with.', + ); - set_transient( Config::PASSWORD_DETECTION_TRANSIENT_PREFIX . '_error_123', $error_message ); + set_transient( Config::PASSWORD_DETECTION_TRANSIENT_PREFIX . '_error_123', $error ); $user = new \WP_User(); $user->ID = 123; @@ -398,7 +420,7 @@ public function test_render_content_shows_transient_error_if_set(): void { $sut->expects( $this->once() ) ->method( 'exit' ); - $this->expectOutputRegex( '@' . $error_message . '@' ); + $this->expectOutputRegex( '@' . $error['message'] . '@' ); $sut->render_content( $user, 'my_cool_token' ); } }