diff --git a/config/index.js b/config/index.js index 8b5094b4..c55bc4a9 100644 --- a/config/index.js +++ b/config/index.js @@ -876,7 +876,47 @@ const conf = convict({ env: 'RECOVERY_CODE_NOTIFY_LOW_COUNT' } } - } + }, + verificationReminders: { + rolloutRate: { + doc: 'Rollout rate for verification reminder emails, in the range 0 .. 1', + default: 1, + env: 'VERIFICATION_REMINDERS_ROLLOUT_RATE', + format: Number, + }, + firstInterval: { + doc: 'Time since account creation after which the first reminder is sent', + default: '1 day', + env: 'VERIFICATION_REMINDERS_FIRST_INTERVAL', + format: 'duration', + }, + secondInterval: { + doc: 'Time since account creation after which the second reminder is sent', + default: '5 days', + env: 'VERIFICATION_REMINDERS_SECOND_INTERVAL', + format: 'duration', + }, + redis: { + prefix: { + default: 'verificationReminders:', + doc: 'Key prefix for the verification reminders Redis pool', + env: 'VERIFICATION_REMINDERS_REDIS_PREFIX', + format: String, + }, + maxConnections: { + default: 10, + doc: 'Maximum connection count for the verification reminders Redis pool', + env: 'VERIFICATION_REMINDERS_REDIS_MAX_CONNECTIONS', + format: 'nat', + }, + minConnections: { + default: 1, + doc: 'Minimum connection count for the verification reminders Redis pool', + env: 'VERIFICATION_REMINDERS_REDIS_MIN_CONNECTIONS', + format: 'nat', + }, + }, + }, }); // handle configuration files. you can specify a CSV list of configuration diff --git a/docs/api.md b/docs/api.md index 3de98878..17126fa8 100644 --- a/docs/api.md +++ b/docs/api.md @@ -1785,10 +1785,10 @@ not just the one being attached to the Firefox account. Opaque alphanumeric token to be included in verification links. -* `reminder`: *string, max(32), alphanum, optional* +* `reminder`: *string, regex(/^(?:first|second)$/), optional* - Deprecated. + Indicates that verification originates from a reminder email. * `type`: *string, max(32), alphanum, optional* diff --git a/docs/metrics-events.md b/docs/metrics-events.md index 0f7a921e..ea6b2a6b 100644 --- a/docs/metrics-events.md +++ b/docs/metrics-events.md @@ -168,6 +168,7 @@ in a sign-in or sign-up flow: |`signinCode.consumed`|A sign-in code has been consumed on the server.| |`account.confirmed`|Sign-in to an existing account has been confirmed via email.| |`account.verified`|A new account has been verified via email.| +|`account.reminder.${reminder}`|A new account has been verified via a reminder email.| |`account.keyfetch`|Sync encryption keys have been fetched.| |`account.signed`|A certificate has been signed.| |`account.reset`|An account has been reset.| diff --git a/lib/metrics/amplitude.js b/lib/metrics/amplitude.js index a93b3445..e998cabc 100644 --- a/lib/metrics/amplitude.js +++ b/lib/metrics/amplitude.js @@ -105,6 +105,11 @@ module.exports = (log, config) => { throw new TypeError('Missing argument'); } + const verificationReminders = require('../verification-reminders')(log, config); + verificationReminders.keys.forEach(key => { + EMAIL_TYPES[`verificationReminder${key[0].toUpperCase()}${key.substr(1)}Email`] = 'registration'; + }); + const transformEvent = initialize(config.oauth.clientIds, EVENTS, FUZZY_EVENTS); return receiveEvent; diff --git a/lib/routes/account.js b/lib/routes/account.js index cadf9707..633a8476 100644 --- a/lib/routes/account.js +++ b/lib/routes/account.js @@ -23,7 +23,7 @@ const MS_ONE_DAY = MS_ONE_HOUR * 24; const MS_ONE_WEEK = MS_ONE_DAY * 7; const MS_ONE_MONTH = MS_ONE_DAY * 30; -module.exports = (log, db, mailer, Password, config, customs, signinUtils, push) => { +module.exports = (log, db, mailer, Password, config, customs, signinUtils, push, verificationReminders) => { const tokenCodeConfig = config.signinConfirmation.tokenVerificationCode; const tokenCodeLifetime = tokenCodeConfig && tokenCodeConfig.codeLifetime || MS_ONE_HOUR; const tokenCodeLength = tokenCodeConfig && tokenCodeConfig.codeLength || 8; @@ -279,6 +279,8 @@ module.exports = (log, db, mailer, Password, config, customs, signinUtils, push) tokenVerificationId: tokenVerificationId }); } + + return verificationReminders.create(account.uid); }) .catch((err) => { log.error('mailer.sendVerifyCode.1', { err: err}); diff --git a/lib/routes/emails.js b/lib/routes/emails.js index d40029a9..7589e950 100644 --- a/lib/routes/emails.js +++ b/lib/routes/emails.js @@ -14,7 +14,9 @@ const validators = require('./validators'); const HEX_STRING = validators.HEX_STRING; -module.exports = (log, db, mailer, config, customs, push) => { +module.exports = (log, db, mailer, config, customs, push, verificationReminders) => { + const REMINDER_PATTERN = new RegExp(`^(?:${verificationReminders.keys.join('|')})$`); + return [ { method: 'GET', @@ -271,8 +273,7 @@ module.exports = (log, db, mailer, config, customs, push) => { uid: isA.string().max(32).regex(HEX_STRING).required(), code: isA.string().min(32).max(32).regex(HEX_STRING).required(), service: validators.service, - // TODO: drop this param once it is no longer sent by clients - reminder: isA.string().max(32).alphanum().optional(), + reminder: isA.string().regex(REMINDER_PATTERN).optional(), type: isA.string().max(32).alphanum().optional(), marketingOptIn: isA.boolean() } @@ -281,7 +282,7 @@ module.exports = (log, db, mailer, config, customs, push) => { handler: async function (request) { log.begin('Account.RecoveryEmailVerify', request); - const { code, marketingOptIn, service, type, uid } = request.payload; + const { code, marketingOptIn, reminder, service, type, uid } = request.payload; // verify_code because we don't know what type this is yet, but // we want to record right away before anything could fail, so @@ -429,7 +430,9 @@ module.exports = (log, db, mailer, config, customs, push) => { // Force it so that we emit the appropriate newsletter state. marketingOptIn: marketingOptIn || false, uid - }) + }), + reminder ? request.emitMetricsEvent(`account.reminder.${reminder}`, { uid }) : null, + verificationReminders.delete(uid), ]); }) .then(() => { diff --git a/lib/routes/index.js b/lib/routes/index.js index 5a72bc0a..e3c40f28 100644 --- a/lib/routes/index.js +++ b/lib/routes/index.js @@ -23,6 +23,7 @@ module.exports = function ( const pushbox = require('../pushbox')(log, config); const devicesImpl = require('../devices')(log, db, push); const signinUtils = require('./utils/signin')(log, config, customs, db, mailer); + const verificationReminders = require('../verification-reminders')(log, config); // The routing modules themselves. const defaults = require('./defaults')(log, db); const idp = require('./idp')(log, serverPublicKeys); @@ -34,11 +35,12 @@ module.exports = function ( config, customs, signinUtils, - push + push, + verificationReminders, ); const oauth = require('./oauth')(log, config, oauthdb); const devicesSessions = require('./devices-and-sessions')(log, db, config, customs, push, pushbox, devicesImpl, oauthdb); - const emails = require('./emails')(log, db, mailer, config, customs, push); + const emails = require('./emails')(log, db, mailer, config, customs, push, verificationReminders); const password = require('./password')( log, db, diff --git a/lib/senders/email.js b/lib/senders/email.js index 04a27483..2344dded 100644 --- a/lib/senders/email.js +++ b/lib/senders/email.js @@ -38,6 +38,7 @@ module.exports = function (log, config, oauthdb) { // Fallback to a stub implementation if redis is disabled get: () => P.resolve() }; + const verificationReminders = require('../verification-reminders')(log, config); // Email template to UTM campaign map, each of these should be unique and // map to exactly one email template. @@ -589,6 +590,44 @@ module.exports = function (log, config, oauthdb) { })); }; + + verificationReminders.keys.forEach(key => { + // Template names are generated in the form `verificationReminderFirstEmail`, + // where `First` is the key derived from config, with an initial capital letter. + const template = `verificationReminder${key[0].toUpperCase()}${key.substr(1)}Email`; + const subject = key === 'first' ? gettext('Hello again') : gettext('Still there?'); + + templateNameToCampaignMap[template] = `${key}-verification-reminder`; + templateNameToContentMap[template] = 'activate'; + + Mailer.prototype[template] = async function (message) { + const { code, email, uid } = message; + + log.trace(`mailer.${template}`, { code, email, uid }); + + const query = { code, reminder: key, uid }; + const links = this._generateLinks(this.verificationUrl, email, query, template); + const headers = { + 'X-Link': links.link, + 'X-Verify-Code': message.code + }; + + return this.send(Object.assign({}, message, { + headers, + subject, + template, + templateValues: { + email, + link: links.link, + oneClickLink: links.oneClickLink, + privacyUrl: links.privacyUrl, + supportUrl: links.supportUrl, + supportLinkAttributes: links.supportLinkAttributes, + }, + })); + }; + }); + Mailer.prototype.unblockCodeEmail = function (message) { log.trace('mailer.unblockCodeEmail', { email: message.email, uid: message.uid }); diff --git a/lib/senders/templates/_versions.json b/lib/senders/templates/_versions.json index 38664b3b..5ccb86ce 100644 --- a/lib/senders/templates/_versions.json +++ b/lib/senders/templates/_versions.json @@ -11,6 +11,8 @@ "recoveryEmail": 1, "sms.installFirefox": 1, "unblockCodeEmail": 1, + "verificationReminderFirstEmail": 2, + "verificationReminderSecondEmail": 2, "verifyEmail": 2, "verifyPrimaryEmail": 3, "verifyLoginEmail": 1, diff --git a/lib/senders/templates/index.js b/lib/senders/templates/index.js index 9f10f869..3c90ce41 100644 --- a/lib/senders/templates/index.js +++ b/lib/senders/templates/index.js @@ -79,6 +79,8 @@ module.exports = { 'recovery', 'sms.installFirefox', 'unblock_code', + 'verification_reminder_first', + 'verification_reminder_second', 'verify', 'verify_login', 'verify_login_code', diff --git a/lib/senders/templates/verification_reminder_first.html b/lib/senders/templates/verification_reminder_first.html new file mode 100644 index 00000000..950e9bd1 --- /dev/null +++ b/lib/senders/templates/verification_reminder_first.html @@ -0,0 +1,83 @@ + + + + + {{t "Firefox Accounts"}} + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + +
+

