mikker / passwordless

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

Allow the user to change/update their email? #106

Closed duarme closed 1 year ago

duarme commented 2 years ago

Hello there and congrats on this gem! :) I'm trying it out, migrating from devise, and so I'm trying to replicate the most important functionalities.

The one the puzzles me is reconfirmable, the one about a user that needs to update their email address. Devise stores the new one in a dedicated attribute User#unconfirmed_email then sends a confirmation email to the previous email. Only upon confirmation, the unconfirmed_email becomes the email.

How would you go about reproducing this feature the passwordless way?

I'm thinking about adding an unconfirmed_email attribute to PasswordlessSession instead and then sending a confirmation email with a magic link that points (redirects?) to a PasswordlessSessionsController#update action.

Does that sound right? 🤔
How would you do it better please?

rickychilcott commented 2 years ago

This general approach could work.

Do you think in addition to authing from their old email, should they also auth from their new email once?

duarme commented 2 years ago

Now that you mention it, yes, definitely. Otherwise, a malicious user could set up an account and then assign it to another person, that then would receive notifications, newsletters, etc.

Another approach would be to send the confirmation email directly to the new email. But that would mean that any person getting a hold of the session on the victim's device, could steal their account. Am I being paranoid here?

To recap there are two possible approaches:

1. Simple

We send a single confirmation link to the new email.

2. Secure

We send a confirmation link to the previous email, and then a second one to the new one. Only after both magic links are used, we confirm the unconfirmed_email.

Which one would you choose?

rickychilcott commented 2 years ago

If a user is already authenticated then yes option 1 would be fine. I was just clarifying that you'd need to authenticate with the old email and then confirm the new email -- that's less from a security perspective and more from a usability perspective. What would happen if you fat-fingered your email address when you were changing it?

Another approach would be to send the confirmation email directly to the new email. But that would mean that any person getting a hold of the session on the victim's device, could steal their account. Am I being paranoid here?

Sure, that's true. But isn't that typical? I guess most auth providers do often require you to re-auth if you were changing your password or email (or any critical account settings). So that could steer us to option 2.

I'm supportive of this, but the real question is whether or not @mikker is willing to accept a PR and ultimately support this extension of logic as part of passwordless? @duarme do you have an interest in working up a proof of concept PR and then @mikker might be able to see what sort of migration path and configurability would be needed to support this as a first-class citizen?

mikker commented 2 years ago

I'm not sure this is in scope for this gem. I'm deliberately trying to keep this library to a minimum of functionality and handling the email field on the User model seems out of scope. As Passwordless has essentially no control over the User model (contrary to Devise) I don't immediately love adding functionality around handling a field on it.

I like your discussion here and I agree this is a sensible thing to get right, but ultimately feel like it is up to the individual application to work out how their flow should be.

mikker commented 2 years ago

FWIW, in my own app I've made a EmailChangeProposal model that saves the proposed change, then I send an e-mail to the old one. When approved, it changes the User's email and signs them in anew.

duarme commented 2 years ago

Fair enough! Everyone using this gem will eventually have to build something like this, though. How about a wiki entry with a short tutorial on the best practice to accomplish this?

PS: I'd rather not have a 3rd model 🤔