From b1966e8b8f5f3d8aee91a39ed07a557a093b3924 Mon Sep 17 00:00:00 2001 From: narenin Date: Tue, 2 Jul 2024 11:28:16 +0530 Subject: [PATCH 1/6] Fixed : A password change should not destroy a user's session data --- src/wp-includes/user.php | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/wp-includes/user.php b/src/wp-includes/user.php index 38ff198286b3e..49243b0da5d03 100644 --- a/src/wp-includes/user.php +++ b/src/wp-includes/user.php @@ -2779,12 +2779,14 @@ function wp_update_user( $userdata ) { $default_cookie_life = apply_filters( 'auth_cookie_expiration', ( 2 * DAY_IN_SECONDS ), $user_id, false ); $remember = false; + $token = ''; if ( false !== $logged_in_cookie && ( $logged_in_cookie['expiration'] - time() ) > $default_cookie_life ) { $remember = true; + $token = $logged_in_cookie['token']; } - wp_set_auth_cookie( $user_id, $remember ); + wp_set_auth_cookie( $user_id, $remember, $token ); } } From 545870bb16851ec7a40d4a2e6bf56f5817c7ebef Mon Sep 17 00:00:00 2001 From: John Blackbourn Date: Thu, 16 Jan 2025 15:25:22 +0100 Subject: [PATCH 2/6] Tests. --- tests/phpunit/tests/user.php | 63 ++++++++++++++++++++++++++++++++++++ 1 file changed, 63 insertions(+) diff --git a/tests/phpunit/tests/user.php b/tests/phpunit/tests/user.php index 884200530f1d9..e9743eae63bde 100644 --- a/tests/phpunit/tests/user.php +++ b/tests/phpunit/tests/user.php @@ -63,9 +63,36 @@ public static function wpSetUpBeforeClass( WP_UnitTest_Factory $factory ) { public function set_up() { parent::set_up(); + add_action( 'set_auth_cookie', array( $this, 'action_set_auth_cookie' ), 10, 6 ); + add_action( 'set_logged_in_cookie', array( $this, 'action_set_logged_in_cookie' ), 10 ); + add_action( 'clear_auth_cookie', array( $this, 'action_clear_auth_cookie' ) ); + + $_COOKIE = []; $this->author = clone self::$_author; } + final public function action_set_auth_cookie( + string $cookie, + int $expire, + int $expiration, + int $user_id, + string $scheme, + string $token + ): void { + $_COOKIE[ SECURE_AUTH_COOKIE ] = $cookie; + $_COOKIE[ AUTH_COOKIE ] = $cookie; + } + + final public function action_set_logged_in_cookie( string $cookie ): void { + $_COOKIE[ LOGGED_IN_COOKIE ] = $cookie; + } + + final public function action_clear_auth_cookie(): void { + unset( $_COOKIE[ LOGGED_IN_COOKIE ] ); + unset( $_COOKIE[ SECURE_AUTH_COOKIE ] ); + unset( $_COOKIE[ AUTH_COOKIE ] ); + } + public function test_get_users_of_blog() { // Add one of each user role. $nusers = array( @@ -1122,6 +1149,42 @@ public function test_changing_password_invalidates_password_reset_key() { $this->assertEmpty( $user->user_activation_key ); } + /** + * @ticket 61366 + * @dataProvider data_remember_user + */ + public function test_changing_own_password_retains_current_session( bool $remember ) { + $user = $this->author; + $manager = WP_Session_Tokens::get_instance( $user->ID ); + $expiry = $remember ? ( 2 * WEEK_IN_SECONDS ) : ( 2 * DAY_IN_SECONDS ); + $token = $manager->create( time() + $expiry ); + $pass = $user->user_pass; + + wp_set_current_user( $user->ID ); + wp_set_auth_cookie( $user->ID, $remember, '', $token ); + + $userdata = array( + 'ID' => $user->ID, + 'user_pass' => 'my_new_password', + ); + $updated = wp_update_user( $userdata, $manager ); + $valid = wp_validate_auth_cookie(); + $cookie = wp_parse_auth_cookie(); + + $this->assertNotWPError( $updated ); + $this->assertNotSame( $pass, get_userdata( $user->ID )->user_pass ); + $this->assertSame( $user->ID, $valid ); + $this->assertSame( $token, $cookie['token'] ); + $this->assertCount( 1, $manager->get_all() ); + } + + public function data_remember_user() { + return array( + array( true ), + array( false ), + ); + } + public function test_search_users_login() { $users = get_users( array( From e9b6dbddad1644a522ee0c160763442f3980da6e Mon Sep 17 00:00:00 2001 From: John Blackbourn Date: Thu, 16 Jan 2025 15:27:58 +0100 Subject: [PATCH 3/6] Coding standards. --- tests/phpunit/tests/user.php | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/tests/phpunit/tests/user.php b/tests/phpunit/tests/user.php index e9743eae63bde..5f33f3a8536dc 100644 --- a/tests/phpunit/tests/user.php +++ b/tests/phpunit/tests/user.php @@ -63,11 +63,12 @@ public static function wpSetUpBeforeClass( WP_UnitTest_Factory $factory ) { public function set_up() { parent::set_up(); - add_action( 'set_auth_cookie', array( $this, 'action_set_auth_cookie' ), 10, 6 ); + add_action( 'set_auth_cookie', array( $this, 'action_set_auth_cookie' ), 10, 6 ); add_action( 'set_logged_in_cookie', array( $this, 'action_set_logged_in_cookie' ), 10 ); - add_action( 'clear_auth_cookie', array( $this, 'action_clear_auth_cookie' ) ); + add_action( 'clear_auth_cookie', array( $this, 'action_clear_auth_cookie' ) ); + + $_COOKIE = array(); - $_COOKIE = []; $this->author = clone self::$_author; } From db653e9102a19be8e7b3ae3e463d193843e74b66 Mon Sep 17 00:00:00 2001 From: John Blackbourn Date: Thu, 16 Jan 2025 15:39:30 +0100 Subject: [PATCH 4/6] Fix the token re-use. --- src/wp-includes/user.php | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/src/wp-includes/user.php b/src/wp-includes/user.php index 47d6854f321a0..635f82c5ed353 100644 --- a/src/wp-includes/user.php +++ b/src/wp-includes/user.php @@ -2780,8 +2780,6 @@ function wp_update_user( $userdata ) { $current_user = wp_get_current_user(); if ( $current_user->ID === $user_id ) { if ( isset( $plaintext_pass ) ) { - wp_clear_auth_cookie(); - /* * Here we calculate the expiration length of the current auth cookie and compare it to the default expiration. * If it's greater than this, then we know the user checked 'Remember Me' when they logged in. @@ -2790,15 +2788,20 @@ function wp_update_user( $userdata ) { /** This filter is documented in wp-includes/pluggable.php */ $default_cookie_life = apply_filters( 'auth_cookie_expiration', ( 2 * DAY_IN_SECONDS ), $user_id, false ); + wp_clear_auth_cookie(); + $remember = false; $token = ''; + if ( false !== $logged_in_cookie ) { + $token = $logged_in_cookie['token']; + } + if ( false !== $logged_in_cookie && ( (int) $logged_in_cookie['expiration'] - time() ) > $default_cookie_life ) { $remember = true; - $token = $logged_in_cookie['token']; } - wp_set_auth_cookie( $user_id, $remember, $token ); + wp_set_auth_cookie( $user_id, $remember, '', $token ); } } From 9778efc5469bd068ca168cf59c4cdb74335c28d1 Mon Sep 17 00:00:00 2001 From: John Blackbourn Date: Thu, 16 Jan 2025 15:53:39 +0100 Subject: [PATCH 5/6] Tidying up. --- tests/phpunit/tests/user.php | 16 ++++++++++++---- 1 file changed, 12 insertions(+), 4 deletions(-) diff --git a/tests/phpunit/tests/user.php b/tests/phpunit/tests/user.php index 5f33f3a8536dc..1d616a967cae1 100644 --- a/tests/phpunit/tests/user.php +++ b/tests/phpunit/tests/user.php @@ -1164,19 +1164,27 @@ public function test_changing_own_password_retains_current_session( bool $rememb wp_set_current_user( $user->ID ); wp_set_auth_cookie( $user->ID, $remember, '', $token ); + $cookie = $_COOKIE[ AUTH_COOKIE ]; $userdata = array( 'ID' => $user->ID, 'user_pass' => 'my_new_password', ); $updated = wp_update_user( $userdata, $manager ); - $valid = wp_validate_auth_cookie(); - $cookie = wp_parse_auth_cookie(); + $parsed = wp_parse_auth_cookie(); + // Check the prerequisites: $this->assertNotWPError( $updated ); $this->assertNotSame( $pass, get_userdata( $user->ID )->user_pass ); - $this->assertSame( $user->ID, $valid ); - $this->assertSame( $token, $cookie['token'] ); + + // Check the session token: + $this->assertSame( $token, $parsed['token'] ); $this->assertCount( 1, $manager->get_all() ); + + // Check that the newly set auth cookie is valid: + $this->assertSame( $user->ID, wp_validate_auth_cookie() ); + + // Check that, despite the session token reuse, the old auth cookie should now be invalid because the password changed: + $this->assertFalse( wp_validate_auth_cookie( $cookie ) ); } public function data_remember_user() { From 262f5992ecbcc9025dc67eb4c166b990aefcef22 Mon Sep 17 00:00:00 2001 From: John Blackbourn Date: Thu, 16 Jan 2025 15:55:13 +0100 Subject: [PATCH 6/6] More coding standards. --- tests/phpunit/tests/user.php | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/tests/phpunit/tests/user.php b/tests/phpunit/tests/user.php index 1d616a967cae1..9e1faa2dca3f0 100644 --- a/tests/phpunit/tests/user.php +++ b/tests/phpunit/tests/user.php @@ -81,7 +81,7 @@ final public function action_set_auth_cookie( string $token ): void { $_COOKIE[ SECURE_AUTH_COOKIE ] = $cookie; - $_COOKIE[ AUTH_COOKIE ] = $cookie; + $_COOKIE[ AUTH_COOKIE ] = $cookie; } final public function action_set_logged_in_cookie( string $cookie ): void { @@ -1169,8 +1169,8 @@ public function test_changing_own_password_retains_current_session( bool $rememb 'ID' => $user->ID, 'user_pass' => 'my_new_password', ); - $updated = wp_update_user( $userdata, $manager ); - $parsed = wp_parse_auth_cookie(); + $updated = wp_update_user( $userdata, $manager ); + $parsed = wp_parse_auth_cookie(); // Check the prerequisites: $this->assertNotWPError( $updated );