Skip to content

Commit e5d59c3

Browse files
authored
Merge pull request #19850 from mozilla/normalize-email-bounce-addrs-2
task(auth): Normalize email bounce lookup
2 parents 7103b79 + 5b1b3eb commit e5d59c3

16 files changed

Lines changed: 386 additions & 93 deletions

File tree

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,25 @@
1+
SET NAMES utf8mb4 COLLATE utf8mb4_bin;
2+
3+
CALL assertPatchLevel('180');
4+
5+
CREATE PROCEDURE `fetchEmailBounces_4` (
6+
IN `inEmail` VARCHAR(255)
7+
)
8+
BEGIN
9+
SELECT
10+
e.email,
11+
t.emailType,
12+
e.bounceType,
13+
e.bounceSubType,
14+
e.createdAt,
15+
e.diagnosticCode
16+
FROM emailBounces e
17+
JOIN emailTypes t
18+
ON t.id = e.emailTypeId
19+
WHERE
20+
e.email LIKE inEmail
21+
ORDER BY createdAt DESC
22+
LIMIT 20;
23+
END;
24+
25+
UPDATE dbMetadata SET value = '181' WHERE name = 'schema-patch-level';
Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
-- -- Drop new column and new constraint
2+
3+
-- -- Drop new stored procedures
4+
-- DROP PROCEDURE `fetchEmailBounces_4`;
5+
6+
-- -- Decrement the schema version
7+
-- UPDATE dbMetadata SET value = '180' WHERE name = 'schema-patch-level';
Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,3 @@
11
{
2-
"level": 180
2+
"level": 181
33
}

packages/functional-tests/lib/email.ts

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
* file, You can obtain one at http://mozilla.org/MPL/2.0/. */
44

55
import got from 'got';
6+
import mysql from 'mysql2/promise';
67

78
function wait() {
89
return new Promise((r) => setTimeout(r, 50));
@@ -253,4 +254,40 @@ export class EmailClient {
253254
await this.clear(email);
254255
return code;
255256
}
257+
258+
/** Creates a bounce record. Note, this only works on localhost. For stage / prod, we expect we can generate a real bounce. */
259+
async createBounce(
260+
email: string,
261+
bounceType = 1,
262+
bounceSubType = 1,
263+
emailTypeId = 39,
264+
diagnosticCode = ''
265+
): Promise<void> {
266+
try {
267+
// create the connection to database
268+
const connection = await mysql.createConnection({
269+
host: 'localhost',
270+
user: 'root',
271+
database: 'fxa',
272+
// Since this is local, wait up to a second and fail other wise.
273+
connectTimeout: 1000,
274+
});
275+
276+
// execute will internally call prepare and query
277+
connection.execute(
278+
`INSERT INTO emailBounces (email, bounceType, bounceSubType, createdAt, emailTypeId, diagnosticCode)
279+
VALUES (?, ?, ?, ?, ?, ?);`,
280+
[
281+
email,
282+
bounceType,
283+
bounceSubType,
284+
Date.now(),
285+
emailTypeId,
286+
diagnosticCode,
287+
]
288+
);
289+
} catch (err) {
290+
console.log('Could not create bounce record in local db!');
291+
}
292+
}
256293
}

packages/functional-tests/lib/testAccountTracker.ts

