search

mikker / passwordless

🗝 Authentication for your Rails app without the icky-ness of passwords
MIT License
1.26k stars 85 forks source link

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

Closed mzrnsh closed 7 months ago

mzrnsh commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

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

dkhgh commented 1 year ago

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 commented 1 year ago

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