[24.0] Disable password reset for deleted users [GCC2024_COFEST] #18459
Conversation
Co-authored-by: John Davis <[email protected]>
laperlej
changed the title
| self.user_manager.delete(user) | ||
| assert user.deleted is True | ||
| message = self.user_manager.send_reset_email(self.trans, {"email": user_email}) | ||
| assert message == "Failed to produce password reset token. User not found." |
There was a problem hiding this comment.
Is this message getting out of the API ? We should never reveal whether or not a user exists at an instance, that's a security and privacy concern.
There was a problem hiding this comment.
That message is returned when we fail to produce a password reset token.
Should we change it to just "Failed to produce password reset token"?
There was a problem hiding this comment.
I think we should return nothing, as Failed to produce password reset token is also an admission that we know about the user. (or not, in this case)
There was a problem hiding this comment.
But we need to indicate to the user that they won't be able to reset their password (for whatever reason: either the account doesn't exist/is deleted etc.
Do we just pretend that a reset email is sent even though it isn't? Or maybe return this:
"Failed to submit email. Please contact the administrator: {util.unicodify(e)}" (from L610 in lib/galaxy/managers/users.py)
There was a problem hiding this comment.
I don't agree, services generally don't ack that an account exists. Most services I know just say something along the lines of "If an account exists for this email address a confirmation email will be dispatched".
There was a problem hiding this comment.
Do we just pretend that a reset email is sent even though it isn't?
That's what I meant to say here, my bad. So regardless of whether the email is sent or not, we just alert:
"If an account exists for this email address a confirmation email will be dispatched" (or something similar) once the user clicks reset password
There was a problem hiding this comment.
@ahmedhamidawan yeah, that is the correct message
Currently a deleted user can use the reset password functionality. Added a check in the send_reset_email function, deleted users will behave the same way as users that cannot be found. Returning the following error: "Failed to produce password reset token. User not found."
fixes #18195
How to test the changes?
(Select all options that apply)
License