Skip to content

Commit

Permalink
Store the two-factor details in the user session at login time (#528)
Browse files Browse the repository at this point in the history 
* Store the two factor login timestamp and provider in the user session.

* Create the user session directly attaching data to it, rather than using filters.

* Add a method to determine if the current login session is two factored.

* Use an anonymous function attached to a callback to set the user session information.

* Tests: Add a test that validates that is_current_user_session_two_factor() doesn't return true when the cookie is set from outside of the two-factor handler.

* Make is_current_user_session_two_factor() more readable.

* Make Two_Factor_Core::login_form_validate_2fa() testable, by not calling exit;

* Tests: Add tests that validate that the 2fa status is appropriately set in the user session.

* Remove misplaced `@covers` annotation.

Co-authored-by: Ian Dunn <[email protected]>

* Correct an @Covers annotation.

Co-authored-by: Ian Dunn <[email protected]>

* Update to reflect #546

* Tests: Simplify the cookie management.

* Be more explicit about the return value.

* Use a static anonymous function, as $this isn't needed.

* Simplify the wrapper by allowing _login_form_validate_2fa() to always be exited after calling.

* Simplify by passing $provider as a string always.

* Update @SInCE tags.

---------

Co-authored-by: Ian Dunn <[email protected]>
  • Loading branch information
@dd32 @iandunn
dd32 and iandunn authored Apr 19, 2023
1 parent ed75664 commit 836ef62
Show file tree
Hide file tree
Showing 2 changed files with 174 additions and 22 deletions.
95 changes: 73 additions & 22 deletions class-two-factor-core.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1082,28 +1082,66 @@ public static function is_user_rate_limited( $user ) {
}

/**
* Login form validation.
* Determine if the current user session is logged in with 2FA.
*
* @since 0.9.0
*
* @return int|false The last time the two-factor was validated on success, false if not currently using a 2FA session.
*/
public static function is_current_user_session_two_factor() {
$user_id = get_current_user_id();
$token = wp_get_session_token();
$manager = WP_Session_Tokens::get_instance( $user_id );
$session = $manager->get( $token );

if ( empty( $session['two-factor-login'] ) ) {
return false;
}

return (int) $session['two-factor-login'];
}

/**
* Login form validation handler.
*
* @since 0.1-dev
*/
public static function login_form_validate_2fa() {
$wp_auth_id = ! empty( $_REQUEST['wp-auth-id'] ) ? absint( $_REQUEST['wp-auth-id'] ) : 0;
$nonce = ! empty( $_REQUEST['wp-auth-nonce'] ) ? wp_unslash( $_REQUEST['wp-auth-nonce'] ) : '';
$provider = ! empty( $_REQUEST['provider'] ) ? wp_unslash( $_REQUEST['provider'] ) : false;
$provider = ! empty( $_REQUEST['provider'] ) ? wp_unslash( $_REQUEST['provider'] ) : '';
$redirect_to = ! empty( $_REQUEST['redirect_to'] ) ? wp_unslash( $_REQUEST['redirect_to'] ) : '';
$is_post_request = ( 'POST' === strtoupper( $_SERVER['REQUEST_METHOD'] ) );
$user = get_user_by( 'id', $wp_auth_id );

if ( ! $wp_auth_id || ! $nonce ) {
if ( ! $wp_auth_id || ! $nonce || ! $user ) {
return;
}

$user = get_userdata( $wp_auth_id );
if ( ! $user ) {
return;
}
self::_login_form_validate_2fa( $user, $nonce, $provider, $redirect_to, $is_post_request );
exit;
}

/**
* Login form validation.
*
* This function exists for unit testing, as `exit` prevents testing.
* This function expects the caller exiting after calling.
*
* @since 0.9.0
*
* @param WP_User $user The WP_User instance.
* @param string $nonce The nonce provided.
* @param string $provider The provider to use, if known.
* @param string $redirect_to The redirection location.
* @param bool $is_post_request Whether the incoming request was a POST request or not.
* @return void
*/
public static function _login_form_validate_2fa( $user, $nonce = '', $provider = '', $redirect_to = '', $is_post_request = false ) {
// Validate the request.
if ( true !== self::verify_login_nonce( $user->ID, $nonce ) ) {
wp_safe_redirect( home_url() );
exit;
return;
}

if ( $provider ) {
Expand All @@ -1124,8 +1162,8 @@ public static function login_form_validate_2fa() {
wp_die( esc_html__( 'Failed to create a login nonce.', 'two-factor' ) );
}

self::login_html( $user, $login_nonce['key'], $_REQUEST['redirect_to'], '', $provider );
exit;
self::login_html( $user, $login_nonce['key'], $redirect_to, '', $provider );
return;
}

// If the form hasn't been submitted, just display the auth form.
Expand All @@ -1135,8 +1173,8 @@ public static function login_form_validate_2fa() {
wp_die( esc_html__( 'Failed to create a login nonce.', 'two-factor' ) );
}

self::login_html( $user, $login_nonce['key'], $_REQUEST['redirect_to'], '', $provider );
exit;
self::login_html( $user, $login_nonce['key'], $redirect_to, '', $provider );
return;
}

// Rate limit two factor authentication attempts.
Expand All @@ -1159,8 +1197,8 @@ public static function login_form_validate_2fa() {
wp_die( esc_html__( 'Failed to create a login nonce.', 'two-factor' ) );
}

self::login_html( $user, $login_nonce['key'], $_REQUEST['redirect_to'], esc_html( $error->get_error_message() ), $provider );
exit;
self::login_html( $user, $login_nonce['key'], $redirect_to, esc_html( $error->get_error_message() ), $provider );
return;
}

// Ask the provider to verify the second factor.
Expand All @@ -1177,16 +1215,16 @@ public static function login_form_validate_2fa() {
self::reset_compromised_password( $user );
self::send_password_reset_emails( $user );
self::show_password_reset_error();
exit;
return;
}

$login_nonce = self::create_login_nonce( $user->ID );
if ( ! $login_nonce ) {
wp_die( esc_html__( 'Failed to create a login nonce.', 'two-factor' ) );
}

self::login_html( $user, $login_nonce['key'], $_REQUEST['redirect_to'], esc_html__( 'ERROR: Invalid verification code.', 'two-factor' ), $provider );
exit;
self::login_html( $user, $login_nonce['key'], $redirect_to, esc_html__( 'ERROR: Invalid verification code.', 'two-factor' ), $provider );
return;
}

