-
-
Notifications
You must be signed in to change notification settings - Fork 88
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
Comments
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. |
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 👌 |
I don't see a benefit, happy to change the response code. PR welcome 😊 |
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: (Yep, I landed on this repo by looking for Devise alternatives) |
Didn't know about this Devise option! If we do end up adding one, let's copy the name at least from Devise 👍 |
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.
The text was updated successfully, but these errors were encountered: