fxa(email): Add email normalization check to email libs #19951
Conversation
There was a problem hiding this comment.
Pull request overview
This PR extends the shared email-sender bounce checking to understand and normalize email aliases, aligning its behavior with the auth-server’s alias-aware bounce logic.
Changes:
- Extend
BouncesConfigandBouncetypes to support alias-aware checks (aliasCheckEnabled,emailAliasNormalization, optionalemailonBounce). - Introduce
checkBouncesWithAliasesinBounces, usingEmailNormalizationto query both normalized root and wildcard alias addresses in parallel, and merge/deduplicate results. - Add comprehensive Jest tests in
bounces.spec.tscovering alias checks, threshold behavior, deduplication, and non-configured domain behavior.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| libs/accounts/email-sender/src/bounces.ts | Adds alias-aware bounce checking using EmailNormalization, new config fields, and deduplication of normalized/wildcard query results before applying existing bounce rules. |
| libs/accounts/email-sender/src/bounces.spec.ts | Adds tests for alias-aware scenarios (on/off switch, thresholds, deduplication, and non-configured domains) to validate the new logic and config options. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| // For non-configured domain, both queries should use the original email | ||
| // (no transformation applied) | ||
| expect(db.emailBounces.findByEmail).toHaveBeenCalledTimes(2); | ||
| expect(db.emailBounces.findByEmail).toHaveBeenCalledWith( | ||
| 'test+alias@other.com' |
There was a problem hiding this comment.
This test’s description and inline comment state that for non-configured domains, both bounce queries should use the original email, but the expectations only assert that findByEmail is called twice and that it was called with 'test+alias@other.com' at least once. As written, an implementation that uses the original email once and a transformed value the second time would still pass. To fully verify the intended behavior, consider asserting each call explicitly (for example, using toHaveBeenNthCalledWith for both calls, or checking the argument list of all invocations) so the test will fail if the wildcard query ever starts transforming the address for non-configured domains.
| * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ | ||
|
|
||
| import { AppError } from '@fxa/accounts/errors'; | ||
| import { EmailNormalization } from 'fxa-shared/email/email-normalization'; |
There was a problem hiding this comment.
We should not reference fxa-shared from libs... Can we just move this function here?
dschom
left a comment
dschom
left a comment
There was a problem hiding this comment.
Do we need to actually use this routine in auth-server now to full fill the ticket?
Because
test+123@gmail.com→test@gmail.com), but the shared email-sender library inlibs/accounts/email-sender/does notThis pull request
checkBouncesWithAliasesmethod that queries both the normalized root email and a wildcard alias pattern in parallelBouncesConfigwithaliasCheckEnabledandemailAliasNormalizationconfig fieldsemailfield toBouncetype for deduplication of merged resultsEmailNormalizationclass fromfxa-sharedIssue
Closes: https://mozilla-hub.atlassian.net/browse/FXA-12829
Checklist
Other Information
aliasCheckEnabled: false). No behavior change unless explicitly enabled.fetchEmailBounces_4) already usesLIKE, so wildcard queries (test+%@domain.com) work without DB changes.packages/fxa-auth-server/lib/bounces.jsto the shared lib.