[Security] Fixed: Strengthen one-time login links.

[herbdool]

This will address two Drupal 7 improvements:

• Changing email address should invalidate one-time login links
• Increase entropy in the one-time login links (especially in the cases where there's an empty password in the database, if a site uses SSO).

In https://www.drupal.org/project/drupal/issues/3306390:

Problem/Motivation
Assume I change email provider or leave a job. I or an attacker requests a password reset link.

I now change the email address on my drupal account, which I've been logged into on my home computer

My former employer or someone with access to my old email account can use the pre-existing one-time login link to gain access to my account (or the whole site if I'm an admin).

Proposed resolution
Invalidate one-time login links if the email address is changed. The easiest fix is to include the current email address as part of the data in user_pass_rehash

From Drupal 7.100: https://www.drupal.org/node/3409960, https://www.drupal.org/node/3409043 (backport from Drupal 10):

The problem is that there is significantly less entropy in the inputs to user_pass_rehash() if the user has an empty password field in the database.

Users may have empty passwords if - for example - a site uses a Single Sign On integration to offload authentication.

Improvements could be made to the implementation to reduce the likelihood that an attacker could brute force e.g. a password reset URL if the user has no password stored in Drupal.

It may also be advisable for sites / projects that do not use Drupal's passwords for authentication to avoid leaving the password field empty in the database, but that can be looked at in a (public) follow-up. It wouldn't help much to set the password field to the same value for all users.

[herbdool]

Related commits (will see if these are necessary):

• https://git.drupalcode.org/project/drupal/-/commit/14b23a6e29883c12746b3233116a68b1d0f11f05
• https://git.drupalcode.org/project/drupal/-/commit/41b03d753a710d4c3d9a8ce0b5fe7d1cd649d127 (Issue #2803921 by roderik, mcdruid, aerozeppelin, David_Rothstein, drumm, pwolanin, locokiter, greggles, cilefen, Fabianx: A valid one-time login link may be leaked by the referer header to 3rd parties - not strictly necessary here. See [D7] A valid one-time login link should not be leaked by the referer header to 3rd parties (d.o 2803921) #6427)
• https://git.drupalcode.org/project/drupal/-/commit/f7d2f47e9ed15ce7840ffafa105a0ea80a46eeca (Issue #889772 by tuutti, stefan.r, opdavies, Sutharsan, Perignon, pjcdawkins, joachim, das-peter, YesCT, David_Rothstein, Zerdiox, hussainweb, Fabianx, mgifford, xjm: Following a password reset link while logged in leaves users unable to change their password - not necessary for this. See [D7] Following a password reset link while logged in should allow changing the password without asking the current password (d.o 889772) #6426)
• https://git.drupalcode.org/project/drupal/-/commit/bdb7dd43533e99dc1b71bec1598c62ee2ffc4dfa (Issue #3326994 by klonos, poker10, BramDriesen: Username enumeration via one time login route - not necessary for this, see [D7] Prevent username enumeration via one time login route #6018)
• https://git.drupalcode.org/project/drupal/-/commit/21932a5f8a7ea90cd9251b9db7196b495c6842c3 (already added)
• https://git.drupalcode.org/project/drupal/-/commit/80cc74478787cf200479623f24fdea2b298ff76d (Issue #2880910 by tatarbj, joseph.olstad, vijaycs85, poker10, klausi, oadaeh, mahalingam_cs, David_Rothstein, mcdruid: [D7] Nothing clears the "5 failed login attempts" security message when a user resets their own password - not necessary for this. See [D7] Clear the "5 failed login attempts" security message when a user resets their own password (d.o #2880910) #6425)
• https://git.drupalcode.org/project/drupal/-/commit/88bc1a551c88d1bdf7bf30950d26b8e0fd9e6243 (already added in issue 5659)
• https://git.drupalcode.org/project/drupal/-/commit/1b0d6d22d336d23c0c3bb29cd0468a03ab3fbb4a

There were some big changes in this PR which mean the D7 tests need to adapt to different behaviour https://github.com/backdrop/backdrop/pull/122/files

[quicksketch]

This looks great @herbdool!

[stpaultim]

Any clear guidance on testing this issue?

Does this make UI changes or is a background thing?

[klonos]

The respective Drupal issues are either bugs or tasks, so not sure that this is a new feature request. It does need a change record though (same as the one released for D7.100: https://www.drupal.org/node/3409960), so OK if only targeted for 1.28.0 I guess.

[herbdool]

@stpaultim you can test it by seeing if changing an email address will expire a one time login for that user.

Let me expand. First create a one time login link for a user. Then in a private window use that URL. The user should be logged in. Log out.

Then create a second one time login link for that user. But before using the URL, change the email address for that user. Then go to the URL in a private window. This time the URL should not work; it should be expired.

[quicksketch]

I don't see a good reason why this would need to be in a minor release. As a security-hardening, I think this should be allowed in any bug-fix release as well.

[jenlampton]

Code looks good to me, still needs manual testing. (Failed to test on the sandbox since it won't sent pw reset emails --- needs local testing)

[quicksketch]

I verified this does not affect existing password-reset tokens or bee uli, both of which call user_pass_reset_url() (which then in turn calls user_pass_rehash(), the changed function). And this PR includes a safety fallback even if $user_mail is not provided, it automatically loads it based on the user name.

This all looks good to me!

[quicksketch]

Merged backdrop/backdrop#4673 into 1.x and 1.27.x. Thanks @herbdool, @stpaultim, and @jenlampton!

[klonos]

Reopening this in order to create a change record. Assigning it to me as well. I'll prepare a draft soon as I get a chance.

[quicksketch]

Change record created here: https://docs.backdropcms.org/change-records/user-password-reset-links-strengthened