Lines changed: 18 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ import { SessionStatus } from 'fxa-auth-client/lib/client';
1717
enum EmailPrefix {
1818
BLOCKED = 'blocked',
1919
BOUNCED = 'bounced',
20+
BOUNCED_ALIAS = 'bounced+',
2021
FORCED_PWD_CHANGE = 'forcepwdchange',
2122
SIGNIN = 'signin',
2223
SIGNUP = 'signup',
@@ -85,9 +86,12 @@ export class TestAccountTracker {
8586
* @param prefix email prefix to use when generating email
8687
* @returns email
8788
*/
88-
generateEmail(prefix: EmailPrefix = EmailPrefix.SIGNIN): string {
89+
generateEmail(
90+
prefix: EmailPrefix = EmailPrefix.SIGNIN,
91+
domain = 'restmail.net'
92+
): string {
8993
const id = crypto.randomBytes(8).toString('hex');
90-
return `${prefix}${id}@restmail.net`;
94+
return `${prefix}${id}@${domain}`;
9195
}
9296

9397
/**
@@ -107,6 +111,15 @@ export class TestAccountTracker {
107111
return this.generateAccountDetails(EmailPrefix.BOUNCED);
108112
}
109113

114+
/**
115+
* Creates a new email address with the 'bounced' prefix and a new randomized
116+
* password
117+
* @returns AccountDetails
118+
*/
119+
generateBouncedAliasAccountDetails(domain: string): AccountDetails {
120+
return this.generateAccountDetails(EmailPrefix.BOUNCED_ALIAS, domain);
121+
}
122+
110123
/**
111124
* Creates a new email address with the 'signup' prefix and a new
112125
* randomized password
@@ -141,10 +154,11 @@ export class TestAccountTracker {
141154
* @returns AccountDetails
142155
*/
143156
generateAccountDetails(
144-
prefix: EmailPrefix = EmailPrefix.SIGNIN
157+
prefix: EmailPrefix = EmailPrefix.SIGNIN,
158+
domain = 'restmail.net'
145159
): AccountDetails {
146160
const account = {
147-
email: this.generateEmail(prefix),
161+
email: this.generateEmail(prefix, domain),
148162
password: this.generatePassword(),
149163
};
150164
this.accounts.push(account);

packages/functional-tests/pages/signin.ts

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -86,6 +86,12 @@ export class SigninPage extends BaseLayout {
8686
});
8787
}
8888

89+
get emailReturnedWarning() {
90+
return this.page.getByText(
91+
'Your confirmation email was just returned. Mistyped email?'
92+
);
93+
}
94+
8995
// for backwards compatibility with Backbone
9096
// not currently implemented in React, see FXA-8827
9197
get permissionsHeading() {
@@ -152,10 +158,7 @@ export class SigninPage extends BaseLayout {
152158
* assertions up to the caller.
153159
* @param Credentials
154160
*/
155-
async emailFirstSignin({
156-
email,
157-
password,
158-
}: Credentials) {
161+
async emailFirstSignin({ email, password }: Credentials) {
159162
if (this.page.url() !== this.url) {
160163
// avoid navigating if we don't need to.
161164
// Checking url is faster than an extra navigation

packages/functional-tests/tests/signin/signinBlocked.spec.ts

Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,54 @@ test.describe('severity-2 #smoke', () => {
4040
await removeAccount(settings, deleteAccount, page, credentials.password);
4141
});
4242

43+
test('email bounced', async ({
44+
target,
45+
page,
46+
pages: {
47+
deleteAccount,
48+
secondaryEmail,
49+
settings,
50+
signin,
51+
signup,
52+
signinUnblock,
53+
},
54+
testAccountTracker,
55+
}) => {
56+
// We might have to wait a bit for the bounce to get picked up. So
57+
// we will flag this as a slow test. If on local, we manually create
58+
// a hard bounce record. On local, this will insert a new bounce
59+
// record. On stage prod, this will silently fail, and we will rely
60+
// on the email to actually bounce.
61+
if (target.name === 'local') {
62+
await target.emailClient.createBounce('[email protected]');
63+
} else {
64+
test.slow();
65+
}
66+
67+
// Here, we check that an email alias ie `bounced+123` will trip a bounce
68+
// check when the bounce record is based on the non-aliased email form.
69+
const credentials =
70+
testAccountTracker.generateBouncedAliasAccountDetails('mozilla.com');
71+
72+
// Start the sign up process
73+
await page.goto(target.contentServerUrl);
74+
await signin.fillOutEmailFirstForm(credentials.email);
75+
await signup.fillOutSignupForm(credentials.password);
76+
77+
// The service that polls for bounces does so on approximately a 5 second interval.
78+
// Add a little timeout to make sure that catches the bounce, and the polling
79+
// mechanism can have a chance to redirect.
80+
if (target.name === 'local') {
81+
await page.waitForTimeout(1000);
82+
} else {
83+
await page.waitForTimeout(9000);
84+
}
85+
86+
// This indicates the email bounced. The front end starts polling for bounces,
87+
// it should etecte one, and then kick the user back to the sign in page.
88+
await expect(signin.emailReturnedWarning).toBeVisible();
89+
});
90+
4391
test('incorrect code entered', async ({
4492
target,
4593
page,

packages/fxa-auth-server/config/index.ts

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -612,6 +612,14 @@ const convictConf = convict({
612612
default: true,
613613
env: 'BOUNCES_DELETE_ACCOUNT',
614614
},
615+
emailAliasNormalization: {
616+
default: JSON.stringify([
617+
{ domain: 'mozilla.com', regex: '\\+.*', replace: '%' },
618+
]),
619+
doc: 'List of email domain configurations for alias normalization. Each entry should have domain, regex, and replace properties. Example: [{domain: "mozilla.com", regex: "\\+[^@]+" }].',
620+
env: 'BOUNCES_EMAIL_ALIAS_NORMALIZATION',
621+
format: String,
622+
},
615623
},
616624
connectionTimeout: {
617625
doc: 'Milliseconds to wait for the connection to establish (default is 2 minutes)',
@@ -2318,7 +2326,9 @@ const convictConf = convict({
23182326
format: Array,
23192327
},
23202328
emailAliasNormalization: {
2321-
default: '',
2329+
default: JSON.stringify([
2330+
{ domain: 'mozilla.com', regex: '\\+.*', replace: '' },
2331+
]),
23222332
doc: 'List of email domain configurations for alias normalization. Each entry should have domain, regex, and replace properties. Example: [{domain: "mozilla.com", regex: "\\+[^@]+", replace: ""}]',
23232333
env: 'RATE_LIMIT__EMAIL_ALIAS_NORMALIZATION',
23242334
format: String,

packages/fxa-auth-server/lib/bounces.js

Lines changed: 15 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
'use strict';
66

77
const { AppError: error } = require('@fxa/accounts/errors');
8+
const { EmailNormalization } = require('fxa-shared/email/email-normalization');
89

910
module.exports = (config, db) => {
1011
const configBounces = (config.smtp && config.smtp.bounces) || {};
@@ -28,12 +29,24 @@ module.exports = (config, db) => {
2829
[BOUNCE_TYPE_COMPLAINT]: error.emailComplaint,
2930
};
3031

31-
function checkBounces(email, template) {
32+
const emailNormalization = new EmailNormalization(
33+
configBounces.emailAliasNormalization
34+
);
35+
36+
async function checkBounces(email, template) {
3237
if (ignoreTemplates.includes(template)) {
3338
return;
3439
}
3540

36-
return db.emailBounces(email).then(applyRules);
41+
// This strips out 'alias' stuff from an email and replace
42+
// them with wildcards, allowing us to turn up bounce records
43+
// on email aliases.
44+
const normalizedEmail = emailNormalization.normalizeEmailAliases(
45+
email,
46+
'%'
47+
);
48+
const bounces = await db.emailBounces(normalizedEmail);
49+
return applyRules(bounces);
3750
}
3851

3952
// Relies on the order of the bounces array to be sorted by date,

0 commit comments

Comments
 (0)