two-factor
two-factor copied to clipboard
Optionally Force 2fa
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:
Users who fall within the required buckets will see an interface like this:
Hey folks, apologies - this is nowhere near review ready and I hit the wrong button 🤦♂️
@mikeselander This looks great! Will you open a new pull request later or should I re-open this one?
@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 :)
@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!
@kasparsd sorry for the slow reply - this has been a busier week than I expected but I will swing back to this.
@mikeselander Do you have time to continue on this, or shall I pick it up? :)
@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!
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:
Users who fall within the required buckets will see an interface like this:
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 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.
When will this be finished? Kind of critical before we can use the addon. Thanks
@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.
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.
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?
What's the status on this feature?
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
What's the current status of this feature?
Would love to have this merged +1
@kasparsd any thoughts on a review of this PR and whether it's ready for consideration in v0.8.0?
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.
I look forward to when this gets included. Any updates on the state of this?
@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.
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:
- 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.
- 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.
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.
🤔 Wouldn't removing the user's capabilities solve that (assuming the plugin is written correctly)? Or are you thinking of something different?
I've fixed the merge conflicts here!
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.
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' ];
}
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.