Flush 'user_activation_key' after successfully login

[nsinelnikov]

Hi all,

Let's imagine the next steps:

1. User goes to {site_url}/wp-login.php?action=lostpassword for getting reset password link to its email.

2. Then go to email and open the reset password link with an expiration time (DAY_IN_SECONDS by default). It has been resolved a long time ago. But then he remembers his old password and login using a second web browser with its username and old password. At the same time, the link to reset the password remains active in the first browser for a whole day.

3. If it's a public laptop anybody can use the reset password link and login with new credentials and make some hacker things.

Suggestions:

Flush the 'user_activation_key' after successful login:

wp-includes/user.php::line 113 before

do_action( 'wp_login', $user->user_login, $user );

Can be added this line:

global $wpdb;
$wpdb->update(
    $wpdb->users,
    array(
	'user_activation_key' => '',
    ),
    array( 'ID' => $user->ID )
);

Best Regards!

Flushing 'user_activation_key' after successful login

Trac ticket: https://core.trac.wordpress.org/ticket/58901

Flushing 'user_activation_key' after successful login

Trac ticket: https://core.trac.wordpress.org/ticket/58901

The proper PR is ​here

[rajinsharwar]

Looks great to me. Placing this for 6.4

[rajinsharwar]

I believe we will be needing unit tests for this.

[Rahmohn]

Update to clear the user_activation_key in the user object and add a test

[Rahmohn]

Hi @nsinelnikov

I've added a patch that updates to clear the user_activation_key property from the user object and adds a test.

[oglekler]

I wonder if clean_user_cache( $user_id ) is a better way to clear the cache.

[Rahmohn]

Replying to oglekler:

I wonder if clean_user_cache( $user_id ) is a better way to clear the cache.

Hi @oglekler

If I'm not wrong, `clean_user_cache( $user_id ) just removes the cache for the user ID, login, nicename, email and meta but the link to reset the password will still be valid. That way, cleaning the user cache after a successful login doesn't seem to help in this case.

[oglekler]

I was suggesting use clean_user_cache( $user_id ); instead of $user->__set( 'user_activation_key', '' );

I've tested the last patch, it works as expected, but I was unable to actually apply the patch, tests need a rebase.

[hellofromTonya]

In 58901.diff​, directly invoking the __set() magic method is not preferred. And 'user_activation_key' is a dynamic property (i.e. undeclared property of WP_User) that is incompatible with PHP 8.2 and beyond.

I think this needs more consideration of how to reset the cached activation key. More test reports before and after would also be helpful.

Tomorrow being Beta 1. Moving it to 6.5 to give the ticket more time.