Skip to content

Commit 6f5ce04

Browse files
vbudhramvbudhram
authored
Merge pull request #19951 from mozilla/fxa-12829
fxa(email): Add email normalization check to email libs
2 parents 28e1975 + 0b1274d commit 6f5ce04

4 files changed

Lines changed: 391 additions & 1 deletion

File tree

‎libs/accounts/email-sender/src/bounces.spec.ts‎

Lines changed: 168 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -327,4 +327,172 @@ describe('Bounces', () => {
327327
});
328328
});
329329
});
330+
331+
describe('checkBouncesWithAliases', () => {
332+
const aliasNormalizationConfig = JSON.stringify([
333+
{ domain: 'example.com', regex: '\\+.*', replace: '' },
334+
]);
335+
336+
it('uses regular check when aliasCheckEnabled is false', async () => {
337+
const config: BouncesConfig = {
338+
...defaultBouncesConfig,
339+
aliasCheckEnabled: false,
340+
emailAliasNormalization: aliasNormalizationConfig,
341+
};
342+
const db: BounceDb = {
343+
emailBounces: {
344+
findByEmail: jest.fn().mockResolvedValue([]),
345+
},
346+
};
347+
const bounces = new Bounces(config, db);
348+
349+
await bounces.check('[email protected]', 'verifyEmail');
350+
351+
// Should only call once with the original email
352+
expect(db.emailBounces.findByEmail).toHaveBeenCalledTimes(1);
353+
expect(db.emailBounces.findByEmail).toHaveBeenCalledWith(
354+
'[email protected]'
355+
);
356+
});
357+
358+
it('queries both normalized and wildcard emails when aliasCheckEnabled is true', async () => {
359+
const config: BouncesConfig = {
360+
...defaultBouncesConfig,
361+
aliasCheckEnabled: true,
362+
emailAliasNormalization: aliasNormalizationConfig,
363+
};
364+
const db: BounceDb = {
365+
emailBounces: {
366+
findByEmail: jest.fn().mockResolvedValue([]),
367+
},
368+
};
369+
const bounces = new Bounces(config, db);
370+
371+
await bounces.check('[email protected]', 'verifyEmail');
372+
373+
// Should call twice: once for normalized, once for wildcard
374+
expect(db.emailBounces.findByEmail).toHaveBeenCalledTimes(2);
375+
expect(db.emailBounces.findByEmail).toHaveBeenCalledWith(
376+
'[email protected]'
377+
);
378+
expect(db.emailBounces.findByEmail).toHaveBeenCalledWith(
379+
'test+%@example.com'
380+
);
381+
});
382+
383+
it('throws error when alias bounces exceed threshold', async () => {
384+
const config: BouncesConfig = {
385+
...defaultBouncesConfig,
386+
aliasCheckEnabled: true,
387+
emailAliasNormalization: aliasNormalizationConfig,
388+
hard: { 0: daysInMs(30) },
389+
};
390+
const bouncedAt = mockNow - daysInMs(10);
391+
const db: BounceDb = {
392+
emailBounces: {
393+
findByEmail: jest.fn().mockImplementation((email: string) => {
394+
if (email === '[email protected]') {
395+
return Promise.resolve([
396+
{
397+
email: '[email protected]',
398+
bounceType: BOUNCE_TYPE_HARD,
399+
createdAt: bouncedAt,
400+
},
401+
]);
402+
}
403+
return Promise.resolve([]);
404+
}),
405+
},
406+
};
407+
const bounces = new Bounces(config, db);
408+
409+
// Email with alias should fail because root email has a bounce
410+
await expect(
411+
bounces.check('[email protected]', 'verifyEmail')
412+
).rejects.toMatchObject(AppError.emailBouncedHard(bouncedAt));
413+
});
414+
415+
it('merges and deduplicates bounces from both queries', async () => {
416+
const config: BouncesConfig = {
417+
...defaultBouncesConfig,
418+
aliasCheckEnabled: true,
419+
emailAliasNormalization: aliasNormalizationConfig,
420+
hard: { 2: daysInMs(30) }, // Allow 2 bounces before throwing
421+
};
422+
const bounce1At = mockNow - daysInMs(5);
423+
const bounce2At = mockNow - daysInMs(10);
424+
const duplicateBounceAt = mockNow - daysInMs(15);
425+
426+
const db: BounceDb = {
427+
emailBounces: {
428+
findByEmail: jest.fn().mockImplementation((email: string) => {
429+
if (email === '[email protected]') {
430+
return Promise.resolve([
431+
{
432+
email: '[email protected]',
433+
bounceType: BOUNCE_TYPE_HARD,
434+
createdAt: bounce1At,
435+
},
436+
{
437+
email: '[email protected]',
438+
bounceType: BOUNCE_TYPE_HARD,
439+
createdAt: duplicateBounceAt,
440+
},
441+
]);
442+
}
443+
if (email === 'test+%@example.com') {
444+
return Promise.resolve([
445+
{
446+
email: '[email protected]',
447+
bounceType: BOUNCE_TYPE_HARD,
448+
createdAt: bounce2At,
449+
},
450+
// Duplicate entry (same email and createdAt as normalized query)
451+
{
452+
email: '[email protected]',
453+
bounceType: BOUNCE_TYPE_HARD,
454+
createdAt: duplicateBounceAt,
455+
},
456+
]);
457+
}
458+
return Promise.resolve([]);
459+
}),
460+
},
461+
};
462+
const bounces = new Bounces(config, db);
463+
464+
// Should throw because we have 3 unique hard bounces (one duplicate removed)
465+
await expect(
466+
bounces.check('[email protected]', 'verifyEmail')
467+
).rejects.toMatchObject(AppError.emailBouncedHard(bounce1At));
468+
});
469+
470+
it('does not apply alias normalization for domains not in config', async () => {
471+
const config: BouncesConfig = {
472+
...defaultBouncesConfig,
473+
aliasCheckEnabled: true,
474+
emailAliasNormalization: aliasNormalizationConfig, // Only example.com configured
475+
};
476+
const db: BounceDb = {
477+
emailBounces: {
478+
findByEmail: jest.fn().mockResolvedValue([]),
479+
},
480+
};
481+
const bounces = new Bounces(config, db);
482+
483+
await bounces.check('[email protected]', 'verifyEmail');
484+
485+
// For non-configured domain, both queries should use the original email
486+
// (no transformation applied)
487+
expect(db.emailBounces.findByEmail).toHaveBeenCalledTimes(2);
488+
expect(db.emailBounces.findByEmail).toHaveBeenNthCalledWith(
489+
1,
490+
'[email protected]'
491+
);
492+
expect(db.emailBounces.findByEmail).toHaveBeenNthCalledWith(
493+
2,
494+
'[email protected]'
495+
);
496+
});
497+
});
330498
});

