-
Notifications
You must be signed in to change notification settings - Fork 978
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
Existence of user account queryable through password reset #18475
Comments
Nice catch. I remember we have fixed this long ago, but seems we reintroduced it. |
This prevents existence of a user account from being queryable through password reset. We now return `None` and display a generic message regardless of a prt being created or not. Fixes galaxyproject#18475
I think it's reasonable to change the message here, but I just want to point out that if we really want to prevent leaking emails we would have to significantly change (impede) how registration works, explicitly putting the activation email in the required loop (i.e. -- you can't just register and immediately be logged in, you must click the link in the email to start your first session). Without this, the act of registering an already registered email leaks as well. |
This is a good point. So, how do we communicate to a user who has an account but may have forgotten about it and is trying to register again? Should the system send an email informing that user that they have an account and ask whether they want to reset their password? Plus, display a generic message, e.g. "an activation message has been sent to the provided email address"? |
The two tricky parts while tightening this seem to be:
|
I think that's fine, it's probably only small or behind-firewall instances that don't have user activation enabled. We can still mention in the user activation settings that having it disabled allows this information leak mechanism. |
We could also consider enabling activation by default. Or at least heartily recommend it for production settings since it complicates basic setup. |
Originally posted by @mvdbeek in #18459 (comment)
The text was updated successfully, but these errors were encountered: