[D7] Username enumeration via one time login route

Thanks for the MR!

I would prefer to avoid changing frontend strings in this D7 phase if possible. Probably it will be better to reuse an existing string "The one-time login link you clicked is invalid.". See the proposed resolution in the D10 issue here: #3327294: Username enumeration via one time login route when logged in as another user. We could probably introduce a check if the link is valid and only then output the existing message, otherwise we can output: "The one-time login link you clicked is invalid.".

D10 issue was committed. We can check the approach to see if the similar can be used in D7.

Uploading a patch which backports possible parts from the D10 patch (different approach as in the current MR). I have not created a separate validation function (as in D10, see validatePathParameters()), because the D7 code is a bit different with more conditions (it is checking hashes from URL or session - $session_reset_hash and $hashed_pass), so it would not be easy to refactor it to one simple function as in D10. This patch uses the same approach to verify a valid hash as it uses a few lines below (see https://git.drupalcode.org/project/drupal/-/blob/7.x/modules/user/user.p...), which is not super ideal, but on the other hand, it is not an invasive change and it fixes the problem with enumeration. So I think this approach is worth a try.

Tested the patch also manually:

1. If a logged in user uses an invalid reset link (like /user/reset/1/1/1), then:

• before patch (username enumeration): Another user (XXX) is already logged into the site on this computer, but you tried to use a one-time link for user ZZZ. Please logout and try using the link again.
• after patch (no username enumeration): The one-time login link you clicked is invalid.

2. If a logged in user uses a valid reset link (like /user/reset/1/1673978256/rCRqJ0X800xD0eKVFhBmMnkdpDgKtKkzYIGav7sBqSg), then the message would be the same (unchanged) before and after patch: Another user (XXX) is already logged into the site on this computer, but you tried to use a one-time link for user ZZZ. Please logout and try using the link again.

The benefit of this approach is that we do not need to change any frontend-facing strings.

@BramDriesen thanks for the review!

I do not think that the message is different in D7 vs D10. If we look at UserController::resetPass() (link) there is a condition which checks if user is authenticated. If yes and the password reset link is for another user, there are only two options:

If the link is valid:

$this->messenger()->addWarning($this->t('Another user (%other_user) is already logged into the site on this computer, but you tried to use a one-time link for user %resetting_user. <a href=":logout">Log out</a> and try using the link again.',

If the link is invalid:

$this->messenger()->addError($this->t('The one-time login link you clicked is invalid.'));

The same situation is in D7 code in user_pass_reset() (when user is logged in), see: https://git.drupalcode.org/project/drupal/-/blob/7.x/modules/user/user.p...

I tried to make the smallest impact as possible, so the patch only changes the behavior when the link is invalid - this previously displayed the message with username:

drupal_set_message(t('Another user (%other_user) is already logged into the site on this computer, but you tried to use a one-time link for user %resetting_user. Please <a href="!logout">logout</a> and try using the link again.', array('%other_user' => $user->name, '%resetting_user' => $reset_link_account->name, '!logout' => url('user/logout'))), 'warning');

But after the patch applied, the message from else is displayed in such case:

drupal_set_message(t('The one-time login link you clicked is invalid.'), 'error');

The message you mention (You have tried to use a one-time login link that has either been used or is no longer valid. Please request a new one using the form below.) is displayed only when user is logged out (so the password reset form is accessible), which is not the case we are fixing here (D7 does not seems to output username if user is logged out, only if is logged in).

Adding a tag for the final review from another D7 maintainer before commit.