mikker / passwordless

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

Added security note to README.md #105

Closed duarme closed 2 years ago

duarme commented 2 years ago

The example provided in the "Providing your own templates" chapter is still useful because it shows how to override a template, but it would be better to advise against actually using it.

mikker commented 2 years ago

Thank you for this. I've actually gone back and forth on this detail. If you also have a registration page and a uniqueness requirement on the email field, a malicious user can essentially use that registration form for the same purpose to check for existence of specific accounts.

Just to add that for this note to be true, you also need to not have a public registration form and then if you do, you might as well make the experience for the user better by actually telling them if they put in the right e-mail. Right?

mikker commented 2 years ago

Before we merge – do we want to add this detail to the note or are we already making things way too complex?

duarme commented 2 years ago

If you also have a registration page and a uniqueness requirement on the email field, a malicious user can essentially use that registration form for the same purpose to check for existence of specific accounts.

@mikker 🤔 I didn't think of this, you're right. It doesn't seem hard to fix, though. The public registration form should always reply with the same success message, something like: "We've sent the magic link to your email address".

Then there are 2 possibilities:

  1. New user: we actually send the magic link to confirm the registration.
  2. Existing user: we send the same magic link to sign in.

I think that this could improve the UX because some users confuse the sign-up page with the sign-in. In both messages an "if you didn't request this, you can ignore this email" notice can be included.

This way you're not exposing your data, and you can have a public registration form.

It seems simple enough, am I missing something? What do you think?

mikker commented 2 years ago

That's mostly how I do in my own app. However, that's getting into the individual app's specifics and I don't think something like that would be common enough for it to be the default in this gem. If it's something you want it should be easy enough to build from the parts included here 😊

duarme commented 2 years ago

Absolutely, I agree. I'm merely talking about this note for README.md 🙂 Something like:

Please note that, from a security standpoint, this is a bad practice because you'd be giving information about which users are registered on your system. It is recommended to use a single message similar to the default one: "If we found you in the system, we've sent you an email". The best practice is to never expose which emails are registered on your system.

Would work?

mikker commented 2 years ago

Sure 👍

duarme commented 2 years ago

Security note updated 🙂

mikker commented 2 years ago

Thanks!