two-factor icon indicating copy to clipboard operation
two-factor copied to clipboard

Optionally Force 2fa

Open mikeselander opened this issue 6 years ago • 29 comments

Fixes #255.

Adds a network-level and site level (if non-multisite) setting to require two-factor authentication on a per-role or global level. If 2fa forcing is on and a user matches the criteria, we force the user into a 2fa settings screen and they cannot use the site until they add valid 2fa to their account.

Super administrators can edit the settings with this interface via network (or site if single site) settings: screen shot 2018-09-04 at 1 40 41 pm

Users who fall within the required buckets will see an interface like this: login-wout-2fa

mikeselander avatar Sep 03 '18 22:09 mikeselander

Hey folks, apologies - this is nowhere near review ready and I hit the wrong button 🤦‍♂️

mikeselander avatar Sep 03 '18 22:09 mikeselander

@mikeselander This looks great! Will you open a new pull request later or should I re-open this one?

kasparsd avatar Sep 04 '18 06:09 kasparsd

@kasparsd it still needs quite a bit of smoothing out which I'll be working on today. I'll re-open this once it's in a better state :)

mikeselander avatar Sep 04 '18 15:09 mikeselander

@kasparsd thank you for the comments! I know this was a lot to read through so I appreciate it 🙏

We could really simplify the implementation if we just redirected users to their profile page and didn't allow any other admin view until they configure the two factor. The current AJAX based implementation feels relatively fragile and coupled to existing providers.

I considered that approach (certainly would have saved me a lot of time 😄) but I think the downfall is in the user experience. If I am a user and I am constantly forced into a page with 20-30 input boxes, I'm going to end up confused.

Even if I am shown a banner I'm unlikely to read it thoroughly enough to understand and if I'm scrolled down to the section there will still be other controls in sight so it won't be clear to me what to do, exactly. I felt that the only way to be very clear what needed to happen was to display an isolated page with only the necessary information displayed.

I'd be willing to look at a non-AJAX approach if you have ideas for something that supported the same isolated view or clarity of action!

mikeselander avatar Sep 10 '18 01:09 mikeselander

@kasparsd sorry for the slow reply - this has been a busier week than I expected but I will swing back to this.

mikeselander avatar Sep 15 '18 03:09 mikeselander

@mikeselander Do you have time to continue on this, or shall I pick it up? :)

rmccue avatar Oct 16 '18 03:10 rmccue

@rmccue I won't have time to spend on this for a quite a while yet :( I wouldn't argue if you had time to pick this up!

mikeselander avatar Oct 18 '18 16:10 mikeselander

Fixes #255.

Adds a network-level and site level (if non-multisite) setting to require two-factor authentication on a per-role or global level. If 2fa forcing is on and a user matches the criteria, we force the user into a 2fa settings screen and they cannot use the site until they add valid 2fa to their account.

Super administrators can edit the settings with this interface via network (or site if single site) settings: screen shot 2018-09-04 at 1 40 41 pm

Users who fall within the required buckets will see an interface like this: login-wout-2fa

I don't see where to specify these requirements in Two Factor, or is this just a mock-up / in a dev-build right now?

alternativesurfer avatar Dec 05 '18 21:12 alternativesurfer

@alternativesurfer you've checked out this branch, correct? That UI will only work with this Pull Request checked out - it's not yet landed in the plugin.

mikeselander avatar Dec 05 '18 21:12 mikeselander

When will this be finished? Kind of critical before we can use the addon. Thanks

adambirds avatar Dec 11 '18 18:12 adambirds

@adambirds I personally do not have much time for this and will not for several months. If someone else can pick this up it would get done much faster.

mikeselander avatar Dec 11 '18 19:12 mikeselander

What we've done for this is just use the front-end shortcode from #261 (fixed on our https://github.com/wpgwinnett/two-factor master branch).

Here's our process:

  • choose the roles that need to have forced 2FA
  • choose the page that has the front-end shortcode
  • when someone logs in, check their role and if it needs 2FA, redirect them to the page with the front-end shortcode

They can't go anywhere on our site until they setup 2FA.

screely-1558726048742

This meets @kasparsd's requirement of

simplify the implementation...and didn't allow any other...view until they configure the two factor. The current AJAX based implementation feels relatively fragile and coupled to existing providers.

and

@mikeselander's concern for

isolated view or clarity of action

Happy to submit a PR if this works for you.

@adambirds does this help?

naomicbush avatar May 24 '19 19:05 naomicbush

What's the status on this feature?

Onyx808 avatar Aug 18 '19 21:08 Onyx808

