Accept internationalized emails. Fix uppercase and anchor bugs in email auth. - #4695
Accept internationalized emails. Fix uppercase and anchor bugs in email auth.#4695FranjoMindek wants to merge 22 commits into
Conversation
GitHub Actions
Inactive
GitHub Actions
Inactive
@wasp.sh/spec
@wasp.sh/wasp-cli
@wasp.sh/wasp-cli-darwin-arm64-unknown
@wasp.sh/wasp-cli-darwin-x64-unknown
@wasp.sh/wasp-cli-linux-arm64-glibc
@wasp.sh/wasp-cli-linux-x64-glibc
@wasp.sh/wasp-cli-linux-x64-musl
commit: |
GitHub Actions
Inactive
GitHub Actions
Inactive
![pullfrog[bot]](https://avatars.githubusercontent.com/in/1768019?s=60&v=4){kind=small}
There was a problem hiding this comment.
Caution
The new Unicode acceptance can route password-reset tokens to a different mailbox after provider-ID lowercasing; this must be fixed before merge.
Reviewed changes Reviewed the shared email validator, generated client and server integration, auth-flow regression tests, and regenerated golden outputs.
- Shared validation — Moves email syntax validation into the isomorphic auth library, adds Unicode and uppercase support, anchors the grammar, and enforces octet limits.
- Generated auth forms — Reuses the shared validator in login, signup, and forgot-password forms while replacing the browser's ASCII-only
type="email"validation. - Regression coverage — Adds validator unit cases and kitchen-sink signup, verification, login, and malformed-input e2e scenarios.
- Generated outputs — Updates conditional auth code generation, snapshots, checksums, and the changelog.
GPT Sol ｜ 𝕏
GitHub Actions
Inactive
GitHub Actions
Failure
Deploying wasp-docs-on-main with
|
| Latest commit: |
8fc0d8e
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://fad4ce01.wasp-docs-on-main.pages.dev |
| Branch Preview URL: | https://fix-email-validation-unicode.wasp-docs-on-main.pages.dev |
GitHub Actions
Inactive
GitHub Actions
Inactive
GitHub Actions
Inactive
GitHub Actions
Inactive
…unicode-and-anchoring # Conflicts: # waspc/ChangeLog.md
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
GitHub Actions
Inactive
FranjoMindek
changed the title
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
GitHub Actions
Inactive
There was a problem hiding this comment.
✅ No new issues found.
Reviewed changes Reviewed the validator type-contract correction and generated checksum refresh since the previous Pullfrog pass.
- Restored the boolean contract — Changed
isValidEmailfrom an unsoundinput is stringpredicate back toboolean, preserving correct false-branch typing for invalid strings. - Refreshed generated package metadata — Updated the auth-package checksums in all affected golden outputs to match the declaration change.
GPT Sol ｜ 𝕏
GitHub Actions
Inactive
| <FormItemGroup> | ||
| <FormLabel>E-mail</FormLabel> | ||
| <FormInput type="email" {...register("email")} /> | ||
| <FormInput type="text" inputMode="email" {...register("email")} /> |
There was a problem hiding this comment.
But type=email exists, what am I missing here? https://developer.mozilla.org/en-US/docs/Web/HTML/Reference/Elements/input/email
There was a problem hiding this comment.
type=email only supports HTML5 emails, which means no unicode.
If we use it, we can't support unicode.
There was a problem hiding this comment.
Edited description to better clarify, there is a line about it in the docs but it was disconnected from this change.
There was a problem hiding this comment.
We should comment on thi sin the code, so it is clear why this was done! The good ol' "Don't answer the reviewer, fix it for every next person who will hvae the same questoin".
There was a problem hiding this comment.
The auth ui one has the comment.
/**
* Deliberately avoids setting `type="email"`.
* Browsers validate email input against the HTML5 grammar, which is ASCII-only
* and would reject internationalized addresses that the server accepts.
* `inputMode` keeps the email keyboard on mobile.
*
* @see {@link https://github.com/whatwg/html/issues/4562 WHATWG international email addresses issue}
*/
export const emailInputProps = {
type: 'text',
inputMode: 'email',
autoComplete: 'email',
} as constSadly this isn't exported so we can't re-use it.
Wasp docs do mention this quirk to users.
Didn't think we would want to also add it to custom signup in kitchen-sink.
| // look at https://github.com/JoshData/python-email-validator for inspiration. | ||
|
|
||
| /** | ||
| * The syntax we accept is the HTML5 `input[type=email]` grammar, |
There was a problem hiding this comment.
This reads to me like a signal that we might want to follow HTML5's lead and accept only ASCII emails? We don't have to be better than HTML5 standard?
There was a problem hiding this comment.
There is an open issue about adding unicode email support to HTML though.
Email providers support unicode, browsers are just lagging behind.
We also had users encounter this problem. So it is an real issue.
There was a problem hiding this comment.
I had same toughts as Miho, why do we have to be better than html5's standard.
Do other auth solutions allow unicode? If they do, then ok let's do it.
| @@ -1,3 +1 @@ | |||
| /** | |||
| * NOTE: this is a placeholder file for future code exported for both runtimes. | |||
|
I feel like we are introducing quite a bit of complexity in our codebase for edge case emails that even the HTML5 input form rejects. That's a pretty strong signal that we are out of the 80/20 territory. I'm down with cleaning up the e-mail validation (anchoring, lowercasing etc.) and I think this is long overdue. But I'm more in favor of going for a simpler built-in validation and working on allowing users to override our validation rules for their specific case. What do you think about that? |
Not supporting unicode by default is an actual issue our users encountered. Supprting better auth UI and server-side validation should also come, but it shouldn't be a bandaid to bad defaults. |
|
Ok fair, if that's the direction the browsers are heading - we should too. I see that Laravel and Django support unicode e-mails in their validators, so that's another good signal. I'll review with that in mind one more time. |
| Even though Wasp offers premade [Auth UI](../ui.md) for your authentication flows, there are times when you might want more customization, so we also give you the option to create your own UI and call Wasp's auth actions from your own code, similar to how Auth UI does it under the hood. | ||
|
|
||
| :::note Handling the internationalized email input | ||
| We use `type="text"` with `inputMode=email` because it allows unicode/international characters, while `type=email` only allows ASCII / english letters. |
There was a problem hiding this comment.
todo: fix typo, first / has no spaces
There was a problem hiding this comment.
Also keep unicode with capitcal U to be consistent, so Unicode?
There was a problem hiding this comment.
Ok so I guess if they use type=email, no big deal, they are just limiting input field more than our validator, right?
Btw you use double quotes around text but not around email.
The fact a user reporeted an issue is not argument per se, in this case. Quesoitn is, what is normal these days? If others support it (other farmeworks / auth libraries / pages) then let's do it, otherwise we odn't have to. |
Personally, that seems like a somewhat restricted view. But anyways, it's a mixed bag:
I tried to keep the language simple (instead of saying RFC yada yada). Some trends I can see:
I was hesitant about introducing another package as client bundle. Weirdly, some support unicode server side but users can't use it because the client ships All in all, I would either:
|
Martinsos
left a comment
Martinsos
left a comment
There was a problem hiding this comment.
@FranjoMindek check the comments but in genreal LGTM!
I guess main questoin is: is it worth it, and should we use external library instead. I don't think answer is super clear, but since we alrady were doing validatoin ourselves, and this seems to be strictly better, I say let's go for it and we can later consider switcihng to library if we feel there is a need.
| <FormItemGroup> | ||
| <FormLabel>E-mail</FormLabel> | ||
| <FormInput type="email" {...register("email")} /> | ||
| <FormInput type="text" inputMode="email" {...register("email")} /> |
There was a problem hiding this comment.
Aha this is example app and this is "custom signup code".
I am interested, if they are doing this on their own, writing custom signup page, how likely are they to know to use type text an inputMode email (or their AI)?
Do they also have to know to do the trimming? WHy didn't we also set autocomplete field here?
| @@ -0,0 +1,63 @@ | |||
| // TODO: If we ever need a more quality email validator, its worth to | |||
There was a problem hiding this comment.
Doesn't sound very encouraging in an auth library :D.
Maybe say that if we ever want to tighten up the email validation fuirther, we can check this one.
But could you also provide a bit of context why would we want to? I think you know now, but later somebody is reading this and has no idea if this TODO is somethign we should do soon but didn't have time, or likely won't ever wwant to do because it is overengineering, ... .
| * the possibly dangerous format characters. What still gets through is | ||
| * handled separately later. |
There was a problem hiding this comment.
What does it mean "what still gets through is handled separately later"? What can get through and where it handles separately later and why?
| * | ||
| * @see {@link https://github.com/whatwg/html/issues/4562 WHATWG international email addresses issue} | ||
| */ | ||
| const HTML5_EMAIL_WITH_UNICODE_REGEX = |
There was a problem hiding this comment.
Hah uf damn :D. What a regex :D. Do we have a good test suite for it? I have to yet see it but I hope we do.
There is a lot of repetition for this \p{L}\p{M}\p{Nd}, maybe extract that to a a named var and reuse it so we both understand wht it is + make regex easier to read?
|
|
||
| /** | ||
| * Characters that render as nothing, so that two addresses spelled | ||
| * differently look identical on screen. |
There was a problem hiding this comment.
I get that its characters that render as nothing, but why the remark "so that two addresses spelled differently look identical on screen" -> "so" indicates some kind of cause or something, but I don't get it. Maybe best to just drop this second part of the commend because it seems to be talking about usage of this regex and don't see why you would address that here.
| * Deliberately avoids setting `type="email"`. | ||
| * Browsers validate email input against the HTML5 grammar, which is ASCII-only | ||
| * and would reject internationalized addresses that the server accepts. | ||
| * `inputMode` keeps the email keyboard on mobile. |
There was a problem hiding this comment.
Main thing I dind't like is that your first sentence and the rest are somewhat disconnected. The fact that type="email" is the one that says use HTML5 grammar, you never say that. and I personally dind't know taht before this PR. Plus it is just hard to read even when you know about it.
| * Deliberately avoids setting `type="email"`. | |
| * Browsers validate email input against the HTML5 grammar, which is ASCII-only | |
| * and would reject internationalized addresses that the server accepts. | |
| * `inputMode` keeps the email keyboard on mobile. | |
| * We deliberately don't use the usual `type="email"` here because | |
| * that one validates against HTML5 grammar which is stricter than our | |
| * email validation rules (we additionally allow unicode chars). | |
| * | |
| * We relax `type` to `"text"`, but then set `inputMode` and `autoComplete` | |
| * to `"email"` to compensate and keep the right experience (e.g. on mobile). |
|
|
||
| export const emailFieldRules = { | ||
| required: 'Email is required', | ||
| // `type="email"` used to strip surrounding whitespace for us, `type="text"` doesn't. |
There was a problem hiding this comment.
Sounds like comment ad dressing old code that is weird. I don't thikn you need any kind of comment here, it is clear you are trimming because it needs trimming.
| Even though Wasp offers premade [Auth UI](../ui.md) for your authentication flows, there are times when you might want more customization, so we also give you the option to create your own UI and call Wasp's auth actions from your own code, similar to how Auth UI does it under the hood. | ||
|
|
||
| :::note Handling the internationalized email input | ||
| We use `type="text"` with `inputMode=email` because it allows unicode/international characters, while `type=email` only allows ASCII / english letters. |
| Even though Wasp offers premade [Auth UI](../ui.md) for your authentication flows, there are times when you might want more customization, so we also give you the option to create your own UI and call Wasp's auth actions from your own code, similar to how Auth UI does it under the hood. | ||
|
|
||
| :::note Handling the internationalized email input | ||
| We use `type="text"` with `inputMode=email` because it allows unicode/international characters, while `type=email` only allows ASCII / english letters. |
There was a problem hiding this comment.
Ok so I guess if they use type=email, no big deal, they are just limiting input field more than our validator, right?
Btw you use double quotes around text but not around email.
| Because Wasp supports Unicode email addresses, browsers would reject their syntax with `input[type=email]`. | ||
| If you are building your own form, please use `type="text"` with `inputMode="email"` instead. | ||
| Keep in mind that `type="text"` doesn't strip surrounding whitespace the way `type="email"` does, so trim the address before you send it. |
There was a problem hiding this comment.
This is again somewhat hard to read for me.
Maybe do something like:
Since Wasp's definition of valid email addresses is wider than the HTML5 grammar (we also support unicode!), you will want to not use the usual `type="email"` attribute on `input` element (which follows HTML5 grammar and doesnt accept unicode) but instead use ...4 participants
Fixes #1392.
Started as "Accept internationalized emails" PR, but fixes some bugs along the way.
Description
user@example.complainaddressuser@münchen.dejürgen@example.comFRANJO@gmail.comfranjo@Gmail.coma@b.com <script>alert(1)</script>I am not an email, ask a@b.com<65 chars>@example.comFour problems:
No Unicode support. E.g.
user@münchen.deis rejected.The regex was never anchored.
input.match(validEmailRegex)with no^/$, so any string containing something address-shaped passed. This is also whyjürgen@example.comappeared to work: the substringrgen@example.commatched. Anchoring the old regex and re-running rejects it, confirming the pass was accidental.The regex was lowercase-only. Every character class was
[a-z0-9...]. Validation runs onreq.bodyatsignup.ts:47, while lowercasing only happens increateProviderIdatsignup.ts:49, so the validator sees whatever casing the user typed.Defect 2 partly masks this one, which is why it is easy to miss: since the regex is unanchored, an address is accepted as long as some substring of it matches. In practice an address passes only if its domain is entirely lowercase and the run of characters immediately before the
@is lowercase. Measured againstmain:mainacceptsFRANJO.mindek@gmail.commindek@gmail.comFRANJO.Mindek@gmail.comindek@gmail.comFRANJO@gmail.comFRANJO.M@gmail.comfranjo@Gmail.comfranjo@gmail.COMNo length limits. A local part of any length was accepted.
What this PR does
isValidEmailis moved into@wasp.sh/lib-auth, so the client form and the server run the same check.The new grammar is the HTML5
input[type=email]grammar widened to accept Unicode letters, marks and digits, plus the RFC 5321 size limits. It is anchored and case-insensitive.The auth form drops
type="email"in favour oftype="text"+inputMode="email", and validates viareact-hook-formwith the sameisValidEmail. This is becausetype="email"forces HTML5 validation, which does not allow unicode. By using correctinputModewe still keep the sam UX for mobile users (email keyboard) while providing better email support.Why this shape
Rejecting one locks a user out of the app entirely, whereas accepting an undeliverable one just means no verification email arrives.
Type of change
Checklist
I tested my change in a Wasp app to verify that it works as intended.
Built
examples/kitchen-sink(email auth) and a scratch app with username-only auth, to check both branches of the new conditional codegen compile. Ran the kitchen-sink Playwright suite inbuildmode (44/44 passing), which exercises signup withjürgen-<uuid>@münchen.testend to end: form submit → server validation → verification email delivered through SMTP to Mailcrab → verification link → login.🧪 Tests and apps:
I added unit tests for my change.
(if you fixed a bug) I added a regression test for the bug I fixed.
waspc/data/Generator/libs/auth/tests/email.test.ts, 45 tests, with one group per defect:internationalized addresses,case,anchoring,size limits. Thecasegroup covers uppercase confined to the domain and uppercase confined to the local part separately, since defect 3 manifests differently in each.(if you added/updated a feature) I added/updated e2e tests in
examples/kitchen-sink/e2e-tests.(if you added/updated a feature) I updated the starter templates in
waspc/data/Cli/templates, as needed.(if you added/updated a feature) I updated the example apps in
examples/, as needed.examples/tutorials) I updated the tutorial in the docs (and vice versa).📜 Documentation:
(if you added/updated a feature) I added/updated the documentation in
web/docs/.The docs don't describe the email validation rules, so there was nothing to update.
🆕 Changelog: (if change is more than just code/docs improvement)
I updated
waspc/ChangeLog.mdwith a user-friendly description of the change.(if you did a breaking change) I added a step to the current migration guide in
web/docs/migration-guides/.I bumped the
versioninwaspc/waspc.cabalto reflect the changes I introduced.Already at
0.26.0, which is unreleased.