Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Enable 2FA by default #1213

Merged
merged 14 commits into from
Apr 30, 2019
Merged

Enable 2FA by default #1213

merged 14 commits into from
Apr 30, 2019

Conversation

joshbetz
Copy link
Contributor

@joshbetz joshbetz commented Apr 11, 2019

Enables two-factor by default and requires it for anyone with edit_posts capabilities.

Deploy Notes

We need to deploy a fix to two-factor that lets it work with Jetpack SSO: WordPress/two-factor#276 / #1214

Testing

  1. Load this PR on a sandbox.
  2. You may need to comment out the line that disables this for proxied users.
  3. See that most of the sidebar options are gone except for Profile.
  4. Enable two-factor in your profile.
  5. See that sidebar options are back as expected.

@joshbetz
Copy link
Contributor Author

We should just need Automattic/jetpack#12029 to make this all work.

@joshbetz
Copy link
Contributor Author

joshbetz commented Apr 12, 2019

Instead of the previous Jetpack PR, we need WordPress/two-factor#276 / #1214

two-factor.php Outdated Show resolved Hide resolved
@joshbetz joshbetz marked this pull request as ready for review April 12, 2019 16:55
@joshbetz
Copy link
Contributor Author

I'm thinking we get this deployed with wpcom_vip_force_two_factor defaulting to false for now. So, nobody is locked out, but you can still enable 2FA or opt-in to force it on your site.

Then we announce via the lobby that you can filter that now to force 2FA for any users you want and starting some time in the future (maybe 1 month from now?) we will default it for all users that can edit posts.

Copy link
Member

@mjangda mjangda left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Spotted a couple of minor things.

two-factor.php Outdated Show resolved Hide resolved
two-factor.php Outdated Show resolved Hide resolved
two-factor.php Outdated Show resolved Hide resolved
two-factor.php Show resolved Hide resolved
@mjangda
Copy link
Member

mjangda commented Apr 23, 2019

We'll want to test this out with the following scenarios:

  • Applications using the Jetpack Force 2fa plugin
  • Applications using the force 2fa setting in Jetpack
  • Applications manually enabling two-factor and our filter

two-factor.php Outdated Show resolved Hide resolved
joshbetz and others added 6 commits April 23, 2019 14:33
The only caps a restricted user needs are `read` and `edit_user` (only for theirself).
The issue in question has been fixed, so U2F should be safe to use now.
@joshbetz joshbetz force-pushed the update/enforce-2fa branch from a831d3b to 8bfe566 Compare April 23, 2019 19:33
If we call current_user_can inside map_map_cap, we create a callback loop. By calling it outside, we avoid that issue.
Before this date, we just output an admin notice to users that will need to enable 2FA. The logic to display the date got a little complex, but
ensuring the notice is in their local timezone is worth it.

After the date specified, the admin notice switches to say 2FA is required and only the most basic functions are available.

This is a date that's a month from now in the middle-ish of the day in the middle of the week (so plenty of support will be around in case there are questions).
@joshbetz
Copy link
Contributor Author

In 1f8dc49 we time gate forcing for everyone with edit_posts after Thursday, May 23, 2019 at 18:00 UTC / 2 PM ET. Happy to change that date and time as necessary.

@joshbetz
Copy link
Contributor Author

Whatever date we decide, we should set up a couple Slack reminders :)

This allows clients to override the time gate and force two factor at any time. Previously
two factor could only be force enabled after the date specified.
If the timezone string is not set, use UTC as a backup.
@joshbetz
Copy link
Contributor Author

Tested variations sites that are manually loading two-factor and our filter.

wpcom_vip_load_plugin( 'two-factor' );
add_action( 'muplugins_loaded', 'wpcom_enable_two_factor_plugin' );

Also, sites that load another copy of two-factor in the admin.

I didn't experience any issues. The sites I tested on a sandbox transparently switched to our version of two-factor and showed the admin notice as expected.

@joshbetz
Copy link
Contributor Author

Also tested sites with jetpack-force-2fa enabled, the require two-step setting in Jetpack, and both at the same time.

@joshbetz
Copy link
Contributor Author

@mjangda I think we should be all set here as long as we're OK with May 23 @ 1800 UTC for forcing 2FA for everyone.

```
Notice: wp_register_script was called incorrectly. Scripts and styles should not be registered or enqueued until the wp_enqueue_scripts, admin_enqueue_scripts, or login_enqueue_scripts hooks.
```
two-factor.php Show resolved Hide resolved
two-factor.php Show resolved Hide resolved
two-factor.php Show resolved Hide resolved
two-factor.php Outdated Show resolved Hide resolved
Make the message more prominent, consistent, and add more context.
@mjangda
Copy link
Member

mjangda commented Apr 26, 2019

Only blocker now is being able to link directly to the "Two-Factor Options". Everything else is good to go.

Copy link
Member

@mjangda mjangda left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍 🎉 🎂

@mjangda
Copy link
Member

mjangda commented Apr 26, 2019

To be deployed on Tuesday.

@joshbetz joshbetz merged commit 652461b into master Apr 30, 2019
@joshbetz joshbetz deleted the update/enforce-2fa branch April 30, 2019 13:23
@joshbetz
Copy link
Contributor Author

r138198-deploy

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants