Protect initial login link against abuse and username leaking

Makes sense and patch looks good. We should also write a test for this changing behaviour though.

Thanks for the patches.

1) Patch from #4 seems fine.

2) For tests patch from #8, noticed a nitpick:

+++ b/core/modules/user/tests/src/Functional/UserPasswordResetTest.php
@@ -81,6 +81,14 @@ public function testUserPasswordReset() {
+    // Fail to do a first login if the request time was 60 seconds older than the allowed limit.

Nitpick: Over 80 chars.

3) Both patches applied cleanly to 9.1 dev:

[mac:kristen:drupal-9.1.x-dev]$ patch -p1 < 3097238-initial-reset-harden-4.patch 
patching file core/assets/scaffold/files/robots.txt
patching file core/modules/user/src/Controller/UserController.php
Hunk #1 succeeded at 234 (offset -4 lines).
patching file robots.txt

[mac:kristen:drupal-9.1.x-dev]$ patch -p1 < 3097238-test-only.patch 
patching file core/modules/user/tests/src/Functional/UserPasswordResetTest.php

4) Verified the robots.txt files were updated with the disallows

5) Seems like the best way to manually test this is to do a password reset and then let it sit and then try it after it expires, yes?

I tried manually testing as follows:

1) Tested on simplytest.me and added these two modules as it doesn't send email or provide access via drush:

https://www.drupal.org/project/user_pwreset_timeout
https://www.drupal.org/project/maillog

2) Set password_reset_timeout to 60 seconds for easy testing (via /admin/config/people/accounts)

3) Added a new user and sent the email and grabbed the reset URL

4) Used the reset URL in another browser while logged out before the 60 seconds timeout and it worked as expected

5) Repeated 3

6) Waited a few minutes and then used the reset URL, expecting the reset URL to not work but the reset URL did work

7) Thought maybe it was the user_pwreset_timeout module not working so tested the same idea as above but with the password reset process and it worked as expected (reset URL worked if less than 60 seconds, and reset URL didn't work if more than 60 seconds)

8) Cleared cache and repeated 3 & 6 in case I did something wrong but the same thing happened (reset URL worked when I was expecting it not to)

I can't completely rule out that the user_pwreset_timeout module isn't working but my testing didn't work. I might try again later to make sure I didn't set things up wrong.

Set timeout to 60 seconds

Grab reset URL

Wait more than 60 seconds and use reset URL

Do password reset and wait more than 60 seconds to use reset URL

Things:

1) Re-rolled the patch from #4 because of #3123285: Actually exclude user register, login, logout, and password pages from search results in robots.txt (current rules are broken). (The other user/ URLS have had their trailing slash removed, but ours should keep the trailing slash. Also: can we be allowed to alphabetize the links in this 'user' section of robots.txt please? It's hard to know where to add, because both sections are un-alphabetized AND inconsistent with each other.)

2)
Manual testing works for me. I repeated Kristen Pol's failing test steps 3 and 6 (with the user_pwreset_timeout module installed, but on my localhost instead of simplytest.me) and they did work for me. (If I had to guess: maybe simplytest.me applied the patch from #8? The actual fix is in #4.)

3)
Re. #6: Upon thinking it over, I'm not surprised that no existing no functional tests broke. What this change does, is bring things in line with the behavior of "password reset" links, which keep behaving the same way and don't need additional test coverage. (And the test from test-only patch #8 is already almost-literally present already: https://git.drupalcode.org/project/drupal/-/blob/81768d40f9d4a98cb9a8e8b... )

What we could test, is the presence of a full 'user reset' link in the registration e-mail. which includes a timestamp that we'll use now. (It's a bit arbitrary because the timestamp has always been a mandatory part of the link; we just never used it until now. So the test change in the interdiff doesn't really test changed functionality. The added comments make clear that we don't need to test anything else.)

4)
Since we always use the timestamp in said link, also for user registration... an extra one-line change was added to UserController, and the logic of displaying "Set" vs "Reset" is moved inside UserPasswordResetForm now. I'm not really sure how to treat the quote-unquote "deprecation" of the NULL value for $expiration_date there.

I'm opening a merge request, with 2 commits, 1. the re-rolled patch, 2. the interdiff.

Sorry for the commit noise. Still getting used to the MR workflow.

Now done:

• Added a Change Notice (with a code blurb that is very small, because... noone will care about this change).
• Rebased on 9.3.x (no changes); added a third commit mentioning that change notice and a todo.

Reroll against 9.4.5.

Composer patching doesn't like changes to robots.txt and I'm unclear on why: I'm guessing I have a version of core that scaffolds robots.txt every time rather than including it. So here's a patch that doesn't include robots.txt.

Only a partial solution that prevents the crawling of the login link: Shy One Time
Since bots, crawlers & co like to simply ignore the robots.txt.

This is a reroll against 9.5.0 for anyone who needs it.

Rebased MR #151 against 11.x Branch , Setting NR.

Serving as the Ghost of Drupal Past, as I am sure everyone remembers ;) José added this not long ago ;) in #18719: Request New Password Security with a little dabbling from me but even I can't recall the reason for no timeout on first login. Re-reading the issue, it was introduced in #14 but there's no reasoning given. Considering some use cases here... for example you might be registering on an event website months ahead, get a link and never bother to go through with the actual account creation until the event comes. if we consider this a valid use case then maybe we should add instructions on how to obtain a fresh reset link -- AFAIK currently the only way in the web UI is to visit user/reset, enter the username and click... so maybe we should consider adding username prefill functionality to the user reset page and add instructions to the initial user mail?

This seems like something that could use an issue summary update.

Is the same approach from 3 years ago still desired?

The issue summary still seems solid to me.

Reroll of #25 against Drupal 10.2.x

Reroll of #34 against 10.3.x

Merged the latest 11.x.

The Needs Review Queue Bot tested this issue. It no longer applies to Drupal core. Therefore, this issue status is now "Needs work".

This does not mean that the patch necessarily needs to be re-rolled or the MR rebased. Read the Issue Summary, the issue tags and the latest discussion here to determine what needs to be done.

Consult the Drupal Contributor Guide to find step-by-step guides for working with issues.

I merged the latest 11.x

The Needs Review Queue Bot tested this issue. It no longer applies to Drupal core. Therefore, this issue status is now "Needs work".

This does not mean that the patch necessarily needs to be re-rolled or the MR rebased. Read the Issue Summary, the issue tags and the latest discussion here to determine what needs to be done.

Consult the Drupal Contributor Guide to find step-by-step guides for working with issues.

I merged the latest 11.x.

Left some comments on the MR.

I think we need a followup for the todo (remove deprecated code branch in UserPasswordResetForm).

@prudloff looks like this is very close to the finish-line?

Here's a patch that applies to current main