Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
163 changes: 163 additions & 0 deletions libs/accounts/email-sender/src/bounces.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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'
Comment on lines +485 to +489
Copy link

Copilot AI Jan 27, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copilot uses AI. Check for mistakes.
);
});
});
});
64 changes: 63 additions & 1 deletion libs/accounts/email-sender/src/bounces.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,16 +3,20 @@
* 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';
Copy link
Contributor

@dschom dschom Jan 28, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should not reference fxa-shared from libs... Can we just move this function here?


export type BouncesConfig = {
enabled: boolean;
aliasCheckEnabled?: boolean;
emailAliasNormalization?: string;
hard: Record<number, number>;
soft: Record<number, number>;
complaint: Record<number, number>;
ignoreTemplates: string[];
};

export type Bounce = {
email?: string;
bounceType: number;
createdAt: number;
};
Expand All @@ -34,6 +38,7 @@ export const BOUNCE_TYPE_COMPLAINT = 3;

export class Bounces {
private readonly bounceRules: Record<number, any>;
private readonly emailNormalization: EmailNormalization;

constructor(
private readonly config: BouncesConfig,
Expand All @@ -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) {
Expand All @@ -58,10 +66,64 @@ export class Bounces {
return undefined;
}

const bounces = await this.db.emailBounces.findByEmail(email);
let bounces: Array<Bounce>;

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<Array<Bounce>> {
// 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<string>();
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<Bounce>) {
const tallies: Record<number, any> = {
[BOUNCE_TYPE_HARD]: {
Expand Down
Loading