From bb96a02bf2f86c904b579b2c265587fe23ff7ecf Mon Sep 17 00:00:00 2001
From: Dion Hulse <dion@wordpress.org>
Date: Fri, 28 Apr 2023 15:49:32 +1000
Subject: [PATCH] Simplify UI Text / more consistent text and UI. (#521)

* Replace strings with more user-friendly wordings.

* Hide the Language dropdown when we're on a 2FA page. See #520

* Try an alternative simpler login UI for the 'Or continue with...' buttons.

* Try using a GitHub-style 'Having Problems?' + list style.

* Rename backup codes to 'Recovery Codes', to dissuade their regular use.
---
 class-two-factor-core.php                     | 63 +++++--------------
 providers/class-two-factor-backup-codes.php   | 41 +++++++-----
 providers/class-two-factor-email.php          |  9 +++
 providers/class-two-factor-fido-u2f.php       |  9 +++
 providers/class-two-factor-provider.php       | 15 +++++
 providers/class-two-factor-totp.php           | 24 ++++---
 .../class-two-factor-backup-codes.php         |  4 +-
 tests/providers/class-two-factor-totp.php     |  2 +-
 8 files changed, 94 insertions(+), 73 deletions(-)

diff --git a/class-two-factor-core.php b/class-two-factor-core.php
index df61c3eb..0b814f3b 100644
--- a/class-two-factor-core.php
+++ b/class-two-factor-core.php
@@ -734,6 +734,9 @@ public static function login_html( $user, $login_nonce, $redirect_to, $error_msg
 			include_once TWO_FACTOR_DIR . 'includes/function.login-header.php';
 		}
 
+		// Disable the language switcher.
+		add_filter( 'login_display_language_dropdown', '__return_false' );
+
 		login_header();
 
 		if ( ! empty( $error_msg ) ) {
@@ -756,45 +759,12 @@ public static function login_html( $user, $login_nonce, $redirect_to, $error_msg
 
 				<?php $provider->authentication_page( $user ); ?>
 		</form>
-
-		<?php
-		if ( 1 === count( $backup_providers ) ) :
-			$backup_provider_key = key( $backup_providers );
-			$backup_provider     = $backup_providers[ $backup_provider_key ];
-			$login_url           = self::login_url(
-				array(
-					'action'        => 'validate_2fa',
-					'provider'      => $backup_provider_key,
-					'wp-auth-id'    => $user->ID,
-					'wp-auth-nonce' => $login_nonce,
-					'redirect_to'   => $redirect_to,
-					'rememberme'    => $rememberme,
-				)
-			);
-			?>
+		<?php if ( $backup_providers ) : ?>
 			<div class="backup-methods-wrap">
-				<p class="backup-methods">
-					<a href="<?php echo esc_url( $login_url ); ?>">
-						<?php
-						echo esc_html(
-							sprintf(
-								// translators: %s: Two-factor method name.
-								__( 'Or, use your backup method: %s &rarr;', 'two-factor' ),
-								$backup_provider->get_label()
-							)
-						);
-						?>
-					</a>
+				<p>
+					<?php esc_html_e( 'Having Problems?', 'two-factor' ); ?>
 				</p>
-			</div>
-			<?php elseif ( 1 < count( $backup_providers ) ) : ?>
-			<div class="backup-methods-wrap">
-				<p class="backup-methods">
-					<a href="javascript:;" onclick="document.querySelector('ul.backup-methods').style.display = 'block';">
-						<?php esc_html_e( 'Or, use a backup method…', 'two-factor' ); ?>
-					</a>
-				</p>
-				<ul class="backup-methods">
+				<ul>
 					<?php
 					foreach ( $backup_providers as $backup_provider_key => $backup_provider ) :
 						$login_url = self::login_url(
@@ -810,34 +780,33 @@ public static function login_html( $user, $login_nonce, $redirect_to, $error_msg
 						?>
 						<li>
 							<a href="<?php echo esc_url( $login_url ); ?>">
-								<?php echo esc_html( $backup_provider->get_label() ); ?>
+								<?php echo esc_html( $backup_provider->get_alternative_provider_label() ); ?>
 							</a>
 						</li>
 					<?php endforeach; ?>
 				</ul>
 			</div>
 		<?php endif; ?>
+
 		<style>
 			/* @todo: migrate to an external stylesheet. */
 			.backup-methods-wrap {
-			margin-top: 16px;
-			padding: 0 24px;
+				margin-top: 16px;
+				padding: 0 24px;
 			}
 			.backup-methods-wrap a {
-			color: #999;
-			text-decoration: none;
+				text-decoration: none;
 			}
-			ul.backup-methods {
-			display: none;
-			padding-left: 1.5em;
+			.backup-methods-wrap ul {
+				list-style-position: inside;
 			}
 			/* Prevent Jetpack from hiding our controls, see https://github.com/Automattic/jetpack/issues/3747 */
 			.jetpack-sso-form-display #loginform > p,
 			.jetpack-sso-form-display #loginform > div {
-			display: block;
+				display: block;
 			}
 			#login form p.two-factor-prompt {
-			margin-bottom: 1em;
+				margin-bottom: 1em;
 			}
 			.input.authcode {
 				letter-spacing: .3em;
diff --git a/providers/class-two-factor-backup-codes.php b/providers/class-two-factor-backup-codes.php
index 3e09f9a8..bebbb94c 100644
--- a/providers/class-two-factor-backup-codes.php
+++ b/providers/class-two-factor-backup-codes.php
@@ -100,7 +100,7 @@ public function admin_notices() {
 					echo wp_kses(
 						sprintf(
 						/* translators: %s: URL for code regeneration */
-							__( 'Two-Factor: You are out of backup codes and need to <a href="%s">regenerate!</a>', 'two-factor' ),
+							__( 'Two-Factor: You are out of recovery codes and need to <a href="%s">regenerate!</a>', 'two-factor' ),
 							esc_url( get_edit_user_link( $user->ID ) . '#two-factor-backup-codes' )
 						),
 						array( 'a' => array( 'href' => true ) )
@@ -118,7 +118,16 @@ public function admin_notices() {
 	 * @since 0.1-dev
 	 */
 	public function get_label() {
-		return _x( 'Backup Verification Codes (Single Use)', 'Provider Label', 'two-factor' );
+		return _x( 'Recovery Codes', 'Provider Label', 'two-factor' );
+	}
+
+	/**
+	 * Returns the "continue with" text provider for the login screen.
+	 *
+	 * @since 0.9.0
+	 */
+	public function get_alternative_provider_label() {
+		return __( 'Use a recovery code', 'two-factor' );
 	}
 
 	/**
@@ -148,24 +157,26 @@ public function user_options( $user ) {
 		$count = self::codes_remaining_for_user( $user );
 		?>
 		<p id="two-factor-backup-codes">
-			<button type="button" class="button button-two-factor-backup-codes-generate button-secondary hide-if-no-js">
-				<?php esc_html_e( 'Generate Verification Codes', 'two-factor' ); ?>
-			</button>
-			<span class="two-factor-backup-codes-count">
+			<p class="two-factor-backup-codes-count">
 			<?php
 				echo esc_html(
 					sprintf(
-					/* translators: %s: count */
-						_n( '%s unused code remaining.', '%s unused codes remaining.', $count, 'two-factor' ),
+						/* translators: %s: count */
+						_n( '%s unused code remaining, each recovery code can only be used once.', '%s unused codes remaining, each recovery code can only be used once.', $count, 'two-factor' ),
 						$count
 					)
 				);
 			?>
-				</span>
+			</p>
+			<p>
+				<button type="button" class="button button-two-factor-backup-codes-generate button-secondary hide-if-no-js">
+					<?php esc_html_e( 'Generate new recovery codes', 'two-factor' ); ?>
+				</button>
+			</p>
 		</p>
 		<div class="two-factor-backup-codes-wrapper" style="display:none;">
 			<ol class="two-factor-backup-codes-unused-codes"></ol>
-			<p class="description"><?php esc_html_e( 'Write these down!  Once you navigate away from this page, you will not be able to view these codes again.', 'two-factor' ); ?></p>
+			<p class="description"><?php esc_html_e( 'Write these down! Once you navigate away from this page, you will not be able to view these codes again.', 'two-factor' ); ?></p>
 			<p>
 				<a class="button button-two-factor-backup-codes-download button-secondary hide-if-no-js" href="javascript:void(0);" id="two-factor-backup-codes-download-link" download="two-factor-backup-codes.txt"><?php esc_html_e( 'Download Codes', 'two-factor' ); ?></a>
 			<p>
@@ -258,7 +269,7 @@ public function rest_generate_codes( $request ) {
 		$count = self::codes_remaining_for_user( $user );
 		$title = sprintf(
 			/* translators: %s: the site's domain */
-			__( 'Two-Factor Backup Codes for %s', 'two-factor' ),
+			__( 'Two-Factor Recovery Codes for %s', 'two-factor' ),
 			home_url( '/' )
 		);
 
@@ -274,11 +285,11 @@ public function rest_generate_codes( $request ) {
 
 		$i18n = array(
 			/* translators: %s: count */
-			'count' => esc_html( sprintf( _n( '%s unused code remaining.', '%s unused codes remaining.', $count, 'two-factor' ), $count ) ),
+			'count' => esc_html( sprintf( _n( '%s unused code remaining, each recovery code can only be used once.', '%s unused codes remaining, each recovery code can only be used once.', $count, 'two-factor' ), $count ) ),
 		);
 
 		if ( $request->get_param( 'enable_provider' ) && ! Two_Factor_Core::enable_provider_for_user( $user_id, 'Two_Factor_Backup_Codes' ) ) {
-			return new WP_Error( 'db_error', __( 'Unable to enable Backup Codes provider for this user.', 'two-factor' ), array( 'status' => 500 ) );
+			return new WP_Error( 'db_error', __( 'Unable to enable recovery codes for this user.', 'two-factor' ), array( 'status' => 500 ) );
 		}
 
 		return array(
@@ -313,9 +324,9 @@ public static function codes_remaining_for_user( $user ) {
 	public function authentication_page( $user ) {
 		require_once ABSPATH . '/wp-admin/includes/template.php';
 		?>
-		<p class="two-factor-prompt"><?php esc_html_e( 'Enter a backup verification code.', 'two-factor' ); ?></p>
+		<p class="two-factor-prompt"><?php esc_html_e( 'Enter a recovery code.', 'two-factor' ); ?></p><br/>
 		<p>
-			<label for="authcode"><?php esc_html_e( 'Verification Code:', 'two-factor' ); ?></label>
+			<label for="authcode"><?php esc_html_e( 'Recovery Code:', 'two-factor' ); ?></label>
 			<input type="text" inputmode="numeric" name="two-factor-backup-code" id="authcode" class="input authcode" value="" size="20" pattern="[0-9 ]*" placeholder="1234 5678" data-digits="8" />
 		</p>
 		<?php
diff --git a/providers/class-two-factor-email.php b/providers/class-two-factor-email.php
index efdc2f73..c6f5881f 100644
--- a/providers/class-two-factor-email.php
+++ b/providers/class-two-factor-email.php
@@ -54,6 +54,15 @@ public function get_label() {
 		return _x( 'Email', 'Provider Label', 'two-factor' );
 	}
 
+	/**
+	 * Returns the "continue with" text provider for the login screen.
+	 *
+	 * @since 0.9.0
+	 */
+	public function get_alternative_provider_label() {
+		return __( 'Send a code to your email', 'two-factor' );
+	}
+
 	/**
 	 * Generate the user token.
 	 *
diff --git a/providers/class-two-factor-fido-u2f.php b/providers/class-two-factor-fido-u2f.php
index 4dace439..52d28f57 100644
--- a/providers/class-two-factor-fido-u2f.php
+++ b/providers/class-two-factor-fido-u2f.php
@@ -104,6 +104,15 @@ public function get_label() {
 		return _x( 'FIDO U2F Security Keys', 'Provider Label', 'two-factor' );
 	}
 
+	/**
+	 * Returns the "continue with" text provider for the login screen.
+	 *
+	 * @since 0.9.0
+	 */
+	public function get_alternative_provider_label() {
+		return __( 'Use your security key', 'two-factor' );
+	}
+
 	/**
 	 * Register script dependencies used during login and when
 	 * registering keys in the WP admin.
diff --git a/providers/class-two-factor-provider.php b/providers/class-two-factor-provider.php
index 0fb063c7..2e9f2a8f 100644
--- a/providers/class-two-factor-provider.php
+++ b/providers/class-two-factor-provider.php
@@ -49,6 +49,21 @@ protected function __construct() {
 	 */
 	abstract public function get_label();
 
+	/**
+	 * Returns the "continue with" text provider for the login screen.
+	 *
+	 * @since 0.9.0
+	 *
+	 * @return string
+	 */
+	public function get_alternative_provider_label() {
+		return sprintf(
+			/* translators: the two factor provider name */
+			__( 'Use %s', 'two-factor' ),
+			$this->get_label()
+		);
+	}
+
 	/**
 	 * Prints the name of the provider.
 	 *
diff --git a/providers/class-two-factor-totp.php b/providers/class-two-factor-totp.php
index 5066c1ed..533f7075 100644
--- a/providers/class-two-factor-totp.php
+++ b/providers/class-two-factor-totp.php
@@ -110,7 +110,16 @@ public function register_rest_routes() {
 	 * Returns the name of the provider.
 	 */
 	public function get_label() {
-		return _x( 'Time Based One-Time Password (TOTP)', 'Provider Label', 'two-factor' );
+		return _x( 'Authenticator app', 'Provider Label', 'two-factor' );
+	}
+
+	/**
+	 * Returns the "continue with" text provider for the login screen.
+	 *
+	 * @since 0.9.0
+	 */
+	public function get_alternative_provider_label() {
+		return __( 'Use your authenticator app', 'two-factor' );
 	}
 
 	/**
@@ -369,16 +378,15 @@ public function user_two_factor_options( $user ) {
 
 		<?php else : ?>
 			<p class="success">
-				<?php esc_html_e( 'Secret key is configured and registered. It is not possible to view it again for security reasons.', 'two-factor' ); ?>
+				<?php esc_html_e( 'An authenticator app is currently configured. You will need to re-scan the QR code on all devices if reset.', 'two-factor' ); ?>
 			</p>
 			<p>
-				<a class="button reset-totp-key" href="#"><?php esc_html_e( 'Reset Key', 'two-factor' ); ?></a>
-				<em class="description">
-					<?php esc_html_e( 'You will have to re-scan the QR code on all devices as the previous codes will stop working.', 'two-factor' ); ?>
-				</em>
+				<button type="button" class="button button-secondary reset-totp-key hide-if-no-js">
+					<?php esc_html_e( 'Reset authenticator app', 'two-factor' ); ?>
+				</button>
 				<script>
 					( function( $ ) {
-						$( 'a.reset-totp-key' ).click( function( e ) {
+						$( '.button.reset-totp-key' ).click( function( e ) {
 							e.preventDefault();
 
 							wp.apiRequest( {
@@ -652,7 +660,7 @@ public function authentication_page( $user ) {
 		require_once ABSPATH . '/wp-admin/includes/template.php';
 		?>
 		<p class="two-factor-prompt">
-			<?php esc_html_e( 'Please enter the code generated by your authenticator app.', 'two-factor' ); ?>
+			<?php esc_html_e( 'Enter the code generated by your authenticator app.', 'two-factor' ); ?>
 		</p>
 		<p>
 			<label for="authcode"><?php esc_html_e( 'Authentication Code:', 'two-factor' ); ?></label>
diff --git a/tests/providers/class-two-factor-backup-codes.php b/tests/providers/class-two-factor-backup-codes.php
index 5e74fbb3..a926044e 100644
--- a/tests/providers/class-two-factor-backup-codes.php
+++ b/tests/providers/class-two-factor-backup-codes.php
@@ -47,7 +47,7 @@ public function test_get_instance() {
 	 * @covers Two_Factor_Backup_Codes::get_label
 	 */
 	public function test_get_label() {
-		$this->assertStringContainsString( 'Backup Verification Codes', $this->provider->get_label() );
+		$this->assertStringContainsString( 'Recovery Codes', $this->provider->get_label() );
 	}
 
 	/**
@@ -60,7 +60,7 @@ public function test_authentication_page() {
 		$this->provider->authentication_page( false );
 		$contents = ob_get_clean();
 
-		$this->assertStringContainsString( 'Enter a backup verification code.', $contents );
+		$this->assertStringContainsString( 'Enter a recovery code.', $contents );
 	}
 
 	/**
diff --git a/tests/providers/class-two-factor-totp.php b/tests/providers/class-two-factor-totp.php
index 08f4986a..8c961f12 100644
--- a/tests/providers/class-two-factor-totp.php
+++ b/tests/providers/class-two-factor-totp.php
@@ -58,7 +58,7 @@ public function test_get_instance() {
 	 * @covers Two_Factor_Totp::get_label
 	 */
 	public function test_get_label() {
-		$this->assertStringContainsString( 'Time Based One-Time Password (TOTP)', $this->provider->get_label() );
+		$this->assertStringContainsString( 'Authenticator app', $this->provider->get_label() );
 	}
 
 	/**