search

pow-auth / pow

Robust, modular, and extendable user authentication system
https://powauth.com
MIT License
1.58k stars 152 forks source link

Extract `validate_email` and its private functions into a new library? #728

Closed eahanson closed 2 months ago

eahanson commented 3 months ago

I found the Pow.Ecto.Schema.Changeset.validate_email function to be very useful in my app; since my app doesn't use Pow, I just copied the code into my app, but I think that if it were extracted into its own library, it would be pretty helpful to the community.

If you agree and want to make an empty repo for it, I'd be happy to do the extraction. Alternatively, I can extract the code into a new library in my own org and link back to Pow in its readme.

danschultzer commented 3 months ago

Sure, though I think it needs to be fleshed out quite at bit more. I did a cursory look and there is already prior work with an email validator library in hex (written in erlang): https://github.com/rbkmoney/email_validator

I think the ABNF approach is much better as that is what is used in the RFCs.

eahanson commented 2 months ago

I tried out rbkmoney/email_validator and it worked great in my app. I also tried it in Pow and all the tests passed except for two that correctly failed on invalid email addresses but had a slightly different error message for some reason that I didn't look into. I also tried it on a list of ~3000 email addresses from a podcast crawler I wrote and I didn't see any email addresses that were rejected incorrectly.

So thanks for pointing out rbkmoney/email_validator, and I think I'll use it instead of Pow's code. And I'm guessing that it would be safe for Pow to switch to it as its default validator.

danschultzer commented 2 months ago

Great to know, thanks @eahanson! I wanted to do a bit better than the "just send an email and see if it works" approach matching for @, but I suspected a few edge cases wouldn't be covered with my version of an email validator. The ABNF approach makes a lot more sense than the hand-rolled parsing I did. I wish this could be part of OTP or Elixir core. I'll look into switching this in Pow, adding a TODO for now.