-
Notifications
You must be signed in to change notification settings - Fork 11.6k
[12.x] Add Email::domainIs()
and Email::domainIsNot()
validation rules
#57284
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
@ash-jc-allen I can see this being very useful for business-oriented applications. Syntax wise I think your original proposal is more ergonomic than the alternative. Just wondering, is there a use for having an allowlist and a denylist? Logically, if you have an allowlist, the denylist is completely overruled. Because if not, and just thinking out loud here, maybe it’s an idea to only have a single array holding the domains and a flag for checking allow/deny? This could have some cognitive overhead for maintenance though. |
It seems like a nice intention-revealing rule.
I'm trying to construct an example here. Maybe something along the lines of:
Or maybe:
It'd hinge a bit on whether one of them can override the other. Also, on how ->domainIs() behaves when your config returns an empty array because maybe in that environment nothing is required. |
I like this feature! One thing to note is that currently most of the Email class validation methods have feature parity with string equivalents. It's not immediately clear from your PR summary whether this would allow |
Reminds me a bit of #56580, but maybe you have more luck as your PR has a smaller scope 🤞🏻 |
My two cents, but I'm not a big fan of this addition. We lose parity with string, and in any case, it doesn't allow for the correct email validation, see RFC 5321 (i.e., DNS validation with an MX or A (RFC 5321 §5). record check). I preferred to have an official way to validate an email on Laravel, which performs a DNS validation query to ensure that the email is valid (which is often expected in enterprise for customer qualification). If we want to make this addition, it should be clearly specified that this addition only allows the email to be validated syntactically (RFC 5322) and match the domain and shouldn't be use for customer qualification (like B2B). |
{ | ||
$domain = explode('@', $value)[1]; | ||
|
||
return Str::of($domain)->is($domains, true); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
return Str::of($domain)->is($domains, true); | |
return Str::of($value)->after('@')->is($domains, true); |
@SirMishaa there already exists a way for verifying the dns record of an email address: Edit: I almost always use |
I think I would prefer people just using a closure based custom rule for this. Otherwise I think it opens us up to tons of other rules... think about the URL rule... |
@taylorotwell That's totally cool, and I understand. I'm just glad I tried to make the contribution; I'd have only kicked myself if I didn't try 😄 |
Hey! This PR proposes two new validation rules that can be used to validate the domain of an email address.
At the moment, you can use a combination of the
email
rule and theends_with
/doesnt_end_with
rules to validate that a string is an email and uses (or doesn't use) a given domain name.But with the two methods I'm proposing, you'll also be able to use wildcards so pattern matching can be used easily.
For example, to check the domain ends with
laravel.com
(including any subdomains), oramazing-domain.co.uk
, you could do:And similarly, to validate that an email doesn't end with a given domain, you can use this:
Example Use Cases
I can see this feature being used for things like rejecting registration form submissions if the domain is from a temporary/disposable email provider. Personally, I'd like to keep a config file containing domains that I'd reject. I'd then use it like so:
I can also see it being used for things like only allowing users from your organisation to be invited to a web app.
As I say, I know that this is already pretty much achievable with the existing rules I mentioned above(except for the wildcards), or with a custom rule. So I don't know how valuable this might be to other people. But I thought I'd PR it on the off chance, because I'd only kick myself if I didn't give it a shot. I think it reads pretty nicely and makes it obvious to the reader what the rule is trying to achieve, and it's something I'd personally use.
Alternative approach
Another approach I'd considered rather than the two separate methods was to use a single
domain
method with named arguments like so:And:
I couldn't make up my mind which approach would be better, so I opted for the two separate methods.
This is my first time delving into this part of the framework, so I do apologise if I've done this completely wrong! But if you do think it's something which might be useful for others, then please give me a shout if you'd like me to change anything 😄