‎libs/accounts/email-sender/src/bounces.ts‎

Lines changed: 63 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,16 +3,20 @@
33
* file, You can obtain one at http://mozilla.org/MPL/2.0/. */
44

55
import { AppError } from '@fxa/accounts/errors';
6+
import { EmailNormalization } from './email-normalization';
67

78
export type BouncesConfig = {
89
enabled: boolean;
10+
aliasCheckEnabled?: boolean;
11+
emailAliasNormalization?: string;
912
hard: Record<number, number>;
1013
soft: Record<number, number>;
1114
complaint: Record<number, number>;
1215
ignoreTemplates: string[];
1316
};
1417

1518
export type Bounce = {
19+
email?: string;
1620
bounceType: number;
1721
createdAt: number;
1822
};
@@ -34,6 +38,7 @@ export const BOUNCE_TYPE_COMPLAINT = 3;
3438

3539
export class Bounces {
3640
private readonly bounceRules: Record<number, any>;
41+
private readonly emailNormalization: EmailNormalization;
3742

3843
constructor(
3944
private readonly config: BouncesConfig,
@@ -44,6 +49,9 @@ export class Bounces {
4449
[BOUNCE_TYPE_SOFT]: Object.freeze(config.soft || {}),
4550
[BOUNCE_TYPE_COMPLAINT]: Object.freeze(config.complaint || {}),
4651
};
52+
this.emailNormalization = new EmailNormalization(
53+
config.emailAliasNormalization || '[]'
54+
);
4755
}
4856

4957
async check(email: string, template: string) {
@@ -58,10 +66,64 @@ export class Bounces {
5866
return undefined;
5967
}
6068

61-
const bounces = await this.db.emailBounces.findByEmail(email);
69+
let bounces: Array<Bounce>;
70+
71+
if (this.config.aliasCheckEnabled) {
72+
bounces = await this.checkBouncesWithAliases(email);
73+
} else {
74+
bounces = await this.db.emailBounces.findByEmail(email);
75+
}
76+
6277
return this.applyRules(bounces);
6378
}
6479

80+
private async checkBouncesWithAliases(email: string): Promise<Array<Bounce>> {
81+
// Given an email alias like [email protected]:
82+
// We look for bounces to the 'root' email -> `[email protected]`
83+
// And look for bounces to the alias with a wildcard -> `test+%@domain.com`
84+
//
85+
// This prevents us from picking up false positives when we replace the alias
86+
// with a wildcard, and doesn't miss the root email bounces either. We have to
87+
// use both because just using the wildcard would miss bounces sent to the root
88+
// and just using the root with a wildcard would pickup false positives.
89+
//
90+
// So, [email protected] would match:
91+
// - [email protected] Covered by normalized email
92+
// - [email protected] Covered by wildcard email
93+
// - [email protected] Covered by wildcard email
94+
// but not
95+
// - [email protected] Not picked up by wildcard since we include the '+'
96+
const normalizedEmail = this.emailNormalization.normalizeEmailAliases(
97+
email,
98+
''
99+
);
100+
const wildcardEmail = this.emailNormalization.normalizeEmailAliases(
101+
email,
102+
'+%'
103+
);
104+
105+
const [normalizedBounces, wildcardBounces] = await Promise.all([
106+
this.db.emailBounces.findByEmail(normalizedEmail),
107+
this.db.emailBounces.findByEmail(wildcardEmail),
108+
]);
109+
110+
// Merge and deduplicate by email+createdAt
111+
// There shouldn't be any overlap, but just in case
112+
const seen = new Set<string>();
113+
const merged = [...normalizedBounces, ...wildcardBounces].filter(
114+
(bounce) => {
115+
const key = `${bounce.email || ''}:${bounce.createdAt}`;
116+
if (seen.has(key)) {
117+
return false;
118+
}
119+
seen.add(key);
120+
return true;
121+
}
122+
);
123+
124+
return merged.sort((a, b) => b.createdAt - a.createdAt);
125+
}
126+
65127
private applyRules(bounces: Array<Bounce>) {
66128
const tallies: Record<number, any> = {
67129
[BOUNCE_TYPE_HARD]: {

‎libs/accounts/email-sender/src/email-normalization.spec.ts‎

Lines changed: 97 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,97 @@
1+
/* This Source Code Form is subject to the terms of the Mozilla Public
2+
* License, v. 2.0. If a copy of the MPL was not distributed with this
3+
* file, You can obtain one at http://mozilla.org/MPL/2.0/. */
4+
5+
import { EmailNormalization } from './email-normalization';
6+
7+
describe('EmailNormalization', () => {
8+
it('trims and lowercases the email before processing', () => {
9+
const emailNormalization = new EmailNormalization(JSON.stringify([]));
10+
11+
const result = emailNormalization.normalizeEmailAliases(
12+
' [email protected] '
13+
);
14+
15+
expect(result).toBe('[email protected]');
16+
});
17+
18+
it('throws on bad json', () => {
19+
expect(() => new EmailNormalization('{')).toThrow(
20+
'emailTransforms must be JSON formatted string'
21+
);
22+
});
23+
24+
it('throws on invalid email: missing @', () => {
25+
const emailNormalization = new EmailNormalization(JSON.stringify([]));
26+
27+
expect(() =>
28+
emailNormalization.normalizeEmailAliases('not-an-email')
29+
).toThrow('Invalid Email!');
30+
});
31+
32+
it('throws on invalid email: too many @', () => {
33+
const emailNormalization = new EmailNormalization(JSON.stringify([]));
34+
35+
expect(() => emailNormalization.normalizeEmailAliases('a@b@c')).toThrow(
36+
'Invalid Email!'
37+
);
38+
});
39+
40+
it('applies transforms only for matching domain', () => {
41+
const transforms = JSON.stringify([
42+
{ domain: 'example.com', regex: '\\.', replace: '' },
43+
{ domain: 'other.com', regex: 'foo', replace: 'bar' },
44+
]);
45+
const emailNormalization = new EmailNormalization(transforms);
46+
47+
const result1 = emailNormalization.normalizeEmailAliases(
48+
'[email protected]'
49+
);
50+
const result2 = emailNormalization.normalizeEmailAliases(
51+
'[email protected]'
52+
);
53+
54+
expect(result1).toBe('[email protected]');
55+
expect(result2).toBe('[email protected]');
56+
});
57+
58+
it('applies multiple transforms for the same domain in order', () => {
59+
const transforms = JSON.stringify([
60+
{ domain: 'example.com', regex: '\\.', replace: '' }, // foo.bar -> foobar
61+
{ domain: 'example.com', regex: '^foo', replace: 'baz' }, // foobar -> bazbar
62+
]);
63+
const emailNormalization = new EmailNormalization(transforms);
64+
65+
const result = emailNormalization.normalizeEmailAliases(
66+
'[email protected]'
67+
);
68+
69+
expect(result).toBe('[email protected]');
70+
});
71+
72+
it('returns the normalized local-part plus the (lowercased) domain', () => {
73+
const transforms = JSON.stringify([
74+
{ domain: 'example.com', regex: '\\+.*', replace: '' },
75+
]);
76+
const emailNormalization = new EmailNormalization(transforms);
77+
78+
const result = emailNormalization.normalizeEmailAliases(
79+
'[email protected]'
80+
);
81+
82+
expect(result).toBe('[email protected]');
83+
});
84+
85+
it('does nothing if there are no matching transforms', () => {
86+
const transforms = JSON.stringify([
87+
{ domain: 'other.com', regex: '\\.', replace: '' },
88+
]);
89+
const emailNormalization = new EmailNormalization(transforms);
90+
91+
const result = emailNormalization.normalizeEmailAliases(
92+
'[email protected]'
93+
);
94+
95+
expect(result).toBe('[email protected]');
96+
});
97+
});

0 commit comments

Comments
 (0)