{{t "Hello again."}}

+

{{{t "A few days ago you created a Firefox Account, but never verified it. A verified account lets you access your tabs, bookmarks, passwords and history on any device connected to it. Simply confirm this email address to activate your account."}}}

+
+ + + + +
+ + {{t "Activate now"}} + +
+
+
+

{{t "This is an automated email; if you received it in error, no action is required."}} {{{t "For more information, please visit Mozilla Support."}}}

+
+

Mozilla. 331 E Evelyn Ave, Mountain View, CA 94041 +
+ {{t "Mozilla Privacy Policy" }}

+
+ +
+
+ + + +
+ +
+ + + diff --git a/lib/senders/templates/verification_reminder_first.txt b/lib/senders/templates/verification_reminder_first.txt new file mode 100644 index 00000000..6cc89ba7 --- /dev/null +++ b/lib/senders/templates/verification_reminder_first.txt @@ -0,0 +1,9 @@ +{{t "Hello again."}} + +{{t "A few days ago you created a Firefox Account, but never verified it. A verified account lets you access your tabs, bookmarks, passwords and history on any device connected to it. Simply confirm this email address to activate your account."}} +{{t "Activate now:"}} {{{link}}} + +{{t "This is an automated email; if you received it in error, no action is required."}} {{{t "For more information, please visit %(supportUrl)s"}}} + +Mozilla. 331 E Evelyn Ave, Mountain View, CA 94041 +{{t "Mozilla Privacy Policy" }} {{{privacyUrl}}} diff --git a/lib/senders/templates/verification_reminder_second.html b/lib/senders/templates/verification_reminder_second.html new file mode 100644 index 00000000..2d37bbb0 --- /dev/null +++ b/lib/senders/templates/verification_reminder_second.html @@ -0,0 +1,83 @@ + + + + + {{t "Firefox Accounts"}} + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + +
+

{{t "Still there?"}}

+

{{{t "A week ago you created a Firefox Account, but never verified it. We’re worried about you. Confirm this email address to activate your account and let us know you're okay."}}}

+
+ + + + +
+ + {{t "Activate now"}} + +
+
+
+

{{t "This is an automated email; if you received it in error, no action is required."}} {{{t "For more information, please visit Mozilla Support."}}}

+
+

Mozilla. 331 E Evelyn Ave, Mountain View, CA 94041 +
+ {{t "Mozilla Privacy Policy" }}

