-
Notifications
You must be signed in to change notification settings - Fork 1k
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
[24.0] Return generic message for password reset email #18479
[24.0] Return generic message for password reset email #18479
Conversation
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
lib/galaxy/managers/users.py
Outdated
else: | ||
return "Failed to produce password reset token. User not found." | ||
return None |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We no longer need the else
clause: None
will be returned by the outer conditional. To simplify, you could just have return None
as the last line in the method's body, outside the conditionals' scope.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Or just drop the else:
clause completely, as returning None is the default.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, but ( I think) mypy will fail with a missing return statement. The argument, I think, goes like this: if there is no previous return statement, it's OK to let it fall through to the default None; but if there is a return statement in a previous clause, adding an explicit return None
improves readability. (I don't remember the source, but I do support the argument)
The unit test needs to be fixed too: this line will fail https://github.com/galaxyproject/galaxy/blob/dev/test/unit/app/managers/test_UserManager.py#L243 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good! Thank you for addressing the comments!
Co-authored-by: John Davis <[email protected]>
Co-authored-by: Marius van den Beek <[email protected]>
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 #18475
How to test the changes?
(Select all options that apply)
License