self::delete_login_nonce( $user->ID );
Expand All @@ -1198,15 +1236,29 @@ public static function login_form_validate_2fa() {
$rememberme = true;
}

$session_information_callback = static function( $session, $user_id ) use( $provider, $user ) {
if ( $user->ID === $user_id ) {
$session['two-factor-login'] = time();
$session['two-factor-provider'] = $provider->get_key();
}

return $session;
};

add_filter( 'attach_session_information', $session_information_callback, 10, 2 );

/*
* NOTE: This filter removal is not normally required, this is included for protection against
* a plugin/two factor provider which runs the `authenticate` filter during it's validation.
* Such a plugin would cause self::filter_authenticate_block_cookies() to run and add this filter.
*/
remove_filter( 'send_auth_cookies', '__return_false', PHP_INT_MAX );

wp_set_auth_cookie( $user->ID, $rememberme );

do_action( 'two_factor_user_authenticated', $user );
do_action( 'two_factor_user_authenticated', $user, $provider );

remove_filter( 'attach_session_information', $session_information_callback );

// Must be global because that's how login_header() uses it.
global $interim_login;
Expand All @@ -1231,12 +1283,11 @@ public static function login_form_validate_2fa() {
<?php endif; ?>
</body></html>
<?php
exit;
return;
}
$redirect_to = apply_filters( 'login_redirect', $_REQUEST['redirect_to'], $_REQUEST['redirect_to'], $user );
wp_safe_redirect( $redirect_to );

exit;
$redirect_to = apply_filters( 'login_redirect', $redirect_to, $redirect_to, $user );
wp_safe_redirect( $redirect_to );
}

/**
Expand Down
101 changes: 101 additions & 0 deletions tests/class-two-factor-core.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,8 @@ class Test_ClassTwoFactorCore extends WP_UnitTestCase {
*/
public static function wpSetUpBeforeClass() {
set_error_handler( array( 'Test_ClassTwoFactorCore', 'error_handler' ) ); // phpcs:ignore WordPress.PHP.DevelopmentFunctions.error_log_set_error_handler
add_action( 'set_auth_cookie', [ __CLASS__, 'set_auth_cookie' ] );
add_action( 'set_logged_in_cookie', [ __CLASS__, 'set_logged_in_cookie' ] );
}

