From 097baedb9eed49458568dcb0f4eb7973c46dd5a9 Mon Sep 17 00:00:00 2001 From: Ivo Plamenac Date: Mon, 4 Jul 2022 22:28:44 -0700 Subject: [PATCH] fix(auth): user Sent Unnecessary Finish Setting Up Account Reminder Email Because: * users are receiving unnecessary emails after creating a password (likely due to an error occuring after their account is verified or an issue with not dropping the key from redis) This commit: * adds an additional attempt to drop the key from redis if an error occurs after the account is verified and adds a check to see if the account is verified before sending the email Closes #12802 --- .../fxa-auth-server/lib/routes/account.ts | 13 +++++- packages/fxa-auth-server/lib/senders/email.js | 35 ++++++++-------- .../scripts/verification-reminders.js | 1 + .../test/local/routes/account.js | 14 +++++++ .../test/local/senders/index.js | 41 ++++++++++++++++++- 5 files changed, 86 insertions(+), 18 deletions(-) diff --git a/packages/fxa-auth-server/lib/routes/account.ts b/packages/fxa-auth-server/lib/routes/account.ts index da85892ff49..9ad5c58016e 100644 --- a/packages/fxa-auth-server/lib/routes/account.ts +++ b/packages/fxa-auth-server/lib/routes/account.ts @@ -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. @@ -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; } } diff --git a/packages/fxa-auth-server/lib/senders/email.js b/packages/fxa-auth-server/lib/senders/email.js index f9390a23f84..cf65f03fff2 100644 --- a/packages/fxa-auth-server/lib/senders/email.js +++ b/packages/fxa-auth-server/lib/senders/email.js @@ -658,6 +658,7 @@ module.exports = function (log, config, bounces) { flowId, flowBeginTime, deviceId, + accountVerified, } = message; log.trace(`mailer.${template}`, { email, uid }); @@ -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, + }, + }); + } }; }); diff --git a/packages/fxa-auth-server/scripts/verification-reminders.js b/packages/fxa-auth-server/scripts/verification-reminders.js index a508c2635fb..16a5027459c 100755 --- a/packages/fxa-auth-server/scripts/verification-reminders.js +++ b/packages/fxa-auth-server/scripts/verification-reminders.js @@ -182,6 +182,7 @@ async function run() { acceptLanguage: account.locale, code: account.emailCode, email: account.email, + accountVerified: account.verifierSetAt > 0, token: token, flowBeginTime, flowId, diff --git a/packages/fxa-auth-server/test/local/routes/account.js b/packages/fxa-auth-server/test/local/routes/account.js index fbc8d3aa86a..603241b9793 100644 --- a/packages/fxa-auth-server/test/local/routes/account.js +++ b/packages/fxa-auth-server/test/local/routes/account.js @@ -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', () => { diff --git a/packages/fxa-auth-server/test/local/senders/index.js b/packages/fxa-auth-server/test/local/senders/index.js index c2a9bc5cb75..8613192f899 100644 --- a/packages/fxa-auth-server/test/local/senders/index.js +++ b/packages/fxa-auth-server/test/local/senders/index.js @@ -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; @@ -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); + }); + }); }); });