mikker / passwordless

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

require email presence in SIGN_IN #129

Open yshmarov opened 1 year ago

yshmarov commented 1 year ago

if the "send magic link" form is submitted without an email, there will be an SMTP error.

Screenshot 2022-11-08 at 13 23 35

this is because we do not guard against submitting a nil email.

fchatterji commented 1 year ago

Hey, i'd like to contribute on this issue if possible. Is it still ongoing? I see the first step has already been merged.

mikker commented 1 year ago

Please go ahead 😊 PRs welcome

fchatterji commented 1 year ago

@mikker Ok, I'm not really sure the second step is needed. Usually you would put the validation in the model, and the controller would render the error just fine. But here, the user of the gem creates the user model.

We could add a validation in the controller, render :new and use flash messages to add the error. But it seems overly complex. I think the user of the gem should be responsible for validating that he doesn´t allow empty emails on his own backend.

What do you think?

yshmarov commented 1 year ago

hey @fchatterji ! Having just frontend presence validation is not enough. This way, a user can input an invalid email, successfully submit the form, and get Net::SMTPSyntaxError in passwordless/sessions # create.

I think we can add some validation in find_authenticable to check if email satisfies URI::MailTo::EMAIL_REGEXP...

fchatterji commented 1 year ago

Hey @yshmarov, ok thanks, I see what you mean, here's my PR: https://github.com/mikker/passwordless/pull/134