Optionally Force 2fa

[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:

Users who fall within the required buckets will see an interface like this:

[mikeselander]

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

[kasparsd]

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

[mikeselander]

@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 :)

[alexclst]

There should be a way to hard-code the force settings from wp-config.php, so that we can enforce these in a way that normal admin users cannot change. While filters are more capable, I think that also having a define we can add would further harden security. Otherwise one admin could get 2FA set up, and then disable the requirement right away.

[jeffpaul]

@iandunn @joehoyle @georgestephanis is there a preferred approach to either getting this updated and towards merge (or perhaps closed with a fresh PR and new approach and then towards merge)? If so, I can try and help pull some engineering time to get this to done.

[eric-michel]

We've been using the "use email if nothing selected" method of forcing 2FA:

function force_2fa_for_users($providers, $user_id) {
    // Force Email provider if no providers are enabled
    if (empty($providers) && class_exists('Two_Factor_Email')) {
      $providers[] = 'Two_Factor_Email';
    }
    
    return $providers;
  }

This works quite well (although I recently found that if the user selected FIDO U2F but enters no keys, 2FA gets disabled) Ultimately, I think an official "require 2FA feature" could be barely more complicated than the function I've shown above. Adding a filter to turn it on + the ability for a developer to set a wp-config setting and forcing email as a fallback method when no other is available is fully sufficient.

I'd go so far as to say requiring 2FA should be on by default with the option to disable that feature, but I understand not wanting to change the behavior for existing installations.

[masteradhoc]

This feature has been merged inside the Humanmade Fork for quite some time and seems in use there. Good to recheck and see if we can merge this. See humanmade#1

[github-actions]

The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the props-bot label.

Unlinked Accounts

The following contributors have not linked their GitHub and WordPress.org accounts: @mikeselander, @tcrsavage, @alternativesurfer, @adambirds, @naomicbush, @Onyx808, @brianjohnpenner, @NewJenk.

Contributors, please read how to link your accounts to ensure your work is properly credited in WordPress releases.

If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message.

Unlinked contributors: mikeselander, tcrsavage, alternativesurfer, adambirds, naomicbush, Onyx808, brianjohnpenner, NewJenk.

Co-authored-by: joehoyle <joehoyle@git.wordpress.org>
Co-authored-by: shadyvb <shadyvb@git.wordpress.org>
Co-authored-by: mikelittle <mikelittle@git.wordpress.org>
Co-authored-by: kasparsd <kasparsd@git.wordpress.org>
Co-authored-by: rmccue <rmccue@git.wordpress.org>
Co-authored-by: BrookeDot <brookedot@git.wordpress.org>
Co-authored-by: flowdee <flowdee@git.wordpress.org>
Co-authored-by: jeffpaul <jeffpaul@git.wordpress.org>
Co-authored-by: alexclst <alexclst@git.wordpress.org>
Co-authored-by: iandunn <iandunn@git.wordpress.org>
Co-authored-by: joshbetz <betzster@git.wordpress.org>
Co-authored-by: eric-michel <ytfeldrawkcab@git.wordpress.org>
Co-authored-by: masteradhoc <masteradhoc@git.wordpress.org>
Co-authored-by: adriantoll <adriantoll@git.wordpress.org>
Co-authored-by: georgestephanis <georgestephanis@git.wordpress.org>
Co-authored-by: apermo <apermo@git.wordpress.org>

To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook.

[BrookeDot]

@masteradhoc Thanks for pointing to that fork. I took a quick look so may have missed it, but does the HM fork include a filter to enable/disable this functionality?

My quick read shows that it's enabled by default. While in the case of this feature plugin it should likely be a simple opt-in. My general preference would be with a filters for minimum capability and enabled/disabled, but also I can see the value of having it in the UI.

In either case, I think a revisit of #255 and a fresh PR is the way to go. I would like to see agreement on:

• Filters and WP-config option or just filters (I vote filters only)
• Enabled or disabled by default? (I vote disabled)
• Include UI (I vote yes)

@jeffpaul as you were the most recent comment in regards to a fresh PR. and @mikeselander as you opened the original PR even if by your own admission it was supposed to be as a draft PR.

If we can get some consensus I can take a look at likely forking the Human Made approach back into core but will likely need some help writing the tests.

[masteradhoc]

@BrookeDot your help would be very appreciated! I think as well that we want to:

• give admins in the settings page an option to force it for user roles of their choice
• filter to activate/disable this behaviour is already there as far as i saw but good to doublecheck
• constant over wp config is for sure a great idea

So far my opinion :)

[BrookeDot]

filter to activate/disable this behaviour is already there as far as i saw but good to doublecheck

🤦‍♀️ The file was callapsed because it was new, but does contain the main logic with filters:
https://github.com/humanmade/two-factor/blob/252e3e0891ecf08adaf910d1ea5debd20ad39170/class.two-factor-force.php

Looking at:

/**
	 * Get which user roles have two-factor forced.
	 *
	 * @since 0.1-dev
	 *
	 * @return array
	 */
	public static function get_forced_user_roles() {
		/**
		 * User roles which have two-factor forced.
		 *
		 * @param array $roles Roles which are required to use 2fa.
		 */
		return (array) apply_filters( 'two_factor_forced_user_roles', get_site_option( self::FORCED_ROLES_META_KEY, false ) );
	}