/**
Expand All @@ -36,6 +38,17 @@ public static function wpSetUpBeforeClass() {
*/
public static function wpTearDownAfterClass() {
restore_error_handler();
remove_action( 'set_auth_cookie', [ __CLASS__, 'set_auth_cookie' ] );
remove_action( 'set_logged_in_cookie', [ __CLASS__, 'set_logged_in_cookie' ] );
}

/**
* Cleanup after each test.
*/
public function tearDown(): void {
parent::tearDown();

unset( $_COOKIE[ AUTH_COOKIE ], $_COOKIE[ LOGGED_IN_COOKIE ] );
}

/**
Expand All @@ -56,6 +69,20 @@ public static function error_handler( $errno, $errstr ) {
return true;
}

/**
* Simulate the auth cookie having being sent.
*/
public static function set_auth_cookie( $auth_cookie ) {
$_COOKIE[ AUTH_COOKIE ] = $auth_cookie;
}

/**
* Simulate the logged_in cookie having being sent.
*/
public static function set_logged_in_cookie( $logged_in_cookie ) {
$_COOKIE[ LOGGED_IN_COOKIE ] = $logged_in_cookie;
}

/**
* Get a dummy user object.
*
Expand Down Expand Up @@ -783,4 +810,78 @@ public function test_show_password_reset_error() {

$this->assertStringContainsString( 'check your email for instructions on regaining access', $contents );
}

/**
* Validate that a non-2fa login doesn't set the session two-factor data.
*
* @covers Two_Factor_Core::is_current_user_session_two_factor()
*/
public function test_is_current_user_session_two_factor_without_two_factor() {
$user = $this->get_dummy_user();

// Assert no cookies are set.
$this->assertArrayNotHasKey( AUTH_COOKIE, $_COOKIE );
$this->assertArrayNotHasKey( LOGGED_IN_COOKIE, $_COOKIE );

// Assert user not logged in is false.
$this->assertFalse( Two_Factor_Core::is_current_user_session_two_factor() );

// Set the cookie without going through two-factor, and fill in $_COOKIE.
wp_set_auth_cookie( $user->ID );

$this->assertNotEmpty( $_COOKIE[ AUTH_COOKIE ] );
$this->assertNotEmpty( $_COOKIE[ LOGGED_IN_COOKIE ] );

// Validate that the session is not flagged as 2FA
$this->assertFalse( Two_Factor_Core::is_current_user_session_two_factor() );

$manager = WP_Session_Tokens::get_instance( $user->ID );
$token = wp_get_session_token();
$session = $manager->get( $token );

// Validate the Session data is not set.
$this->assertArrayNotHasKey( 'two-factor-provider', $session );
}

/**
* Validate that a simulated 2fa login sets the session two-factor data.
*
* @covers Two_Factor_Core::is_current_user_session_two_factor()
* @covers Two_Factor_Core::_login_form_validate_2fa()
*/
public function test_is_current_user_session_two_factor_with_two_factor() {
$user = $this->get_dummy_user( array( 'Two_Factor_Dummy' => 'Two_Factor_Dummy' ) );

// Assert no cookies are set.
$this->assertArrayNotHasKey( AUTH_COOKIE, $_COOKIE );
$this->assertArrayNotHasKey( LOGGED_IN_COOKIE, $_COOKIE );

// Assert user not logged in is false.
$this->assertFalse( Two_Factor_Core::is_current_user_session_two_factor() );

// Simulate a 2FA login.
$login_nonce = Two_Factor_Core::create_login_nonce( $user->ID );

$this->assertNotFalse( $login_nonce );

ob_start();
Two_Factor_Core::_login_form_validate_2fa( $user, $login_nonce['key'], 'Two_Factor_Dummy', '', true );
ob_end_clean();

$this->assertNotEmpty( $_COOKIE[ AUTH_COOKIE ] );
$this->assertNotEmpty( $_COOKIE[ LOGGED_IN_COOKIE ] );

// Validate that the session is flagged as 2FA, the return value being int.
$this->assertNotFalse( Two_Factor_Core::is_current_user_session_two_factor() );

$manager = WP_Session_Tokens::get_instance( $user->ID );
$token = wp_get_session_token();
$session = $manager->get( $token );

// Validate that the session provider is as expected.
$this->assertArrayHasKey( 'two-factor-provider', $session );
$this->assertEquals( 'Two_Factor_Dummy', $session['two-factor-provider'] );

}

}

0 comments on commit 836ef62

Please sign in to comment.