Skip to content

Commit

Permalink
Simplify UI Text / more consistent text and UI. (#521)
Browse files Browse the repository at this point in the history
* 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.
  • Loading branch information
dd32 authored Apr 28, 2023
1 parent f027b04 commit bb96a02
Show file tree
Hide file tree
Showing 8 changed files with 94 additions and 73 deletions.
63 changes: 16 additions & 47 deletions class-two-factor-core.php
Original file line number Diff line number Diff line change
Expand Up @@ -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 ) ) {
Expand All @@ -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(
Expand All @@ -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;
Expand Down
41 changes: 26 additions & 15 deletions providers/class-two-factor-backup-codes.php
Original file line number Diff line number Diff line change
Expand Up @@ -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 ) )
Expand All @@ -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' );
}

/**
Expand Down Expand Up @@ -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>
Expand Down Expand Up @@ -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( '/' )
);

Expand All @@ -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(
Expand Down Expand Up @@ -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
Expand Down
9 changes: 9 additions & 0 deletions providers/class-two-factor-email.php
Original file line number Diff line number Diff line change
Expand Up @@ -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.
*
Expand Down
9 changes: 9 additions & 0 deletions providers/class-two-factor-fido-u2f.php
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
15 changes: 15 additions & 0 deletions providers/class-two-factor-provider.php
Original file line number Diff line number Diff line change
Expand Up @@ -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.
*
Expand Down
24 changes: 16 additions & 8 deletions providers/class-two-factor-totp.php
Original file line number Diff line number Diff line change
Expand Up @@ -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' );
}

/**
Expand Down Expand Up @@ -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( {
Expand Down Expand Up @@ -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>
Expand Down
4 changes: 2 additions & 2 deletions tests/providers/class-two-factor-backup-codes.php
Original file line number Diff line number Diff line change
Expand Up @@ -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() );
}

/**
Expand All @@ -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 );
}

/**
Expand Down
2 changes: 1 addition & 1 deletion tests/providers/class-two-factor-totp.php
Original file line number Diff line number Diff line change
Expand Up @@ -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() );
}

/**
Expand Down

0 comments on commit bb96a02

Please sign in to comment.