From 53a10e70c180289e95fec7a2cda0c95f0a0a61a8 Mon Sep 17 00:00:00 2001 From: Dion Hulse Date: Thu, 3 Nov 2022 13:09:19 +1000 Subject: [PATCH 1/8] Don't send valid auth cookies, only to then destroy them. --- class-two-factor-core.php | 34 +++++++++++++++++++++++++++++++++- 1 file changed, 33 insertions(+), 1 deletion(-) diff --git a/class-two-factor-core.php b/class-two-factor-core.php index 99d3595c..3b3449e1 100644 --- a/class-two-factor-core.php +++ b/class-two-factor-core.php @@ -57,6 +57,11 @@ class Two_Factor_Core { */ private static $password_auth_tokens = array(); + /** + * Keep track of who wp_set_auth_cookie() is running for. + */ + private static $last_auth_cookie_user = false; + /** * Set up filters and actions. * @@ -68,6 +73,8 @@ public static function add_hooks( $compat ) { add_action( 'plugins_loaded', array( __CLASS__, 'load_textdomain' ) ); add_action( 'init', array( __CLASS__, 'get_providers' ) ); add_action( 'wp_login', array( __CLASS__, 'wp_login' ), 10, 2 ); + add_action( 'set_auth_cookie', array( __CLASS__, 'set_auth_cookie' ), 10, 4 ); + add_action( 'send_auth_cookies', array( __CLASS__, 'send_auth_cookies' ) ); add_action( 'login_form_validate_2fa', array( __CLASS__, 'login_form_validate_2fa' ) ); add_action( 'login_form_backup_2fa', array( __CLASS__, 'backup_2fa' ) ); add_action( 'show_user_profile', array( __CLASS__, 'user_two_factor_options' ) ); @@ -433,13 +440,35 @@ public static function wp_login( $user_login, $user ) { // Invalidate the current login session to prevent from being re-used. self::destroy_current_session_for_user( $user ); - // Also clear the cookies which are no longer valid. + // Clear any cookies which are no longer valid. wp_clear_auth_cookie(); self::show_two_factor_login( $user ); exit; } + // Don't send auth cookies in the first place. + public static function send_auth_cookies( $send_cookies ) { + if ( + $send_cookies && + self::$last_auth_cookie_user && + self::is_user_using_two_factor( self::$last_auth_cookie_user ) && + ! apply_filters( 'two_factor_authentication_succeeded', false ) + ) { + $send_cookies = false; + + // Reset the tracked user. + self::$last_auth_cookie_user = false; + } + + return $send_cookies; + } + + // Keep track of the last user the cookies were generated for. + public static function set_auth_cookie( $auth_cookie, $expire, $expiration, $user_id ) { + self::$last_auth_cookie_user = $user_id; + } + /** * Destroy the known password-based authentication sessions for the current user. * @@ -897,6 +926,9 @@ public static function login_form_validate_2fa() { $rememberme = true; } + // TODO. + add_filter( 'two_factor_authentication_succeeded', '__return_true' ); + wp_set_auth_cookie( $user->ID, $rememberme ); do_action( 'two_factor_user_authenticated', $user ); From c21109469294628004b566f9d7f08f43ec06db6a Mon Sep 17 00:00:00 2001 From: Dion Hulse Date: Thu, 3 Nov 2022 13:26:07 +1000 Subject: [PATCH 2/8] Catch wp_clear_auth_cookie() usage as well. --- class-two-factor-core.php | 16 ++++++++++++---- 1 file changed, 12 insertions(+), 4 deletions(-) diff --git a/class-two-factor-core.php b/class-two-factor-core.php index 3b3449e1..d83e0577 100644 --- a/class-two-factor-core.php +++ b/class-two-factor-core.php @@ -73,6 +73,7 @@ public static function add_hooks( $compat ) { add_action( 'plugins_loaded', array( __CLASS__, 'load_textdomain' ) ); add_action( 'init', array( __CLASS__, 'get_providers' ) ); add_action( 'wp_login', array( __CLASS__, 'wp_login' ), 10, 2 ); + add_action( 'clear_auth_cookie', array( __CLAS__, 'clear_auth_cookie' ) ); add_action( 'set_auth_cookie', array( __CLASS__, 'set_auth_cookie' ), 10, 4 ); add_action( 'send_auth_cookies', array( __CLASS__, 'send_auth_cookies' ) ); add_action( 'login_form_validate_2fa', array( __CLASS__, 'login_form_validate_2fa' ) ); @@ -449,6 +450,11 @@ public static function wp_login( $user_login, $user ) { // Don't send auth cookies in the first place. public static function send_auth_cookies( $send_cookies ) { + if ( ! $send_cookies ) { + return $send_cookies; + } + + // Don't send auth cookies for an authenticated user, unless they've passed the two-factor check. if ( $send_cookies && self::$last_auth_cookie_user && @@ -456,19 +462,21 @@ public static function send_auth_cookies( $send_cookies ) { ! apply_filters( 'two_factor_authentication_succeeded', false ) ) { $send_cookies = false; - - // Reset the tracked user. - self::$last_auth_cookie_user = false; } return $send_cookies; } - // Keep track of the last user the cookies were generated for. + // Keep track of the last user the cookies were generated for, called from wp_set_auth_cookie(). public static function set_auth_cookie( $auth_cookie, $expire, $expiration, $user_id ) { self::$last_auth_cookie_user = $user_id; } + // Clear the tracked user, called from wp_clear_auth_cookie() + public static function clear_auth_cookie() { + self::$last_auth_cookie_user = 0; + } + /** * Destroy the known password-based authentication sessions for the current user. * From f558b136ef306d853aa3317ba8af6939d1c40cc0 Mon Sep 17 00:00:00 2001 From: Dion Hulse Date: Thu, 3 Nov 2022 15:00:21 +1000 Subject: [PATCH 3/8] Remove the filter, rather than using a flag. --- class-two-factor-core.php | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/class-two-factor-core.php b/class-two-factor-core.php index d83e0577..a95bcf37 100644 --- a/class-two-factor-core.php +++ b/class-two-factor-core.php @@ -458,8 +458,7 @@ public static function send_auth_cookies( $send_cookies ) { if ( $send_cookies && self::$last_auth_cookie_user && - self::is_user_using_two_factor( self::$last_auth_cookie_user ) && - ! apply_filters( 'two_factor_authentication_succeeded', false ) + self::is_user_using_two_factor( self::$last_auth_cookie_user ) ) { $send_cookies = false; } @@ -934,8 +933,8 @@ public static function login_form_validate_2fa() { $rememberme = true; } - // TODO. - add_filter( 'two_factor_authentication_succeeded', '__return_true' ); + // Allow authentication cookies to be set for this user. + remove_action( 'send_auth_cookies', array( __CLASS__, 'send_auth_cookies' ) ); wp_set_auth_cookie( $user->ID, $rememberme ); From 578260c4f643f45c81212aa4ef0cd79a077bb00e Mon Sep 17 00:00:00 2001 From: Dion Hulse Date: Thu, 3 Nov 2022 15:05:31 +1000 Subject: [PATCH 4/8] When wp_set_auth_cookie() is called outside of login, redirect the user through wp-login.php for two-factor verification. --- class-two-factor-core.php | 65 ++++++++++++++++++++++++++++++++++++++- 1 file changed, 64 insertions(+), 1 deletion(-) diff --git a/class-two-factor-core.php b/class-two-factor-core.php index a95bcf37..2af7e19a 100644 --- a/class-two-factor-core.php +++ b/class-two-factor-core.php @@ -73,9 +73,10 @@ public static function add_hooks( $compat ) { add_action( 'plugins_loaded', array( __CLASS__, 'load_textdomain' ) ); add_action( 'init', array( __CLASS__, 'get_providers' ) ); add_action( 'wp_login', array( __CLASS__, 'wp_login' ), 10, 2 ); - add_action( 'clear_auth_cookie', array( __CLAS__, 'clear_auth_cookie' ) ); + add_action( 'clear_auth_cookie', array( __CLASS__, 'clear_auth_cookie' ) ); add_action( 'set_auth_cookie', array( __CLASS__, 'set_auth_cookie' ), 10, 4 ); add_action( 'send_auth_cookies', array( __CLASS__, 'send_auth_cookies' ) ); + add_action( 'login_form_show_2fa', array( __CLASS__, 'login_form_show_2fa' ) ); add_action( 'login_form_validate_2fa', array( __CLASS__, 'login_form_validate_2fa' ) ); add_action( 'login_form_backup_2fa', array( __CLASS__, 'backup_2fa' ) ); add_action( 'show_user_profile', array( __CLASS__, 'user_two_factor_options' ) ); @@ -461,6 +462,13 @@ public static function send_auth_cookies( $send_cookies ) { self::is_user_using_two_factor( self::$last_auth_cookie_user ) ) { $send_cookies = false; + + // If we're not on wp-login.php, redirect there. This is for when `wp_set_auth_cookie()` is called outside of wp-login.php. + if ( ! did_action( 'login_init' ) ) { + $user = get_userdata( self::$last_auth_cookie_user ); + self::redirect_to_two_factor( $user ); + exit; + } } return $send_cookies; @@ -566,6 +574,61 @@ public static function show_two_factor_login( $user ) { self::login_html( $user, $login_nonce['key'], $redirect_to ); } + /** + * Display the Two Factor login. + */ + public static function login_form_show_2fa() { + $wp_auth_id = filter_input( INPUT_GET, 'wp-auth-id', FILTER_SANITIZE_NUMBER_INT ); + $nonce = filter_input( INPUT_GET, 'wp-auth-nonce', FILTER_CALLBACK, array( 'options' => 'sanitize_key' ) ); + $redirect_to = isset( $_REQUEST['redirect_to'] ) ? $_REQUEST['redirect_to'] : wp_get_referer(); + $user = get_user_by( 'id', $wp_auth_id ); + + if ( ! $wp_auth_id || ! $nonce ) { + return; + } + + $user = get_userdata( $wp_auth_id ); + if ( ! $user ) { + return; + } + + if ( true !== self::verify_login_nonce( $user->ID, $nonce ) ) { + wp_safe_redirect( home_url() ); + exit; + } + + + self::login_html( $user, $nonce, $redirect_to ); + exit; + } + + /** + * Redirect the user to the two-factor authentication. + */ + public static function redirect_to_two_factor( $user ) { + if ( ! $user ) { + $user = wp_get_current_user(); + } + + $login_nonce = self::create_login_nonce( $user->ID ); + if ( ! $login_nonce ) { + wp_die( esc_html__( 'Failed to create a login nonce.', 'two-factor' ) ); + } + + $redirect_to = isset( $_REQUEST['redirect_to'] ) ? $_REQUEST['redirect_to'] : admin_url(); + + wp_safe_redirect( self::login_url( + array( + 'action' => 'show_2fa', + 'wp-auth-id' => $user->ID, + 'wp-auth-nonce' => $login_nonce['key'], + 'redirect_to' => $redirect_to, + ) + ) ); + exit; + } + + /** * Display the Backup code 2fa screen. * From 7c413b334a065ba6a2faf3569de489ba2bde7537 Mon Sep 17 00:00:00 2001 From: Dion Hulse Date: Thu, 3 Nov 2022 15:11:00 +1000 Subject: [PATCH 5/8] Documentation & WordPress 6.2+ filter usage. --- class-two-factor-core.php | 39 ++++++++++++++++++++++++++++++--------- 1 file changed, 30 insertions(+), 9 deletions(-) diff --git a/class-two-factor-core.php b/class-two-factor-core.php index 2af7e19a..7c6979f2 100644 --- a/class-two-factor-core.php +++ b/class-two-factor-core.php @@ -59,8 +59,10 @@ class Two_Factor_Core { /** * Keep track of who wp_set_auth_cookie() is running for. + * + * @var int */ - private static $last_auth_cookie_user = false; + private static $last_auth_cookie_user = 0; /** * Set up filters and actions. @@ -75,7 +77,7 @@ public static function add_hooks( $compat ) { add_action( 'wp_login', array( __CLASS__, 'wp_login' ), 10, 2 ); add_action( 'clear_auth_cookie', array( __CLASS__, 'clear_auth_cookie' ) ); add_action( 'set_auth_cookie', array( __CLASS__, 'set_auth_cookie' ), 10, 4 ); - add_action( 'send_auth_cookies', array( __CLASS__, 'send_auth_cookies' ) ); + add_action( 'send_auth_cookies', array( __CLASS__, 'send_auth_cookies' ), 10, 2 ); add_action( 'login_form_show_2fa', array( __CLASS__, 'login_form_show_2fa' ) ); add_action( 'login_form_validate_2fa', array( __CLASS__, 'login_form_validate_2fa' ) ); add_action( 'login_form_backup_2fa', array( __CLASS__, 'backup_2fa' ) ); @@ -449,23 +451,34 @@ public static function wp_login( $user_login, $user ) { exit; } - // Don't send auth cookies in the first place. - public static function send_auth_cookies( $send_cookies ) { + /** + * Maybe abort sending authentication cookies, and prompt for two-factor instead. + * + * @param bool $send_cookies Whether to send the authentication cookies. + * @param int $user_id The User ID having cookies sent for. WordPress 6.2+. + * @return Filtered `$send_cookies`. + */ + public static function send_auth_cookies( $send_cookies, $user_id = null ) { if ( ! $send_cookies ) { return $send_cookies; } + // WordPress < 6.2 compat. See https://core.trac.wordpress.org/ticket/56971. + if ( is_null( $user_id ) ) { + $user_id = self::$last_auth_cookie_user; + } + // Don't send auth cookies for an authenticated user, unless they've passed the two-factor check. if ( $send_cookies && - self::$last_auth_cookie_user && - self::is_user_using_two_factor( self::$last_auth_cookie_user ) + $user_id && + self::is_user_using_two_factor( $user_id ) ) { $send_cookies = false; // If we're not on wp-login.php, redirect there. This is for when `wp_set_auth_cookie()` is called outside of wp-login.php. if ( ! did_action( 'login_init' ) ) { - $user = get_userdata( self::$last_auth_cookie_user ); + $user = get_userdata( $user_id ); self::redirect_to_two_factor( $user ); exit; } @@ -474,12 +487,20 @@ public static function send_auth_cookies( $send_cookies ) { return $send_cookies; } - // Keep track of the last user the cookies were generated for, called from wp_set_auth_cookie(). + /** + * Keep track of the last user a cookie was generated for. + * + * This is used for when context is not provided via send_auth_cookies. + */ public static function set_auth_cookie( $auth_cookie, $expire, $expiration, $user_id ) { self::$last_auth_cookie_user = $user_id; } - // Clear the tracked user, called from wp_clear_auth_cookie() + /** + * Keep track of the last user a cookie was generated for. + * + * When clearing cookies, there is not user context. + */ public static function clear_auth_cookie() { self::$last_auth_cookie_user = 0; } From ce690cd8019f52dd30e97eb4b0be667c8196e363 Mon Sep 17 00:00:00 2001 From: Dion Hulse Date: Thu, 3 Nov 2022 15:18:52 +1000 Subject: [PATCH 6/8] Correct PHPDoc format. --- 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 7c6979f2..1f517e26 100644 --- a/class-two-factor-core.php +++ b/class-two-factor-core.php @@ -456,7 +456,7 @@ public static function wp_login( $user_login, $user ) { * * @param bool $send_cookies Whether to send the authentication cookies. * @param int $user_id The User ID having cookies sent for. WordPress 6.2+. - * @return Filtered `$send_cookies`. + * @return bool Filtered `$send_cookies`. */ public static function send_auth_cookies( $send_cookies, $user_id = null ) { if ( ! $send_cookies ) { From c68cdb4626b74b7c8151232c4ad47e8666d11d22 Mon Sep 17 00:00:00 2001 From: Dion Hulse Date: Thu, 3 Nov 2022 15:20:46 +1000 Subject: [PATCH 7/8] Respect the redirect location if a cookie set was intercepted. --- class-two-factor-core.php | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/class-two-factor-core.php b/class-two-factor-core.php index 1f517e26..c624487a 100644 --- a/class-two-factor-core.php +++ b/class-two-factor-core.php @@ -601,7 +601,7 @@ public static function show_two_factor_login( $user ) { public static function login_form_show_2fa() { $wp_auth_id = filter_input( INPUT_GET, 'wp-auth-id', FILTER_SANITIZE_NUMBER_INT ); $nonce = filter_input( INPUT_GET, 'wp-auth-nonce', FILTER_CALLBACK, array( 'options' => 'sanitize_key' ) ); - $redirect_to = isset( $_REQUEST['redirect_to'] ) ? $_REQUEST['redirect_to'] : wp_get_referer(); + $redirect_to = ! empty( $_REQUEST['redirect_to'] ) ? $_REQUEST['redirect_to'] : wp_get_referer(); $user = get_user_by( 'id', $wp_auth_id ); if ( ! $wp_auth_id || ! $nonce ) { @@ -618,7 +618,6 @@ public static function login_form_show_2fa() { exit; } - self::login_html( $user, $nonce, $redirect_to ); exit; } @@ -636,7 +635,7 @@ public static function redirect_to_two_factor( $user ) { wp_die( esc_html__( 'Failed to create a login nonce.', 'two-factor' ) ); } - $redirect_to = isset( $_REQUEST['redirect_to'] ) ? $_REQUEST['redirect_to'] : admin_url(); + $redirect_to = isset( $_REQUEST['redirect_to'] ) ? $_REQUEST['redirect_to'] : ''; wp_safe_redirect( self::login_url( array( From c31d18755c4dd12bb863d40609449cadcf58d0d7 Mon Sep 17 00:00:00 2001 From: Dion Hulse Date: Tue, 6 Dec 2022 14:17:29 +1000 Subject: [PATCH 8/8] Update class-two-factor-core.php Co-authored-by: Ian Dunn --- 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 c624487a..8cab506a 100644 --- a/class-two-factor-core.php +++ b/class-two-factor-core.php @@ -497,7 +497,7 @@ public static function set_auth_cookie( $auth_cookie, $expire, $expiration, $use } /** - * Keep track of the last user a cookie was generated for. + * Reset the last user a cookie was generated for. * * When clearing cookies, there is not user context. */