On WordPress VIP we do this with filters instead of providing a UI. Just pointing this out in case it's useful to others: https://github.com/Automattic/vip-go-mu-plugins/blob/master/two-factor.php#L153

BrookeDot avatar Jan 28 '20 10:01 BrookeDot

What's the current status of this feature?

brianjohnpenner avatar May 04 '20 20:05 brianjohnpenner

Would love to have this merged +1

flowdee avatar Jan 29 '21 15:01 flowdee

@kasparsd any thoughts on a review of this PR and whether it's ready for consideration in v0.8.0?

jeffpaul avatar May 10 '21 15:05 jeffpaul

Per yesterday's bug scrub, given our narrow focus in 0.8.0 on the U2F deprecation we're going to punt this to a future release but note that it is a worthwhile concept.

jeffpaul avatar Mar 24 '22 18:03 jeffpaul

I look forward to when this gets included. Any updates on the state of this?

alexclst avatar Sep 19 '22 21:09 alexclst

@alexclst this is still in need of someone picking up the existing work in the PR and updating based on feedback in https://github.com/WordPress/two-factor/pull/239#pullrequestreview-153752223.

jeffpaul avatar Sep 20 '22 02:09 jeffpaul

I agree with Kaspars that this would be much simpler if we just redirected to the profile page instead of setting up an interstitial UI. I also agree with Mike, though, about the poor UX of that page.

I think the best way forward would be to:

  1. Remove all the extra code in this PR, so that it just removes capabilities and does the redirect. That would make this simple and lean.
  2. Open a new issue to improve the UI/UX, including maybe moving it to its own page. That would improve it for all flows, instead of just the one where 2FA is being enforced.

iandunn avatar Nov 14 '22 16:11 iandunn

There's a fairly significant disadvantage to using the profile page, which is that it requires rendering the entire admin. This potentially increases the attack surface and could lead to undesirable behaviour for plugins that aren't aware of the 2FA plugin. Keeping it siloed either on the frontend or login ensures nothing from the admin is accidentally leaked that way.

rmccue avatar Nov 14 '22 17:11 rmccue

🤔 Wouldn't removing the user's capabilities solve that (assuming the plugin is written correctly)? Or are you thinking of something different?

iandunn avatar Nov 14 '22 18:11 iandunn

I've fixed the merge conflicts here!

joehoyle avatar Aug 04 '23 15:08 joehoyle

Maybe this is obvious, but removing caps only serves to motivate people to enable Two Factor. It doesn't enhance security because anyone with the username and password can just enable 2FA to get full access. If you have a privileged user that never enables 2FA (maybe they don't login for a long time), their account is still vulnerable via a pwned password.

I would suggest an alternative way to think about this might be to enable Email by default for anyone that doesn't have other factors and then require everyone to have at least 1 factor enabled. (So you could disable email after enable TOTP, for example.)

I know this gets complicated if you want to enable Email as a factor, so maybe the default factor could be configurable. For example, maybe you want an admin to be able to generate a backup code and deliver it out of band.

joshbetz avatar Sep 09 '23 01:09 joshbetz

Could be as simple as:

	add_filter( 'two_factor_enabled_providers_for_user', 'force_2fa', 10, 2 );
	function force_2fa( $enabled, $user_id ) {
		if ( ! $force_2fa ) {
			return $enabled;
		}
		
		if ( count( $enabled ) ) {
			return $enabled;
		}

		return [ 'Two_Factor_Email' ];
	}

joshbetz avatar Sep 09 '23 01:09 joshbetz

Could be as simple as:

	add_filter( 'two_factor_enabled_providers_for_user', 'force_2fa', 10, 2 );
	function force_2fa( $enabled, $user_id ) {
		if ( ! $force_2fa ) {
			return $enabled;
		}
		
		if ( count( $enabled ) ) {
			return $enabled;
		}

		return [ 'Two_Factor_Email' ];
	}

I think there's a minor typo here, guessing $force_2fa should be $user_id, like this:

add_filter( 'two_factor_enabled_providers_for_user', 'force_2fa', 10, 2 );
function force_2fa( $enabled, $user_id ) {
	if ( ! $user_id ) {
		return $enabled;
	}

	if ( count( $enabled ) ) {
		return $enabled;
	}

	return [ 'Two_Factor_Email' ];
}

I've quickly tested this locally and it seems to work great - a really nice way to quickly and conveniently enforce email 2fa. It also automatically pre-selects email on the user settings screen, so if a user wants to change 2fa settings they'll see that email is pre-selected.

NewJenk avatar Dec 02 '23 16:12 NewJenk