Skip to content

Commit

Permalink
Merge pull request #13520 from mozilla/fxa-5052/unnecessary-reminder-…
Browse files Browse the repository at this point in the history
…emails

fix(auth): user Sent Unnecessary Finish Setting Up Account Reminder Email
  • Loading branch information
IvoJP authored Jul 6, 2022
2 parents 37dc446 + 097baed commit 82f1382
Show file tree
Hide file tree
Showing 5 changed files with 86 additions and 18 deletions.
13 changes: 12 additions & 1 deletion packages/fxa-auth-server/lib/routes/account.ts
Original file line number Diff line number Diff line change
Expand Up @@ -578,12 +578,13 @@ export class AccountHandler {
this.log.begin('Account.finishSetup', request);
const form = request.payload as any;
const authPW = form.authPW;
let uid;
try {
const payload = (await jwt.verify(form.token, {
typ: 'fin+JWT',
ignoreExpiration: true,
})) as any;
const uid = payload.uid;
uid = payload.uid;
form.uid = payload.uid;
const account = await this.db.account(uid);
// Only proceed if the account is in the stub state.
Expand Down Expand Up @@ -628,6 +629,16 @@ export class AccountHandler {
this.log.error('Account.finish_setup.error', {
err,
});

// if it errored out after verifiying the account
// remove the uid from the list of accounts to send reminders to.
if (!!uid) {
const account = await this.db.account(uid);
if (account.verifierSetAt > 0) {
await this.subscriptionAccountReminders.delete(uid);
}
}

throw err;
}
}
Expand Down
35 changes: 19 additions & 16 deletions packages/fxa-auth-server/lib/senders/email.js
Original file line number Diff line number Diff line change
Expand Up @@ -658,6 +658,7 @@ module.exports = function (log, config, bounces) {
flowId,
flowBeginTime,
deviceId,
accountVerified,
} = message;

log.trace(`mailer.${template}`, { email, uid });
Expand All @@ -682,22 +683,24 @@ module.exports = function (log, config, bounces) {
'X-Link': links.link,
};

return this.send({
...message,
headers,
layout: 'subscription',
template,
templateValues: {
email,
...links,
oneClickLink: links.oneClickLink,
privacyUrl: links.privacyUrl,
termsOfServiceDownloadURL: links.termsOfServiceDownloadURL,
supportUrl: links.supportUrl,
supportLinkAttributes: links.supportLinkAttributes,
reminderShortForm: true,
},
});
if (!accountVerified) {
return this.send({
...message,
headers,
layout: 'subscription',
template,
templateValues: {
email,
...links,
oneClickLink: links.oneClickLink,
privacyUrl: links.privacyUrl,
termsOfServiceDownloadURL: links.termsOfServiceDownloadURL,
supportUrl: links.supportUrl,
supportLinkAttributes: links.supportLinkAttributes,
reminderShortForm: true,
},
});
}
};
});

Expand Down
1 change: 1 addition & 0 deletions packages/fxa-auth-server/scripts/verification-reminders.js
Original file line number Diff line number Diff line change
Expand Up @@ -182,6 +182,7 @@ async function run() {
acceptLanguage: account.locale,
code: account.emailCode,
email: account.email,
accountVerified: account.verifierSetAt > 0,
token: token,
flowBeginTime,
flowId,
Expand Down
14 changes: 14 additions & 0 deletions packages/fxa-auth-server/test/local/routes/account.js
Original file line number Diff line number Diff line change
Expand Up @@ -1595,6 +1595,20 @@ describe('/account/finish_setup', () => {
assert.equal(err.errno, 110);
}
});

it('removes the reminder if it errors after account is verified', async () => {
const { route, mockRequest, subscriptionAccountReminders } = setup({
verifierSetAt: Date.now(),
});

try {
await runTest(route, mockRequest);
assert.fail('should have errored');
} catch (err) {
assert.equal(err.errno, 110);
assert.calledOnce(subscriptionAccountReminders.delete);
}
});
});

describe('/account/login', () => {
Expand Down
41 changes: 40 additions & 1 deletion packages/fxa-auth-server/test/local/senders/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -241,7 +241,10 @@ describe('lib/senders/index', () => {
return email.sendPostAddLinkedAccountEmail(EMAILS, acct, {});
})
.then(() => {
assert.equal(email._ungatedMailer.postAddLinkedAccountEmail.callCount, 1);
assert.equal(
email._ungatedMailer.postAddLinkedAccountEmail.callCount,
1
);

const args =
email._ungatedMailer.postAddLinkedAccountEmail.getCall(0).args;
Expand Down Expand Up @@ -461,5 +464,41 @@ describe('lib/senders/index', () => {
});
});
});

describe('subscriptionAccountReminder Emails', () => {
it('should send an email if the account is unverified', async () => {
const mailer = await createSender(config);
await mailer.sendSubscriptionAccountReminderFirstEmail(EMAILS, acct, {
email: 'test@test.com',
uid: '123',
productId: 'abc',
productName: 'testProduct',
token: 'token',
flowId: '456',
lowBeginTime: 123,
deviceId: 'xyz',
accountVerified: false,
});

assert.equal(mailer._ungatedMailer.mailer.sendMail.callCount, 1);
});

it('should not send an email if the account is verified', async () => {
const mailer = await createSender(config);
await mailer.sendSubscriptionAccountReminderFirstEmail(EMAILS, acct, {
email: 'test@test.com',
uid: '123',
productId: 'abc',
productName: 'testProduct',
token: 'token',
flowId: '456',
lowBeginTime: 123,
deviceId: 'xyz',
accountVerified: true,
});

assert.equal(mailer._ungatedMailer.mailer.sendMail.callCount, 0);
});
});
});
});

0 comments on commit 82f1382

Please sign in to comment.