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. Maybe we can put 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.

Clear the activation key if exists in the DB during login.

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

[rajinsharwar]

So, here, I am just flushing the activation_key using the $wpdb update, just the way we are doing in case of a successful password reset. Also, I don't think we need to clear any sort of user cache, because, we are just logging the user in using the old password, and removing the activation key, we are not changing anything in the User Data.

Also, I added an initial check if $user->user_activation_key is empty or not, and then ran that update statement, to avoid unnecessary running of query in the case where activation_key will be already empty (In most cases)

Feel free to share any suggestions and any test reports! Let's get this committed before the Beta 1!

[rajinsharwar]

I have tested the PR 5918.

✅ Initially, the 'activation_key' is empty.

✅ Requested reset email, the 'activation_key' is set.

✅ Logged in with old password, the 'activation_key' is flushed.

This is working fine. Marking this for 'commit'

[audrasjb]

In 58333:

Login and Registration: Flush user_activation_key after successfully login.

This changeset ensures the user_activation_key is flushed after successful login, so reset password links can not be used anymore after the user successfully log into their dashboard.

Props nsinelnikov, rajinsharwar, Rahmohn, oglekler, hellofromTonya.
Fixes #58901.
See #32429

COmmitted in https://core.trac.wordpress.org/changeset/58333

[SergeyBiryukov]

In 58341:

Login and Registration: Declare globals at the top of wp_signon() for consistency.

Follow-up to [10437], [32637], [58333].

See #58901.