-
Notifications
You must be signed in to change notification settings - Fork 38
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[D7] Prevent username enumeration via one time login route #6018
Comments
I'm thinking that this should be dealt with as part of #4696, with the same setting we are to introduce there. Waiting for feedback on that issue (or here) before I mark this issue here as a duplicate and close it in favor of that other issue. |
I am suggesting to change the message in this use case to the following:
|
Is this not a duplicate of #5971 (which was fixed and merged last week)? |
Yes it is @BWPanda 🤦🏼 ...I did a search of the terms I guess that people will find this issue here when they search in the future, and that'll lead them to #5971 😉 Closing... |
I'm reopening this because the D7 backport has a couple useful lines https://git.drupalcode.org/project/drupal/-/commit/bdb7dd43533e99dc1b71bec1598c62ee2ffc4dfa. Such as checking that the timestamp is valid:
|
If #6420 is merged first we'll have to update this PR to add |
Since #6420 has now been committed does backdrop/backdrop#4674 need update to add |
@izmeez this has been updated. |
This is another related issue. While testing the PR tugboat, before logging in, request a new password for a non-existing user works to not identify if the user or email exist. However, request for a known user, e.g. admin returns: |
There are issues with the PR. I will leave comment there. |
The PR does include the extra checks added to Drupal 7. It would still help to know the overall goal. |
@stpaultim @izmeez Okay, I'm going to describe how to test this PR. And what it does. It makes sure that if someone goes to So that's the first thing you can test. And the other thing to test is to be logged in as one user (test1), then create a one-time login link for a second user (might need to do this with bee "You cannot use a password reset link while logged into the site." Note that this PR looks a bit different than the original Drupal 7 PR since Backdrop had already removed the username from the message. But it still includes the improvement of checking the timestamp in the reset URL to better know if it's a valid one-time link or not. You can also see that the PR also includes an automated test that does everything that I describe above so I know that it's already passing those tests. |
This is the respective issue for https://www.drupal.org/project/drupal/issues/2828724 (D9.5) and its D7 counterpart https://www.drupal.org/project/drupal/issues/3326994.
...
In the summary above, I have only kept the bit that is relevant to Backdrop, and have confirmed that it applies to us: Trying to access
/user/reset/1/1/1
while logged in with another user account, reveals the username of user1:The text was updated successfully, but these errors were encountered: