alleyinteractive / wp-alleyvate

Defaults for WordPress sites by Alley.
GNU General Public License v2.0
16 stars 2 forks source link

[FEATURE] Force 2FA for All Users Who Can Edit Content #55

Closed kevinfodness closed 2 months ago

kevinfodness commented 7 months ago

Description

As a security best practice, we should force 2FA for all users who can edit content (basically anyone with a role other than subscriber). This should affect WordPress VIP sites, sites that use Jetpack, and sites that use the Two Factor plugin.

Use Case

When a user attempts to access a site without having set up two-factor authentication, they should be blocked from editing anything other than their profile until they activate two-factor authentication. Essentially, they should be downgraded to Subscriber status.

Acceptance Criteria

kevinfodness commented 7 months ago

We can likely crib from what Jetpack 2FA does: https://github.com/Automattic/jetpack-force-2fa/blob/master/jetpack-force-2fa.php

kevinfodness commented 7 months ago

Actually, maybe not, that's pretty Jetpack-specific.

kevinfodness commented 7 months ago

VIP's two factor settings might be a more appropriate place to borrow from: https://github.com/Automattic/vip-go-mu-plugins-built/blob/7703f4cd3c41fc65c67114ca4d7131e28653dc05/two-factor.php

kevinfodness commented 7 months ago

Alley folks: Example of implementation in code: https://l.alley.dev/27d7d96371

anubisthejackle commented 3 months ago

@mogmarsh @kevinfodness Looking at the example implementation, I'm assuming that this is an example using the Two Factor plugin.

If I'm not mistaken, the final implementation for this feature will need to combine both the Automattic and the Alley implementation, right?

mogmarsh commented 3 months ago

@anubisthejackle, VIP forces it, so I think we only need this for the Alley implementation on non-VIP sites. The goal is to hold those non-VIP sites to the same standard.

kevinfodness commented 3 months ago

For sites on VIP, this feature should make what VIP already does more strict. From the docs:

By default, two-factor authentication is required on the VIP Platform for all WordPress user accounts with an Administrator role or custom roles with the manage_options capability.

We want to enforce this for all users with the edit_posts capability. We can use VIP's example for how to do this as a basis (they helpfully show how to do this for all users with edit_posts, which is what we want): https://docs.wpvip.com/security/two-factor-authentication/#h-enforce-two-factor-authentication-for-user-roles-and-capabilities

However, we will need a logic fork for "is this on VIP" vs. not. If on VIP, use the VIP filter, else use functionality that works natively with the Two Factor plugin. Since VIP uses the Two Factor plugin under the hood, we don't want to use the native Two Factor hooks if we're on VIP, otherwise the logic will run twice (once on VIP's hook, once on the plugin's hook).