Skip to content
New issue

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

Sign up for GitHub

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

Already on GitHub? Sign in to your account

ReAuth: resolve fatal, code cleanup #567

Merged
merged 17 commits into from
May 17, 2023
Merged

ReAuth: resolve fatal, code cleanup #567

Show file tree
Hide file tree
Changes from 12 commits
Commits
Show all changes
17 commits
Select commit Hold shift + click to select a range
b5626ae
Centralise the $provider logic into a central method.
dd32 May 15, 2023
535f741
Add update_current_user_session() and get_current_user_session() help…
dd32 May 15, 2023
ad7db92
A non-two-factor user can update their two-factor details.
dd32 May 15, 2023
1aa854b
Use the generic Cheating error for missing provider actions.
dd32 May 15, 2023
02c131c
add test to show bug
iandunn May 12, 2023
1093899
test working w/ wiped out raw data
iandunn May 12, 2023
3bd84c8
Tests: Make use of Two_Factor_Core::get_current_user_session().
dd32 May 15, 2023
ac7813b
Return early
dd32 May 15, 2023
5e83e1d
Whitespace
dd32 May 15, 2023
30d8382
Clarify phpdoc
dd32 May 15, 2023
fb4b3ca
Lowercase instanceof
dd32 May 15, 2023
77a6b23
If the session is already two-factor, don't overwrite those data poin…
dd32 May 15, 2023
58a7d8b
Test: Clarify test description.
dd32 May 16, 2023
a5b3479
When a provider instance is passed, still verify that it's enabled fo…
dd32 May 17, 2023
b040cad
Tests: Add test coverage for Two_Factor_Core::get_provider_for_user, …
dd32 May 17, 2023
5ea7ba3
Ensure that when a provider object is passed, that the singleton inst…
dd32 May 17, 2023
d1941d6
Tests: Check the primary provider is returned with a valid (but not a…
dd32 May 17, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
164 changes: 114 additions & 50 deletions class-two-factor-core.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -447,6 +447,41 @@ public static function get_available_providers_for_user( $user = null ) {
return $configured_providers;
}

/**
* Fetch the provider for the request based on the user preferences.
*
* @param int|WP_User $user Optonal. User ID, or WP_User object of the the user. Defaults to current user.
* @param null|string|object $preferred_provider Optional. The name of the provider, the provider, or empty.
* @return null|object The provider
*/
public static function get_provider_for_user( $user = null, $preferred_provider = null ) {
if ( $preferred_provider && $preferred_provider instanceof Two_Factor_Provider ) {
return $preferred_provider;
}

$user = self::fetch_user( $user );
if ( ! $user ) {
return null;
}

// Default to the currently logged in provider.
if ( ! $preferred_provider && get_current_user_id() === $user->ID ) {
$session = self::get_current_user_session();
if ( ! empty( $session['two-factor-provider'] ) ) {
$preferred_provider = $session['two-factor-provider'];
}
}

if ( is_string( $preferred_provider ) ) {
$providers = self::get_available_providers_for_user( $user );
if ( isset( $providers[ $preferred_provider ] ) ) {
return $providers[ $preferred_provider ];
}
}

return self::get_primary_provider_for_user( $user );
}

/**
* Gets the Two-Factor Auth provider for the specified|current user.
*
Expand Down Expand Up @@ -733,10 +768,9 @@ public static function clear_password_reset_notice( $user ) {
* @param string|object $provider An override to the provider.
*/
public static function login_html( $user, $login_nonce, $redirect_to, $error_msg = '', $provider = null, $action = 'validate_2fa' ) {
if ( empty( $provider ) ) {
$provider = self::get_primary_provider_for_user( $user->ID );
} elseif ( is_string( $provider ) && method_exists( $provider, 'get_instance' ) ) {
$provider = call_user_func( array( $provider, 'get_instance' ) );
$provider = self::get_provider_for_user( $user, $provider );
if ( ! $provider ) {
wp_die( __( 'Cheatin’ uh?', 'two-factor' ) );
}

$provider_key = $provider->get_key();
Expand Down Expand Up @@ -1095,10 +1129,7 @@ public static function is_user_rate_limited( $user ) {
* @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 );
$session = self::get_current_user_session();

if ( empty( $session['two-factor-login'] ) ) {
return false;
Expand All @@ -1123,8 +1154,8 @@ public static function current_user_can_update_two_factor_options( $context = 'd
return false;
}

// If the current user is not a two-factor user, not having a two-factor session is okay.
if ( ! self::is_user_using_two_factor( $user_id ) && ! $is_two_factor_session ) {
// If the current user is not using two-factor, they can adjust the settings.
if ( ! self::is_user_using_two_factor( $user_id ) ) {
return true;
}

Expand Down Expand Up @@ -1224,11 +1255,9 @@ public static function _login_form_validate_2fa( $user, $nonce = '', $provider =
return;
}

$providers = self::get_available_providers_for_user( $user );
if ( $provider && isset( $providers[ $provider ] ) ) {
$provider = $providers[ $provider ];
} else {
$provider = self::get_primary_provider_for_user( $user->ID );
$provider = self::get_provider_for_user( $user, $provider );
if ( ! $provider ) {
wp_die( __( 'Cheatin’ uh?', 'two-factor' ) );
}

// Run the provider processing.
Expand Down Expand Up @@ -1347,21 +1376,10 @@ public static function _login_form_revalidate_2fa( $provider = '', $redirect_to
return;
}

$user = wp_get_current_user();
$session_token = wp_get_session_token();
$session_manager = WP_Session_Tokens::get_instance( $user->ID );
$session = $session_manager->get( $session_token );
$providers = self::get_available_providers_for_user( $user );

// Default to the currently logged in provider.
if ( ! $provider && ! empty( $session['two-factor-provider'] ) ) {
$provider = $session['two-factor-provider'];
}

if ( $provider && isset( $providers[ $provider ] ) ) {
$provider = $providers[ $provider ];
} else {
$provider = self::get_primary_provider_for_user( $user->ID );
$user = wp_get_current_user();
$provider = self::get_provider_for_user( $user, $provider );
if ( ! $provider ) {
wp_die( __( 'Cheatin’ uh?', 'two-factor' ) );
}

// Run the provider processing.
Expand All @@ -1378,10 +1396,11 @@ public static function _login_form_revalidate_2fa( $provider = '', $redirect_to
return;
}

$session['two-factor-provider'] = get_class( $provider );
$session['two-factor-login'] = time();

$session_manager->update( $session_token, $session );
// Update the session metadata with the revalidation details.
self::update_current_user_session( array(
'two-factor-provider' => $provider->get_key(),
'two-factor-login' => time(),
) );

// Must be global because that's how login_header() uses it.
global $interim_login;
Expand Down Expand Up @@ -1825,27 +1844,72 @@ public static function user_two_factor_options_update( $user_id ) {
update_user_meta( $user_id, self::PROVIDER_USER_META_KEY, $new_provider );
}

// Have we enabled new providers? Set this as a 2FA session, so they can continue to edit.
if (
! $existing_providers &&
$enabled_providers &&
! self::is_current_user_session_two_factor() &&
$user_id === get_current_user_id()
) {
$token = wp_get_session_token();
if ( $token ) {
$manager = WP_Session_Tokens::get_instance( $user_id );
$session = $manager->get( $token );

$session['two-factor-provider'] = ''; // Set the key, but not the provider, as no provider has been used yet.
$session['two-factor-login'] = time();

$manager->update( $token, $session );
// Have we changed the two-factor settings for the current user? Alter their session metadata.
if ( $user_id === get_current_user_id() ) {

if ( $enabled_providers && ! $existing_providers && ! self::is_current_user_session_two_factor() ) {
// We've enabled two-factor from a non-two-factor session, set the key but not the provider, as no provider has been used yet.
self::update_current_user_session( array(
'two-factor-provider' => '',
'two-factor-login' => time(),
) );
} elseif ( $existing_providers && ! $enabled_providers ) {
// We've disabled two-factor, remove session metadata.
self::update_current_user_session( array(
'two-factor-provider' => null,
'two-factor-login' => null,
) );
}
}
}
}

/**
* Update the current user session metadata.
*
* Any values set in $data that are null will be removed from the user session metadata.
*
* @param array $data The data to append/remove from the current session.
* @return bool
*/
public static function update_current_user_session( $data = array() ) {
$user_id = get_current_user_id();
$token = wp_get_session_token();
if ( ! $user_id || ! $token ) {
return false;
}

$manager = WP_Session_Tokens::get_instance( $user_id );
$session = $manager->get( $token );

// Add any session data.
$session = array_merge( $session, $data );

// Remove any set null fields.
foreach ( array_filter( $data, 'is_null' ) as $key => $null ) {
unset( $session[ $key ] );
}

return $manager->update( $token, $session );
}

/**
* Fetch the current user session metadata.
*
* @return false|array The session array, false on error.
*/
public static function get_current_user_session() {
$user_id = get_current_user_id();
$token = wp_get_session_token();
if ( ! $user_id || ! $token ) {
return false;
}

$manager = WP_Session_Tokens::get_instance( $user_id );

return $manager->get( $token );
}

/**
* Should the login session persist between sessions.
*
Expand Down
36 changes: 36 additions & 0 deletions tests/class-two-factor-core.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -862,6 +862,42 @@ public function test_enabling_two_factor_is_factored_session() {

}

/**
* Validate that disabling all providers [invalidate] the two-factor session.
dd32 marked this conversation as resolved.
Show resolved Hide resolved
*
* @covers Two_Factor_Core::current_user_can_update_two_factor_options()
* @covers Two_Factor_Core::user_two_factor_options_update()
*/
public function test_disabling_two_factor_is_not_factored_session() {
$user = self::factory()->user->create_and_get();

// Setup a 2FA session.
wp_set_current_user( $user->ID );
wp_set_auth_cookie( $user->ID );

$key = '_nonce_user_two_factor_options';
$nonce = wp_create_nonce( 'user_two_factor_options' );
$_POST[ $key ] = $nonce;
$_REQUEST[ $key ] = $nonce;

$_POST[ Two_Factor_Core::ENABLED_PROVIDERS_USER_META_KEY ] = [ 'Two_Factor_Dummy' => 'Two_Factor_Dummy' ];

Two_Factor_Core::user_two_factor_options_update( $user->ID );

$this->assertNotFalse( Two_Factor_Core::is_current_user_session_two_factor() );

// Disable all providers, and test that the session is invalidated.
$_POST[ Two_Factor_Core::ENABLED_PROVIDERS_USER_META_KEY ] = [];
Two_Factor_Core::user_two_factor_options_update( $user->ID );

$this->assertFalse( Two_Factor_Core::is_current_user_session_two_factor() );

$session = Two_Factor_Core::get_current_user_session();

$this->assertArrayNotHasKey( 'two-factor-login', $session );
$this->assertArrayNotHasKey( 'two-factor-provider', $session );
}

/**
* Validate that a non-2fa login doesn't set the session two-factor data.
*
Expand Down