Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Simplify the Two Factor settings in user profile #654

Open
wants to merge 14 commits into
base: master
Choose a base branch
from
Open

Simplify the Two Factor settings in user profile #654

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
14 commits
Select commit Hold shift + click to select a range
90d5957
Prototype
kasparsd Sep 19, 2024
2eee45d
Merge remote-tracking branch 'origin/master' into profile-ux
kasparsd Dec 3, 2024
8b9c5af
Remove test code
kasparsd Dec 3, 2024
d369a3a
Primary is determined dynamically
kasparsd Dec 3, 2024
cc830ce
Allow unsetting the primary provider for default behaviour
kasparsd Dec 3, 2024
ab43eb3
Introduce a dedicated helper to fetch the user selection
kasparsd Dec 3, 2024
26331d3
Return a boolean per tests
kasparsd Dec 3, 2024
26d7cb3
Merge branch 'master' into profile-ux
kasparsd Dec 18, 2024
f77f2ac
Merge remote-tracking branch 'origin/master' into profile-ux
kasparsd Jan 9, 2025
1cf0889
Per suggestion from @jeffpaul
kasparsd Jan 9, 2025
1166098
Move to own section for visual seperation
kasparsd Jan 9, 2025
701a260
Consistent casing with the other methods
kasparsd Jan 9, 2025
f1b53ef
Mention the TOTP at least somewhere
kasparsd Jan 9, 2025
54f34e5
Match the updated title
kasparsd Jan 9, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
71 changes: 48 additions & 23 deletions class-two-factor-core.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -571,6 +571,25 @@ public static function get_provider_for_user( $user = null, $preferred_provider
return self::get_primary_provider_for_user( $user );
}

/**
* Get the name of the primary provider selected by the user
* and enabled for the user.
*
* @param WP_User|int $user User ID or instance.
*
* @return string|null
*/
private static function get_primary_provider_key_selected_for_user( $user ) {
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a dedicated helper to get the selected option without attempting to resolve if the method is active or not.

$primary_provider = get_user_meta( $user->ID, self::PROVIDER_USER_META_KEY, true );
$available_providers = self::get_available_providers_for_user( $user );

if ( ! empty( $primary_provider ) && ! empty( $available_providers[ $primary_provider ] ) ) {
return $primary_provider;
}

return null;
}

/**
* Gets the Two-Factor Auth provider for the specified|current user.
*
Expand All @@ -594,7 +613,7 @@ public static function get_primary_provider_for_user( $user = null ) {
} elseif ( 1 === count( $available_providers ) ) {
$provider = key( $available_providers );
} else {
$provider = get_user_meta( $user->ID, self::PROVIDER_USER_META_KEY, true );
$provider = self::get_primary_provider_key_selected_for_user( $user );

// If the provider specified isn't enabled, just grab the first one that is.
if ( ! isset( $available_providers[ $provider ] ) ) {
Expand Down Expand Up @@ -1788,12 +1807,7 @@ public static function user_two_factor_options( $user ) {
wp_enqueue_style( 'user-edit-2fa', plugins_url( 'user-edit.css', __FILE__ ), array(), TWO_FACTOR_VERSION );

$enabled_providers = array_keys( self::get_available_providers_for_user( $user ) );
$primary_provider = self::get_primary_provider_for_user( $user->ID );

$primary_provider_key = null;
if ( ! empty( $primary_provider ) && is_object( $primary_provider ) ) {
$primary_provider_key = $primary_provider->get_key();
}
$primary_provider_key = self::get_primary_provider_key_selected_for_user( $user );

// This is specific to the current session, not the displayed user.
$show_2fa_options = self::current_user_can_update_two_factor_options();
Expand Down Expand Up @@ -1822,6 +1836,7 @@ public static function user_two_factor_options( $user ) {
}
?>
<h2><?php esc_html_e( 'Two-Factor Options', 'two-factor' ); ?></h2>

<?php foreach ( $notices as $notice_type => $notice ) : ?>
<div class="<?php echo esc_attr( $notice_type ? 'notice inline notice-' . $notice_type : '' ); ?>">
<p><?php echo wp_kses_post( $notice ); ?></p>
Expand All @@ -1832,21 +1847,17 @@ public static function user_two_factor_options( $user ) {
</p>
<?php wp_nonce_field( 'user_two_factor_options', '_nonce_user_two_factor_options', false ); ?>
<input type="hidden" name="<?php echo esc_attr( self::ENABLED_PROVIDERS_USER_META_KEY ); ?>[]" value="<?php /* Dummy input so $_POST value is passed when no providers are enabled. */ ?>" />
<table class="wp-list-table widefat fixed striped table-view-list two-factor-methods-table">
<thead>
<tr>
<th class="col-enabled" scope="col"><?php esc_html_e( 'Enabled', 'two-factor' ); ?></th>
<th class="col-primary" scope="col"><?php esc_html_e( 'Primary', 'two-factor' ); ?></th>
<th class="col-name" scope="col"><?php esc_html_e( 'Type', 'two-factor' ); ?></th>
</tr>
</thead>

