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.

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

Add a filter to limit two-factor providers available to each user #669

Open
wants to merge 23 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
23 commits
Select commit Hold shift + click to select a range
afa6a18
Add a filter to adjust the providers available to each user
kasparsd Feb 14, 2025
eccac72
Use the new helper when getting providers for user
kasparsd Feb 14, 2025
6eed739
More references
kasparsd Feb 14, 2025
acba8c0
Better description to differentiate between available, enabled and su…
kasparsd Feb 14, 2025
d2c9ecc
Move the form into a helper for conditional rendering
kasparsd Feb 14, 2025
1aa2bfc
We have the user context so pass it along
kasparsd Feb 14, 2025
19938fb
The static hell is there
kasparsd Feb 14, 2025
927408c
Enabled providers returns just the keys so we need the supported ones…
kasparsd Feb 14, 2025
80d7f98
Add a unit test for the new filter
kasparsd Feb 14, 2025
f801c3a
Check for known things instead
kasparsd Feb 14, 2025
c06eff4
Pass the missing data
kasparsd Feb 14, 2025
a8cd102
Per linter
kasparsd Feb 14, 2025
6e4f0a6
Document the new filter
kasparsd Feb 14, 2025
e810859
Add the missing data
kasparsd Feb 14, 2025
63323e4
Move the fieldset logic to the caller where we know the state
kasparsd Feb 14, 2025
58e30e0
Add a helper to check if the method is supported for the user
kasparsd Feb 14, 2025
966285f
Don’t show the settings if the method is not supported for the user
kasparsd Feb 14, 2025
b7d593b
Per linter
kasparsd Feb 14, 2025
2bb236f
Reference the sub-class instead
kasparsd Feb 14, 2025
5422ba1
Keep together similar tests
kasparsd Feb 14, 2025
5f7783d
Test the new provider resolver
kasparsd Feb 14, 2025
aa8cfda
Test the filter impact to the new helper
kasparsd Feb 14, 2025
b2b605e
This is for the current session and not the user edited
kasparsd Feb 14, 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
137 changes: 96 additions & 41 deletions class-two-factor-core.php
Original file line number Diff line number Diff line change
Expand Up @@ -249,9 +249,12 @@ private static function get_providers_classes( $providers ) {
}

/**
* Get all enabled two-factor providers with keys as the original
* Get all registered two-factor providers with keys as the original
* provider class names and the values as the provider class instances.
*
* @see Two_Factor_Core::get_enabled_providers_for_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.

Cross-linked the related methods for easier discovery and comparison.

* @see Two_Factor_Core::get_supported_providers_for_user()
*
* @since 0.1-dev
*
* @return array
Expand Down Expand Up @@ -297,6 +300,28 @@ public static function get_providers() {
return $providers;
}

/**
* Get providers available for user which may not be enabled or configured.
*
* @see Two_Factor_Core::get_enabled_providers_for_user()
* @see Two_Factor_Core::get_available_providers_for_user()
*
* @param WP_User|int|null $user User ID.
* @return array List of provider instances indexed by provider key.
*/
public static function get_supported_providers_for_user( $user = null ) {
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 the new method and the associated filter.

$user = self::fetch_user( $user );
$providers = self::get_providers();

/**
* List of providers available to user which may not be enabled or configured.
*
* @param array $providers List of available provider instances indexed by provider key.
* @param int|WP_User $user User ID.
*/
return apply_filters( 'two_factor_providers_for_user', $providers, $user );
}

