Require email presence in SIGN_IN - #238
Conversation
|
@mikker There's a failure about Rails 8.0.0.beta1 and the Ruby version ... I'm not sure how to fix it. Should create a Gemfile for rails 8.0? https://github.com/mikker/passwordless/actions/runs/11238743396/job/31244211533?pr=238 |
|
Thank you for yet another contribution! There's no requirement currently that the passwordless_for field is an email address. So we'll at least need to put this behind an config option and default it to off to not be a breaking change. Coming back to this issue, I'm also not entirely convinced this is something we need. We don't control how users are created and I think this is more of a thing to validate on creation. What exactly is Passwordless' worry if someone inputs something that's not an email address? @yshmarov do you have any thoughts on this? |
|
Thanks for the feedback @mikker You've raised some excellent points I hadn't fully considered. I agree that adding strict email validation would be too restrictive and potentially breaking for many cases. So I could:
For users who do want email validation, we could add an optional configuration: Passwordless.configure do |config|
config.validate_email = false # default to false for backward compatibility
endThis approach would address the original concern of validating emails while maitaining the gem's flexibility and avoid breaking changes. What are your thoughts? |
|
@mikker 👋🏽 |
fixes #129
Our current implementation lacks robust server-side email validation, potentially allowing invalid or empty email addresses to be processed. This can lead to errors like
Net::SMTPSyntaxErrorand a poor user experience.Solution
This PR introduces enhanced email validation in the
Passwordless::SessionsController, specifically in thefind_authenticatablemethod. It adds checks for:URI::MailTo::EMAIL_REGEXPChanges
find_authenticatablemethod to explicitly check for blank emails and validate email formatcreateaction to catch and display appropriate error messagesTest Coverage
Added the following test cases: