Allow to limit providers per user

[ocean90]

Is your enhancement related to a problem? Please describe.

I'd like to limit the available providers per user. Example code:

/**
 * Limit the possible providers to email and time-based one-time password.
 */
add_filter(
	'two_factor_providers',
	static fn( array $providers ): array =>
		array_intersect_key(
			$providers,
			[
				'Two_Factor_Email' => '',
				'Two_Factor_Totp'  => '',
			]
		)
);

/**
 * Require two factor authentication via email for all users without the manage_extended_two_factor capability.
 */
add_filter(
	'two_factor_enabled_providers_for_user',
	static fn( $providers, $user_id ) =>
		user_can( $user_id, 'manage_extended_two_factor' ) ? $providers : [ 'Two_Factor_Email' ],
	10,
	2
);
add_filter(
	'two_factor_primary_provider_for_user',
	static fn( $provider, $user_id ) =>
		user_can( $user_id, 'manage_extended_two_factor' ) ? $provider : 'Two_Factor_Email',
	10,
	2
);

While this does prevent users without the manage_extended_two_factor capability to use the TOTP provider, the UI still renders the TOTP option.

That's because the table uses the Two_Factor_Core::get_providers() method to render the list of providers which isn't user-specific.

two-factor/class-two-factor-core.php

Line 1837 in 6a95e7f

<?php foreach ( self::get_providers() as $provider_key => $object ) : ?>

Proposed Solution

I'm not sure if Two_Factor_Core::get_providers() should get an argument for a user or if there should be a wrapper for Two_Factor_Core::get_providers() like Two_Factor_Core::get_providers_for_user( $user ).

Looking at the current usage of Two_Factor_Core::get_providers(), in all cases we'd have a user ID available.

Designs

No response

Describe alternatives you've considered

No response

Please confirm that you have searched existing issues in this repository.

Yes

[kasparsd]

This sounds useful!

The only time we need a list of all available providers without the user context is during the uninstall of the plugin. Ideally, it would return all providers, even those that could have been enabled at some point in the past, to ensure that all meta data is deleted. Sounds like extending get_providers() to accept the user ID input and add a new filter would work for this.

However, I'm wondering about a scenario where this filter disables all the providers that are currently configured/enabled for the user. It would essentially disable the two-factor for the user, right? Can we rely on the code using the filter to account for that?

[kasparsd]

Eventually needing a canonical list of available providers for the settings page #249 is one reason why having a dedicated helper function for getting the available providers for user might be a better. This would ensure a filter doesn't accidentally remove a provider if the user ID argument is not provided.

[r-a-y]

/**
 * Limit the possible providers to email and time-based one-time password.
 */
add_filter(
	'two_factor_providers',
	static fn( array $providers ): array =>
		array_intersect_key(
			$providers,
			[
				'Two_Factor_Email' => '',
				'Two_Factor_Totp'  => '',
			]
		)
);

Just wanted to note that this portion of ocean90's code snippet was working before v0.10.0, but v0.10.0 broke this functionality due to:

two-factor/class-two-factor-core.php

Lines 210 to 215 in 636dc0b

	$additional_providers = apply_filters( 'two_factor_providers', $providers );
	// Merge them with the default providers.
	if ( ! empty( $additional_providers ) ) {
	return array_merge( $providers, $additional_providers );
	}

It's no longer possible to disable a core-included provider since v0.10.0. Please reconsider allowing developers to filter the core-included providers like in v0.9.1. A good example is the Two Factor WebAuthn plugin used to remove the deprecated U2F provider, but since v0.10.0, this doesn't work. I was also using this filter to re-order the provider list.

Update - This problem is already being discussed here: #651 (comment)

[kasparsd]

@r-a-y Do you mind reviewing the solution in #651? The latest iteration removes the merge here https://github.com/WordPress/two-factor/pull/651/files#diff-ee04f1d323104504c6bfa38dd11ef43e78d1dbac2883293af0b5c310d16e9519L203-L217 -- please see the inline comments for context.