+
+ +
+
+ + + +
+ +
+ + + diff --git a/lib/senders/templates/verification_reminder_second.txt b/lib/senders/templates/verification_reminder_second.txt new file mode 100644 index 00000000..5f21cb78 --- /dev/null +++ b/lib/senders/templates/verification_reminder_second.txt @@ -0,0 +1,9 @@ +{{t "Still there?"}} + +{{t "A week ago you created a Firefox Account, but never verified it. We’re worried about you. Confirm this email address to activate your account and let us know you're okay."}} +{{t "Activate now:"}} {{{link}}} + +{{t "This is an automated email; if you received it in error, no action is required."}} {{{t "For more information, please visit %(supportUrl)s"}}} + +Mozilla. 331 E Evelyn Ave, Mountain View, CA 94041 +{{t "Mozilla Privacy Policy" }} {{{privacyUrl}}} diff --git a/lib/verification-reminders.js b/lib/verification-reminders.js new file mode 100644 index 00000000..c356042c --- /dev/null +++ b/lib/verification-reminders.js @@ -0,0 +1,143 @@ +/* This Source Code Form is subject to the terms of the Mozilla Public + * License, v. 2.0. If a copy of the MPL was not distributed with this + * file, You can obtain one at https://mozilla.org/MPL/2.0/. */ + +// This module implements logic for managing account verification reminders. +// +// Reminder records are stored in Redis sorted sets on account creation and +// removed when an acount is verified. A separate script, running on the +// fxa-admin box, processes reminder records in a cron job by pulling the +// records that have ticked passed an expiry limit set in config and sending +// the appropriate reminder email to the address associated with each account. +// +// Right now, config determines how many reminder emails are sent and what +// the expiry intervals for them are. Ultimately though, that might be a good +// candidate to control with feature flags. +// +// More detail on sorted sets: +// +// * https://redis.io/topics/data-types#sorted-sets +// * https://redis.io/topics/data-types-intro#redis-sorted-sets + +'use strict'; + +const P = require('./promise'); + +const INTERVAL_PATTERN = /^([a-z]+)Interval$/; + +/** + * Initialise the verification reminders module. + * + * @param {Object} log + * @param {Object} config + * @returns {VerificationReminders} + */ +module.exports = (log, config) => { + const redis = require('fxa-shared/redis')({ + ...config.redis, + ...config.verificationReminders.redis, + enabled: true, + }, log); + + const { rolloutRate } = config.verificationReminders; + + const { keys, intervals } = Object.entries(config.verificationReminders).reduce(({ keys, intervals }, [ key, value ]) => { + const matches = INTERVAL_PATTERN.exec(key); + if (matches && matches.length === 2) { + const key = matches[1]; + keys.push(key); + intervals[key] = value; + } + return { keys, intervals }; + }, { keys: [], intervals: {} }); + + /** + * @typedef {Object} VerificationReminders + * @property {Array} keys + * @property {Function} create + * @property {Function} delete + * @property {Function} process + * + * Each method below returns a promise that resolves to an object, + * the shape of which is determined by config. If config has settings + * for `firstInterval` and `secondInterval` (as at time of writing), + * the shape of those objects would be `{ first, second }`. + */ + return { + keys: keys.slice(), + + /** + * Create verification reminder records for an account. + * + * @param {String} uid + * @returns {Promise} - Each property on the resolved object will be the number + * of elements added to that sorted set, i.e. the result of + * [`redis.zadd`](https://redis.io/commands/zadd). + */ + async create (uid) { + try { + if (rolloutRate <= 1 && Math.random() < rolloutRate) { + const now = Date.now(); + const result = await P.props(keys.reduce((result, key) => { + result[key] = redis.zadd(key, now, uid); + return result; + }, {})); + log.info('verificationReminders.create', { uid }); + return result; + } + } catch (err) { + log.error('verificationReminders.create.error', { err, uid }); + throw err; + } + }, + + /** + * Delete verification reminder records for an account. + * + * @param {String} uid + * @returns {Promise} - Each property on the resolved object will be the number of + * elements removed from that sorted set, i.e. the result of + * [`redis.zrem`](https://redis.io/commands/zrem). + */ + async delete (uid) { + try { + const result = await P.props(keys.reduce((result, key) => { + result[key] = redis.zrem(key, uid); + return result; + }, {})); + log.info('verificationReminders.delete', { uid }); + return result; + } catch (err) { + log.error('verificationReminders.delete.error', { err, uid }); + throw err; + } + }, + + /** + * Read and remove all verification reminders that have + * ticked past the expiry intervals set in config. + * + * @returns {Promise} - Each property on the resolved object will be an array of uids that + * were found to have ticked past the relevant expiry interval, i.e. + * the result of [`redis.zrangebyscore`](https://redis.io/commands/zrangebyscore). + */ + async process () { + try { + const now = Date.now(); + return await P.props(keys.reduce((result, key) => { + const cutoff = now - intervals[key]; + result[key] = redis.zrangebyscore(key, 0, cutoff); + setImmediate(async () => { + await result[key]; + redis.zremrangebyscore(key, 0, cutoff); + }); + log.info('verificationReminders.process', { key, now, cutoff }); + return result; + }, {})); + } catch (err) { + log.error('verificationReminders.process.error', { err }); + throw err; + } + }, + }; +}; diff --git a/npm-shrinkwrap.json b/npm-shrinkwrap.json index a60b9811..b3dd2e17 100644 --- a/npm-shrinkwrap.json +++ b/npm-shrinkwrap.json @@ -8447,17 +8447,29 @@ } }, "fxa-shared": { - "version": "1.0.19", - "resolved": "https://registry.npmjs.org/fxa-shared/-/fxa-shared-1.0.19.tgz", - "integrity": "sha512-WuKS50Z/Il+bjnlp6qHqc3qr6mS7q1sYQ8rRcFx+4R39m2FjbxwSG8f1BnF74q3rdb8nCH0BsmSNFsJ1QNaN8w==", + "version": "1.0.21", + "resolved": "https://registry.npmjs.org/fxa-shared/-/fxa-shared-1.0.21.tgz", + "integrity": "sha512-jIvoGmulWWDa6i5MjfxBjIZrk3WiBU/tZziKN8VQtptnfKGEGOFRPtbnpWX3lElLmf/46+yOXcSyKkIoA2WzTg==", "requires": { "accept-language": "2.0.17", + "ajv": "6.10.0", "bluebird": "3.5.3", "generic-pool": "3.6.1", "moment": "2.20.1", "redis": "2.8.0" }, "dependencies": { + "ajv": { + "version": "6.10.0", + "resolved": "https://registry.npmjs.org/ajv/-/ajv-6.10.0.tgz", + "integrity": "sha512-nffhOpkymDECQyR0mnsUtoCE8RlX38G0rYP+wgLWFyZuUyuuojSSvi/+euOiQBIn63whYwYVIIH1TvE3tu4OEg==", + "requires": { + "fast-deep-equal": "^2.0.1", + "fast-json-stable-stringify": "^2.0.0", + "json-schema-traverse": "^0.4.1", + "uri-js": "^4.2.2" + } + }, "bluebird": { "version": "3.5.3", "resolved": "https://registry.npmjs.org/bluebird/-/bluebird-3.5.3.tgz", @@ -9749,9 +9761,9 @@ } }, "ieee754": { - "version": "1.1.12", - "resolved": "https://registry.npmjs.org/ieee754/-/ieee754-1.1.12.tgz", - "integrity": "sha512-GguP+DRY+pJ3soyIiGPTvdiVXjZ+DbXOxGpXn3eMvNW4x4irjqXm4wHKscC+TfxSJ0yw/S1F24tqdMNsMZTiLA==" + "version": "1.1.13", + "resolved": "https://registry.npmjs.org/ieee754/-/ieee754-1.1.13.tgz", + "integrity": "sha512-4vf7I2LYV/HaWerSo3XmlMkp5eZ83i+/CDluXi/IGTs/O1sejBNhTtnxzmRZfvOUqj7lZjqHkeTvpgSFDlWZTg==" }, "ignore": { "version": "3.3.10", @@ -11213,9 +11225,9 @@ } }, "nan": { - "version": "2.13.1", - "resolved": "https://registry.npmjs.org/nan/-/nan-2.13.1.tgz", - "integrity": "sha512-I6YB/YEuDeUZMmhscXKxGgZlFnhsn5y0hgOZBadkzfTRrZBtJDZeg6eQf7PYMIEclwmorTKK8GztsyOUSVBREA==", + "version": "2.13.2", + "resolved": "https://registry.npmjs.org/nan/-/nan-2.13.2.tgz", + "integrity": "sha512-TghvYc72wlMGMVMluVo9WRJc0mB8KxxF/gZ4YYFy7V2ZQX9l7rgbPg7vjS9mt6U5HXODVFVI2bOduCzwOMv/lw==", "optional": true }, "nanomatch": { diff --git a/package.json b/package.json index 098e51f3..67450b35 100644 --- a/package.json +++ b/package.json @@ -52,7 +52,7 @@ "fxa-geodb": "1.0.4", "fxa-jwtool": "0.7.2", "fxa-notifier-aws": "1.0.0", - "fxa-shared": "1.0.19", + "fxa-shared": "1.0.21", "generic-pool": "3.2.0", "google-libphonenumber": "2.0.10", "grunt-nunjucks-2-html": "3.1.0", diff --git a/test/local/devices.js b/test/local/devices.js index 77fa7f61..48f7e1c4 100644 --- a/test/local/devices.js +++ b/test/local/devices.js @@ -270,8 +270,6 @@ describe('lib/devices:', () => { is_placeholder: false }, 'event data was correct'); - assert.equal(log.info.callCount, 0, 'log.info was not called'); - assert.equal(log.notifyAttachedServices.callCount, 1, 'log.notifyAttachedServices was called once'); args = log.notifyAttachedServices.args[0]; assert.equal(args.length, 3, 'log.notifyAttachedServices was passed three arguments'); @@ -314,10 +312,10 @@ describe('lib/devices:', () => { assert.equal(log.activityEvent.callCount, 1, 'log.activityEvent was called once'); assert.equal(log.activityEvent.args[0][0].is_placeholder, true, 'is_placeholder was correct'); - assert.equal(log.info.callCount, 1, 'log.info was called once'); - assert.equal(log.info.args[0].length, 2); - assert.equal(log.info.args[0][0], 'device:createPlaceholder'); - assert.deepEqual(log.info.args[0][1], { + assert.equal(log.info.callCount, 2); + assert.equal(log.info.args[1].length, 2); + assert.equal(log.info.args[1][0], 'device:createPlaceholder'); + assert.deepEqual(log.info.args[1][1], { uid: credentials.uid, id: result.id }, 'argument was event data'); @@ -368,8 +366,6 @@ describe('lib/devices:', () => { is_placeholder: false }, 'event data was correct'); - assert.equal(log.info.callCount, 0, 'log.info was not called'); - assert.equal(log.notifyAttachedServices.callCount, 0, 'log.notifyAttachedServices was not called'); assert.equal(push.notifyDeviceConnected.callCount, 0, 'push.notifyDeviceConnected was not called'); @@ -424,8 +420,6 @@ describe('lib/devices:', () => { is_placeholder: false }, 'event data was correct'); - assert.equal(log.info.callCount, 0, 'log.info was not called'); - assert.equal(log.notifyAttachedServices.callCount, 1, 'log.notifyAttachedServices was called once'); args = log.notifyAttachedServices.args[0]; assert.equal(args.length, 3, 'log.notifyAttachedServices was passed three arguments'); @@ -458,10 +452,10 @@ describe('lib/devices:', () => { assert.equal(log.activityEvent.callCount, 1, 'log.activityEvent was called once'); assert.equal(log.activityEvent.args[0][0].is_placeholder, true, 'is_placeholder was correct'); - assert.equal(log.info.callCount, 1, 'log.info was called once'); - assert.equal(log.info.args[0].length, 2); - assert.equal(log.info.args[0][0], 'device:createPlaceholder'); - assert.deepEqual(log.info.args[0][1], { + assert.equal(log.info.callCount, 2); + assert.equal(log.info.args[1].length, 2); + assert.equal(log.info.args[1][0], 'device:createPlaceholder'); + assert.deepEqual(log.info.args[1][1], { uid: credentials.uid, id: result.id }, 'argument was event data'); @@ -512,8 +506,6 @@ describe('lib/devices:', () => { is_placeholder: false }, 'event data was correct'); - assert.equal(log.info.callCount, 0, 'log.info was not called'); - assert.equal(log.notifyAttachedServices.callCount, 0, 'log.notifyAttachedServices was not called'); assert.equal(push.notifyDeviceConnected.callCount, 0, 'push.notifyDeviceConnected was not called'); diff --git a/test/local/email/bounce.js b/test/local/email/bounce.js index bce05dcd..f1e6c047 100644 --- a/test/local/email/bounce.js +++ b/test/local/email/bounce.js @@ -93,9 +93,9 @@ describe('bounce messages', () => { assert.equal(mockDB.deleteAccount.callCount, 2); assert.equal(mockDB.accountRecord.args[0][0], 'test@example.com'); assert.equal(mockDB.accountRecord.args[1][0], 'foobar@example.com'); - assert.equal(log.info.callCount, 6); - assert.equal(log.info.args[5][0], 'accountDeleted'); - assert.equal(log.info.args[5][1].email, 'foobar@example.com'); + assert.equal(log.info.callCount, 7); + assert.equal(log.info.args[6][0], 'accountDeleted'); + assert.equal(log.info.args[6][1].email, 'foobar@example.com'); assert.equal(mockMsg.del.callCount, 1); }); }); diff --git a/test/local/ip_profiling.js b/test/local/ip_profiling.js index da88c23b..19affeb7 100644 --- a/test/local/ip_profiling.js +++ b/test/local/ip_profiling.js @@ -43,7 +43,7 @@ function makeRoutes (options = {}, requireMocks) { customs, signinUtils, mocks.mockPush(), - mocks.mockDevices() + mocks.mockVerificationReminders(), ); } diff --git a/test/local/metrics/amplitude.js b/test/local/metrics/amplitude.js index c53e345e..36545df8 100644 --- a/test/local/metrics/amplitude.js +++ b/test/local/metrics/amplitude.js @@ -19,11 +19,11 @@ describe('metrics/amplitude', () => { }); it('throws if log argument is missing', () => { - assert.throws(() => amplitudeModule(null, { oauth: { clientIds: {} } })); + assert.throws(() => amplitudeModule(null, { oauth: { clientIds: {} }, verificationReminders: {} })); }); it('throws if config argument is missing', () => { - assert.throws(() => amplitudeModule({}, { oauth: { clientIds: null } })); + assert.throws(() => amplitudeModule({}, { oauth: { clientIds: null }, verificationReminders: {} })); }); describe('instantiate', () => { @@ -37,7 +37,12 @@ describe('metrics/amplitude', () => { 0: 'amo', 1: 'pocket' } - } + }, + verificationReminders: { + firstInterval: 1000, + secondInterval: 2000, + thirdInterval: 3000, + }, }); }); @@ -826,6 +831,131 @@ describe('metrics/amplitude', () => { }); }); + describe('email.verificationReminderFirstEmail.bounced', () => { + beforeEach(() => { + return amplitude('email.verificationReminderFirstEmail.bounced', mocks.mockRequest({})); + }); + + it('did not call log.error', () => { + assert.equal(log.error.callCount, 0); + }); + + it('called log.amplitudeEvent correctly', () => { + assert.equal(log.amplitudeEvent.callCount, 1); + const args = log.amplitudeEvent.args[0]; + assert.equal(args[0].event_type, 'fxa_email - bounced'); + assert.equal(args[0].event_properties.email_type, 'registration'); + }); + }); + + describe('email.verificationReminderFirstEmail.sent', () => { + beforeEach(() => { + return amplitude('email.verificationReminderFirstEmail.sent', mocks.mockRequest({}), { + templateVersion: 1, + }); + }); + + it('did not call log.error', () => { + assert.equal(log.error.callCount, 0); + }); + + it('called log.amplitudeEvent correctly', () => { + assert.equal(log.amplitudeEvent.callCount, 1); + const args = log.amplitudeEvent.args[0]; + assert.equal(args[0].event_type, 'fxa_email - sent'); + assert.equal(args[0].event_properties.email_type, 'registration'); + assert.equal(args[0].event_properties.email_template, 'verificationReminderFirstEmail'); + }); + }); + + describe('email.verificationReminderSecondEmail.bounced', () => { + beforeEach(() => { + return amplitude('email.verificationReminderSecondEmail.bounced', mocks.mockRequest({})); + }); + + it('did not call log.error', () => { + assert.equal(log.error.callCount, 0); + }); + + it('called log.amplitudeEvent correctly', () => { + assert.equal(log.amplitudeEvent.callCount, 1); + const args = log.amplitudeEvent.args[0]; + assert.equal(args[0].event_type, 'fxa_email - bounced'); + assert.equal(args[0].event_properties.email_type, 'registration'); + }); + }); + + describe('email.verificationReminderSecondEmail.sent', () => { + beforeEach(() => { + return amplitude('email.verificationReminderSecondEmail.sent', mocks.mockRequest({})); + }); + + it('did not call log.error', () => { + assert.equal(log.error.callCount, 0); + }); + + it('called log.amplitudeEvent correctly', () => { + assert.equal(log.amplitudeEvent.callCount, 1); + const args = log.amplitudeEvent.args[0]; + assert.equal(args[0].event_type, 'fxa_email - sent'); + assert.equal(args[0].event_properties.email_type, 'registration'); + }); + }); + + describe('email.verificationReminderThirdEmail.bounced', () => { + beforeEach(() => { + return amplitude('email.verificationReminderThirdEmail.bounced', mocks.mockRequest({})); + }); + + it('did not call log.error', () => { + assert.equal(log.error.callCount, 0); + }); + + it('called log.amplitudeEvent correctly', () => { + assert.equal(log.amplitudeEvent.callCount, 1); + const args = log.amplitudeEvent.args[0]; + assert.equal(args[0].event_type, 'fxa_email - bounced'); + assert.equal(args[0].event_properties.email_type, 'registration'); + }); + }); + + describe('email.verificationReminderThirdEmail.sent', () => { + beforeEach(() => { + return amplitude('email.verificationReminderThirdEmail.sent', mocks.mockRequest({})); + }); + + it('did not call log.error', () => { + assert.equal(log.error.callCount, 0); + }); + + it('called log.amplitudeEvent correctly', () => { + assert.equal(log.amplitudeEvent.callCount, 1); + const args = log.amplitudeEvent.args[0]; + assert.equal(args[0].event_type, 'fxa_email - sent'); + assert.equal(args[0].event_properties.email_type, 'registration'); + }); + }); + + describe('email.verificationReminderFourthEmail.bounced', () => { + beforeEach(() => { + return amplitude('email.verificationReminderFourthEmail.bounced', mocks.mockRequest({})); + }); + + it('did not call log.amplitudeEvent', () => { + assert.equal(log.amplitudeEvent.callCount, 0); + }); + }); + + describe('email.verificationReminderFourthEmail.sent', () => { + beforeEach(() => { + return amplitude('email.verificationReminderFourthEmail.sent', mocks.mockRequest({})); + }); + + it('did not call log.amplitudeEvent', () => { + assert.equal(log.amplitudeEvent.callCount, 0); + }); + }); + describe('email.verifyEmail.bounced', () => { beforeEach(() => { return amplitude('email.verifyEmail.bounced', mocks.mockRequest({})); diff --git a/test/local/metrics/events.js b/test/local/metrics/events.js index b39db366..22ddc872 100644 --- a/test/local/metrics/events.js +++ b/test/local/metrics/events.js @@ -10,12 +10,14 @@ const log = { activityEvent: sinon.spy(), amplitudeEvent: sinon.spy(), error: sinon.spy(), - flowEvent: sinon.spy() + flowEvent: sinon.spy(), + info: sinon.spy(), }; const events = require('../../../lib/metrics/events')(log, { oauth: { clientIds: {} - } + }, + verificationReminders: {}, }); const mocks = require('../../mocks'); const P = require('../../../lib/promise'); diff --git a/test/local/routes/account.js b/test/local/routes/account.js index c9737102..6353619e 100644 --- a/test/local/routes/account.js +++ b/test/local/routes/account.js @@ -51,6 +51,7 @@ const makeRoutes = function (options = {}, requireMocks) { signinUtils.checkPassword = options.checkPassword; } const push = options.push || require('../../../lib/push')(log, db, {}); + const verificationReminders = options.verificationReminders || mocks.mockVerificationReminders(); return proxyquire('../../../lib/routes/account', requireMocks || {})( log, db, @@ -59,7 +60,8 @@ const makeRoutes = function (options = {}, requireMocks) { config, customs, signinUtils, - push + push, + verificationReminders, ); }; @@ -384,6 +386,7 @@ describe('/account/create', () => { }); const mockMailer = mocks.mockMailer(); const mockPush = mocks.mockPush(); + const verificationReminders = mocks.mockVerificationReminders(); const accountRoutes = makeRoutes({ config: { securityHistory: { @@ -403,7 +406,8 @@ describe('/account/create', () => { } }; }, - push: mockPush + push: mockPush, + verificationReminders, }); const route = getRoute(accountRoutes, '/account/create'); @@ -418,23 +422,26 @@ describe('/account/create', () => { mockRequest, route, sessionTokenId, - uid + uid, + verificationReminders, }; } it('should create a sync account', () => { - const mocked = setup(); - const clientAddress = mocked.clientAddress; - const emailCode = mocked.emailCode; - const keyFetchTokenId = mocked.keyFetchTokenId; - const mockDB = mocked.mockDB; - const mockLog = mocked.mockLog; - const mockMailer = mocked.mockMailer; - const mockMetricsContext = mocked.mockMetricsContext; - const mockRequest = mocked.mockRequest; - const route = mocked.route; - const sessionTokenId = mocked.sessionTokenId; - const uid = mocked.uid; + const { + clientAddress, + emailCode, + keyFetchTokenId, + mockDB, + mockLog, + mockMailer, + mockMetricsContext, + mockRequest, + route, + sessionTokenId, + uid, + verificationReminders, + } = setup(); const now = Date.now(); sinon.stub(Date, 'now').callsFake(() => now); @@ -560,17 +567,24 @@ describe('/account/create', () => { assert.equal(args[2].service, 'sync'); assert.equal(args[2].uid, uid); + assert.equal(verificationReminders.create.callCount, 1); + args = verificationReminders.create.args[0]; + assert.lengthOf(args, 1); + assert.equal(args[0], uid); + assert.equal(mockLog.error.callCount, 0); }).finally(() => Date.now.restore()); }); it('should create a non-sync account', () => { - const mocked = setup(); - const mockLog = mocked.mockLog; - const mockMailer = mocked.mockMailer; - const mockRequest = mocked.mockRequest; - const route = mocked.route; - const uid = mocked.uid; + const { + mockLog, + mockMailer, + mockRequest, + route, + uid, + verificationReminders, + } = setup(); const now = Date.now(); sinon.stub(Date, 'now').callsFake(() => now); @@ -598,15 +612,20 @@ describe('/account/create', () => { assert.equal(mockMailer.sendVerifyCode.callCount, 1, 'mailer.sendVerifyCode was called'); args = mockMailer.sendVerifyCode.args[0]; assert.equal(args[2].service, 'foo'); + + assert.equal(verificationReminders.create.callCount, 1); + assert.equal(mockLog.error.callCount, 0); }).finally(() => Date.now.restore()); }); it('should return an error if email fails to send', () => { - const mocked = setup(); - const mockMailer = mocked.mockMailer; - const mockRequest = mocked.mockRequest; - const route = mocked.route; + const { + mockMailer, + mockRequest, + route, + verificationReminders, + } = setup(); mockMailer.sendVerifyCode = sinon.spy(() => P.reject()); @@ -615,14 +634,18 @@ describe('/account/create', () => { assert.equal(err.output.payload.code, 422); assert.equal(err.output.payload.errno, 151); assert.equal(err.output.payload.error, 'Unprocessable Entity'); + + assert.equal(verificationReminders.create.callCount, 0); }); }); it('should return a bounce error if send fails with one', () => { - const mocked = setup(); - const mockMailer = mocked.mockMailer; - const mockRequest = mocked.mockRequest; - const route = mocked.route; + const { + mockMailer, + mockRequest, + route, + verificationReminders, + } = setup(); mockMailer.sendVerifyCode = sinon.spy(() => P.reject(error.emailBouncedHard(42))); @@ -631,6 +654,8 @@ describe('/account/create', () => { assert.equal(err.output.payload.code, 400); assert.equal(err.output.payload.errno, 134); assert.equal(err.output.payload.error, 'Bad Request'); + + assert.equal(verificationReminders.create.callCount, 0); }); }); }); @@ -1612,8 +1637,8 @@ describe('/account/destroy', () => { assert.equal(args[0].email, email, 'db.deleteAccount was passed email record'); assert.deepEqual(args[0].uid, uid, 'email record had correct uid'); - assert.equal(mockLog.info.callCount, 1); - args = mockLog.info.args[0]; + assert.equal(mockLog.info.callCount, 2); + args = mockLog.info.args[1]; assert.lengthOf(args, 2); assert.equal(args[0], 'accountDeleted.byRequest'); assert.equal(args[1].email, email); diff --git a/test/local/routes/emails.js b/test/local/routes/emails.js index 03e0e09c..5aa4a0d8 100644 --- a/test/local/routes/emails.js +++ b/test/local/routes/emails.js @@ -52,13 +52,15 @@ const makeRoutes = function (options = {}, requireMocks) { check: function () { return P.resolve(true); } }; const push = options.push || require('../../../lib/push')(log, db, {}); + const verificationReminders = options.verificationReminders || mocks.mockVerificationReminders(); return proxyquire('../../../lib/routes/emails', requireMocks || {})( log, db, options.mailer || {}, config, customs, - push + push, + verificationReminders, ); }; @@ -453,6 +455,7 @@ describe('/recovery_email/verify_code', () => { const mockMailer = mocks.mockMailer(); const mockPush = mocks.mockPush(); const mockCustoms = mocks.mockCustoms(); + const verificationReminders = mocks.mockVerificationReminders(); const accountRoutes = makeRoutes({ checkPassword: function () { return P.resolve(true); @@ -462,9 +465,22 @@ describe('/recovery_email/verify_code', () => { db: mockDB, log: mockLog, mailer: mockMailer, - push: mockPush + push: mockPush, + verificationReminders, }); const route = getRoute(accountRoutes, '/recovery_email/verify_code'); + + afterEach(() => { + mockDB.verifyTokens.resetHistory(); + mockDB.verifyEmail.resetHistory(); + mockLog.activityEvent.resetHistory(); + mockLog.flowEvent.resetHistory(); + mockLog.notifyAttachedServices.resetHistory(); + mockMailer.sendPostVerifyEmail.resetHistory(); + mockPush.notifyAccountUpdated.resetHistory(); + verificationReminders.delete.resetHistory(); + }); + describe('verifyTokens rejects with INVALID_VERIFICATION_CODE', () => { it('without a reminder payload', () => { @@ -513,17 +529,13 @@ describe('/recovery_email/verify_code', () => { assert.ok(Array.isArray(args[1]), 'second argument should have been devices array'); assert.equal(args[2], 'accountVerify', 'third argument should have been reason'); + assert.equal(verificationReminders.delete.callCount, 1); + args = verificationReminders.delete.args[0]; + assert.lengthOf(args, 1); + assert.equal(args[0], uid); + assert.equal(JSON.stringify(response), '{}'); - }) - .then(() => { - mockDB.verifyTokens.resetHistory(); - mockDB.verifyEmail.resetHistory(); - mockLog.activityEvent.resetHistory(); - mockLog.flowEvent.resetHistory(); - mockLog.notifyAttachedServices.resetHistory(); - mockMailer.sendPostVerifyEmail.resetHistory(); - mockPush.notifyAccountUpdated.resetHistory(); - }); + }); }); it('with marketingOptIn', () => { @@ -542,43 +554,26 @@ describe('/recovery_email/verify_code', () => { assert.equal(args[0].user_properties.newsletter_state, 'subscribed', 'newsletter_state was correct'); assert.equal(JSON.stringify(response), '{}'); - }) - .then(() => { - delete mockRequest.payload.marketingOptIn; - mockDB.verifyTokens.resetHistory(); - mockDB.verifyEmail.resetHistory(); - mockLog.activityEvent.resetHistory(); - mockLog.flowEvent.resetHistory(); - mockLog.notifyAttachedServices.resetHistory(); - mockMailer.sendPostVerifyEmail.resetHistory(); - mockPush.notifyAccountUpdated.resetHistory(); - }); + }); }); it('with a reminder payload', () => { mockRequest.payload.reminder = 'second'; return runTest(route, mockRequest, (response) => { - assert.equal(mockLog.activityEvent.callCount, 1, 'activityEvent was called once'); + assert.equal(mockLog.activityEvent.callCount, 1); - assert.equal(mockLog.flowEvent.callCount, 2, 'flowEvent was called twice'); - assert.equal(mockLog.flowEvent.args[0][0].event, 'email.verify_code.clicked', 'first event was email.verify_code.clicked'); - assert.equal(mockLog.flowEvent.args[1][0].event, 'account.verified', 'second event was account.verified'); + assert.equal(mockLog.flowEvent.callCount, 3); + assert.equal(mockLog.flowEvent.args[0][0].event, 'email.verify_code.clicked'); + assert.equal(mockLog.flowEvent.args[1][0].event, 'account.verified'); + assert.equal(mockLog.flowEvent.args[2][0].event, 'account.reminder.second'); - assert.equal(mockMailer.sendPostVerifyEmail.callCount, 1, 'sendPostVerifyEmail was called once'); - assert.equal(mockPush.notifyAccountUpdated.callCount, 1, 'mockPush.notifyAccountUpdated should have been called once'); + assert.equal(verificationReminders.delete.callCount, 1); + assert.equal(mockMailer.sendPostVerifyEmail.callCount, 1); + assert.equal(mockPush.notifyAccountUpdated.callCount, 1); assert.equal(JSON.stringify(response), '{}'); - }) - .then(() => { - mockDB.verifyTokens.resetHistory(); - mockDB.verifyEmail.resetHistory(); - mockLog.activityEvent.resetHistory(); - mockLog.flowEvent.resetHistory(); - mockLog.notifyAttachedServices.resetHistory(); - mockMailer.sendPostVerifyEmail.resetHistory(); - mockPush.notifyAccountUpdated.resetHistory(); - }); + }); }); }); @@ -597,10 +592,7 @@ describe('/recovery_email/verify_code', () => { assert.equal(mockLog.activityEvent.callCount, 0, 'log.activityEvent was not called'); assert.equal(mockPush.notifyAccountUpdated.callCount, 0, 'mockPush.notifyAccountUpdated should not have been called'); assert.equal(mockPush.notifyDeviceConnected.callCount, 0, 'mockPush.notifyDeviceConnected should not have been called (no devices)'); - }) - .then(() => { - mockDB.verifyTokens.resetHistory(); - }); + }); }); it('email verification with associated device', () => { @@ -618,10 +610,7 @@ describe('/recovery_email/verify_code', () => { assert.equal(mockLog.activityEvent.callCount, 0, 'log.activityEvent was not called'); assert.equal(mockPush.notifyAccountUpdated.callCount, 0, 'mockPush.notifyAccountUpdated should not have been called'); assert.equal(mockPush.notifyDeviceConnected.callCount, 1, 'mockPush.notifyDeviceConnected should have been called'); - }) - .then(() => { - mockDB.verifyTokens.resetHistory(); - }); + }); }); it('sign-in confirmation', () => { @@ -650,12 +639,7 @@ describe('/recovery_email/verify_code', () => { assert.equal(args[0].toString('hex'), uid, 'first argument should have been uid'); assert.ok(Array.isArray(args[1]), 'second argument should have been devices array'); assert.equal(args[2], 'accountConfirm', 'third argument should have been reason'); - }) - .then(() => { - mockDB.verifyTokens.resetHistory(); - mockLog.activityEvent.resetHistory(); - mockPush.notifyAccountUpdated.resetHistory(); - }); + }); }); it('secondary email verification', () => { @@ -678,13 +662,7 @@ describe('/recovery_email/verify_code', () => { assert.equal(args[2].secondaryEmail, dbData.secondEmail, 'correct secondary email was passed'); assert.equal(args[2].service, mockRequest.payload.service); assert.equal(args[2].uid, uid); - }) - .then(() => { - mockDB.verifyEmail.resetHistory(); - mockLog.activityEvent.resetHistory(); - mockMailer.sendPostVerifySecondaryEmail.resetHistory(); - mockPush.notifyAccountUpdated.resetHistory(); - }); + }); }); }); }); @@ -799,8 +777,8 @@ describe('/recovery_email', () => { assert.equal(args[1].email, TEST_EMAIL_ADDITIONAL, 'call db.createEmail with correct email'); assert.equal(mockMailer.sendVerifySecondaryEmail.callCount, 1, 'call mailer.sendVerifySecondaryEmail'); - assert.equal(mockLog.info.callCount, 1); - args = mockLog.info.args[0]; + assert.equal(mockLog.info.callCount, 5); + args = mockLog.info.args[4]; assert.lengthOf(args, 2); assert.equal(args[0], 'accountDeleted.unverifiedSecondaryEmail'); assert.equal(args[1].normalizedEmail, TEST_EMAIL); diff --git a/test/local/routes/recovery-codes.js b/test/local/routes/recovery-codes.js index 861d91d9..c440b437 100644 --- a/test/local/routes/recovery-codes.js +++ b/test/local/routes/recovery-codes.js @@ -63,8 +63,8 @@ describe('recovery codes', () => { assert.equal(args[0], UID, 'called with uid'); assert.equal(args[1], 8, 'called with recovery code count'); - assert.equal(log.info.callCount, 1); - args = log.info.args[0]; + assert.equal(log.info.callCount, 2); + args = log.info.args[1]; assert.equal(args[0], 'account.recoveryCode.replaced'); assert.equal(args[1].uid, UID); }); diff --git a/test/local/routes/token-codes.js b/test/local/routes/token-codes.js index 34e918db..376d065b 100644 --- a/test/local/routes/token-codes.js +++ b/test/local/routes/token-codes.js @@ -45,8 +45,8 @@ describe('/session/verify/token', () => { }); it('called log.info correctly', () => { - assert.equal(log.info.callCount, 1); - const args = log.info.args[0]; + assert.equal(log.info.callCount, 2); + const args = log.info.args[1]; assert.equal(args.length, 2); assert.equal(args[0], 'account.token.code.verified'); }); diff --git a/test/local/routes/utils/signin.js b/test/local/routes/utils/signin.js index 58bfa1bf..a69aaaaa 100644 --- a/test/local/routes/utils/signin.js +++ b/test/local/routes/utils/signin.js @@ -399,8 +399,8 @@ describe('checkCustomsAndLoadAccount', () => { assert.calledWithExactly(request.emitMetricsEvent.getCall(0), 'account.login.blocked'); assert.calledWithExactly(request.emitMetricsEvent.getCall(1), 'account.login.invalidUnblockCode'); - assert.calledOnce(log.info); - assert.calledWithMatch(log.info, 'Account.login.unblockCode.expired'); + assert.equal(log.info.callCount, 2); + assert.equal(log.info.args[1][0], 'Account.login.unblockCode.expired'); assert.calledOnce(customs.flag); assert.calledWithExactly(customs.flag, CLIENT_ADDRESS, { diff --git a/test/local/senders/email.js b/test/local/senders/email.js index 3be097aa..bc2d0793 100644 --- a/test/local/senders/email.js +++ b/test/local/senders/email.js @@ -35,6 +35,8 @@ const messageTypes = [ 'postVerifySecondaryEmail', 'recoveryEmail', 'unblockCodeEmail', + 'verificationReminderFirstEmail', + 'verificationReminderSecondEmail', 'verifyEmail', 'verifyLoginEmail', 'verifyLoginCodeEmail', @@ -56,6 +58,8 @@ const typesContainSupportLinks = [ 'postRemoveTwoStepAuthenticationEmail', 'postVerifyEmail', 'recoveryEmail', + 'verificationReminderFirstEmail', + 'verificationReminderSecondEmail', 'verifyEmail', 'verifyLoginCodeEmail', 'verifyPrimaryEmail', @@ -251,7 +255,7 @@ describe( }; it( - `Contains template header for ${ type}`, + `Contains template header for ${type}`, () => { mailer.mailer.sendMail = stubSendMail(emailConfig => { assert.equal(emailConfig.from, config.get('smtp.sender'), 'from header is correct'); @@ -286,7 +290,7 @@ describe( }); it( - `test privacy link is in email template output for ${ type}`, + `test privacy link is in email template output for ${type}`, () => { const privacyLink = mailer.createPrivacyLink(type); @@ -300,7 +304,7 @@ describe( if (type === 'verifySecondaryEmail') { it( - `contains correct type ${ type}`, + `contains correct type ${type}`, () => { mailer.mailer.sendMail = stubSendMail(emailConfig => { assert.ok(includes(emailConfig.headers['X-Link'], 'type=secondary')); @@ -316,7 +320,7 @@ describe( } it( - `If sesConfigurationSet is not defined, then outgoing email does not contain X-SES* headers, for type ${ type}`, + `If sesConfigurationSet is not defined, then outgoing email does not contain X-SES* headers, for type ${type}`, () => { assert.ok('sesConfigurationSet' in mailer, 'configuration key exists'); mailer.mailer.sendMail = stubSendMail(emailConfig => { @@ -331,7 +335,7 @@ describe( ); it( - `If sesConfigurationSet is defined, then outgoing email will contain X-SES* headers, for type ${ type}`, + `If sesConfigurationSet is defined, then outgoing email will contain X-SES* headers, for type ${type}`, () => { assert.ok('sesConfigurationSet' in mailer, 'configuration key exists'); const savedSesConfigurationSet = mailer.sesConfigurationSet; @@ -354,7 +358,7 @@ describe( if (includes(typesContainSupportLinks, type)) { it( - `test support link is in email template output for ${ type}`, + `test support link is in email template output for ${type}`, () => { const supportTextLink = mailer.createSupportLink(type); @@ -369,7 +373,7 @@ describe( if (includes(typesContainPasswordResetLinks, type)) { it( - `reset password link is in email template output for ${ type}`, + `reset password link is in email template output for ${type}`, () => { const resetPasswordLink = mailer.createPasswordResetLink(message.email, type); @@ -386,7 +390,7 @@ describe( if (includes(typesContainPasswordChangeLinks, type)) { it( - `password change link is in email template output for ${ type}`, + `password change link is in email template output for ${type}`, () => { const passwordChangeLink = mailer.createPasswordChangeLink(message.email, type); @@ -403,7 +407,7 @@ describe( if (includes(typesContainUnblockCode, type)) { it( - `unblock code is in email template output for ${ type}`, + `unblock code is in email template output for ${type}`, () => { mailer.mailer.sendMail = stubSendMail(emailConfig => { assert.ok(includes(emailConfig.html, message.unblockCode)); @@ -428,7 +432,7 @@ describe( if (includes(typesContainTokenCode, type)) { it( - `login code is in email template output for ${ type}`, + `login code is in email template output for ${type}`, () => { mailer.mailer.sendMail = stubSendMail(emailConfig => { assert.ok(includes(emailConfig.html, message.tokenCode)); @@ -441,7 +445,7 @@ describe( if (includes(typesContainReportSignInLinks, type)) { it( - `report sign-in link is in email template output for ${ type}`, + `report sign-in link is in email template output for ${type}`, () => { mailer.mailer.sendMail = stubSendMail(emailConfig => { const reportSignInLink = @@ -457,7 +461,7 @@ describe( } if (includes(typesContainRevokeAccountRecoveryLinks, type)) { - it(`revoke account recovery link is in email template output for ${ type}`, () => { + it(`revoke account recovery link is in email template output for ${type}`, () => { mailer.mailer.sendMail = stubSendMail(emailConfig => { const link = mailer.createRevokeAccountRecoveryLink(type, message); assert.ok(includes(emailConfig.html, link)); @@ -470,7 +474,7 @@ describe( } if (includes(typesContainCreateAccountRecoveryLinks, type)) { - it(`create account recovery link is in email template output for ${ type}`, () => { + it(`create account recovery link is in email template output for ${type}`, () => { mailer.mailer.sendMail = stubSendMail(emailConfig => { const link = mailer._generateCreateAccountRecoveryLinks(message, type).link; assert.ok(includes(emailConfig.html, link)); @@ -484,7 +488,7 @@ describe( if (includes(typesContainAndroidStoreLinks, type)) { it( - `Android store link is in email template output for ${ type}`, + `Android store link is in email template output for ${type}`, () => { const androidStoreLink = mailer.androidUrl; @@ -500,7 +504,7 @@ describe( if (includes(typesContainIOSStoreLinks, type)) { it( - `IOS store link is in email template output for ${ type}`, + `IOS store link is in email template output for ${type}`, () => { const iosStoreLink = mailer.iosUrl; @@ -516,7 +520,7 @@ describe( if (includes(typesContainPasswordManagerInfoLinks, type)) { it( - `password manager info link is in email template output for ${ type}`, + `password manager info link is in email template output for ${type}`, () => { const passwordManagerInfoUrl = mailer._generateLinks(config.get('smtp').passwordManagerInfoUrl, message.email, {}, type).passwordManagerInfoUrl; @@ -532,7 +536,7 @@ describe( } if (includes(typesContainManageSettingsLinks, type)) { - it(`account settings info link is in email template output for ${ type}`, () => { + it(`account settings info link is in email template output for ${type}`, () => { const accountSettingsUrl = mailer._generateSettingLinks(message, type).link; mailer.mailer.sendMail = stubSendMail(emailConfig => { @@ -546,7 +550,7 @@ describe( } if (includes(typesContainRecoveryCodeLinks, type)) { - it(`recovery code settings info link is in email template output for ${ type}`, () => { + it(`recovery code settings info link is in email template output for ${type}`, () => { const url = mailer._generateLowRecoveryCodesLinks(message, type).link; mailer.mailer.sendMail = stubSendMail(emailConfig => { @@ -569,7 +573,7 @@ describe( if (type === 'verifySecondaryEmail') { it( - `original user email data is in template for ${ type}`, + `original user email data is in template for ${type}`, () => { const message = getLocationMessage(defaultLocation); message.primaryEmail = 'user@email.com'; @@ -585,7 +589,7 @@ describe( } it( - `ip data is in template for ${ type}`, + `ip data is in template for ${type}`, () => { const message = getLocationMessage(defaultLocation); @@ -599,21 +603,21 @@ describe( ); it( - `location is correct with city, country, stateCode for ${ type}`, + `location is correct with city, country, stateCode for ${type}`, () => { const location = defaultLocation; const message = getLocationMessage(defaultLocation); mailer.mailer.sendMail = stubSendMail(emailConfig => { - assert.ok(includes(emailConfig.html, `${location.city }, ${ location.stateCode }, ${ location.country}`)); - assert.ok(includes(emailConfig.text, `${location.city }, ${ location.stateCode }, ${ location.country}`)); + assert.ok(includes(emailConfig.html, `${location.city}, ${location.stateCode}, ${location.country}`)); + assert.ok(includes(emailConfig.text, `${location.city}, ${location.stateCode}, ${location.country}`)); }); return mailer[type](message); } ); it( - `location is correct with city, country for ${ type}`, + `location is correct with city, country for ${type}`, () => { const location = Object.assign({}, defaultLocation); delete location.stateCode; @@ -621,30 +625,30 @@ describe( mailer.mailer.sendMail = stubSendMail(emailConfig => { - assert.ok(includes(emailConfig.html, `${location.city }, ${ location.country}`)); - assert.ok(includes(emailConfig.text, `${location.city }, ${ location.country}`)); + assert.ok(includes(emailConfig.html, `${location.city}, ${location.country}`)); + assert.ok(includes(emailConfig.text, `${location.city}, ${location.country}`)); }); return mailer[type](message); } ); it( - `location is correct with stateCode, country for ${ type}`, + `location is correct with stateCode, country for ${type}`, () => { const location = Object.assign({}, defaultLocation); delete location.city; const message = getLocationMessage(location); mailer.mailer.sendMail = stubSendMail(emailConfig => { - assert.ok(includes(emailConfig.html, `${location.stateCode }, ${ location.country}`)); - assert.ok(includes(emailConfig.text, `${location.stateCode }, ${ location.country}`)); + assert.ok(includes(emailConfig.html, `${location.stateCode}, ${location.country}`)); + assert.ok(includes(emailConfig.text, `${location.stateCode}, ${location.country}`)); }); return mailer[type](message); } ); it( - `location is correct with country for ${ type}`, + `location is correct with country for ${type}`, () => { const location = Object.assign({}, defaultLocation); delete location.city; @@ -661,7 +665,7 @@ describe( ); it( - `device name is correct for ${ type}`, + `device name is correct for ${type}`, () => { const message = getLocationMessage(defaultLocation); message.uaBrowser = 'Firefox'; @@ -716,10 +720,9 @@ describe( }); } - if (type === 'verifyEmail') { - it( - 'passes the OAuth relier name to the template', - () => { + switch (type) { + case 'verifyEmail': + it('passes the OAuth relier name to the template', () => { mailer.mailer.sendMail = stubSendMail(emailConfig => { assert.equal(oauthClientInfo.fetch.callCount, 1); assert.equal(oauthClientInfo.fetch.args[0][0], 'foo'); @@ -728,11 +731,8 @@ describe( }); message.service = 'foo'; return mailer[type](message); - } - ); - it( - 'works without a service', - () => { + }); + it('works without a service', () => { mailer.mailer.sendMail = stubSendMail(emailConfig => { assert.isFalse(oauthClientInfo.fetch.called); assert.ok(! includes(emailConfig.html, 'and continue to')); @@ -740,12 +740,11 @@ describe( }); delete message.service; return mailer[type](message); - } - ); - } else if (type === 'verifyLoginEmail') { - it( - 'test verify token email', - () => { + }); + break; + + case 'verifyLoginEmail': + it('test verify token email', () => { mailer.mailer.sendMail = stubSendMail(emailConfig => { const verifyLoginUrl = config.get('smtp').verifyLoginUrl; assert.equal(emailConfig.subject, 'Confirm new sign-in to Firefox'); @@ -753,22 +752,20 @@ describe( assert.ok(emailConfig.text.indexOf(verifyLoginUrl) > 0); }); return mailer[type](message); - } - ); - } else if (type === 'newDeviceLoginEmail') { - it( - 'test new device login email', - () => { + }); + break; + + case 'newDeviceLoginEmail': + it('test new device login email', () => { mailer.mailer.sendMail = stubSendMail(emailConfig => { assert.equal(emailConfig.subject, 'New sign-in to Firefox'); }); return mailer[type](message); - } - ); - } else if (type === 'postVerifyEmail') { - it( - `test utm params for ${ type}`, - () => { + }); + break; + + case 'postVerifyEmail': + it(`test utm params for ${type}`, () => { const syncLink = mailer._generateUTMLink(config.get('smtp').syncUrl, {}, type, 'connect-device'); const androidLink = mailer._generateUTMLink(config.get('smtp').androidUrl, {}, type, 'connect-android'); const iosLink = mailer._generateUTMLink(config.get('smtp').iosUrl, {}, type, 'connect-ios'); @@ -780,19 +777,51 @@ describe( assert.ok(includes(emailConfig.html, 'utm_source=email')); }); return mailer[type](message); - } - ); - } else if (type === 'verifyPrimaryEmail') { - it('test verify token email', () => { - mailer.mailer.sendMail = stubSendMail(emailConfig => { - const verifyPrimaryEmailUrl = config.get('smtp').verifyPrimaryEmailUrl; - assert.ok(emailConfig.html.indexOf(verifyPrimaryEmailUrl) > 0); - assert.ok(emailConfig.text.indexOf(verifyPrimaryEmailUrl) > 0); - assert.ok(! includes(emailConfig.html, 'utm_source=email')); - assert.ok(! includes(emailConfig.text, 'utm_source=email')); }); - return mailer[type](message); - }); + break; + + case 'verifyPrimaryEmail': + it('test verify token email', () => { + mailer.mailer.sendMail = stubSendMail(emailConfig => { + const verifyPrimaryEmailUrl = config.get('smtp').verifyPrimaryEmailUrl; + assert.ok(emailConfig.html.indexOf(verifyPrimaryEmailUrl) > 0); + assert.ok(emailConfig.text.indexOf(verifyPrimaryEmailUrl) > 0); + assert.ok(! includes(emailConfig.html, 'utm_source=email')); + assert.ok(! includes(emailConfig.text, 'utm_source=email')); + }); + return mailer[type](message); + }); + break; + + case 'verificationReminderFirstEmail': + it('emailConfig includes data specific to verificationReminderFirstEmail', () => { + mailer.mailer.sendMail = stubSendMail(emailConfig => { + assert.include(emailConfig.html, 'reminder=first'); + assert.include(emailConfig.text, 'reminder=first'); + assert.include(emailConfig.html, 'utm_campaign=fx-first-verification-reminder'); + assert.include(emailConfig.text, 'utm_campaign=fx-first-verification-reminder'); + assert.include(emailConfig.html, 'utm_content=fx-activate-oneclick'); + assert.include(emailConfig.text, 'utm_content=fx-activate'); + assert.equal(emailConfig.subject, 'Hello again'); + }); + return mailer[type](message); + }); + break; + + case 'verificationReminderSecondEmail': + it('emailConfig includes data specific to verificationReminderSecondEmail', () => { + mailer.mailer.sendMail = stubSendMail(emailConfig => { + assert.include(emailConfig.html, 'reminder=second'); + assert.include(emailConfig.text, 'reminder=second'); + assert.include(emailConfig.html, 'utm_campaign=fx-second-verification-reminder'); + assert.include(emailConfig.text, 'utm_campaign=fx-second-verification-reminder'); + assert.include(emailConfig.html, 'utm_content=fx-activate-oneclick'); + assert.include(emailConfig.text, 'utm_content=fx-activate'); + assert.equal(emailConfig.subject, 'Still there?'); + }); + return mailer[type](message); + }); + break; } } ); @@ -897,15 +926,15 @@ describe( return mailer.send(message) .then(() => { - assert.equal(mockLog.info.callCount, 3); - const emailEventLog = mockLog.info.getCalls()[2]; + assert.equal(mockLog.info.callCount, 4); + const emailEventLog = mockLog.info.getCalls()[3]; assert.equal(emailEventLog.args[0], 'emailEvent'); assert.equal(emailEventLog.args[1].domain, 'other'); assert.equal(emailEventLog.args[1].flow_id, 'wibble'); assert.equal(emailEventLog.args[1].template, 'verifyLoginEmail'); assert.equal(emailEventLog.args[1].type, 'sent'); assert.equal(emailEventLog.args[1].locale, 'en'); - const mailerSend1 = mockLog.info.getCalls()[1]; + const mailerSend1 = mockLog.info.getCalls()[2]; assert.equal(mailerSend1.args[0], 'mailer.send.1'); assert.equal(mailerSend1.args[1].to, message.email); }); diff --git a/test/local/senders/index.js b/test/local/senders/index.js index bfaa42cd..8992a860 100644 --- a/test/local/senders/index.js +++ b/test/local/senders/index.js @@ -305,8 +305,8 @@ describe('lib/senders/index', () => { assert.equal(errorBounces.check.callCount, 2); assert.equal(e.errno, error.ERRNO.BOUNCE_COMPLAINT); - assert.ok(log.info.callCount >= 2); - const msg = log.info.args[1]; + assert.isAtLeast(log.info.callCount, 3); + const msg = log.info.args[2]; assert.equal(msg[0], 'mailer.blocked'); assert.equal(msg[1].errno, e.errno); assert.equal(msg[1].bouncedAt, DATE); @@ -364,8 +364,8 @@ describe('lib/senders/index', () => { assert.equal(errorBounces.check.callCount, 1); assert.equal(e.errno, error.ERRNO.BOUNCE_COMPLAINT); - assert.ok(log.info.callCount >= 2); - const msg = log.info.args[1]; + assert.isAtLeast(log.info.callCount, 3); + const msg = log.info.args[2]; assert.equal(msg[0], 'mailer.blocked'); assert.equal(msg[1].errno, e.errno); assert.equal(msg[1].bouncedAt, DATE); diff --git a/test/local/senders/templates.js b/test/local/senders/templates.js index 12682feb..96b08dd3 100644 --- a/test/local/senders/templates.js +++ b/test/local/senders/templates.js @@ -40,7 +40,7 @@ describe('lib/senders/templates/index:', () => { it('result is correct', () => { assert.equal(typeof result, 'object'); const keys = Object.keys(result); - assert.equal(keys.length, 25); + assert.equal(keys.length, 27); keys.forEach(key => { const fn = result[key]; assert.equal(typeof fn, 'function'); diff --git a/test/local/server.js b/test/local/server.js index a2be0a53..fad4f019 100644 --- a/test/local/server.js +++ b/test/local/server.js @@ -533,6 +533,7 @@ function getConfig () { metrics: { flow_id_expiry: 7200000, flow_id_key: 'wibble' - } + }, + verificationReminders: {}, }; } diff --git a/test/local/verification-reminders.js b/test/local/verification-reminders.js new file mode 100644 index 00000000..8c9675c8 --- /dev/null +++ b/test/local/verification-reminders.js @@ -0,0 +1,193 @@ +/* This Source Code Form is subject to the terms of the Mozilla Public + * License, v. 2.0. If a copy of the MPL was not distributed with this + * file, You can obtain one at https://mozilla.org/MPL/2.0/. */ + +'use strict'; + +const ROOT_DIR = '../..'; +const REMINDERS = [ 'first', 'second', 'third' ]; +const EXPECTED_CREATE_DELETE_RESULT = REMINDERS.reduce((expected, reminder) => { + expected[reminder] = 1; + return expected; +}, {}); + +const { assert } = require('chai'); +const config = require(`${ROOT_DIR}/config`).getProperties(); +const mocks = require('../mocks'); + +describe('lib/verification-reminders:', () => { + let log, mockConfig, redis, verificationReminders; + + beforeEach(() => { + log = mocks.mockLog(); + mockConfig = { + redis: config.redis, + verificationReminders: { + rolloutRate: 1, + firstInterval: 1, + secondInterval: 2, + thirdInterval: 60000, + redis: { + maxConnections: 1, + minConnections: 1, + prefix: 'test-verification-reminders:', + }, + }, + }; + redis = require('fxa-shared/redis')({ + ...config.redis, + ...mockConfig.verificationReminders.redis, + enabled: true, + }, mocks.mockLog()); + verificationReminders = require(`${ROOT_DIR}/lib/verification-reminders`)(log, mockConfig); + }); + + it('returned the expected interface', () => { + assert.isObject(verificationReminders); + assert.lengthOf(Object.keys(verificationReminders), 4); + + assert.deepEqual(verificationReminders.keys, [ 'first', 'second', 'third' ]); + + assert.isFunction(verificationReminders.create); + assert.lengthOf(verificationReminders.create, 1); + + assert.isFunction(verificationReminders.delete); + assert.lengthOf(verificationReminders.delete, 1); + + assert.isFunction(verificationReminders.process); + assert.lengthOf(verificationReminders.process, 0); + }); + + it('called log.info correctly', () => { + assert.equal(log.info.callCount, 1); + const args = log.info.args[0]; + assert.lengthOf(args, 2); + assert.equal(args[0], 'redis.enabled'); + assert.isObject(args[1]); + }); + + describe('create:', () => { + let createResult; + + beforeEach(async () => { + // Clobber keys to assert that misbehaving callers can't wreck the internal behaviour + verificationReminders.keys = []; + createResult = await verificationReminders.create('wibble'); + }); + + afterEach(() => { + return verificationReminders.delete('wibble'); + }); + + it('returned the correct result', async () => { + assert.deepEqual(createResult, EXPECTED_CREATE_DELETE_RESULT); + }); + + REMINDERS.forEach(reminder => { + it(`wrote ${reminder} reminder to redis correctly`, async () => { + const reminders = await redis.zrange(reminder, 0, -1); + assert.deepEqual(reminders, [ 'wibble' ]); + }); + }); + + it('called log.info correctly', () => { + assert.equal(log.info.callCount, 2); + const args = log.info.args[1]; + assert.lengthOf(args, 2); + assert.equal(args[0], 'verificationReminders.create'); + assert.deepEqual(args[1], { uid: 'wibble' }); + }); + + describe('delete:', () => { + let deleteResult; + + beforeEach(async () => { + deleteResult = await verificationReminders.delete('wibble'); + }); + + it('returned the correct result', async () => { + assert.deepEqual(deleteResult, EXPECTED_CREATE_DELETE_RESULT); + }); + + REMINDERS.forEach(reminder => { + it(`removed ${reminder} reminder from redis correctly`, async () => { + const reminders = await redis.zrange(reminder, 0, -1); + assert.lengthOf(reminders, 0); + }); + }); + + it('did not call log.error', () => { + assert.equal(log.error.callCount, 0); + }); + + it('called log.info correctly', () => { + assert.equal(log.info.callCount, 3); + const args = log.info.args[2]; + assert.lengthOf(args, 2); + assert.equal(args[0], 'verificationReminders.delete'); + assert.deepEqual(args[1], { uid: 'wibble' }); + }); + }); + + describe('process:', () => { + let processResult, after; + + beforeEach(done => { + setTimeout(async () => { + processResult = await verificationReminders.process(); + after = Date.now(); + setImmediate(done); + }, 2); + }); + + it('returned the correct result', async () => { + assert.deepEqual(processResult, { + first: [ 'wibble' ], + second: [ 'wibble' ], + third: [], + }); + }); + + REMINDERS.forEach(reminder => { + if (reminder !== 'third') { + it(`removed ${reminder} reminder from redis correctly`, async () => { + const reminders = await redis.zrange(reminder, 0, -1); + assert.lengthOf(reminders, 0); + }); + } else { + it('left the third reminder in redis', async () => { + const reminders = await redis.zrange(reminder, 0, -1); + assert.deepEqual(reminders, [ 'wibble' ]); + }); + } + }); + + it('did not call log.error', () => { + assert.equal(log.error.callCount, 0); + }); + + it('called log.info correctly', () => { + assert.equal(log.info.callCount, 5); + + let args = log.info.args[2]; + assert.lengthOf(args, 2); + assert.equal(args[0], 'verificationReminders.process'); + assert.isObject(args[1]); + assert.equal(args[1].key, 'first'); + assert.isAtMost(args[1].now, after); + assert.isAbove(args[1].now, after - 1000); + assert.equal(args[1].cutoff, args[1].now - mockConfig.verificationReminders.firstInterval); + + args = log.info.args[3]; + assert.equal(args[1].key, 'second'); + assert.equal(args[1].now, log.info.args[2][1].now); + assert.equal(args[1].cutoff, args[1].now - mockConfig.verificationReminders.secondInterval); + + args = log.info.args[4]; + assert.equal(args[1].key, 'third'); + assert.equal(args[1].now, log.info.args[2][1].now); + assert.equal(args[1].cutoff, args[1].now - mockConfig.verificationReminders.thirdInterval); + }); + }); + }); +}); diff --git a/test/mocks.js b/test/mocks.js index 4b199b56..6aceb89d 100644 --- a/test/mocks.js +++ b/test/mocks.js @@ -171,7 +171,8 @@ module.exports = { mockMetricsContext, mockPush, mockPushbox, - mockRequest + mockRequest, + mockVerificationReminders, }; function mockCustoms (errors) { @@ -543,7 +544,8 @@ function mockRequest (data, errors) { const events = require('../lib/metrics/events')(data.log || module.exports.mockLog(), { oauth: { clientIds: data.clientIds || {} - } + }, + verificationReminders: {}, }); const metricsContext = data.metricsContext || module.exports.mockMetricsContext(); @@ -612,3 +614,12 @@ function mockRequest (data, errors) { validateMetricsContext: metricsContext.validate }; } + +function mockVerificationReminders (data = {}) { + return { + keys: [ 'first', 'second', 'third' ], + create: sinon.spy(() => data.create || { first: 1, second: 1, third: 1 }), + delete: sinon.spy(() => data.delete || { first: 1, second: 1, third: 1 }), + process: sinon.spy(() => data.process || { first: [], second: [], third: [] }), + }; +} diff --git a/test/remote/redis_tests.js b/test/remote/redis_tests.js index 4ad312f7..ee5649bf 100644 --- a/test/remote/redis_tests.js +++ b/test/remote/redis_tests.js @@ -161,7 +161,7 @@ describe('reentrant updates of different keys:', () => { let error; before(() => { - const redisPool = require('fxa-shared/redis/pool')({ + const { pool: redisPool } = require('fxa-shared/redis/pool')({ ...config.redis, ...config.redis.sessionTokens }, log);