Skip to content
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

Remove a message that tells whether a user exists #44

Merged
merged 3 commits into from
Dec 15, 2023

Conversation

msimkunas
Copy link
Contributor

This PR removes a message that tells the client whether a user with a given email address exists on the system.

@bennothommo
Copy link
Member

bennothommo commented Dec 6, 2023

@msimkunas I'm assuming this is to stop people from being able to work out which accounts exist on a site or not (user enumeration).

Does this change result in no flash message being shown if the user does not exist? If there's a flash message shown if the user does exist, this will just provide another avenue of enumeration - watch to see if a flash message is thrown.

Ideally, we should be throwing a flash message no matter if the user exists or not, saying something along the lines of "If your account exists, you will receive an email shortly with instructions on resetting your password". This prevents people from being able to work out if the user exists based solely on the message provided. (Ideally, we should also be queueing the email sent as well so that people can't use timing to work out if the user exists or not from the delay of an email being sent).

@msimkunas
Copy link
Contributor Author

@bennothommo I've updated the partial text and changed Mail::send to Mail::queue so that emails are queued.

@LukeTowers
Copy link
Member

@bennothommo LGTM, will let you do the merge once you're happy :)

@bennothommo bennothommo merged commit 729ac41 into wintercms:main Dec 15, 2023
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants