Skip to content

Commit 0d83395

Browse files
authored
Merge pull request #20130 from mozilla/FXA-13125
bug(auth): Two sign in emails being sent
2 parents 1290a70 + 3e02967 commit 0d83395

File tree

2 files changed

+83
-2
lines changed

2 files changed

+83
-2
lines changed

packages/fxa-auth-server/lib/routes/account.ts

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1366,8 +1366,12 @@ export class AccountHandler {
13661366
// For new logins that don't send some other sort of email,
13671367
// send an after-the-fact notification email so that the user
13681368
// is aware that something happened on their account.
1369+
// Only send immediately if the session is already verified (i.e. verification
1370+
// was skipped). When the session is unverified the user will be taken through
1371+
// the token-code flow, and session.js:verify_code sends this email after
1372+
// successful verification — sending it here too would result in a duplicate.
13691373
if (accountRecord.primaryEmail.isVerified) {
1370-
if (sessionToken.tokenVerified || !sessionToken.mustVerify) {
1374+
if (sessionToken.tokenVerified) {
13711375
const geoData = request.app.geo;
13721376
const service =
13731377
(request.payload as any).service || request.query.service;

packages/fxa-auth-server/test/local/routes/account.js

Lines changed: 78 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3192,6 +3192,69 @@ describe('/account/login', () => {
31923192
});
31933193
});
31943194

3195+
it('does not send new device login email during login when session is unverified (deferred to verify_code)', () => {
3196+
// Regression test: when a session is unverified but mustVerify=false (e.g. a non-sync
3197+
// login for an older account), the newDeviceLogin email must NOT be sent during login.
3198+
// It will be sent by session.js:verify_code after the user completes token verification.
3199+
// Previously the condition `tokenVerified || !mustVerify` would send it here too,
3200+
// resulting in two emails.
3201+
const email = 'test@mozilla.com';
3202+
mockDB.accountRecord = function () {
3203+
return Promise.resolve({
3204+
authSalt: hexString(32),
3205+
data: hexString(32),
3206+
email: email,
3207+
emailVerified: true,
3208+
primaryEmail: {
3209+
normalizedEmail: normalizeEmail(email),
3210+
email: email,
3211+
isVerified: true,
3212+
isPrimary: true,
3213+
},
3214+
kA: hexString(32),
3215+
lastAuthAt: function () {
3216+
return Date.now();
3217+
},
3218+
uid: uid,
3219+
wrapWrapKb: hexString(32),
3220+
});
3221+
};
3222+
3223+
// Simulate an unverified session state. This will supress the sending of
3224+
// a 'new device login' email.
3225+
const originalCreateSessionToken = mockDB.createSessionToken;
3226+
mockDB.createSessionToken = sinon.spy(async (opts) => {
3227+
const result = await originalCreateSessionToken(opts);
3228+
result.tokenVerificationId = hexString(16);
3229+
result.tokenVerified = false;
3230+
return result;
3231+
});
3232+
3233+
return runTest(route, mockRequestNoKeys, (response) => {
3234+
mockDB.createSessionToken = originalCreateSessionToken;
3235+
3236+
const tokenData = mockDB.createSessionToken.getCall(0).args[0];
3237+
assert.ok(
3238+
tokenData.tokenVerificationId,
3239+
'sessionToken was created unverified'
3240+
);
3241+
// newDeviceLogin email must NOT be sent during login when the session is
3242+
// unverified — it will be sent by session.js:verify_code after verification.
3243+
assert.equal(
3244+
mockFxaMailer.sendNewDeviceLoginEmail.callCount,
3245+
0,
3246+
'newDeviceLogin email is not sent during login for an unverified session'
3247+
);
3248+
assert.ok(
3249+
!response.verified,
3250+
'response indicates session is not verified'
3251+
);
3252+
3253+
// Restore the original function
3254+
mockDB.createSessionToken = originalCreateSessionToken;
3255+
});
3256+
});
3257+
31953258
it('requires change password verification when the lockedAt field is set', () => {
31963259
const email = 'test@mozilla.com';
31973260
mockDB.accountRecord = function () {
@@ -4510,6 +4573,7 @@ describe('/account/login', () => {
45104573
}
45114574
});
45124575

4576+
// Breaking
45134577
it('should use RP CMS email content for new login email', () => {
45144578
rpConfigManager.fetchCMSData.resetHistory();
45154579
const email = 'test@mozilla.com';
@@ -4533,7 +4597,20 @@ describe('/account/login', () => {
45334597
wrapWrapKb: hexString(32),
45344598
});
45354599
};
4600+
4601+
const originalCreateSessionToken = mockDB.createSessionToken;
4602+
4603+
// Simulate a verified session state. This will result in a new device
4604+
// login email being sent.
4605+
mockDB.createSessionToken = sinon.spy(async (opts) => {
4606+
const result = await originalCreateSessionToken(opts);
4607+
result.tokenVerificationId = null;
4608+
result.tokenVerified = true;
4609+
return result;
4610+
});
4611+
45364612
return runTest(route, mockRequestWithRpCmsConfig, () => {
4613+
mockDB.createSessionToken = originalCreateSessionToken;
45374614
assert.calledOnce(mockFxaMailer.sendNewDeviceLoginEmail);
45384615
const args = mockFxaMailer.sendNewDeviceLoginEmail.args[0];
45394616
const emailMessage = args[0];
@@ -5493,4 +5570,4 @@ describe('/account/metrics_opt', () => {
54935570
assert.calledWith(setMetricsOptStub, uid, 'in');
54945571
});
54955572
});
5496-
});
5573+
});

0 commit comments

Comments
 (0)