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

Add paranoid option to hide user existence when requesting token #131

Closed
mzrnsh opened this issue Dec 23, 2022 · 5 comments · Fixed by #196
Closed

Add paranoid option to hide user existence when requesting token #131

mzrnsh opened this issue Dec 23, 2022 · 5 comments · Fixed by #196

Comments

@mzrnsh
Copy link
Contributor

mzrnsh commented Dec 23, 2022

When you try to sign in with an email that doesn't exist in the database, visually it behaves the same way as when the email exists. My impression is this is for security reasons. But if that's true, then under the hood (but not too deep!), you can still check if any email has a user account if you want: submitting the form with existing emails returns status code 200 and missing ones return 422.

Definitely not an end of the world, but if the goal is to prevent others from checking who is using your app, both scenarios should return 200.

@mikker
Copy link
Owner

mikker commented Dec 25, 2022

Yeah, that was the initial thought and you're right.

However I've changed my mind a bit on the benefits of "not leaking user info", see point 4. So my current thinking is that it's better to sacrifice it for the better UX of telling the user whether a record was found or not.

@mzrnsh
Copy link
Contributor Author

mzrnsh commented Dec 25, 2022

One example where this could be considered a security risk is invite-only apps, with no sign up page to check which emails are already taken.

Those are probably a tiny fraction of Passwordless users (I am one of them 👋), so I understand if we don't want to build a whole new feature for them. But if it's just a matter of a response code returned, could it be worth reconsidering?

Is there any benefit to returning 422 instead of 200? I mean, apart from it already being in the codebase, meaning no changes would be applied, which is a perfectly valid reason in many cases 👌

@mikker
Copy link
Owner

mikker commented Dec 28, 2022

I don't see a benefit, happy to change the response code. PR welcome 😊

@dkhgh
Copy link

dkhgh commented Jan 1, 2023

This could be relevant background info about the security risks: https://wiki.owasp.org/index.php/Testing_for_User_Enumeration_and_Guessable_User_Account_(OWASP-AT-002)

Devise has an option called: config.paranoid, when set to true, it won't show if the provided email was found in the database.

(Yep, I landed on this repo by looking for Devise alternatives)

@mikker
Copy link
Owner

mikker commented Jan 2, 2023

Didn't know about this Devise option! If we do end up adding one, let's copy the name at least from Devise 👍

@mikker mikker changed the title 422 error in case user is not found may give away too much info Add paranoid option to hide user existence when requesting token Sep 21, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants