Add a filter to limit two-factor providers available to each user #669
Conversation
…pported providers
… to resolve the instances
| * provider class names and the values as the provider class instances. | ||
| * | ||
| * @see Two_Factor_Core::get_enabled_providers_for_user() |
There was a problem hiding this comment.
Cross-linked the related methods for easier discovery and comparison.
| * @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 ) { |
There was a problem hiding this comment.
This is the new method and the associated filter.
| $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' ); |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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 ) ) { |
There was a problem hiding this comment.
Display this suggestion only if at least two methods are available.
| <fieldset id="two-factor-options" <?php echo $show_2fa_options ? '' : 'disabled="disabled"'; ?>> | ||
| <?php | ||
| if ( $providers ) { | ||
| self::render_user_providers_form( $user, $providers ); |
There was a problem hiding this comment.
To avoid wrapping a large block into this conditional, I've moved the form to a helper method.
| @@ -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 ) ) { | |||
There was a problem hiding this comment.
Avoid rendering the FIDO U2F settings if the provider is not "supported".
|
@ocean90 Can you please review the implementation and confirm if it matches your requirements? |
Co-authored-by: Dominik Schilling <dominikschilling+git@gmail.com>
|
I don't have an immediate need for this, but props for doing this change, it could be useful in the future! |
What?
Why?
Fixes #647, #662.
How?
get_supported_providers_for_user( $user )which wraps the output ofget_providers()in a filtertwo_factor_providers_for_user.Note that we already similar methods:
get_enabled_providers_for_user( $user = null )for the provider keys that are enabled in the user profile but might not be configured, yet.get_available_providers_for_user( $user = null )for the provider keys that are both enabled and configured.so I chose the name
supportedto designate if a provider is available to a specific user.Testing Instructions
Unit tests have been added to cover the filtering scenario. There are no user settings to toggle this.
Screenshots or screencast
If all providers are disabled for a user we now display a notice instead of an empty table:
Changelog Entry
Add a filter
two_factor_providers_for_userto disable providers for specific users.