Force Two Factor #1

mikeselander · 2018-09-03T22:53:55Z

Adds a network-level setting to require two-factor authentication on a per-role or universal 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 settings:

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

…pecific user roles.

…a or not

…gistered REST constant

…of settings

mikeselander · 2018-09-04T19:49:21Z

@joehoyle can I please get a review and test from you on this functionality?

It's specifically setup for networks at the moment and also worth noting that I've written the code inline with the standards and style setup for the upstream repo. A lot of the style and architectural pattern is pretty gross, but hoping that we can get this upstreamed faster by going that route.

joehoyle · 2018-09-04T20:11:04Z

@mikeselander functinalty wise: it's working great! Though the enable screen looks like this:

Have those items at the bottom shown. I ran npm install and grunt, and then I don't have any errors in console for missing assets. Also if I submit it and there's an error, it seems it doesn't show any validation.

+1 for a non-multisite support too, as central.aws.hmn.md isn't multisite. I'd imagine you could just use the existing functions for that?

mikeselander · 2018-09-04T20:32:35Z

Have those items at the bottom shown.

Odd, I have never seen those 🤔 I'll try to re-produce.

Also if I submit it and there's an error, it seems it doesn't show any validation.

This is an architectural problem with the plugin itself that would require modifying a good bit of internals to fix. The plugin is built to only ever work on a save submission from a user panel and none of them return or throw errors :(

Some of the methods (TOTP for example) do show an error message within their little pane if something has gone wrong.

+1 for a non-multisite support too, as central.aws.hmn.md isn't multisite. I'd imagine you could just use the existing functions for that?

Ya, I can mostly use the same functions and then registering the options for single sites. I'll add that in 👍

mikeselander · 2018-09-04T23:12:26Z

@joehoyle I've fixed this visual issues and added single site support. Mind taking another look through?

rmccue · 2018-09-05T03:22:34Z

Should this be PR'd directly to the upstream repo rather than merged?

joehoyle · 2018-09-05T15:01:42Z

@mikeselander nice, it's looking pretty good to me. I don't know why the plugin shows the "Security Keys" section if you haven't selected U2F.

Out of interest, does the plugin allow you to filter the options for 2fa? I guess it could be a site setting again, but it seems odd site administrators can't whitelist the allowed 2fa authentication methods.

joehoyle · 2018-09-05T15:04:37Z

class.two-factor-core.php

+		$two_factor_forced_roles = self::get_forced_user_roles();
+		$required_roles          = array_filter( $user->roles, function( $role ) use ( $two_factor_forced_roles ) {
+			return in_array( $role, $two_factor_forced_roles, true );
+		}, ARRAY_FILTER_USE_BOTH);


Missing space at the end.

joehoyle · 2018-09-05T15:05:39Z

class.two-factor-core.php

+	 * @return bool
+	 */
+	public static function get_universally_forced_option() {
+		$is_forced = is_multisite() ? get_site_option( self::FORCED_SITE_META_KEY, false ) : get_option( self::FORCED_SITE_META_KEY, false );


You don't need this ternary:

This function is almost identical to get_option(), except that in multisite, it returns the network-wide option. For non-multisite installs, it uses get_option.

joehoyle · 2018-09-05T15:06:34Z

class.two-factor-core.php

+			<tbody>
+				<tr>
+					<th scope="row">
+						<?php esc_html_e( 'Universally force two-factor', 'two-factor' ); ?>


I'm not sure of this working. "Force all users to enable two-factor"?

joehoyle · 2018-09-05T15:09:36Z

@mikeselander actually, I see for enable 2fa you are doing it inline, as opposed to redirecting to a specific URL. For whatever reason this means if I am on hte homepage, it shows like this:

It might be better to have a discrete URL where this is shown (maybe wp-login.php?action=force-2fa for example), rather than trying to show it in-place on all URLs.

joehoyle · 2018-09-05T15:28:07Z

Also noticed I have a fatal on the homepage screenshot:

Fatal error: Uncaught Error: Call to undefined function convert_to_screen() in /usr/src/app/wordpress/wp-admin/includes/class-wp-list-table.php:132 Stack trace: #0 /usr/src/app/content/plugins/two-factor/providers/class.two-factor-fido-u2f-admin.php(189): WP_List_Table->__construct() #1 /usr/src/app/wordpress/wp-includes/class-wp-hook.php(286): Two_Factor_FIDO_U2F_Admin::show_user_profile(Object(WP_User)) #2 /usr/src/app/wordpress/wp-includes/class-wp-hook.php(310): WP_Hook->apply_filters('', Array) #3 /usr/src/app/wordpress/wp-includes/plugin.php(453): WP_Hook->do_action(Array) #4 /usr/src/app/content/plugins/two-factor/class.two-factor-core.php(827): do_action('show_user_secur...', Object(WP_User)) #5 /usr/src/app/content/plugins/two-factor/class.two-factor-core.php(537): Two_Factor_Core::user_two_factor_options(Object(WP_User)) #6 /usr/src/app/content/plugins/two-factor/class.two-factor-core.php(501): Two_Factor_Core::force_2fa_login_html() #7 /usr/src/app/wordpress/wp-includes/class-wp-hook.php(286): Two_Factor_Core:: in /usr/src/app/wordpress/wp-admin/includes/class-wp-list-table.php on line 132

mikeselander · 2018-09-05T16:38:17Z

Also noticed I have a fatal on the homepage screenshot:

Will look and re-produce 👍

@mikeselander nice, it's looking pretty good to me. I don't know why the plugin shows the "Security Keys" section if you haven't selected U2F.

Ya, that's just something that it does ¯\_(ツ)_/¯

Out of interest, does the plugin allow you to filter the options for 2fa? I guess it could be a site setting again, but it seems odd site administrators can't whitelist the allowed 2fa authentication methods.

It does allow you to filter the methods that are available - The filter is two_factor_providers and can be found here: https://github.com/humanmade/two-factor/blob/master/class.two-factor-core.php#L77-L86

mikeselander · 2018-09-05T16:39:11Z

Should this be PR'd directly to the upstream repo rather than merged?

@rmccue I wanted to get Joe's feedback before opening a PR against the upstream repo. I also am wary of how long this could take to get merged into the upstream repo and my client needs it this week.

rmccue · 2018-09-06T02:38:41Z

I wanted to get Joe's feedback before opening a PR against the upstream repo. I also am wary of how long this could take to get merged into the upstream repo and my client needs it this week.

Yeah, just wanted to check if we wanted to fork or not :)

rmccue · 2018-10-16T03:47:37Z

Linking the upstream PR: WordPress#239

Should we close this PR in favour of handling it over there?

Allow user to remove forced 2fa on all roles

shadyvb · 2022-02-14T13:42:15Z

Opening back now, since upstream PR hasn't been merged.

shadyvb · 2022-02-14T14:33:11Z

Travis install seems to be busted here, so will ignore and merge, tested the functionality and it seems to be working as expected.

roborourke · 2022-02-14T14:35:33Z

Yeah I think it's missing a git submodule for the xwp dev library

roborourke · 2022-02-14T14:53:42Z

What are your thoughts on the release version? 0.2.2? technically 0.2.1 introduced a bug by missing a bunch of this plugin :/

We can go for 0.3.0 but we'll need to backport that update through to v7

Update function.login-header.php to be compatible with WP 5.7

Update class-two-factor-email.php

mikeselander added 18 commits September 3, 2018 14:01

Add network-level settings to force 2-factor auth for whole site or s…

c5e1502

…pecific user roles.

Add helper functions to evaluate whether the current user requires 2f…

571bc63

…a or not

Add basic force-2fa-view functionality

48e4ba1

Add form submission and AJAX handling of two-factor settings form

4e2873c

Small self-review cleanups

7d478a9

Load force-2fa script more intelligently

de2d5bb

Replace Back to Site link with Logout link as an escape hatch

28024b8

Do not block REST requests

8ace959

Clean up title styling

0ed9963

Prepare value getters to work with either multi or single site

884baed

Handle REST check with two separate hooks to properly capture late-re…

82ee390

…gistered REST constant

Add hook for providers to save their data against on AJAX submission …

1db7029

…of settings

Remove inline styles and use user-edit CSS instead

5b644f7

Hide unnecessary label on force-2fa view

1ace71e

CS cleanup

bad9706

Add tests for business-critical methods

681da69

Add returns to maybe_force_2fa_settings for better testability

8d531bf

Self-review cleanup

d4712ce

mikeselander requested a review from joehoyle September 4, 2018 19:49

mikeselander added 2 commits September 4, 2018 15:51

Register and save values against a single site'

bcbfec3

Enqueue assets for Fido 2f on forced 2f takeover

0898fdc

joehoyle reviewed Sep 5, 2018

View reviewed changes

mikeselander added 7 commits September 5, 2018 10:45

Add spacing

26824fc

Remove unnecessary ternary

51b93a3

Update language on global 2fa forcing option

2723f88

Split up core class into Force and Core for better separation

8980d90

Add custom path and handling for force-2fa

e8ccab1

Clean up

13a8cf7

Clean up according to es linter on Travis

0b356f3

mikeselander added 2 commits September 17, 2018 17:00

Add phpcs:ignore for incorrectly flagged issues

de8ea84

Properly space these

7b4cce3

mikeselander closed this Oct 18, 2018

tcrsavage and others added 2 commits May 8, 2019 16:09

Allow user to remove forced 2fa on all roles

f7cc27e

Merge pull request #2 from humanmade/force-2fa-fix-empty-role-config

252e3e0

Allow user to remove forced 2fa on all roles

shadyvb mentioned this pull request Feb 14, 2022

Force 2FA #5

Closed

shadyvb reopened this Feb 14, 2022

shadyvb approved these changes Feb 14, 2022

View reviewed changes

shadyvb merged commit 6b59def into master Feb 14, 2022

joehoyle pushed a commit that referenced this pull request Aug 4, 2023

Merge pull request #1 from supermundano/cfaria-patch-1

1e649fe

Update function.login-header.php to be compatible with WP 5.7

joehoyle pushed a commit that referenced this pull request Aug 4, 2023

Merge pull request #1 from EncodeDotHost/nbwpuk-email-patch

9c9e8be

Update class-two-factor-email.php

masteradhoc mentioned this pull request Feb 1, 2026

Optionally Force 2fa WordPress/two-factor#239

Open

Force Two Factor #1

Force Two Factor #1

Uh oh!

Conversation

mikeselander commented Sep 3, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mikeselander commented Sep 4, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

joehoyle commented Sep 4, 2018

Uh oh!

mikeselander commented Sep 4, 2018

Uh oh!

mikeselander commented Sep 4, 2018

Uh oh!

rmccue commented Sep 5, 2018

Uh oh!

joehoyle commented Sep 5, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

joehoyle Sep 5, 2018

Choose a reason for hiding this comment

Uh oh!

joehoyle Sep 5, 2018

Choose a reason for hiding this comment

Uh oh!

mikeselander Sep 5, 2018

Choose a reason for hiding this comment

Uh oh!

joehoyle Sep 5, 2018

Choose a reason for hiding this comment

Uh oh!

joehoyle commented Sep 5, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

joehoyle commented Sep 5, 2018

Uh oh!

mikeselander commented Sep 5, 2018

Uh oh!

mikeselander commented Sep 5, 2018

Uh oh!

rmccue commented Sep 6, 2018

Uh oh!

rmccue commented Oct 16, 2018

Uh oh!

shadyvb commented Feb 14, 2022

Uh oh!

shadyvb commented Feb 14, 2022

Uh oh!

roborourke commented Feb 14, 2022

Uh oh!

roborourke commented Feb 14, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

mikeselander commented Sep 3, 2018 •

edited

Loading

mikeselander commented Sep 4, 2018 •

edited

Loading

joehoyle commented Sep 5, 2018 •

edited

Loading

joehoyle commented Sep 5, 2018 •

edited

Loading