<table class="form-table two-factor-methods-table" role="presentation">
<tbody>
<?php foreach ( self::get_providers() as $provider_key => $object ) : ?>
<tr>
<th scope="row"><input id="enabled-<?php echo esc_attr( $provider_key ); ?>" type="checkbox" name="<?php echo esc_attr( self::ENABLED_PROVIDERS_USER_META_KEY ); ?>[]" value="<?php echo esc_attr( $provider_key ); ?>" <?php checked( in_array( $provider_key, $enabled_providers, true ) ); ?> /></th>
<th scope="row"><input type="radio" name="<?php echo esc_attr( self::PROVIDER_USER_META_KEY ); ?>" value="<?php echo esc_attr( $provider_key ); ?>" <?php checked( $provider_key, $primary_provider_key ); ?> /></th>
<th><?php echo esc_html( $object->get_label() ); ?></th>
<td>
<label class="two-factor-method-label" for="enabled-<?php echo esc_attr( $provider_key ); ?>"><?php echo esc_html( $object->get_label() ); ?></label>
<label class="two-factor-method-label">
<input id="enabled-<?php echo esc_attr( $provider_key ); ?>" type="checkbox" name="<?php echo esc_attr( self::ENABLED_PROVIDERS_USER_META_KEY ); ?>[]" value="<?php echo esc_attr( $provider_key ); ?>" <?php checked( in_array( $provider_key, $enabled_providers, true ) ); ?> />
<?php echo esc_html( sprintf( __( 'Enable %s', 'two-factor' ), $object->get_label() ) ); ?>
</label>
<?php
/**
* Fires after user options are shown.
Expand All @@ -1864,13 +1875,25 @@ public static function user_two_factor_options( $user ) {
</tr>
<?php endforeach; ?>
</tbody>
<tfoot>
</table>
<hr />
<table class="form-table two-factor-primary-method-table" role="presentation">
<tbody>
<tr>
<th class="col-enabled" scope="col"><?php esc_html_e( 'Enabled', 'two-factor' ); ?></th>
<th class="col-primary" scope="col"><?php esc_html_e( 'Primary', 'two-factor' ); ?></th>
<th class="col-name" scope="col"><?php esc_html_e( 'Type', 'two-factor' ); ?></th>
<th><?php esc_html_e( 'Primary Method', 'two-factor' ) ?></th>
<td>
<select name="<?php echo esc_attr( self::PROVIDER_USER_META_KEY ); ?>">
<option value=""><?php echo esc_html( __( 'Default', 'two-factor' ) ); ?></option>
<?php foreach ( self::get_providers() as $provider_key => $object ) : ?>
<option value="<?php echo esc_attr( $provider_key ); ?>" <?php selected( $provider_key, $primary_provider_key ); ?> <?php disabled( ! in_array( $provider_key, $enabled_providers, true ) ); ?>>
<?php echo esc_html( $object->get_label() ); ?>
</option>
<?php endforeach; ?>
</select>
<p class="description"><?php esc_html_e( 'Select the primary method to use for two-factor authentication when signing into this site.', 'two-factor' ) ?></p>
</td>
</tr>
</tfoot>
</tbody>
</table>
</fieldset>
<?php
Expand Down Expand Up @@ -1984,6 +2007,8 @@ public static function user_two_factor_options_update( $user_id ) {
$new_provider = isset( $_POST[ self::PROVIDER_USER_META_KEY ] ) ? $_POST[ self::PROVIDER_USER_META_KEY ] : '';
if ( ! empty( $new_provider ) && in_array( $new_provider, $enabled_providers, true ) ) {
update_user_meta( $user_id, self::PROVIDER_USER_META_KEY, $new_provider );
} else {
delete_user_meta( $user_id, self::PROVIDER_USER_META_KEY );
}

// Have we changed the two-factor settings for the current user? Alter their session metadata.
Expand Down
4 changes: 2 additions & 2 deletions providers/class-two-factor-totp.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -110,7 +110,7 @@ public function register_rest_routes() {
* Returns the name of the provider.
*/
public function get_label() {
return _x( 'Authenticator app', 'Provider Label', 'two-factor' );
return _x( 'Authenticator App', 'Provider Label', 'two-factor' );
}

/**
Expand All @@ -119,7 +119,7 @@ public function get_label() {
* @since 0.9.0
*/
public function get_alternative_provider_label() {
return __( 'Use your authenticator app', 'two-factor' );
return __( 'Use your authenticator app for time-based one-time passwords (TOTP)', 'two-factor' );
}

/**
Expand Down
2 changes: 1 addition & 1 deletion tests/providers/class-two-factor-totp.php
View file
Open in desktop
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( 'Authenticator app', $this->provider->get_label() );
$this->assertStringContainsString( 'Authenticator App', $this->provider->get_label() );
}

/**
Expand Down
13 changes: 0 additions & 13 deletions user-edit.css
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,16 +1,3 @@
.two-factor-methods-table .col-primary,
.two-factor-methods-table .col-enabled {
width: 5%;
}

.two-factor-methods-table .col-name {
width: 90%;
}

.two-factor-methods-table tbody th {
text-align: center;
}

.two-factor-methods-table tbody th,
.two-factor-methods-table tbody td {
vertical-align: top;
Expand Down
Loading