From c5e1502f2ac0853150f875847c6f5c01fb0388f2 Mon Sep 17 00:00:00 2001 From: Mike Selander Date: Mon, 3 Sep 2018 14:01:34 -0600 Subject: [PATCH 01/30] Add network-level settings to force 2-factor auth for whole site or specific user roles. --- class.two-factor-core.php | 138 ++++++++++++++++++++++++++++++++++++++ 1 file changed, 138 insertions(+) diff --git a/class.two-factor-core.php b/class.two-factor-core.php index 1a4d6d2e..2317bcf2 100644 --- a/class.two-factor-core.php +++ b/class.two-factor-core.php @@ -22,6 +22,20 @@ class Two_Factor_Core { */ const ENABLED_PROVIDERS_USER_META_KEY = '_two_factor_enabled_providers'; + /** + * The network forced 2fa user roles key. + * + * @type string + */ + const FORCED_ROLES_META_KEY = '_two_factor_forced_roles'; + + /** + * The network forced 2fa user roles key. + * + * @type string + */ + const FORCED_SITE_META_KEY = '_two_factor_forced_universally'; + /** * The user meta nonce key. * @@ -47,6 +61,17 @@ public static function add_hooks() { add_filter( 'manage_users_columns', array( __CLASS__, 'filter_manage_users_columns' ) ); add_filter( 'wpmu_users_columns', array( __CLASS__, 'filter_manage_users_columns' ) ); add_filter( 'manage_users_custom_column', array( __CLASS__, 'manage_users_custom_column' ), 10, 3 ); + + // Forced 2fa login functionality. + // @todo:: display settings to force 2fa on specific site, if site is not network. + // @todo:: Add action to save said setting if site is not network. + add_action( 'wpmu_options', array( __CLASS__, 'force_two_factor_setting_options' ) ); + add_action( 'update_wpmu_options', array( __CLASS__, 'save_network_force_two_factor_update' ) ); + + // @todo:: Add re-direct to user profile page if criteria is met. + // @todo:: Add notice explaining re-direct. + + // @todo:: maybe, instead, use a login-style page with 2fa settings. } /** @@ -697,4 +722,117 @@ public static function user_two_factor_options_update( $user_id ) { } } } + + /** + * Get whether site has two-factor universally forced or not. + * + * @since 0.1-dev + * + * @return bool + * + * @todo:: make work on single sites also. + */ + public static function get_universally_forced_option() { + /** + * Whether or not site has two-factor universally forced. + * + * @param bool $roles Whether all users on a site are forced to use 2fa. + */ + return (bool) apply_filters( 'two_factor_universally_forced', get_site_option( self::FORCED_SITE_META_KEY, false ) ); + } + + /** + * Get which user roles have two-factor forced. + * + * @since 0.1-dev + * + * @return array + * + * @todo:: make work on single sites also. + */ + 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, [] ) ); + } + + /** + * Add network and site-level fields for forcing 2-factor on users of a role(s). + * + * @since 0.1-dev + */ + public static function force_two_factor_setting_options() { + $forced_roles = self::get_forced_user_roles(); + $is_universally_forced = self::get_universally_forced_option(); + ?> + +

+ + + + + + + + + + + + + +
+ + + +
+ + + $role ) : ?> + +
+ +
+ Date: Mon, 3 Sep 2018 14:16:54 -0600 Subject: [PATCH 02/30] Add helper functions to evaluate whether the current user requires 2fa or not --- class.two-factor-core.php | 39 +++++++++++++++++++++++++++++++++++++++ 1 file changed, 39 insertions(+) diff --git a/class.two-factor-core.php b/class.two-factor-core.php index 2317bcf2..d26317e4 100644 --- a/class.two-factor-core.php +++ b/class.two-factor-core.php @@ -723,6 +723,45 @@ public static function user_two_factor_options_update( $user_id ) { } } + /** + * Check whether the current user requires two_factor or not. + * + * @return bool Whether user should be required to use 2fa. + */ + public static function is_two_factor_forced_for_current_user() { + $id = get_current_user_id(); + return self::is_two_factor_forced( $id ); + } + + /** + * Check whether a user should have 2fa forced on or not. + * + * @param int $user_id User ID to check against. + * @return bool Whether user should be required to use 2fa. + */ + public static function is_two_factor_forced( int $user_id ) : bool { + // If 2fa is forced for all users, always return true. + if ( self::get_universally_forced_option() ) { + return true; + } + + $user = get_user_by( 'id', $user_id ); + + // If we can't pull up user, escape. + if ( ! ( $user instanceof WP_User ) ) { + return false; + } + + // Check whether a user is in a user role that requires two-factor authentication. + $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); + + // If the required_roles is not empty, then the user is in a role that requires two_factor authentication. + return ! empty( $required_roles ); + } + /** * Get whether site has two-factor universally forced or not. * From 48e4ba114d497b8b48a12e2efceed53ebed767c4 Mon Sep 17 00:00:00 2001 From: Mike Selander Date: Mon, 3 Sep 2018 15:32:23 -0600 Subject: [PATCH 03/30] Add basic force-2fa-view functionality --- class.two-factor-core.php | 87 ++++++++++++++++++++++++++++-- includes/function.login-header.php | 6 +++ 2 files changed, 89 insertions(+), 4 deletions(-) diff --git a/class.two-factor-core.php b/class.two-factor-core.php index d26317e4..15833213 100644 --- a/class.two-factor-core.php +++ b/class.two-factor-core.php @@ -68,10 +68,8 @@ public static function add_hooks() { add_action( 'wpmu_options', array( __CLASS__, 'force_two_factor_setting_options' ) ); add_action( 'update_wpmu_options', array( __CLASS__, 'save_network_force_two_factor_update' ) ); - // @todo:: Add re-direct to user profile page if criteria is met. - // @todo:: Add notice explaining re-direct. - - // @todo:: maybe, instead, use a login-style page with 2fa settings. + // @todo:: Add re-direct to 2fa settings page if criteria is met. + add_filter( 'init', array( __CLASS__, 'maybe_force_2fa_settings' ) ); } /** @@ -439,6 +437,87 @@ public static function login_html( $user, $login_nonce, $redirect_to, $error_msg +
+ + +
+ + +

+ +

+ + + + +
+ + + + + Date: Mon, 3 Sep 2018 16:41:55 -0600 Subject: [PATCH 04/30] Add form submission and AJAX handling of two-factor settings form --- assets/js/force-2fa.js | 86 +++++++++++++++++++++++++++++++++++++++ class.two-factor-core.php | 35 +++++++++++++--- 2 files changed, 116 insertions(+), 5 deletions(-) create mode 100644 assets/js/force-2fa.js diff --git a/assets/js/force-2fa.js b/assets/js/force-2fa.js new file mode 100644 index 00000000..6560b255 --- /dev/null +++ b/assets/js/force-2fa.js @@ -0,0 +1,86 @@ +/* global ajaxurl */ + +/** + * Checks that an element has a non-empty `name` and `value` property. + * + * @param {Element} element The element to check + * @return {Boolean} true if the element is an input, false if not + */ +var isValidElement = function( element ) { + return element.name && element.value; +}; + +/** + * Checks if an element’s value can be saved (e.g. not an unselected checkbox). + * + * @param {Element} element The element to check + * @return {Boolean} true if the value should be added, false if not + */ +var isValidValue = function( element ) { + return ( ! [ 'checkbox', 'radio' ].includes( element.type ) || element.checked ); +}; + +/** + * Checks if an input is a checkbox, because checkboxes allow multiple values. + * + * @param {Element} element The element to check + * @return {Boolean} true if the element is a checkbox, false if not + */ +var isCheckbox = function( element ) { + return element.type === 'checkbox' +}; + +/** + * Retrieves input data from a form and returns it as a JSON object. + * + * @param {HTMLFormControlsCollection} elements the form elements + * @return {Object} form data as an object literal + */ +var formToJSON = function( elements ) { + return [].reduce.call( elements, ( data, element ) => { + // Make sure the element has the required properties and should be added. + if ( !isValidElement( element ) || !isValidValue( element ) ) { + return data; + } + + /* + * Some fields allow for more than one value, so we need to check if this + * is one of those fields and, if so, store the values as an array. + */ + if ( isCheckbox( element ) ) { + data[ element.name ] = ( data[ element.name ] || [] ).concat( element.value ); + } else { + data[ element.name ] = element.value; + } + + return data; + }, {} ) +}; + +/** + * A handler function to prevent default submission and run our custom script. + * + * @param {Event} event the submit event triggered by the user + */ +var handleFormSubmit = function( event ) { + event.preventDefault(); + + // Get form data. + var formData = formToJSON( event.target.elements ); + + formData.action = 'two_factor_force_form_submit'; + + // Submit data to WordPress. + jQuery.post( + ajaxurl, + formData, + function() { + window.location.reload(); + } + ); +}; + +window.addEventListener( 'load', function() { + var form = document.querySelector( '#force_2fa_form' ); + form.addEventListener( 'submit', handleFormSubmit ); +} ); diff --git a/class.two-factor-core.php b/class.two-factor-core.php index 15833213..37319aee 100644 --- a/class.two-factor-core.php +++ b/class.two-factor-core.php @@ -67,9 +67,8 @@ public static function add_hooks() { // @todo:: Add action to save said setting if site is not network. add_action( 'wpmu_options', array( __CLASS__, 'force_two_factor_setting_options' ) ); add_action( 'update_wpmu_options', array( __CLASS__, 'save_network_force_two_factor_update' ) ); - - // @todo:: Add re-direct to 2fa settings page if criteria is met. add_filter( 'init', array( __CLASS__, 'maybe_force_2fa_settings' ) ); + add_action( 'wp_ajax_two_factor_force_form_submit', array( __CLASS__, 'handle_force_2fa_submission' ) ); } /** @@ -479,6 +478,15 @@ public static function maybe_force_2fa_settings() { */ public static function force_2fa_login_html() { + wp_enqueue_script( 'jquery' ); + wp_enqueue_script( + 'two_factor_form_script', + plugins_url( 'assets/js/force-2fa.js', __FILE__ ), + [], + '0.1', + false + ); + if ( ! function_exists( 'login_header' ) ) { // We really should migrate login_header() out of `wp-login.php` so it can be called from an includes file. include_once( TWO_FACTOR_DIR . 'includes/function.login-header.php' ); @@ -488,10 +496,10 @@ public static function force_2fa_login_html() { $user = wp_get_current_user(); - // Display the form for updating a user's two-factor options.xw - // @todo:: fill in the action here. + // Display the form for updating a user's two-factor options. ?> -
+

+
@@ -507,8 +515,15 @@ public static function force_2fa_login_html() { width: 100%; max-width: 1000px; } + .login .button-primary { + float: left; + } + + @@ -518,6 +533,16 @@ public static function force_2fa_login_html() { Date: Tue, 4 Sep 2018 09:37:40 -0600 Subject: [PATCH 05/30] Small self-review cleanups --- class.two-factor-core.php | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/class.two-factor-core.php b/class.two-factor-core.php index 37319aee..8086a61b 100644 --- a/class.two-factor-core.php +++ b/class.two-factor-core.php @@ -30,7 +30,7 @@ class Two_Factor_Core { const FORCED_ROLES_META_KEY = '_two_factor_forced_roles'; /** - * The network forced 2fa user roles key. + * The network forced 2fa global key. * * @type string */ @@ -475,7 +475,6 @@ public static function maybe_force_2fa_settings() { * If a user hits this screen, they must setup 2fa and do not get to skip. * * @since 0.1-dev - */ public static function force_2fa_login_html() { wp_enqueue_script( 'jquery' ); @@ -504,7 +503,6 @@ public static function force_2fa_login_html() { -

@@ -521,7 +519,7 @@ public static function force_2fa_login_html() { $role ) : ?>
From de2d5bb23e9d4e00eaaab4bba8ad2a587158519c Mon Sep 17 00:00:00 2001 From: Mike Selander Date: Tue, 4 Sep 2018 10:15:43 -0600 Subject: [PATCH 06/30] Load force-2fa script more intelligently --- class.two-factor-core.php | 26 +++++++++++++++++++------- 1 file changed, 19 insertions(+), 7 deletions(-) diff --git a/class.two-factor-core.php b/class.two-factor-core.php index 8086a61b..53af4689 100644 --- a/class.two-factor-core.php +++ b/class.two-factor-core.php @@ -65,6 +65,7 @@ public static function add_hooks() { // Forced 2fa login functionality. // @todo:: display settings to force 2fa on specific site, if site is not network. // @todo:: Add action to save said setting if site is not network. + add_action( 'init', array( __CLASS__, 'register_scripts' ) ); add_action( 'wpmu_options', array( __CLASS__, 'force_two_factor_setting_options' ) ); add_action( 'update_wpmu_options', array( __CLASS__, 'save_network_force_two_factor_update' ) ); add_filter( 'init', array( __CLASS__, 'maybe_force_2fa_settings' ) ); @@ -80,6 +81,20 @@ public static function load_textdomain() { load_plugin_textdomain( 'two-factor' ); } + /** + * Register scripts. + */ + public static function register_scripts() { + // Script for handling AJAX submission in force 2fa takeover screen. + wp_register_script( + 'two-factor-form-script', + plugins_url( 'assets/js/force-2fa.js', __FILE__ ), + [], + '0.1', + false + ); + } + /** * For each provider, include it and then instantiate it. * @@ -478,13 +493,7 @@ public static function maybe_force_2fa_settings() { */ public static function force_2fa_login_html() { wp_enqueue_script( 'jquery' ); - wp_enqueue_script( - 'two_factor_form_script', - plugins_url( 'assets/js/force-2fa.js', __FILE__ ), - [], - '0.1', - false - ); + wp_enqueue_script( 'two-factor-form-script' ); if ( ! function_exists( 'login_header' ) ) { // We really should migrate login_header() out of `wp-login.php` so it can be called from an includes file. @@ -531,6 +540,9 @@ public static function force_2fa_login_html() { Date: Tue, 4 Sep 2018 10:19:59 -0600 Subject: [PATCH 07/30] Replace Back to Site link with Logout link as an escape hatch --- class.two-factor-core.php | 18 +++++++++++++++++- 1 file changed, 17 insertions(+), 1 deletion(-) diff --git a/class.two-factor-core.php b/class.two-factor-core.php index 53af4689..8ee75b7c 100644 --- a/class.two-factor-core.php +++ b/class.two-factor-core.php @@ -464,6 +464,11 @@ public static function maybe_force_2fa_settings() { return; } + // Should not interrupt logging in or out. + if ( self::is_login_page() ) { + return; + } + // If user is not logged in, they can't 2fa anyway. if ( ! is_user_logged_in() ) { return; @@ -513,7 +518,9 @@ public static function force_2fa_login_html() {

- + + +

@@ -770,7 +759,7 @@ public static function manage_users_custom_column( $output, $column_name, $user_ * @param WP_User $user WP_User object of the logged-in user. */ public static function user_two_factor_options( $user ) { - wp_enqueue_style( 'user-edit-2fa', plugins_url( 'user-edit.css', __FILE__ ) ); + wp_enqueue_style( 'user-edit-2fa' ); $enabled_providers = array_keys( self::get_available_providers_for_user( $user->ID ) ); $primary_provider = self::get_primary_provider_for_user( $user->ID ); diff --git a/user-edit.css b/user-edit.css index 9572fb6f..b70aa20a 100644 --- a/user-edit.css +++ b/user-edit.css @@ -34,4 +34,20 @@ .two-factor-methods-table tbody tr:nth-child(odd) { background-color: #f9f9f9; -} \ No newline at end of file +} + +/* Special modifications for use on force-2fa screen */ +#login { + width: 100%; + max-width: 1000px; +} + +.login .button-primary { + float: left; +} + +.force-2fa-title { + line-height: 1.3; + text-align: center; + padding: 0 10%; +} From 1ace71e4ea4eb09ad4957db4b9b1c6f109992c62 Mon Sep 17 00:00:00 2001 From: Mike Selander Date: Tue, 4 Sep 2018 11:37:25 -0600 Subject: [PATCH 14/30] Hide unnecessary label on force-2fa view --- class.two-factor-core.php | 2 +- user-edit.css | 5 +++++ 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/class.two-factor-core.php b/class.two-factor-core.php index 21f9ce10..08b27dcf 100644 --- a/class.two-factor-core.php +++ b/class.two-factor-core.php @@ -776,7 +776,7 @@ public static function user_two_factor_options( $user ) { -
+ diff --git a/user-edit.css b/user-edit.css index b70aa20a..d70e95fd 100644 --- a/user-edit.css +++ b/user-edit.css @@ -51,3 +51,8 @@ text-align: center; padding: 0 10%; } + +/* Hackity hack to hid the title on the force-2fa view */ +.login .two-factor-main-label { + display: none; +} From bad970648954b20671145984ee80279cc87c8913 Mon Sep 17 00:00:00 2001 From: Mike Selander Date: Tue, 4 Sep 2018 12:49:52 -0600 Subject: [PATCH 15/30] CS cleanup --- assets/js/force-2fa.js | 24 ++++++++++++------------ class.two-factor-core.php | 9 +++++---- 2 files changed, 17 insertions(+), 16 deletions(-) diff --git a/assets/js/force-2fa.js b/assets/js/force-2fa.js index 6560b255..921a5859 100644 --- a/assets/js/force-2fa.js +++ b/assets/js/force-2fa.js @@ -1,4 +1,4 @@ -/* global ajaxurl */ +/* global ajaxurl, jQuery */ /** * Checks that an element has a non-empty `name` and `value` property. @@ -6,7 +6,7 @@ * @param {Element} element The element to check * @return {Boolean} true if the element is an input, false if not */ -var isValidElement = function( element ) { +var isValidElement = function ( element ) { return element.name && element.value; }; @@ -16,7 +16,7 @@ var isValidElement = function( element ) { * @param {Element} element The element to check * @return {Boolean} true if the value should be added, false if not */ -var isValidValue = function( element ) { +var isValidValue = function ( element ) { return ( ! [ 'checkbox', 'radio' ].includes( element.type ) || element.checked ); }; @@ -26,8 +26,8 @@ var isValidValue = function( element ) { * @param {Element} element The element to check * @return {Boolean} true if the element is a checkbox, false if not */ -var isCheckbox = function( element ) { - return element.type === 'checkbox' +var isCheckbox = function ( element ) { + return element.type === 'checkbox'; }; /** @@ -36,10 +36,10 @@ var isCheckbox = function( element ) { * @param {HTMLFormControlsCollection} elements the form elements * @return {Object} form data as an object literal */ -var formToJSON = function( elements ) { - return [].reduce.call( elements, ( data, element ) => { +var formToJSON = function ( elements ) { + return [].reduce.call( elements, function ( data, element ) { // Make sure the element has the required properties and should be added. - if ( !isValidElement( element ) || !isValidValue( element ) ) { + if ( ! isValidElement( element ) || ! isValidValue( element ) ) { return data; } @@ -54,7 +54,7 @@ var formToJSON = function( elements ) { } return data; - }, {} ) + }, {} ); }; /** @@ -62,7 +62,7 @@ var formToJSON = function( elements ) { * * @param {Event} event the submit event triggered by the user */ -var handleFormSubmit = function( event ) { +var handleFormSubmit = function ( event ) { event.preventDefault(); // Get form data. @@ -74,13 +74,13 @@ var handleFormSubmit = function( event ) { jQuery.post( ajaxurl, formData, - function() { + function () { window.location.reload(); } ); }; -window.addEventListener( 'load', function() { +window.addEventListener( 'load', function () { var form = document.querySelector( '#force_2fa_form' ); form.addEventListener( 'submit', handleFormSubmit ); } ); diff --git a/class.two-factor-core.php b/class.two-factor-core.php index 08b27dcf..d361bcb9 100644 --- a/class.two-factor-core.php +++ b/class.two-factor-core.php @@ -539,7 +539,8 @@ public static function force_2fa_login_html() { + do_action( 'login_footer' ); + ?>
@@ -880,9 +881,9 @@ public static function is_two_factor_forced( int $user_id ) : bool { // Check whether a user is in a user role that requires two-factor authentication. $two_factor_forced_roles = self::get_forced_user_roles(); - $required_roles = array_filter( $user->roles, function( $role ) use ( $two_factor_forced_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); + }, ARRAY_FILTER_USE_BOTH); // If the required_roles is not empty, then the user is in a role that requires two_factor authentication. return ! empty( $required_roles ); @@ -995,7 +996,7 @@ public static function save_network_force_two_factor_update() { } // Whitelist roles against valid WordPress role slugs. - $saved_roles = array_filter( $_POST[ self::FORCED_ROLES_META_KEY ], function( $is_role_saved, $role_slug ) { + $saved_roles = array_filter( wp_unslash( $_POST[ self::FORCED_ROLES_META_KEY ] ), function( $is_role_saved, $role_slug ) { return $is_role_saved && in_array( $role_slug, array_keys( get_editable_roles() ), true ); }, ARRAY_FILTER_USE_BOTH ); From 681da695bcd4828f5421420cf01c6d02fdd5b155 Mon Sep 17 00:00:00 2001 From: Mike Selander Date: Tue, 4 Sep 2018 13:20:26 -0600 Subject: [PATCH 16/30] Add tests for business-critical methods --- tests/class.two-factor-core.php | 178 ++++++++++++++++++++++++++++++++ 1 file changed, 178 insertions(+) diff --git a/tests/class.two-factor-core.php b/tests/class.two-factor-core.php index d1f1f266..3807b998 100644 --- a/tests/class.two-factor-core.php +++ b/tests/class.two-factor-core.php @@ -77,6 +77,65 @@ public function test_add_hooks() { array( 'Two_Factor_Core', 'backup_2fa' ) ) ); + $this->assertGreaterThan( + 0, + has_action( + 'init', + array( 'Two_Factor_Core', 'register_scripts' ) + ) + ); + $this->assertGreaterThan( + 0, + has_action( + 'wpmu_options', + array( 'Two_Factor_Core', 'force_two_factor_setting_options' ) + ) + ); + $this->assertGreaterThan( + 0, + has_action( + 'update_wpmu_options', + array( 'Two_Factor_Core', 'save_network_force_two_factor_update' ) + ) + ); + $this->assertGreaterThan( + 0, + has_action( + 'wp_ajax_two_factor_force_form_submit', + array( 'Two_Factor_Core', 'handle_force_2fa_submission' ) + ) + ); + $this->assertGreaterThan( + 0, + has_action( + 'two_factor_ajax_options_update', + array( 'Two_Factor_Core', 'user_two_factor_options_update' ) + ) + ); + $this->assertGreaterThan( + 0, + has_action( + 'parse_request', + array( 'Two_Factor_Core', 'maybe_force_2fa_settings' ) + ) + ); + $this->assertGreaterThan( + 0, + has_action( + 'admin_init', + array( 'Two_Factor_Core', 'maybe_force_2fa_settings' ) + ) + ); + } + + /** + * @covers Two_Factor_Core::register_scripts + */ + public function test_register_scripts() { + Two_Factor_Core::register_scripts(); + + $this->assertTrue( wp_script_is( 'two-factor-form-script', 'registered' ) ); + $this->assertTrue( wp_style_is( 'user-edit-2fa', 'registered' ) ); } /** @@ -180,4 +239,123 @@ public function test_get_primary_provider_for_user_not_logged_in() { public function test_is_user_using_two_factor_not_logged_in() { $this->assertFalse( Two_Factor_Core::is_user_using_two_factor() ); } + + /** + * @covers Two_Factor_Core::maybe_force_2fa_settings + */ + public function test_maybe_force_2fa_settings_logged_in_wrong_role() { + // Set universal value to false. + update_site_option( Two_Factor_Core::FORCED_SITE_META_KEY, 0 ); + // Set role-based value to editors and adminstrators. + update_site_option( Two_Factor_Core::FORCED_ROLES_META_KEY, [ 'editor', 'administrator' ] ); + + $user = new WP_User( $this->factory->user->create( [ 'role' => 'author' ] ) ); + wp_set_current_user( $user->ID ); + + $this->assertFalse( Two_Factor_Core::maybe_force_2fa_settings() ); + } + + /** + * @covers Two_Factor_Core::maybe_force_2fa_settings + */ + public function test_maybe_force_2fa_settings_logged_in_no_requirement() { + // Set universal value to false. + update_site_option( Two_Factor_Core::FORCED_SITE_META_KEY, 0 ); + + $user = new WP_User( $this->factory->user->create() ); + wp_set_current_user( $user->ID ); + + $this->assertFalse( Two_Factor_Core::maybe_force_2fa_settings() ); + } + + /** + * @covers Two_Factor_Core::maybe_force_2fa_settings + */ + public function test_maybe_force_2fa_settings_logged_out() { + wp_logout(); + + $this->assertFalse( Two_Factor_Core::maybe_force_2fa_settings() ); + } + + /** + * @covers Two_Factor_Core::maybe_force_2fa_settings + */ + public function test_maybe_force_2fa_settings_is_rest() { + define( 'REST_REQUEST', true ); + + $this->assertFalse( Two_Factor_Core::maybe_force_2fa_settings() ); + } + + /** + * @covers Two_Factor_Core::maybe_force_2fa_settings + */ + public function test_maybe_force_2fa_settings_is_ajax() { + define( 'DOING_AJAX', true ); + + $this->assertFalse( Two_Factor_Core::maybe_force_2fa_settings() ); + } + + /** + * @covers Two_Factor_Core::is_two_factor_forced + */ + public function test_is_two_factor_forced_universal_option() { + update_site_option( Two_Factor_Core::FORCED_SITE_META_KEY, 1 ); + + $this->assertTrue( Two_Factor_Core::is_two_factor_forced( 123456 ) ); + } + + /** + * @covers Two_Factor_Core::is_two_factor_forced + */ + public function test_is_two_factor_forced_non_existant_user() { + update_site_option( Two_Factor_Core::FORCED_SITE_META_KEY, 0 ); + + $this->assertFalse( Two_Factor_Core::is_two_factor_forced( 123456 ) ); + } + + /** + * @covers Two_Factor_Core::is_two_factor_forced + */ + public function test_is_two_factor_forced_different_role() { + // Set role-based value to editors and adminstrators. + update_site_option( Two_Factor_Core::FORCED_ROLES_META_KEY, [ 'editor', 'administrator' ] ); + + $user = new WP_User( $this->factory->user->create( [ 'role' => 'author' ] ) ); + wp_set_current_user( $user->ID ); + + $this->assertFalse( Two_Factor_Core::is_two_factor_forced( $user->ID ) ); + } + + /** + * @covers Two_Factor_Core::is_two_factor_forced + */ + public function test_is_two_factor_forced_captured_role() { + // Set role-based value to editors and adminstrators. + update_site_option( Two_Factor_Core::FORCED_ROLES_META_KEY, [ 'editor', 'author' ] ); + + $user = new WP_User( $this->factory->user->create( [ 'role' => 'author' ] ) ); + wp_set_current_user( $user->ID ); + + $this->assertTrue( Two_Factor_Core::is_two_factor_forced( $user->ID ) ); + } + + /** + * @covers Two_Factor_Core::get_universally_forced_option + */ + public function test_get_universally_forced_option_multisite() { + // Set role-based value to editors and adminstrators. + update_site_option( Two_Factor_Core::FORCED_SITE_META_KEY, 1 ); + + $this->assertTrue( Two_Factor_Core::get_universally_forced_option() ); + } + + /** + * @covers Two_Factor_Core::get_forced_user_roles + */ + public function test_get_forced_user_roles_multisite() { + // Set role-based value to editors and adminstrators. + update_site_option( Two_Factor_Core::FORCED_ROLES_META_KEY, [ 'author', 'editor', 'administrator' ] ); + + $this->assertEquals( [ 'author', 'editor', 'administrator' ], Two_Factor_Core::get_forced_user_roles() ); + } } From 8d531bf20e2ec0999ae258018ca02d39a247934c Mon Sep 17 00:00:00 2001 From: Mike Selander Date: Tue, 4 Sep 2018 13:21:41 -0600 Subject: [PATCH 17/30] Add returns to maybe_force_2fa_settings for better testability --- class.two-factor-core.php | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/class.two-factor-core.php b/class.two-factor-core.php index d361bcb9..39648243 100644 --- a/class.two-factor-core.php +++ b/class.two-factor-core.php @@ -471,27 +471,27 @@ public static function login_html( $user, $login_nonce, $redirect_to, $error_msg public static function maybe_force_2fa_settings() { // This should not affect AJAX or REST requests, carry on. if ( wp_doing_ajax() || ( defined( 'REST_REQUEST' ) && REST_REQUEST ) ) { - return; + return false; } // Should not interrupt logging in or out. if ( self::is_login_page() ) { - return; + return false; } // If user is not logged in, they can't 2fa anyway. if ( ! is_user_logged_in() ) { - return; + return false; } // 2fa is not forced for current user, nothing to show. if ( ! self::is_two_factor_forced_for_current_user() ) { - return; + return false; } // The current user is already using two-factor, good for them! if ( self::is_user_using_two_factor() ) { - return; + return false; } // We are now forced to display the two-factor settings page. From d4712ce02570daa30941c13814317b978abd9ac4 Mon Sep 17 00:00:00 2001 From: Mike Selander Date: Tue, 4 Sep 2018 13:40:12 -0600 Subject: [PATCH 18/30] Self-review cleanup --- assets/js/force-2fa.js | 6 +++--- class.two-factor-core.php | 12 ++++++++---- 2 files changed, 11 insertions(+), 7 deletions(-) diff --git a/assets/js/force-2fa.js b/assets/js/force-2fa.js index 921a5859..b7486ba6 100644 --- a/assets/js/force-2fa.js +++ b/assets/js/force-2fa.js @@ -44,9 +44,9 @@ var formToJSON = function ( elements ) { } /* - * Some fields allow for more than one value, so we need to check if this - * is one of those fields and, if so, store the values as an array. - */ + * Some fields allow for more than one value, so we need to check if this + * is one of those fields and, if so, store the values as an array. + */ if ( isCheckbox( element ) ) { data[ element.name ] = ( data[ element.name ] || [] ).concat( element.value ); } else { diff --git a/class.two-factor-core.php b/class.two-factor-core.php index 39648243..dcd4b21c 100644 --- a/class.two-factor-core.php +++ b/class.two-factor-core.php @@ -528,7 +528,7 @@ public static function force_2fa_login_html() {

- +

@@ -866,7 +866,7 @@ public static function is_two_factor_forced_for_current_user() { * @param int $user_id User ID to check against. * @return bool Whether user should be required to use 2fa. */ - public static function is_two_factor_forced( int $user_id ) : bool { + public static function is_two_factor_forced( $user_id ) { // If 2fa is forced for all users, always return true. if ( self::get_universally_forced_option() ) { return true; @@ -990,8 +990,10 @@ public static function save_network_force_two_factor_update() { } // Validate and save per-role settings. - if ( ! isset( $_POST[ self::FORCED_ROLES_META_KEY ] ) || - ! is_array( $_POST[ self::FORCED_ROLES_META_KEY ] ) ) { + if ( + ! isset( $_POST[ self::FORCED_ROLES_META_KEY ] ) || + ! is_array( $_POST[ self::FORCED_ROLES_META_KEY ] ) + ) { return; } @@ -1006,6 +1008,8 @@ public static function save_network_force_two_factor_update() { /** * Check whether we're on main login page or not. * + * Why is this not in core yet? + * * @return bool */ public static function is_login_page() { From bcbfec3668a107dbd203cd823cfd817813719a57 Mon Sep 17 00:00:00 2001 From: Mike Selander Date: Tue, 4 Sep 2018 15:51:15 -0600 Subject: [PATCH 19/30] Register and save values against a single site' --- class.two-factor-core.php | 120 +++++++++++++++++++++++++++++++------- 1 file changed, 100 insertions(+), 20 deletions(-) diff --git a/class.two-factor-core.php b/class.two-factor-core.php index dcd4b21c..21effc76 100644 --- a/class.two-factor-core.php +++ b/class.two-factor-core.php @@ -63,17 +63,20 @@ public static function add_hooks() { add_filter( 'manage_users_custom_column', array( __CLASS__, 'manage_users_custom_column' ), 10, 3 ); // Forced 2fa login functionality. - // @todo:: display settings to force 2fa on specific site, if site is not network. - // @todo:: Add action to save said setting if site is not network. add_action( 'init', array( __CLASS__, 'register_scripts' ) ); - add_action( 'wpmu_options', array( __CLASS__, 'force_two_factor_setting_options' ) ); - add_action( 'update_wpmu_options', array( __CLASS__, 'save_network_force_two_factor_update' ) ); add_action( 'wp_ajax_two_factor_force_form_submit', array( __CLASS__, 'handle_force_2fa_submission' ) ); add_action( 'two_factor_ajax_options_update', array( __CLASS__, 'user_two_factor_options_update' ) ); // Handling intercession in 2 separate hooks to allow us to properly parse for REST requests. add_action( 'parse_request', array( __CLASS__, 'maybe_force_2fa_settings' ) ); add_action( 'admin_init', array( __CLASS__, 'maybe_force_2fa_settings' ) ); + + if ( is_multisite() ) { + add_action( 'wpmu_options', array( __CLASS__, 'force_two_factor_setting_options' ) ); + add_action( 'update_wpmu_options', array( __CLASS__, 'save_network_force_two_factor_update' ) ); + } else { + add_action( 'admin_init', array( __CLASS__, 'register_single_site_force_2fa_options' ) ); + } } /** @@ -931,10 +934,7 @@ public static function get_forced_user_roles() { * @since 0.1-dev */ public static function force_two_factor_setting_options() { - $forced_roles = self::get_forced_user_roles(); - $is_universally_forced = self::get_universally_forced_option(); ?> -

@@ -944,10 +944,7 @@ public static function force_two_factor_setting_options() { @@ -956,13 +953,7 @@ public static function force_two_factor_setting_options() { @@ -970,6 +961,38 @@ public static function force_two_factor_setting_options() { + + $role ) : + ?> + +
+ 'boolean', + ] + ); + + // Add per-role force 2fa field. + add_settings_field( + self::FORCED_ROLES_META_KEY, + esc_html__( 'Force two-factor on specific roles', 'two-factor' ), + array( __CLASS__, 'global_force_2fa_by_role_field' ), + 'general', + 'two-factor-force-2fa' + ); + + register_setting( + 'general', + self::FORCED_ROLES_META_KEY, + array( __CLASS__, 'validate_forced_roles' ) + ); } /** From 0898fdc1f33f53b32496287a70ece3b0d7c32404 Mon Sep 17 00:00:00 2001 From: Mike Selander Date: Tue, 4 Sep 2018 16:24:51 -0600 Subject: [PATCH 20/30] Enqueue assets for Fido 2f on forced 2f takeover --- class.two-factor-core.php | 8 ++++++++ providers/css/fido-u2f-admin.css | 11 +++++++++++ 2 files changed, 19 insertions(+) diff --git a/class.two-factor-core.php b/class.two-factor-core.php index 21effc76..0f3d4b66 100644 --- a/class.two-factor-core.php +++ b/class.two-factor-core.php @@ -513,6 +513,14 @@ public static function force_2fa_login_html() { wp_enqueue_script( 'jquery' ); wp_enqueue_script( 'two-factor-form-script' ); + // If Fido is an enabled 2f Provider, enqueue its assets. + $providers = self::get_providers(); + if ( in_array( 'Two_Factor_FIDO_U2F', array_keys( $providers ), true ) ) { + Two_Factor_FIDO_U2F_Admin::enqueue_assets( 'profile.php' ); + wp_enqueue_style( 'common' ); + wp_enqueue_style( 'list-tables' ); + } + if ( ! function_exists( 'login_header' ) ) { // We really should migrate login_header() out of `wp-login.php` so it can be called from an includes file. include_once( TWO_FACTOR_DIR . 'includes/function.login-header.php' ); diff --git a/providers/css/fido-u2f-admin.css b/providers/css/fido-u2f-admin.css index 947dbf43..a1e13694 100644 --- a/providers/css/fido-u2f-admin.css +++ b/providers/css/fido-u2f-admin.css @@ -1,10 +1,21 @@ #security-keys-section .wp-list-table { margin-bottom: 2em; } + #security-keys-section .register-security-key .spinner { float: none; } + #security-keys-section .security-key-status { vertical-align: middle; font-style: italic; } + +.login #security-keys-section p { + line-height: 1.5; + margin: 1em 0; +} + +.login #security-keys-section .button.button-secondary { + display: inline-block; +} From 26824fcd739c1b07ab7bc0c846c95fe03279907b Mon Sep 17 00:00:00 2001 From: Mike Selander Date: Wed, 5 Sep 2018 10:45:15 -0600 Subject: [PATCH 21/30] Add spacing --- class.two-factor-core.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/class.two-factor-core.php b/class.two-factor-core.php index 0f3d4b66..d369690e 100644 --- a/class.two-factor-core.php +++ b/class.two-factor-core.php @@ -894,7 +894,7 @@ public static function is_two_factor_forced( $user_id ) { $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); + }, ARRAY_FILTER_USE_BOTH ); // If the required_roles is not empty, then the user is in a role that requires two_factor authentication. return ! empty( $required_roles ); From 51b93a3df28c340034608d2155f5cdf140e6f254 Mon Sep 17 00:00:00 2001 From: Mike Selander Date: Wed, 5 Sep 2018 10:45:57 -0600 Subject: [PATCH 22/30] Remove unnecessary ternary --- class.two-factor-core.php | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/class.two-factor-core.php b/class.two-factor-core.php index d369690e..0d7c3643 100644 --- a/class.two-factor-core.php +++ b/class.two-factor-core.php @@ -908,14 +908,12 @@ public static function is_two_factor_forced( $user_id ) { * @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 ); - /** * Whether or not site has two-factor universally forced. * * @param bool $is_forced Whether all users on a site are forced to use 2fa. */ - return (bool) apply_filters( 'two_factor_universally_forced', $is_forced ); + return (bool) apply_filters( 'two_factor_universally_forced', get_site_option( self::FORCED_SITE_META_KEY, false ) ); } /** @@ -926,14 +924,12 @@ public static function get_universally_forced_option() { * @return array */ public static function get_forced_user_roles() { - $roles = is_multisite() ? get_site_option( self::FORCED_ROLES_META_KEY, false ) : get_option( self::FORCED_ROLES_META_KEY, false ); - /** * 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', $roles ); + return (array) apply_filters( 'two_factor_forced_user_roles', get_site_option( self::FORCED_ROLES_META_KEY, false ) ); } /** From 2723f88b372d7198a1cdbcdd337765014c1fa028 Mon Sep 17 00:00:00 2001 From: Mike Selander Date: Wed, 5 Sep 2018 10:46:39 -0600 Subject: [PATCH 23/30] Update language on global 2fa forcing option --- class.two-factor-core.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/class.two-factor-core.php b/class.two-factor-core.php index 0d7c3643..d9df8dcb 100644 --- a/class.two-factor-core.php +++ b/class.two-factor-core.php @@ -945,7 +945,7 @@ public static function force_two_factor_setting_options() {
- +
- $role ) : ?> - -
- +
- + @@ -1059,7 +1059,7 @@ public static function register_single_site_force_2fa_options() { // Add global force 2fa field. add_settings_field( self::FORCED_SITE_META_KEY, - esc_html__( 'Universally force two-factor', 'two-factor' ), + esc_html__( 'Force all users to enable two-factor', 'two-factor' ), array( __CLASS__, 'global_force_2fa_field' ), 'general', 'two-factor-force-2fa' From 8980d90cc612d62f36c84ea11d1a68381b6d6976 Mon Sep 17 00:00:00 2001 From: Mike Selander Date: Wed, 5 Sep 2018 12:51:46 -0600 Subject: [PATCH 24/30] Split up core class into Force and Core for better separation --- class.two-factor-core.php | 391 ----------------------------- class.two-factor-force.php | 413 +++++++++++++++++++++++++++++++ tests/class.two-factor-core.php | 162 ------------ tests/class.two-factor-force.php | 194 +++++++++++++++ two-factor.php | 2 + 5 files changed, 609 insertions(+), 553 deletions(-) create mode 100644 class.two-factor-force.php create mode 100644 tests/class.two-factor-force.php diff --git a/class.two-factor-core.php b/class.two-factor-core.php index d9df8dcb..52b15a47 100644 --- a/class.two-factor-core.php +++ b/class.two-factor-core.php @@ -22,20 +22,6 @@ class Two_Factor_Core { */ const ENABLED_PROVIDERS_USER_META_KEY = '_two_factor_enabled_providers'; - /** - * The network forced 2fa user roles key. - * - * @type string - */ - const FORCED_ROLES_META_KEY = '_two_factor_forced_roles'; - - /** - * The network forced 2fa global key. - * - * @type string - */ - const FORCED_SITE_META_KEY = '_two_factor_forced_universally'; - /** * The user meta nonce key. * @@ -61,22 +47,7 @@ public static function add_hooks() { add_filter( 'manage_users_columns', array( __CLASS__, 'filter_manage_users_columns' ) ); add_filter( 'wpmu_users_columns', array( __CLASS__, 'filter_manage_users_columns' ) ); add_filter( 'manage_users_custom_column', array( __CLASS__, 'manage_users_custom_column' ), 10, 3 ); - - // Forced 2fa login functionality. add_action( 'init', array( __CLASS__, 'register_scripts' ) ); - add_action( 'wp_ajax_two_factor_force_form_submit', array( __CLASS__, 'handle_force_2fa_submission' ) ); - add_action( 'two_factor_ajax_options_update', array( __CLASS__, 'user_two_factor_options_update' ) ); - - // Handling intercession in 2 separate hooks to allow us to properly parse for REST requests. - add_action( 'parse_request', array( __CLASS__, 'maybe_force_2fa_settings' ) ); - add_action( 'admin_init', array( __CLASS__, 'maybe_force_2fa_settings' ) ); - - if ( is_multisite() ) { - add_action( 'wpmu_options', array( __CLASS__, 'force_two_factor_setting_options' ) ); - add_action( 'update_wpmu_options', array( __CLASS__, 'save_network_force_two_factor_update' ) ); - } else { - add_action( 'admin_init', array( __CLASS__, 'register_single_site_force_2fa_options' ) ); - } } /** @@ -92,15 +63,6 @@ public static function load_textdomain() { * Register scripts. */ public static function register_scripts() { - // Script for handling AJAX submission in force 2fa takeover screen. - wp_register_script( - 'two-factor-form-script', - plugins_url( 'assets/js/force-2fa.js', __FILE__ ), - [], - '0.1', - false - ); - wp_register_style( 'user-edit-2fa', plugins_url( 'user-edit.css', __FILE__ ) @@ -463,120 +425,6 @@ public static function login_html( $user, $login_nonce, $redirect_to, $error_msg -

-
- - -
- -

- - - -

- - - - -
- - - roles, function( $role ) use ( $two_factor_forced_roles ) { - return in_array( $role, $two_factor_forced_roles, true ); - }, ARRAY_FILTER_USE_BOTH ); - - // If the required_roles is not empty, then the user is in a role that requires two_factor authentication. - return ! empty( $required_roles ); - } - - /** - * Get whether site has two-factor universally forced or not. - * - * @since 0.1-dev - * - * @return bool - */ - public static function get_universally_forced_option() { - /** - * Whether or not site has two-factor universally forced. - * - * @param bool $is_forced Whether all users on a site are forced to use 2fa. - */ - return (bool) apply_filters( 'two_factor_universally_forced', get_site_option( self::FORCED_SITE_META_KEY, false ) ); - } - - /** - * 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 ) ); - } - - /** - * Add network and site-level fields for forcing 2-factor on users of a role(s). - * - * @since 0.1-dev - */ - public static function force_two_factor_setting_options() { - ?> -

- - - - - - - - - - - - - -
- - - -
- - - -
- - - $role ) : - ?> - -
- 'boolean', - ] - ); - - // Add per-role force 2fa field. - add_settings_field( - self::FORCED_ROLES_META_KEY, - esc_html__( 'Force two-factor on specific roles', 'two-factor' ), - array( __CLASS__, 'global_force_2fa_by_role_field' ), - 'general', - 'two-factor-force-2fa' - ); - - register_setting( - 'general', - self::FORCED_ROLES_META_KEY, - array( __CLASS__, 'validate_forced_roles' ) - ); - } - - /** - * Check whether we're on main login page or not. - * - * Why is this not in core yet? - * - * @return bool - */ - public static function is_login_page() { - return isset( $_SERVER['REQUEST_URI'] ) && strpos( esc_url_raw( wp_unslash( $_SERVER['REQUEST_URI'] ) ), 'wp-login.php' ) !== false; - } } diff --git a/class.two-factor-force.php b/class.two-factor-force.php new file mode 100644 index 00000000..a8c986f6 --- /dev/null +++ b/class.two-factor-force.php @@ -0,0 +1,413 @@ + +

+
+ + +
+ +

+ + + +

+ + + + +
+ + + roles, function( $role ) use ( $two_factor_forced_roles ) { + return in_array( $role, $two_factor_forced_roles, true ); + }, ARRAY_FILTER_USE_BOTH ); + + // If the required_roles is not empty, then the user is in a role that requires two_factor authentication. + return ! empty( $required_roles ); + } + + /** + * Get whether site has two-factor universally forced or not. + * + * @since 0.1-dev + * + * @return bool + */ + public static function get_universally_forced_option() { + /** + * Whether or not site has two-factor universally forced. + * + * @param bool $is_forced Whether all users on a site are forced to use 2fa. + */ + return (bool) apply_filters( 'two_factor_universally_forced', get_site_option( self::FORCED_SITE_META_KEY, false ) ); + } + + /** + * 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 ) ); + } + + /** + * Add network and site-level fields for forcing 2-factor on users of a role(s). + * + * @since 0.1-dev + */ + public static function force_two_factor_setting_options() { + ?> +

+ + + + + + + + + + + + + +
+ + + +
+ + + +
+ + + $role ) : + ?> + +
+ 'boolean', + ] + ); + + // Add per-role force 2fa field. + add_settings_field( + self::FORCED_ROLES_META_KEY, + esc_html__( 'Force two-factor on specific roles', 'two-factor' ), + array( __CLASS__, 'global_force_2fa_by_role_field' ), + 'general', + 'two-factor-force-2fa' + ); + + register_setting( + 'general', + self::FORCED_ROLES_META_KEY, + array( __CLASS__, 'validate_forced_roles' ) + ); + } + + /** + * Check whether we're on main login page or not. + * + * Why is this not in core yet? + * + * @return bool + */ + public static function is_login_page() { + return isset( $_SERVER['REQUEST_URI'] ) && strpos( esc_url_raw( wp_unslash( $_SERVER['REQUEST_URI'] ) ), 'wp-login.php' ) !== false; + } +} diff --git a/tests/class.two-factor-core.php b/tests/class.two-factor-core.php index 3807b998..307680a4 100644 --- a/tests/class.two-factor-core.php +++ b/tests/class.two-factor-core.php @@ -84,48 +84,6 @@ public function test_add_hooks() { array( 'Two_Factor_Core', 'register_scripts' ) ) ); - $this->assertGreaterThan( - 0, - has_action( - 'wpmu_options', - array( 'Two_Factor_Core', 'force_two_factor_setting_options' ) - ) - ); - $this->assertGreaterThan( - 0, - has_action( - 'update_wpmu_options', - array( 'Two_Factor_Core', 'save_network_force_two_factor_update' ) - ) - ); - $this->assertGreaterThan( - 0, - has_action( - 'wp_ajax_two_factor_force_form_submit', - array( 'Two_Factor_Core', 'handle_force_2fa_submission' ) - ) - ); - $this->assertGreaterThan( - 0, - has_action( - 'two_factor_ajax_options_update', - array( 'Two_Factor_Core', 'user_two_factor_options_update' ) - ) - ); - $this->assertGreaterThan( - 0, - has_action( - 'parse_request', - array( 'Two_Factor_Core', 'maybe_force_2fa_settings' ) - ) - ); - $this->assertGreaterThan( - 0, - has_action( - 'admin_init', - array( 'Two_Factor_Core', 'maybe_force_2fa_settings' ) - ) - ); } /** @@ -134,7 +92,6 @@ public function test_add_hooks() { public function test_register_scripts() { Two_Factor_Core::register_scripts(); - $this->assertTrue( wp_script_is( 'two-factor-form-script', 'registered' ) ); $this->assertTrue( wp_style_is( 'user-edit-2fa', 'registered' ) ); } @@ -239,123 +196,4 @@ public function test_get_primary_provider_for_user_not_logged_in() { public function test_is_user_using_two_factor_not_logged_in() { $this->assertFalse( Two_Factor_Core::is_user_using_two_factor() ); } - - /** - * @covers Two_Factor_Core::maybe_force_2fa_settings - */ - public function test_maybe_force_2fa_settings_logged_in_wrong_role() { - // Set universal value to false. - update_site_option( Two_Factor_Core::FORCED_SITE_META_KEY, 0 ); - // Set role-based value to editors and adminstrators. - update_site_option( Two_Factor_Core::FORCED_ROLES_META_KEY, [ 'editor', 'administrator' ] ); - - $user = new WP_User( $this->factory->user->create( [ 'role' => 'author' ] ) ); - wp_set_current_user( $user->ID ); - - $this->assertFalse( Two_Factor_Core::maybe_force_2fa_settings() ); - } - - /** - * @covers Two_Factor_Core::maybe_force_2fa_settings - */ - public function test_maybe_force_2fa_settings_logged_in_no_requirement() { - // Set universal value to false. - update_site_option( Two_Factor_Core::FORCED_SITE_META_KEY, 0 ); - - $user = new WP_User( $this->factory->user->create() ); - wp_set_current_user( $user->ID ); - - $this->assertFalse( Two_Factor_Core::maybe_force_2fa_settings() ); - } - - /** - * @covers Two_Factor_Core::maybe_force_2fa_settings - */ - public function test_maybe_force_2fa_settings_logged_out() { - wp_logout(); - - $this->assertFalse( Two_Factor_Core::maybe_force_2fa_settings() ); - } - - /** - * @covers Two_Factor_Core::maybe_force_2fa_settings - */ - public function test_maybe_force_2fa_settings_is_rest() { - define( 'REST_REQUEST', true ); - - $this->assertFalse( Two_Factor_Core::maybe_force_2fa_settings() ); - } - - /** - * @covers Two_Factor_Core::maybe_force_2fa_settings - */ - public function test_maybe_force_2fa_settings_is_ajax() { - define( 'DOING_AJAX', true ); - - $this->assertFalse( Two_Factor_Core::maybe_force_2fa_settings() ); - } - - /** - * @covers Two_Factor_Core::is_two_factor_forced - */ - public function test_is_two_factor_forced_universal_option() { - update_site_option( Two_Factor_Core::FORCED_SITE_META_KEY, 1 ); - - $this->assertTrue( Two_Factor_Core::is_two_factor_forced( 123456 ) ); - } - - /** - * @covers Two_Factor_Core::is_two_factor_forced - */ - public function test_is_two_factor_forced_non_existant_user() { - update_site_option( Two_Factor_Core::FORCED_SITE_META_KEY, 0 ); - - $this->assertFalse( Two_Factor_Core::is_two_factor_forced( 123456 ) ); - } - - /** - * @covers Two_Factor_Core::is_two_factor_forced - */ - public function test_is_two_factor_forced_different_role() { - // Set role-based value to editors and adminstrators. - update_site_option( Two_Factor_Core::FORCED_ROLES_META_KEY, [ 'editor', 'administrator' ] ); - - $user = new WP_User( $this->factory->user->create( [ 'role' => 'author' ] ) ); - wp_set_current_user( $user->ID ); - - $this->assertFalse( Two_Factor_Core::is_two_factor_forced( $user->ID ) ); - } - - /** - * @covers Two_Factor_Core::is_two_factor_forced - */ - public function test_is_two_factor_forced_captured_role() { - // Set role-based value to editors and adminstrators. - update_site_option( Two_Factor_Core::FORCED_ROLES_META_KEY, [ 'editor', 'author' ] ); - - $user = new WP_User( $this->factory->user->create( [ 'role' => 'author' ] ) ); - wp_set_current_user( $user->ID ); - - $this->assertTrue( Two_Factor_Core::is_two_factor_forced( $user->ID ) ); - } - - /** - * @covers Two_Factor_Core::get_universally_forced_option - */ - public function test_get_universally_forced_option_multisite() { - // Set role-based value to editors and adminstrators. - update_site_option( Two_Factor_Core::FORCED_SITE_META_KEY, 1 ); - - $this->assertTrue( Two_Factor_Core::get_universally_forced_option() ); - } - - /** - * @covers Two_Factor_Core::get_forced_user_roles - */ - public function test_get_forced_user_roles_multisite() { - // Set role-based value to editors and adminstrators. - update_site_option( Two_Factor_Core::FORCED_ROLES_META_KEY, [ 'author', 'editor', 'administrator' ] ); - - $this->assertEquals( [ 'author', 'editor', 'administrator' ], Two_Factor_Core::get_forced_user_roles() ); - } } diff --git a/tests/class.two-factor-force.php b/tests/class.two-factor-force.php new file mode 100644 index 00000000..c7a03d53 --- /dev/null +++ b/tests/class.two-factor-force.php @@ -0,0 +1,194 @@ +assertGreaterThan( + 0, + has_action( + 'init', + array( 'Two_Factor_Force', 'register_scripts' ) + ) + ); + $this->assertGreaterThan( + 0, + has_action( + 'wpmu_options', + array( 'Two_Factor_Force', 'force_two_factor_setting_options' ) + ) + ); + $this->assertGreaterThan( + 0, + has_action( + 'update_wpmu_options', + array( 'Two_Factor_Force', 'save_network_force_two_factor_update' ) + ) + ); + $this->assertGreaterThan( + 0, + has_action( + 'wp_ajax_two_factor_force_form_submit', + array( 'Two_Factor_Force', 'handle_force_2fa_submission' ) + ) + ); + $this->assertGreaterThan( + 0, + has_action( + 'two_factor_ajax_options_update', + array( 'Two_Factor_Core', 'user_two_factor_options_update' ) + ) + ); + $this->assertGreaterThan( + 0, + has_action( + 'parse_request', + array( 'Two_Factor_Force', 'maybe_force_2fa_settings' ) + ) + ); + $this->assertGreaterThan( + 0, + has_action( + 'admin_init', + array( 'Two_Factor_Force', 'maybe_force_2fa_settings' ) + ) + ); + } + + /** + * @covers Two_Factor_Force::register_scripts + */ + public function test_register_scripts() { + Two_Factor_Force::register_scripts(); + + $this->assertTrue( wp_script_is( 'two-factor-form-script', 'registered' ) ); + } + + /** + * @covers Two_Factor_Force::maybe_force_2fa_settings + */ + public function test_maybe_force_2fa_settings_logged_in_wrong_role() { + // Set universal value to false. + update_site_option( Two_Factor_Force::FORCED_SITE_META_KEY, 0 ); + // Set role-based value to editors and adminstrators. + update_site_option( Two_Factor_Force::FORCED_ROLES_META_KEY, [ 'editor', 'administrator' ] ); + + $user = new WP_User( $this->factory->user->create( [ 'role' => 'author' ] ) ); + wp_set_current_user( $user->ID ); + + $this->assertFalse( Two_Factor_Force::maybe_force_2fa_settings() ); + } + + /** + * @covers Two_Factor_Force::maybe_force_2fa_settings + */ + public function test_maybe_force_2fa_settings_logged_in_no_requirement() { + // Set universal value to false. + update_site_option( Two_Factor_Force::FORCED_SITE_META_KEY, 0 ); + + $user = new WP_User( $this->factory->user->create() ); + wp_set_current_user( $user->ID ); + + $this->assertFalse( Two_Factor_Force::maybe_force_2fa_settings() ); + } + + /** + * @covers Two_Factor_Force::maybe_force_2fa_settings + */ + public function test_maybe_force_2fa_settings_logged_out() { + wp_logout(); + + $this->assertFalse( Two_Factor_Force::maybe_force_2fa_settings() ); + } + + /** + * @covers Two_Factor_Force::maybe_force_2fa_settings + */ + public function test_maybe_force_2fa_settings_is_rest() { + define( 'REST_REQUEST', true ); + + $this->assertFalse( Two_Factor_Force::maybe_force_2fa_settings() ); + } + + /** + * @covers Two_Factor_Force::maybe_force_2fa_settings + */ + public function test_maybe_force_2fa_settings_is_ajax() { + define( 'DOING_AJAX', true ); + + $this->assertFalse( Two_Factor_Force::maybe_force_2fa_settings() ); + } + + /** + * @covers Two_Factor_Force::is_two_factor_forced + */ + public function test_is_two_factor_forced_universal_option() { + update_site_option( Two_Factor_Force::FORCED_SITE_META_KEY, 1 ); + + $this->assertTrue( Two_Factor_Force::is_two_factor_forced( 123456 ) ); + } + + /** + * @covers Two_Factor_Force::is_two_factor_forced + */ + public function test_is_two_factor_forced_non_existant_user() { + update_site_option( Two_Factor_Force::FORCED_SITE_META_KEY, 0 ); + + $this->assertFalse( Two_Factor_Force::is_two_factor_forced( 123456 ) ); + } + + /** + * @covers Two_Factor_Force::is_two_factor_forced + */ + public function test_is_two_factor_forced_different_role() { + // Set role-based value to editors and adminstrators. + update_site_option( Two_Factor_Force::FORCED_ROLES_META_KEY, [ 'editor', 'administrator' ] ); + + $user = new WP_User( $this->factory->user->create( [ 'role' => 'author' ] ) ); + wp_set_current_user( $user->ID ); + + $this->assertFalse( Two_Factor_Force::is_two_factor_forced( $user->ID ) ); + } + + /** + * @covers Two_Factor_Force::is_two_factor_forced + */ + public function test_is_two_factor_forced_captured_role() { + // Set role-based value to editors and adminstrators. + update_site_option( Two_Factor_Force::FORCED_ROLES_META_KEY, [ 'editor', 'author' ] ); + + $user = new WP_User( $this->factory->user->create( [ 'role' => 'author' ] ) ); + wp_set_current_user( $user->ID ); + + $this->assertTrue( Two_Factor_Force::is_two_factor_forced( $user->ID ) ); + } + + /** + * @covers Two_Factor_Force::get_universally_forced_option + */ + public function test_get_universally_forced_option_multisite() { + // Set role-based value to editors and adminstrators. + update_site_option( Two_Factor_Force::FORCED_SITE_META_KEY, 1 ); + + $this->assertTrue( Two_Factor_Force::get_universally_forced_option() ); + } + + /** + * @covers Two_Factor_Force::get_forced_user_roles + */ + public function test_get_forced_user_roles_multisite() { + // Set role-based value to editors and adminstrators. + update_site_option( Two_Factor_Force::FORCED_ROLES_META_KEY, [ 'author', 'editor', 'administrator' ] ); + + $this->assertEquals( [ 'author', 'editor', 'administrator' ], Two_Factor_Force::get_forced_user_roles() ); + } +} diff --git a/two-factor.php b/two-factor.php index 6cc750d3..b519cc73 100644 --- a/two-factor.php +++ b/two-factor.php @@ -24,5 +24,7 @@ * Include the core that handles the common bits. */ require_once( TWO_FACTOR_DIR . 'class.two-factor-core.php' ); +require_once( TWO_FACTOR_DIR . 'class.two-factor-force.php' ); Two_Factor_Core::add_hooks(); +Two_Factor_Force::add_hooks(); From e8ccab130655f7d6571067fb4eaf235cb11abd36 Mon Sep 17 00:00:00 2001 From: Mike Selander Date: Wed, 5 Sep 2018 14:37:47 -0600 Subject: [PATCH 25/30] Add custom path and handling for force-2fa --- class.two-factor-force.php | 92 +++++++++++++++++++++++++++++--- tests/class.two-factor-force.php | 43 ++++++++------- 2 files changed, 108 insertions(+), 27 deletions(-) diff --git a/class.two-factor-force.php b/class.two-factor-force.php index a8c986f6..280d04a1 100644 --- a/class.two-factor-force.php +++ b/class.two-factor-force.php @@ -33,8 +33,10 @@ public static function add_hooks() { add_action( 'two_factor_ajax_options_update', array( 'Two_Factor_Core', 'user_two_factor_options_update' ) ); // Handling intercession in 2 separate hooks to allow us to properly parse for REST requests. - add_action( 'parse_request', array( __CLASS__, 'maybe_force_2fa_settings' ) ); - add_action( 'admin_init', array( __CLASS__, 'maybe_force_2fa_settings' ) ); + add_filter( 'login_redirect', array( __CLASS__, 'maybe_redirect_login' ), 15, 3 ); + add_action( 'parse_request', array( __CLASS__, 'maybe_redirect_to_2fa_settings' ) ); + add_action( 'admin_init', array( __CLASS__, 'maybe_redirect_to_2fa_settings' ) ); + add_action( 'admin_init', array( __CLASS__, 'maybe_display_2fa_settings' ) ); if ( is_multisite() ) { add_action( 'wpmu_options', array( __CLASS__, 'force_two_factor_setting_options' ) ); @@ -58,6 +60,39 @@ public static function register_scripts() { ); } + /** + * Redirect a suer to the 2fa login screen with redirect parameters. + * + * If a user must have 2fa enabled, we need to send them to the 2fa settings + * takeover. However, we also need to pass in the redirect_to information to + * ensure that the user lands in the right place. + * + * @param string $redirect_to The redirect destination URL. + * @param string $requested_redirect_to The requested redirect destination URL passed as a parameter. + * @param WP_User|WP_Error $user WP_User object if login was successful, WP_Error object otherwise. + * @return string + */ + public static function maybe_redirect_login( $redirect_to, $requested_redirect_to, $user ) { + // If login has failed, do nothing. + if ( $user instanceof WP_Error ) { + return $redirect_to; + } + + // Check if redirect is necessary for user. + if ( ! self::should_user_redirect( $user->ID ) ) { + return $redirect_to; + }; + + // Append redirect_to URL. + return add_query_arg( + [ + 'force_2fa_screen' => 1, + 'redirect_to' => rawurlencode( $requested_redirect_to ), + ], + admin_url() + ); + } + /** * Maybe force the 2fa login page on a user. * @@ -66,13 +101,56 @@ public static function register_scripts() { * a 2-factor authentication of some kind to perform any action on their site. * This occurs both on the front and backend. */ - public static function maybe_force_2fa_settings() { + public static function maybe_redirect_to_2fa_settings() { + if ( ! self::should_user_redirect( get_current_user_id() ) || isset( $_GET['force_2fa_screen'] ) ) { + return; + } + + // We are now forced to display the two-factor settings page. + wp_safe_redirect( add_query_arg( + 'force_2fa_screen', + 1, + admin_url() + ) ); + exit; + } + + /** + * On front and backend requests, display + */ + public static function maybe_display_2fa_settings() { + if ( ! isset( $_GET['force_2fa_screen'] ) || ! $_GET['force_2fa_screen'] ) { + return; + } + + if ( ! self::should_user_redirect( get_current_user_id() ) ) { + $url = admin_url(); + + if ( isset( $_GET['redirect_to'] ) ) { + $url = esc_url_raw( urldecode( $_GET['redirect_to'] ) ); + } + + wp_safe_redirect( $url ); + exit; + } + + self::force_2fa_login_html(); + exit; + } + + /** + * Check whether or not a user should be redirected to the force 2fa screen. + * + * @param int $user_id ID of the user to check against. + * @return bool Whether or not user should be forced to 2fa screen. + */ + public static function should_user_redirect( $user_id ) { // This should not affect AJAX or REST requests, carry on. if ( wp_doing_ajax() || ( defined( 'REST_REQUEST' ) && REST_REQUEST ) ) { return false; } - // Should not interrupt logging in or out. + // Should not interrupt logging in or out or our own screen. if ( self::is_login_page() ) { return false; } @@ -83,7 +161,7 @@ public static function maybe_force_2fa_settings() { } // 2fa is not forced for current user, nothing to show. - if ( ! self::is_two_factor_forced_for_current_user() ) { + if ( ! self::is_two_factor_forced( $user_id ) ) { return false; } @@ -92,9 +170,7 @@ public static function maybe_force_2fa_settings() { return false; } - // We are now forced to display the two-factor settings page. - self::force_2fa_login_html(); - exit; + return true; } /** diff --git a/tests/class.two-factor-force.php b/tests/class.two-factor-force.php index c7a03d53..9b471124 100644 --- a/tests/class.two-factor-force.php +++ b/tests/class.two-factor-force.php @@ -52,14 +52,21 @@ public function test_add_hooks() { 0, has_action( 'parse_request', - array( 'Two_Factor_Force', 'maybe_force_2fa_settings' ) + array( 'Two_Factor_Force', 'maybe_redirect_to_2fa_settings' ) ) ); $this->assertGreaterThan( 0, has_action( 'admin_init', - array( 'Two_Factor_Force', 'maybe_force_2fa_settings' ) + array( 'Two_Factor_Force', 'maybe_redirect_to_2fa_settings' ) + ) + ); + $this->assertGreaterThan( + 0, + has_action( + 'admin_init', + array( 'Two_Factor_Force', 'maybe_display_2fa_settings' ) ) ); } @@ -74,58 +81,56 @@ public function test_register_scripts() { } /** - * @covers Two_Factor_Force::maybe_force_2fa_settings + * @covers Two_Factor_Force::should_user_redirect */ - public function test_maybe_force_2fa_settings_logged_in_wrong_role() { + public function test_should_user_redirect_logged_in_wrong_role() { // Set universal value to false. update_site_option( Two_Factor_Force::FORCED_SITE_META_KEY, 0 ); // Set role-based value to editors and adminstrators. update_site_option( Two_Factor_Force::FORCED_ROLES_META_KEY, [ 'editor', 'administrator' ] ); $user = new WP_User( $this->factory->user->create( [ 'role' => 'author' ] ) ); - wp_set_current_user( $user->ID ); - $this->assertFalse( Two_Factor_Force::maybe_force_2fa_settings() ); + $this->assertFalse( Two_Factor_Force::should_user_redirect( $user->ID ) ); } /** - * @covers Two_Factor_Force::maybe_force_2fa_settings + * @covers Two_Factor_Force::should_user_redirect */ - public function test_maybe_force_2fa_settings_logged_in_no_requirement() { + public function test_should_user_redirect_logged_in_no_requirement() { // Set universal value to false. update_site_option( Two_Factor_Force::FORCED_SITE_META_KEY, 0 ); $user = new WP_User( $this->factory->user->create() ); - wp_set_current_user( $user->ID ); - $this->assertFalse( Two_Factor_Force::maybe_force_2fa_settings() ); + $this->assertFalse( Two_Factor_Force::should_user_redirect( $user->ID ) ); } /** - * @covers Two_Factor_Force::maybe_force_2fa_settings + * @covers Two_Factor_Force::should_user_redirect */ - public function test_maybe_force_2fa_settings_logged_out() { + public function test_should_user_redirect_logged_out() { wp_logout(); - $this->assertFalse( Two_Factor_Force::maybe_force_2fa_settings() ); + $this->assertFalse( Two_Factor_Force::should_user_redirect( 123456 ) ); } /** - * @covers Two_Factor_Force::maybe_force_2fa_settings + * @covers Two_Factor_Force::should_user_redirect */ - public function test_maybe_force_2fa_settings_is_rest() { + public function test_should_user_redirect_is_rest() { define( 'REST_REQUEST', true ); - $this->assertFalse( Two_Factor_Force::maybe_force_2fa_settings() ); + $this->assertFalse( Two_Factor_Force::should_user_redirect( 123456 ) ); } /** - * @covers Two_Factor_Force::maybe_force_2fa_settings + * @covers Two_Factor_Force::should_user_redirect */ - public function test_maybe_force_2fa_settings_is_ajax() { + public function test_should_user_redirect_is_ajax() { define( 'DOING_AJAX', true ); - $this->assertFalse( Two_Factor_Force::maybe_force_2fa_settings() ); + $this->assertFalse( Two_Factor_Force::should_user_redirect( 123456 ) ); } /** From 13a8cf75e22d8ce1ba70f4e43efe8da2e27a08a8 Mon Sep 17 00:00:00 2001 From: Mike Selander Date: Wed, 5 Sep 2018 14:58:42 -0600 Subject: [PATCH 26/30] Clean up --- class.two-factor-core.php | 1 + class.two-factor-force.php | 7 +++---- tests/class.two-factor-core.php | 7 +++++++ tests/class.two-factor-force.php | 7 ------- 4 files changed, 11 insertions(+), 11 deletions(-) diff --git a/class.two-factor-core.php b/class.two-factor-core.php index 52b15a47..10368b7e 100644 --- a/class.two-factor-core.php +++ b/class.two-factor-core.php @@ -44,6 +44,7 @@ public static function add_hooks() { add_action( 'edit_user_profile', array( __CLASS__, 'user_two_factor_options' ) ); add_action( 'personal_options_update', array( __CLASS__, 'user_two_factor_options_update' ) ); add_action( 'edit_user_profile_update', array( __CLASS__, 'user_two_factor_options_update' ) ); + add_action( 'two_factor_ajax_options_update', array( __CLASS__, 'user_two_factor_options_update' ) ); add_filter( 'manage_users_columns', array( __CLASS__, 'filter_manage_users_columns' ) ); add_filter( 'wpmu_users_columns', array( __CLASS__, 'filter_manage_users_columns' ) ); add_filter( 'manage_users_custom_column', array( __CLASS__, 'manage_users_custom_column' ), 10, 3 ); diff --git a/class.two-factor-force.php b/class.two-factor-force.php index 280d04a1..eadad221 100644 --- a/class.two-factor-force.php +++ b/class.two-factor-force.php @@ -30,7 +30,6 @@ public static function add_hooks() { // Forced 2fa login functionality. add_action( 'init', array( __CLASS__, 'register_scripts' ) ); add_action( 'wp_ajax_two_factor_force_form_submit', array( __CLASS__, 'handle_force_2fa_submission' ) ); - add_action( 'two_factor_ajax_options_update', array( 'Two_Factor_Core', 'user_two_factor_options_update' ) ); // Handling intercession in 2 separate hooks to allow us to properly parse for REST requests. add_filter( 'login_redirect', array( __CLASS__, 'maybe_redirect_login' ), 15, 3 ); @@ -61,7 +60,7 @@ public static function register_scripts() { } /** - * Redirect a suer to the 2fa login screen with redirect parameters. + * Redirect a user to the 2fa login screen with redirect parameters. * * If a user must have 2fa enabled, we need to send them to the 2fa settings * takeover. However, we also need to pass in the redirect_to information to @@ -150,7 +149,7 @@ public static function should_user_redirect( $user_id ) { return false; } - // Should not interrupt logging in or out or our own screen. + // Should not interrupt logging in or out. if ( self::is_login_page() ) { return false; } @@ -184,7 +183,7 @@ public static function force_2fa_login_html() { wp_enqueue_script( 'jquery' ); wp_enqueue_script( 'two-factor-form-script' ); - // If Fido is an enabled 2f Provider, enqueue its assets. + // If Fido is a valid 2fa Provider, enqueue its assets. $providers = Two_Factor_Core::get_providers(); if ( in_array( 'Two_Factor_FIDO_U2F', array_keys( $providers ), true ) ) { Two_Factor_FIDO_U2F_Admin::enqueue_assets( 'profile.php' ); diff --git a/tests/class.two-factor-core.php b/tests/class.two-factor-core.php index 307680a4..7d7e05fe 100644 --- a/tests/class.two-factor-core.php +++ b/tests/class.two-factor-core.php @@ -84,6 +84,13 @@ public function test_add_hooks() { array( 'Two_Factor_Core', 'register_scripts' ) ) ); + $this->assertGreaterThan( + 0, + has_action( + 'two_factor_ajax_options_update', + array( 'Two_Factor_Core', 'user_two_factor_options_update' ) + ) + ); } /** diff --git a/tests/class.two-factor-force.php b/tests/class.two-factor-force.php index 9b471124..bf007291 100644 --- a/tests/class.two-factor-force.php +++ b/tests/class.two-factor-force.php @@ -41,13 +41,6 @@ public function test_add_hooks() { array( 'Two_Factor_Force', 'handle_force_2fa_submission' ) ) ); - $this->assertGreaterThan( - 0, - has_action( - 'two_factor_ajax_options_update', - array( 'Two_Factor_Core', 'user_two_factor_options_update' ) - ) - ); $this->assertGreaterThan( 0, has_action( From 0b356f324296dd0848fc33c0e2ea329e447df5ce Mon Sep 17 00:00:00 2001 From: Mike Selander Date: Wed, 5 Sep 2018 14:59:39 -0600 Subject: [PATCH 27/30] Clean up according to es linter on Travis --- assets/js/force-2fa.js | 20 +++++++++++--------- 1 file changed, 11 insertions(+), 9 deletions(-) diff --git a/assets/js/force-2fa.js b/assets/js/force-2fa.js index b7486ba6..0b3775d7 100644 --- a/assets/js/force-2fa.js +++ b/assets/js/force-2fa.js @@ -6,7 +6,7 @@ * @param {Element} element The element to check * @return {Boolean} true if the element is an input, false if not */ -var isValidElement = function ( element ) { +var isValidElement = function( element ) { return element.name && element.value; }; @@ -16,7 +16,7 @@ var isValidElement = function ( element ) { * @param {Element} element The element to check * @return {Boolean} true if the value should be added, false if not */ -var isValidValue = function ( element ) { +var isValidValue = function( element ) { return ( ! [ 'checkbox', 'radio' ].includes( element.type ) || element.checked ); }; @@ -26,8 +26,8 @@ var isValidValue = function ( element ) { * @param {Element} element The element to check * @return {Boolean} true if the element is a checkbox, false if not */ -var isCheckbox = function ( element ) { - return element.type === 'checkbox'; +var isCheckbox = function( element ) { + return 'checkbox' === element.type; }; /** @@ -36,8 +36,9 @@ var isCheckbox = function ( element ) { * @param {HTMLFormControlsCollection} elements the form elements * @return {Object} form data as an object literal */ -var formToJSON = function ( elements ) { - return [].reduce.call( elements, function ( data, element ) { +var formToJSON = function( elements ) { + return [].reduce.call( elements, function( data, element ) { + // Make sure the element has the required properties and should be added. if ( ! isValidElement( element ) || ! isValidValue( element ) ) { return data; @@ -62,12 +63,13 @@ var formToJSON = function ( elements ) { * * @param {Event} event the submit event triggered by the user */ -var handleFormSubmit = function ( event ) { - event.preventDefault(); +var handleFormSubmit = function( event ) { // Get form data. var formData = formToJSON( event.target.elements ); + event.preventDefault(); + formData.action = 'two_factor_force_form_submit'; // Submit data to WordPress. @@ -80,7 +82,7 @@ var handleFormSubmit = function ( event ) { ); }; -window.addEventListener( 'load', function () { +window.addEventListener( 'load', function() { var form = document.querySelector( '#force_2fa_form' ); form.addEventListener( 'submit', handleFormSubmit ); } ); From de8ea843e6e93fb9078130013896033ee846330a Mon Sep 17 00:00:00 2001 From: Mike Selander Date: Mon, 17 Sep 2018 17:00:56 -0600 Subject: [PATCH 28/30] Add phpcs:ignore for incorrectly flagged issues --- class.two-factor-force.php | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/class.two-factor-force.php b/class.two-factor-force.php index eadad221..1c00137c 100644 --- a/class.two-factor-force.php +++ b/class.two-factor-force.php @@ -118,6 +118,7 @@ public static function maybe_redirect_to_2fa_settings() { * On front and backend requests, display */ public static function maybe_display_2fa_settings() { + // phpcs:ignore We are validating that the value exists and are not processing it. if ( ! isset( $_GET['force_2fa_screen'] ) || ! $_GET['force_2fa_screen'] ) { return; } @@ -126,6 +127,7 @@ public static function maybe_display_2fa_settings() { $url = admin_url(); if ( isset( $_GET['redirect_to'] ) ) { + //phpcs:ignore This IS the proper sanitization for URLs. $url = esc_url_raw( urldecode( $_GET['redirect_to'] ) ); } @@ -396,6 +398,7 @@ public static function save_network_force_two_factor_update() { check_admin_referer( 'force_two_factor_options', '_nonce_force_two_factor_options' ); // Validate and save universally forced key. + //phpcs:ignore The value from $_POST is not saved, only 1 or 0 can be. if ( isset( $_POST[ self::FORCED_SITE_META_KEY ] ) && $_POST[ self::FORCED_SITE_META_KEY ] ) { update_site_option( self::FORCED_SITE_META_KEY, 1 ); } else { @@ -411,6 +414,7 @@ public static function save_network_force_two_factor_update() { } // Whitelist roles against valid WordPress role slugs. + //phpcs:ignore Our validation method below only accepts whitelisted strings from `editable_roles`. $roles = self::validate_forced_roles( $_POST[ self::FORCED_ROLES_META_KEY ] ); update_site_option( self::FORCED_ROLES_META_KEY, $roles ); From 7b4cce30d48b7c90591fe4991dd20ea7f92eb7ac Mon Sep 17 00:00:00 2001 From: Mike Selander Date: Mon, 17 Sep 2018 17:01:41 -0600 Subject: [PATCH 29/30] Properly space these --- class.two-factor-force.php | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/class.two-factor-force.php b/class.two-factor-force.php index 1c00137c..086148cc 100644 --- a/class.two-factor-force.php +++ b/class.two-factor-force.php @@ -127,7 +127,7 @@ public static function maybe_display_2fa_settings() { $url = admin_url(); if ( isset( $_GET['redirect_to'] ) ) { - //phpcs:ignore This IS the proper sanitization for URLs. + // phpcs:ignore This IS the proper sanitization for URLs. $url = esc_url_raw( urldecode( $_GET['redirect_to'] ) ); } @@ -398,7 +398,7 @@ public static function save_network_force_two_factor_update() { check_admin_referer( 'force_two_factor_options', '_nonce_force_two_factor_options' ); // Validate and save universally forced key. - //phpcs:ignore The value from $_POST is not saved, only 1 or 0 can be. + // phpcs:ignore The value from $_POST is not saved, only 1 or 0 can be. if ( isset( $_POST[ self::FORCED_SITE_META_KEY ] ) && $_POST[ self::FORCED_SITE_META_KEY ] ) { update_site_option( self::FORCED_SITE_META_KEY, 1 ); } else { @@ -414,7 +414,7 @@ public static function save_network_force_two_factor_update() { } // Whitelist roles against valid WordPress role slugs. - //phpcs:ignore Our validation method below only accepts whitelisted strings from `editable_roles`. + // phpcs:ignore Our validation method below only accepts whitelisted strings from `editable_roles`. $roles = self::validate_forced_roles( $_POST[ self::FORCED_ROLES_META_KEY ] ); update_site_option( self::FORCED_ROLES_META_KEY, $roles ); From f7cc27ee4f402c7b2ff3a00131fd74ede7f6a846 Mon Sep 17 00:00:00 2001 From: Theo Savage Date: Wed, 8 May 2019 16:09:56 +0100 Subject: [PATCH 30/30] Allow user to remove forced 2fa on all roles --- class.two-factor-force.php | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/class.two-factor-force.php b/class.two-factor-force.php index 086148cc..88f59ed3 100644 --- a/class.two-factor-force.php +++ b/class.two-factor-force.php @@ -374,6 +374,10 @@ public static function global_force_2fa_by_role_field() { $forced_roles = self::get_forced_user_roles(); $is_universally_forced = self::get_universally_forced_option(); + ?> + + $role ) : ?>