diff --git a/libs/accounts/email-sender/src/bounces.spec.ts b/libs/accounts/email-sender/src/bounces.spec.ts index 7a7e04514ab..5bf329c326f 100644 --- a/libs/accounts/email-sender/src/bounces.spec.ts +++ b/libs/accounts/email-sender/src/bounces.spec.ts @@ -327,4 +327,167 @@ describe('Bounces', () => { }); }); }); + + describe('checkBouncesWithAliases', () => { + const aliasNormalizationConfig = JSON.stringify([ + { domain: 'example.com', regex: '\\+.*', replace: '' }, + ]); + + it('uses regular check when aliasCheckEnabled is false', async () => { + const config: BouncesConfig = { + ...defaultBouncesConfig, + aliasCheckEnabled: false, + emailAliasNormalization: aliasNormalizationConfig, + }; + const db: BounceDb = { + emailBounces: { + findByEmail: jest.fn().mockResolvedValue([]), + }, + }; + const bounces = new Bounces(config, db); + + await bounces.check('test+alias@example.com', 'verifyEmail'); + + // Should only call once with the original email + expect(db.emailBounces.findByEmail).toHaveBeenCalledTimes(1); + expect(db.emailBounces.findByEmail).toHaveBeenCalledWith( + 'test+alias@example.com' + ); + }); + + it('queries both normalized and wildcard emails when aliasCheckEnabled is true', async () => { + const config: BouncesConfig = { + ...defaultBouncesConfig, + aliasCheckEnabled: true, + emailAliasNormalization: aliasNormalizationConfig, + }; + const db: BounceDb = { + emailBounces: { + findByEmail: jest.fn().mockResolvedValue([]), + }, + }; + const bounces = new Bounces(config, db); + + await bounces.check('test+alias@example.com', 'verifyEmail'); + + // Should call twice: once for normalized, once for wildcard + expect(db.emailBounces.findByEmail).toHaveBeenCalledTimes(2); + expect(db.emailBounces.findByEmail).toHaveBeenCalledWith( + 'test@example.com' + ); + expect(db.emailBounces.findByEmail).toHaveBeenCalledWith( + 'test+%@example.com' + ); + }); + + it('throws error when alias bounces exceed threshold', async () => { + const config: BouncesConfig = { + ...defaultBouncesConfig, + aliasCheckEnabled: true, + emailAliasNormalization: aliasNormalizationConfig, + hard: { 0: daysInMs(30) }, + }; + const bouncedAt = mockNow - daysInMs(10); + const db: BounceDb = { + emailBounces: { + findByEmail: jest.fn().mockImplementation((email: string) => { + if (email === 'test@example.com') { + return Promise.resolve([ + { + email: 'test@example.com', + bounceType: BOUNCE_TYPE_HARD, + createdAt: bouncedAt, + }, + ]); + } + return Promise.resolve([]); + }), + }, + }; + const bounces = new Bounces(config, db); + + // Email with alias should fail because root email has a bounce + await expect( + bounces.check('test+alias@example.com', 'verifyEmail') + ).rejects.toMatchObject(AppError.emailBouncedHard(bouncedAt)); + }); + + it('merges and deduplicates bounces from both queries', async () => { + const config: BouncesConfig = { + ...defaultBouncesConfig, + aliasCheckEnabled: true, + emailAliasNormalization: aliasNormalizationConfig, + hard: { 2: daysInMs(30) }, // Allow 2 bounces before throwing + }; + const bounce1At = mockNow - daysInMs(5); + const bounce2At = mockNow - daysInMs(10); + const duplicateBounceAt = mockNow - daysInMs(15); + + const db: BounceDb = { + emailBounces: { + findByEmail: jest.fn().mockImplementation((email: string) => { + if (email === 'test@example.com') { + return Promise.resolve([ + { + email: 'test@example.com', + bounceType: BOUNCE_TYPE_HARD, + createdAt: bounce1At, + }, + { + email: 'test@example.com', + bounceType: BOUNCE_TYPE_HARD, + createdAt: duplicateBounceAt, + }, + ]); + } + if (email === 'test+%@example.com') { + return Promise.resolve([ + { + email: 'test+alias@example.com', + bounceType: BOUNCE_TYPE_HARD, + createdAt: bounce2At, + }, + // Duplicate entry (same email and createdAt as normalized query) + { + email: 'test@example.com', + bounceType: BOUNCE_TYPE_HARD, + createdAt: duplicateBounceAt, + }, + ]); + } + return Promise.resolve([]); + }), + }, + }; + const bounces = new Bounces(config, db); + + // Should throw because we have 3 unique hard bounces (one duplicate removed) + await expect( + bounces.check('test+alias@example.com', 'verifyEmail') + ).rejects.toMatchObject(AppError.emailBouncedHard(bounce1At)); + }); + + it('does not apply alias normalization for domains not in config', async () => { + const config: BouncesConfig = { + ...defaultBouncesConfig, + aliasCheckEnabled: true, + emailAliasNormalization: aliasNormalizationConfig, // Only example.com configured + }; + const db: BounceDb = { + emailBounces: { + findByEmail: jest.fn().mockResolvedValue([]), + }, + }; + const bounces = new Bounces(config, db); + + await bounces.check('test+alias@other.com', 'verifyEmail'); + + // 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' + ); + }); + }); }); diff --git a/libs/accounts/email-sender/src/bounces.ts b/libs/accounts/email-sender/src/bounces.ts index d54fea90bad..b66fb69531c 100644 --- a/libs/accounts/email-sender/src/bounces.ts +++ b/libs/accounts/email-sender/src/bounces.ts @@ -3,9 +3,12 @@ * 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'; export type BouncesConfig = { enabled: boolean; + aliasCheckEnabled?: boolean; + emailAliasNormalization?: string; hard: Record; soft: Record; complaint: Record; @@ -13,6 +16,7 @@ export type BouncesConfig = { }; export type Bounce = { + email?: string; bounceType: number; createdAt: number; }; @@ -34,6 +38,7 @@ export const BOUNCE_TYPE_COMPLAINT = 3; export class Bounces { private readonly bounceRules: Record; + private readonly emailNormalization: EmailNormalization; constructor( private readonly config: BouncesConfig, @@ -44,6 +49,9 @@ export class Bounces { [BOUNCE_TYPE_SOFT]: Object.freeze(config.soft || {}), [BOUNCE_TYPE_COMPLAINT]: Object.freeze(config.complaint || {}), }; + this.emailNormalization = new EmailNormalization( + config.emailAliasNormalization || '[]' + ); } async check(email: string, template: string) { @@ -58,10 +66,64 @@ export class Bounces { return undefined; } - const bounces = await this.db.emailBounces.findByEmail(email); + let bounces: Array; + + if (this.config.aliasCheckEnabled) { + bounces = await this.checkBouncesWithAliases(email); + } else { + bounces = await this.db.emailBounces.findByEmail(email); + } + return this.applyRules(bounces); } + private async checkBouncesWithAliases(email: string): Promise> { + // Given an email alias like test+123@domain.com: + // We look for bounces to the 'root' email -> `test@domain.com` + // And look for bounces to the alias with a wildcard -> `test+%@domain.com` + // + // This prevents us from picking up false positives when we replace the alias + // with a wildcard, and doesn't miss the root email bounces either. We have to + // use both because just using the wildcard would miss bounces sent to the root + // and just using the root with a wildcard would pickup false positives. + // + // So, test+123@domain.com would match: + // - test@domain.com Covered by normalized email + // - test+123@domain.com Covered by wildcard email + // - test+asdf@domain.com Covered by wildcard email + // but not + // - testing@domain.com Not picked up by wildcard since we include the '+' + const normalizedEmail = this.emailNormalization.normalizeEmailAliases( + email, + '' + ); + const wildcardEmail = this.emailNormalization.normalizeEmailAliases( + email, + '+%' + ); + + const [normalizedBounces, wildcardBounces] = await Promise.all([ + this.db.emailBounces.findByEmail(normalizedEmail), + this.db.emailBounces.findByEmail(wildcardEmail), + ]); + + // Merge and deduplicate by email+createdAt + // There shouldn't be any overlap, but just in case + const seen = new Set(); + const merged = [...normalizedBounces, ...wildcardBounces].filter( + (bounce) => { + const key = `${bounce.email || ''}:${bounce.createdAt}`; + if (seen.has(key)) { + return false; + } + seen.add(key); + return true; + } + ); + + return merged.sort((a, b) => b.createdAt - a.createdAt); + } + private applyRules(bounces: Array) { const tallies: Record = { [BOUNCE_TYPE_HARD]: {