/**
* Enable the dummy method only during debugging.
*
Expand Down Expand Up @@ -483,7 +508,11 @@ public static function fetch_user( $user = null ) {
}

/**
* Get all Two-Factor Auth providers that are enabled for the specified|current user.
* Get two-factor providers that are enabled for the specified (or current) user
* but might not be configured, yet.
*
* @see Two_Factor_Core::get_supported_providers_for_user()
* @see Two_Factor_Core::get_available_providers_for_user()
*
* @param int|WP_User $user Optional. User ID, or WP_User object of the the user. Defaults to current user.
* @return array
Expand All @@ -494,7 +523,7 @@ public static function get_enabled_providers_for_user( $user = null ) {
return array();
}

$providers = self::get_providers();
$providers = self::get_supported_providers_for_user( $user );
$enabled_providers = get_user_meta( $user->ID, self::ENABLED_PROVIDERS_USER_META_KEY, true );
if ( empty( $enabled_providers ) ) {
$enabled_providers = array();
Expand All @@ -511,19 +540,23 @@ public static function get_enabled_providers_for_user( $user = null ) {
}

/**
* Get all Two-Factor Auth providers that are both enabled and configured for the specified|current user.
* Get all two-factor providers that are both enabled and configured
* for the specified (or current) user.
*
* @see Two_Factor_Core::get_supported_providers_for_user()
* @see Two_Factor_Core::get_enabled_providers_for_user()
*
* @param int|WP_User $user Optional. User ID, or WP_User object of the the user. Defaults to current user.
* @return array
* @return array List of provider instances.
*/
public static function get_available_providers_for_user( $user = null ) {
$user = self::fetch_user( $user );
if ( ! $user ) {
return array();
}

$providers = self::get_providers();
$enabled_providers = self::get_enabled_providers_for_user( $user );
$providers = self::get_supported_providers_for_user( $user ); // Returns full objects.
$enabled_providers = self::get_enabled_providers_for_user( $user ); // Returns just the keys.
$configured_providers = array();

foreach ( $providers as $provider_key => $provider ) {
Expand All @@ -538,7 +571,7 @@ public static function get_available_providers_for_user( $user = null ) {
/**
* Fetch the provider for the request based on the user preferences.
*
* @param int|WP_User $user Optional. User ID, or WP_User object of the the user. Defaults to current user.
* @param int|WP_User $user Optional. User ID, or WP_User object of the the user. Defaults to current user.
* @param null|string|object $preferred_provider Optional. The name of the provider, the provider, or empty.
* @return null|object The provider
*/
Expand All @@ -556,7 +589,7 @@ public static function get_provider_for_user( $user = null, $preferred_provider
// Default to the currently logged in provider.
if ( ! $preferred_provider && get_current_user_id() === $user->ID ) {
$session = self::get_current_user_session();
if ( ! empty( $session['two-factor-provider'] ) ) {
if ( ! empty( $session['two-factor-provider'] ) ) {
$preferred_provider = $session['two-factor-provider'];
}
}
Expand Down Expand Up @@ -604,7 +637,7 @@ public static function get_primary_provider_for_user( $user = null ) {
return null;
}

$providers = self::get_providers();
$providers = self::get_supported_providers_for_user( $user );
$available_providers = self::get_available_providers_for_user( $user );

// If there's only one available provider, force that to be the primary.
Expand Down Expand Up @@ -1515,10 +1548,12 @@ public static function _login_form_revalidate_2fa( $nonce = '', $provider = '',
}

// Update the session metadata with the revalidation details.
self::update_current_user_session( array(
'two-factor-provider' => $provider->get_key(),
'two-factor-login' => time(),
) );
self::update_current_user_session(
array(
'two-factor-provider' => $provider->get_key(),
'two-factor-login' => time(),
)
);

do_action( 'two_factor_user_revalidated', $user, $provider );

Expand Down Expand Up @@ -1754,7 +1789,7 @@ public static function show_password_reset_error() {
)
);

login_header( __( 'Password Reset', 'two-factor' ), '', $error );
login_header( __( 'Password Reset', 'two-factor' ), '', $error );
login_footer();
}

Expand Down Expand Up @@ -1804,10 +1839,11 @@ public static function manage_users_custom_column( $output, $column_name, $user_
public static function user_two_factor_options( $user ) {
$notices = [];

$providers = self::get_supported_providers_for_user( $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_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 All @@ -1820,18 +1856,18 @@ public static function user_two_factor_options( $user ) {
);

$notices['warning two-factor-warning-revalidate-session'] = sprintf(
esc_html__( 'To update your Two-Factor options, you must first revalidate your session.', 'two-factor' ) .
esc_html__( 'To update your Two-Factor options, you must first revalidate your session.', 'two-factor' ) .
' <a class="button" href="%s">' . esc_html__( 'Revalidate now', 'two-factor' ) . '</a>',
esc_url( $url )
esc_url( $url )
);
}

printf(
'<fieldset id="two-factor-options" %s>',
$show_2fa_options ? '' : 'disabled="disabled"'
);
if ( empty( $providers ) ) {
$notices['notice two-factor-notice-no-providers-supported'] = esc_html__( 'No providers are available for your account.', 'two-factor' );
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Display a notice if no providers are available for user.

I'm assuming that with the plugin enabled it would be more confusing to hide the section completely. With the section being present users can at least know the option should be available and is simply disabled.

Copy link
Member

Choose a reason for hiding this comment

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

I agree, perhaps iterating on the copy to provide a user with some insight on what to do next? Eg. "No providers are available for your account. Please contact the site administrator for more details." or whatever verbage is used elsewhere in core to point folks to the admin (either site or network)?

}

if ( 1 === count( $enabled_providers ) ) {
// Suggest enabling a backup method if only method is enabled and there are more available.
if ( count( $providers ) > 1 && 1 === count( $enabled_providers ) ) {
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Display this suggestion only if at least two methods are available.

$notices['warning two-factor-warning-suggest-backup'] = esc_html__( 'To prevent being locked out of your account, consider enabling a backup method like Recovery Codes in case you lose access to your primary authentication method.', 'two-factor' );
}
?>
Expand All @@ -1842,15 +1878,44 @@ public static function user_two_factor_options( $user ) {
<p><?php echo wp_kses_post( $notice ); ?></p>
</div>
<?php endforeach; ?>

<fieldset id="two-factor-options" <?php echo $show_2fa_options ? '' : 'disabled="disabled"'; ?>>
<?php
if ( $providers ) {
self::render_user_providers_form( $user, $providers );
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

To avoid wrapping a large block into this conditional, I've moved the form to a helper method.

}
?>
</fieldset>

<?php
/**
* Fires after the Two Factor methods table.
*
* To be used by Two Factor methods to add settings UI.
*
* @param WP_User $user The user.
* @param array $providers List of providers available to the user.
*
* @since 0.1-dev
*/
do_action( 'show_user_security_settings', $user, $providers );
}

private static function render_user_providers_form( $user, $providers ) {
$primary_provider_key = self::get_primary_provider_key_selected_for_user( $user );
$enabled_providers = self::get_enabled_providers_for_user( $user );

?>
<p>
<?php esc_html_e( 'Configure a primary two-factor method along with a backup method, such as Recovery Codes, to avoid being locked out if you lose access to your primary method.', 'two-factor' ); ?>
<?php esc_html_e( 'Configure a primary two-factor method along with a backup method, such as Recovery Codes, to avoid being locked out if you lose access to your primary method.', 'two-factor' ); ?>
</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="form-table two-factor-methods-table" role="presentation">
<tbody>
<?php foreach ( self::get_providers() as $provider_key => $object ) : ?>
<?php foreach ( $providers as $provider_key => $object ) : ?>
<tr>
<th><?php echo esc_html( $object->get_label() ); ?></th>
<td>
Expand Down Expand Up @@ -1880,32 +1945,22 @@ public static function user_two_factor_options( $user ) {
<table class="form-table two-factor-primary-method-table" role="presentation">
<tbody>
<tr>
<th><?php esc_html_e( 'Primary Method', '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 ) : ?>
<?php foreach ( $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>
<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>
</tbody>
</table>
</fieldset>
<?php

/**
* Fires after the Two Factor methods table.
*
* To be used by Two Factor methods to add settings UI.
*
* @since 0.1-dev
*/
do_action( 'show_user_security_settings', $user );
}

/**
Expand All @@ -1920,7 +1975,7 @@ public static function user_two_factor_options( $user ) {
*/
public static function enable_provider_for_user( $user_id, $new_provider ) {
// Ensure the provider is even available.
if ( ! array_key_exists( $new_provider, self::get_providers() ) ) {
if ( ! array_key_exists( $new_provider, self::get_supported_providers_for_user( $user_id ) ) ) {
return false;
}

Expand Down Expand Up @@ -1951,7 +2006,7 @@ public static function enable_provider_for_user( $user_id, $new_provider ) {
*/
public static function disable_provider_for_user( $user_id, $provider_to_delete ) {
// Check if the provider is even enabled.
if ( ! array_key_exists( $provider_to_delete, self::get_providers() ) ) {
if ( ! array_key_exists( $provider_to_delete, self::get_supported_providers_for_user( $user_id ) ) ) {
return false;
}

Expand Down Expand Up @@ -1995,7 +2050,7 @@ public static function user_two_factor_options_update( $user_id ) {
return;
}

$providers = self::get_providers();
$providers = self::get_supported_providers_for_user( $user_id );
$enabled_providers = $_POST[ self::ENABLED_PROVIDERS_USER_META_KEY ];
$existing_providers = self::get_enabled_providers_for_user( $user_id );

Expand Down
4 changes: 4 additions & 0 deletions providers/class-two-factor-fido-u2f-admin.php
Original file line number Diff line number Diff line change
Expand Up @@ -164,6 +164,10 @@ protected static function asset_version() {
* @param WP_User $user WP_User object of the logged-in user.
*/
public static function show_user_profile( $user ) {
if ( ! Two_Factor_FIDO_U2F::is_supported_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.

Avoid rendering the FIDO U2F settings if the provider is not "supported".

return;
}

wp_nonce_field( "user_security_keys-{$user->ID}", '_nonce_user_security_keys' );
$new_key = false;

Expand Down
13 changes: 13 additions & 0 deletions providers/class-two-factor-provider.php
Original file line number Diff line number Diff line change
Expand Up @@ -123,6 +123,19 @@ abstract public function validate_authentication( $user );
*/
abstract public function is_available_for_user( $user );

/**
* If this provider should be available for the user.
*
* @param WP_User|int $user WP_User object, user ID or null to resolve the current user.
*
* @return bool
*/
public static function is_supported_for_user( $user = null ) {
$providers = Two_Factor_Core::get_supported_providers_for_user( $user );

return isset( $providers[ static::class ] );
}

/**
* Generate a random eight-digit string to send out as an auth code.
*
Expand Down
1 change: 1 addition & 0 deletions readme.txt
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ For more history, see [this post](https://georgestephanis.wordpress.com/2013/08/
Here is a list of action and filter hooks provided by the plugin:

- `two_factor_providers` filter overrides the available two-factor providers such as email and time-based one-time passwords. Array values are PHP classnames of the two-factor providers.
- `two_factor_providers_for_user` filter overrides the available two-factor providers for a specific user. Array values are instances of provider classes and the user object `WP_User` is available as the second argument.
- `two_factor_enabled_providers_for_user` filter overrides the list of two-factor providers enabled for a user. First argument is an array of enabled provider classnames as values, the second argument is the user ID.
- `two_factor_user_authenticated` action which receives the logged in `WP_User` object as the first argument for determining the logged in user right after the authentication workflow.
- `two_factor_email_token_ttl` filter overrides the time interval in seconds that an email token is considered after generation. Accepts the time in seconds as the first argument and the ID of the `WP_User` object being authenticated.
Expand Down
Loading
Loading