Brings up the question of if this should be role based (easier to understand for the average user) or if it should be cap based (simplified implementation, as one could just do edit_posts for example instead of selecting multiple roles)

[masteradhoc]

@BrookeDot i think user roles would be quite easier to understand from a UX point.

[eric-michel]

Not sure if this is possible, but it would be amazing if new installations had 2FA forced on by default for all users, while update installations left that setting off by default (since that's how it's operated for so long). I can't imagine many circumstances where I would install a 2FA plugin and not want it to immediately be required.

Either way, just having the ability to easily force 2FA on will be a huge improvement!

[BrookeDot]

Not sure if this is possible, but it would be amazing if new installations had 2FA forced on by default for all users,

It would be possible, it's all just code. But it comes to decisions not options. I would be hesitant to enable it for all users by default, but open to discussion. What may be a middle ground, is on install enable it for every role that has edit_posts capability or above.

This would avoid things like API users, and subscriber only sites having forced 2FA.

[eric-michel]

What may be a middle ground, is on install enable it for every role that has edit_posts capability or above.

That would absolutely work for our purposes. I admit to being pretty myopic on this topic because we build mostly static sites that don't have subscriber accounts. Almost all user accounts are admins, so having a single account with 2FA turned off is as dangerous as having them all turned off (if that one gets compromised). I totally get wanting to give users without admin privileges the option rather than forcing it (since the danger a given compromised account could pose is low).

[adriantoll]

I would argue against requiring 2FA by default, even on new installations. Our use case is making 2FA available to those editors who want it on, but requiring it for administrators. The extra friction of an email with a code is going to frustrating for many of our editors, whereas all our administrator accounts have TOTP enabled, which is a piece of cake with a password manager. The kinds of people we work with - not particularly tech-literate - would have no idea about how to set up TOTP codes so they'd be stuck with email codes. I know we could turn it off on new installations, but from my point of view it would be an extra bit of hassle that I'd rather not have to think about.

Less selfishly and more generally I think changing the behaviour of a plugin that's been around for this long would cause issues - I can't imagine a good way of notifying people of that change. If you had a persistent notification in the dashboard after a new installation that would be annoying because it's only relevant the first time someone sees it.

[georgestephanis]

If #764 merges in, this feels like it would be worth splicing in there if we want to offer it as a visual admin ui option in keeping with core philosophies (decisions not options, etc)

[georgestephanis]

I would argue against requiring 2FA by default, even on new installations. Our use case is making 2FA available to those editors who want it on, but requiring it for administrators. The extra friction of an email with a code is going to frustrating for many of our editors, whereas all our administrator accounts have TOTP enabled, which is a piece of cake with a password manager. The kinds of people we work with - not particularly tech-literate - would have no idea about how to set up TOTP codes so they'd be stuck with email codes. I know we could turn it off on new installations, but from my point of view it would be an extra bit of hassle that I'd rather not have to think about.

I don't think anyone in this plugin or discussion is recommending mandating it by default in the plugin, I think the option is whether to give site admins a visual choice for requiring it for their specific use case. It would of course default to not required, but they could modify that to enable the requirement.

But to be clear: if this option goes in, the default behavior should be no-change and not enforcing 2fa for every account, it's just an option for site admins to require it.

[adriantoll]

@georgestephanis Maybe I misunderstood, but @BrookeDot's comment "what may be a middle ground, is on install enable it for every role that has edit_posts capability or above" sounded like that was the suggestion. Would definitely prefer if not.

[georgestephanis]

No, I believe it was referring to adding an option so a site admin could opt to require it for specific roles.

I don't believe @BrookeDot is suggesting defaulting it to required for users without an admin selecting to.

[BrookeDot]

I don't believe @BrookeDot is suggesting defaulting it to required for users without an admin selecting to.

I was suggesting a default (that could be filtered) of edit_post capability. Once the plugin is enabled MFA is required for anyone who could update a post.

However, after the discussion here about how caps are confusing to most non-devs I would revise the default to be role based, this isn't as good because it doesn't account for custom roles, so perhaps there is a hybrid approach.

But to be clear: if this option goes in, the default behavior should be no-change and not enforcing 2fa for every account, it's just an option for site admins to require it.

I do think that is the most friction less way, especially if this is added to core. Despite my previous commons (and preference) that it's enforced, for a project this size that may not make sense.

[eric-michel]

For full transparency, I definitely was advocating for default-on 2FA for the plugin. A lot of very good arguments were made not to. I support any default-on setup (capability, role, or otherwise) simple because we always turn it on, but ultimately as long is there is a UI for us to turn it on for everyone when we install it, that's really all we need. Everything else is just convenience.

[BrookeDot]

I definitely was advocating for default-on 2FA for the plugin.

After this discussion I think the best way to do this is to ensure that there is a filter that supports this. Then it's just a couple of lines in a feature plugin/theme while not changing the default behavior/expectations of WordPress. It puts the power in the site-owner's hand.

[BrookeDot]

So it looks like there are three PR options:

#255
#764
and #333 (which I think is humanmade#1)

I could use some direction on where to start, my gut says to close #255 and start with #764 (or just rework from Human Made upstream). Thoughts?

[masteradhoc]

@BrookeDot i'll take this as a point to discuss tomorrow on our byweekly meeting in #core-passwords at 4PM UTC. Would be great to have as many joining as possible to decide the direction.