From 5007b4da57a2a9b2d4d28d78e091f7579a40b31e Mon Sep 17 00:00:00 2001 From: Vlad Filippov Date: Tue, 27 Oct 2015 23:26:33 -0400 Subject: [PATCH] feat(email): add verification reminders Fixes #1081 --- .env.dev | 1 + config/index.js | 7 + lib/db.js | 20 +++ lib/pool.js | 4 +- lib/routes/account.js | 19 ++- lib/verification-reminders.js | 78 +++++++++++ npm-shrinkwrap.json | 2 +- test/local/pool_tests.js | 5 +- test/local/verification_reminder_db_tests.js | 123 +++++++++++++++++ test/local/verification_reminder_tests.js | 137 +++++++++++++++++++ 10 files changed, 390 insertions(+), 6 deletions(-) create mode 100644 lib/verification-reminders.js create mode 100644 test/local/verification_reminder_db_tests.js create mode 100644 test/local/verification_reminder_tests.js diff --git a/.env.dev b/.env.dev index 9e36d69ba..22b6bbde3 100644 --- a/.env.dev +++ b/.env.dev @@ -11,3 +11,4 @@ LOCKOUT_ENABLED=true SNS_TOPIC_ARN=disabled TRUSTED_JKUS=http://127.0.0.1:8080/.well-known/public-keys,http://127.0.0.1:10139/.well-known/public-keys STATSD_SAMPLE_RATE=1 +VERIFICATION_REMINDER_RATE=1 diff --git a/config/index.js b/config/index.js index 738c3aa97..940f9daa0 100644 --- a/config/index.js +++ b/config/index.js @@ -281,6 +281,13 @@ var conf = convict({ default: '' } }, + verificationReminders: { + rate: { + doc: 'Rate of users getting the verification reminder. If "0" then the feature is disabled. If "1" all users get it.', + default: 0, + env: 'VERIFICATION_REMINDER_RATE' + } + }, useHttps: { doc: 'set to true to serve directly over https', env: 'USE_TLS', diff --git a/lib/db.js b/lib/db.js index 8643c0dac..b84b00da2 100644 --- a/lib/db.js +++ b/lib/db.js @@ -758,6 +758,26 @@ module.exports = function ( ) } + // VERIFICATION REMINDERS + + DB.prototype.createVerificationReminder = function (reminderData) { + log.trace({ + op: 'DB.createVerificationReminder', + reminderData: reminderData + }) + + return this.pool.post('/verificationReminders', reminderData) + } + + DB.prototype.deleteVerificationReminder = function (reminderData) { + log.trace({ + op: 'DB.deleteVerificationReminder', + reminderData: reminderData + }) + + return this.pool.del('/verificationReminders', reminderData) + } + return DB } diff --git a/lib/pool.js b/lib/pool.js index bfb897352..a8f33e686 100644 --- a/lib/pool.js +++ b/lib/pool.js @@ -83,8 +83,8 @@ Pool.prototype.get = function (path) { return this.request('GET', path) } -Pool.prototype.del = function (path) { - return this.request('DELETE', path) +Pool.prototype.del = function (path, data) { + return this.request('DELETE', path, data) } Pool.prototype.head = function (path) { diff --git a/lib/routes/account.js b/lib/routes/account.js index 69db14272..097e7f280 100644 --- a/lib/routes/account.js +++ b/lib/routes/account.js @@ -40,6 +40,8 @@ module.exports = function ( ) ] + var verificationReminder = require('../verification-reminders')(log, db) + function isOpenIdProviderAllowed(id) { if (typeof(id) !== 'string') { return false } var hostname = url.parse(id).hostname @@ -223,7 +225,16 @@ module.exports = function ( redirectTo: form.redirectTo, resume: form.resume, acceptLanguage: request.app.acceptLanguage - }).catch(function (err) { + }) + .then(function () { + // only create reminder if sendVerifyCode succeeds + verificationReminder.create({ + uid: account.uid.toString('hex') + }).catch(function (err) { + log.error({ op: 'Account.verificationReminder.create', err: err }) + }) + }) + .catch(function (err) { log.error({ op: 'mailer.sendVerifyCode.1', err: err }) }) } @@ -1081,6 +1092,12 @@ module.exports = function ( // send a push notification to all devices that the account changed push.notifyUpdate(uid, 'accountVerify') + // remove verification reminders + verificationReminder.delete({ + uid: account.uid.toString('hex') + }).catch(function (err) { + log.error({ op: 'Account.RecoveryEmailVerify', err: err }) + }) return db.verifyEmail(account) .then( diff --git a/lib/verification-reminders.js b/lib/verification-reminders.js new file mode 100644 index 000000000..df0fd8639 --- /dev/null +++ b/lib/verification-reminders.js @@ -0,0 +1,78 @@ +/* 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 http://mozilla.org/MPL/2.0/. */ + +var config = require('../config') +var P = require('./promise') +var reminderConfig = config.get('verificationReminders') + +var LOG_REMINDERS_CREATED = 'verification-reminders.created' +var LOG_REMINDERS_DELETED = 'verification-reminders.deleted' +var LOG_REMINDERS_ERROR_CREATE = 'verification-reminder.create' +var LOG_REMINDERS_ERROR_DELETE = 'verification-reminder.delete' + +module.exports = function (log, db) { + /** + * shouldRemind + * + * Determines if we should create a reminder for this user to verify their account. + * + * @returns {boolean} + */ + function shouldRemind() { + // random between 0 and 100, inclusive + var rand = Math.floor(Math.random() * (100 + 1)) + return rand < (reminderConfig.rate * 100) + } + + return { + /** + * Create a new reminder + * @param reminderData + * @param {string} reminderData.uid - The uid to remind. + */ + create: function createReminder(reminderData) { + if (! shouldRemind()) { + // resolves if not part of the verification roll out + return P.resolve(false) + } + + reminderData.type = 'first' + var firstReminder = db.createVerificationReminder(reminderData) + reminderData.type = 'second' + var secondReminder = db.createVerificationReminder(reminderData) + + return P.all([firstReminder, secondReminder]) + .then( + function () { + log.increment(LOG_REMINDERS_CREATED) + }, + function (err) { + log.error({ op: LOG_REMINDERS_ERROR_CREATE, err: err }) + } + ) + }, + /** + * Delete the reminder. Used if the user verifies their account. + * + * @param reminderData + * @param {string} reminderData.uid - The uid for the reminder. + */ + 'delete': function deleteReminder(reminderData) { + reminderData.type = 'first' + var firstReminder = db.deleteVerificationReminder(reminderData) + reminderData.type = 'second' + var secondReminder = db.deleteVerificationReminder(reminderData) + + return P.all([firstReminder, secondReminder]) + .then( + function () { + log.increment(LOG_REMINDERS_DELETED) + }, + function (err) { + log.error({ op: LOG_REMINDERS_ERROR_DELETE, err: err }) + } + ) + } + } +} diff --git a/npm-shrinkwrap.json b/npm-shrinkwrap.json index 04eecc6cd..80f96dea0 100644 --- a/npm-shrinkwrap.json +++ b/npm-shrinkwrap.json @@ -462,7 +462,7 @@ "fxa-auth-db-mysql": { "version": "0.62.0", "from": "git+https://github.com/mozilla/fxa-auth-db-mysql.git#master", - "resolved": "git+https://github.com/mozilla/fxa-auth-db-mysql.git#978e74fa56bc465cc56d21bf529e7b6187ab4d79", + "resolved": "git+https://github.com/mozilla/fxa-auth-db-mysql.git#06b8db63eabf287aee761338eb54655758f37e4e", "dependencies": { "bluebird": { "version": "2.1.3", diff --git a/test/local/pool_tests.js b/test/local/pool_tests.js index 460e6b75a..cb7621031 100644 --- a/test/local/pool_tests.js +++ b/test/local/pool_tests.js @@ -273,14 +273,15 @@ test( var pool = new Pool('http://example.com/') sinon.stub(pool, 'request', function () {}) - pool.del('foo') + pool.del('foo', 'bar') t.equal(pool.request.callCount, 1, 'pool.request was called once') var args = pool.request.getCall(0).args - t.equal(args.length, 2, 'pool.request was passed three arguments') + t.equal(args.length, 3, 'pool.request was passed three arguments') t.equal(args[0], 'DELETE', 'first argument to pool.request was POST') t.equal(args[1], 'foo', 'second argument to pool.request was correct') + t.equal(args[2], 'bar', 'second argument can be data') t.end() } diff --git a/test/local/verification_reminder_db_tests.js b/test/local/verification_reminder_db_tests.js new file mode 100644 index 000000000..7552c804d --- /dev/null +++ b/test/local/verification_reminder_db_tests.js @@ -0,0 +1,123 @@ +/* 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 http://mozilla.org/MPL/2.0/. */ + +require('ass') +var tap = require('tap') +var test = tap.test +var uuid = require('uuid') +var log = { trace: console.log, info: console.log } + +var config = require('../../config').getProperties() +var TestServer = require('../test_server') +var Token = require('../../lib/tokens')(log) +var DB = require('../../lib/db')( + config.db.backend, + log, + Token.error, + Token.SessionToken, + Token.KeyFetchToken, + Token.AccountResetToken, + Token.PasswordForgotToken, + Token.PasswordChangeToken +) + +var zeroBuffer16 = Buffer('00000000000000000000000000000000', 'hex') +var zeroBuffer32 = Buffer('0000000000000000000000000000000000000000000000000000000000000000', 'hex') + +function createTestAccount() { + return { + uid: uuid.v4('binary'), + email: 'reminder' + Math.random() + '@bar.com', + emailCode: zeroBuffer16, + emailVerified: false, + verifierVersion: 1, + verifyHash: zeroBuffer32, + authSalt: zeroBuffer32, + kA: zeroBuffer32, + wrapWrapKb: zeroBuffer32, + acceptLanguage: 'bg-BG,en-US;q=0.7,ar-BH;q=0.3' + } +} + +var mockLog = require('../mocks').mockLog + +var dbServer, reminderConfig +var dbConn = TestServer.start(config) + .then( + function (server) { + dbServer = server + reminderConfig = process.env.VERIFICATION_REMINDER_RATE + process.env.VERIFICATION_REMINDER_RATE = 1 + return DB.connect(config[config.db.backend]) + } + ) + +test( + 'create', + function (t) { + var thisMockLog = mockLog({ + increment: function (name) { + t.equal(name, 'verification-reminders.created') + } + }) + + dbConn.then(function (db) { + var account = createTestAccount() + var reminder = { uid: account.uid.toString('hex') } + + var verificationReminder = require('../../lib/verification-reminders')(thisMockLog, db) + return verificationReminder.create(reminder).then( + function () { + t.end() + }, + function () { + t.fail() + } + ) + }) + } +) + +test( + 'delete', + function (t) { + var thisMockLog = mockLog({ + increment: function (name) { + if (name === 'verification-reminders.deleted') { + t.ok(true, 'correct log message') + } + } + }) + + dbConn.then(function (db) { + var verificationReminder = require('../../lib/verification-reminders')(thisMockLog, db) + var account = createTestAccount() + var reminder = { uid: account.uid.toString('hex') } + + return verificationReminder.create(reminder) + .then(function () { + return verificationReminder.delete(reminder) + }) + .then(function () { + t.end() + }, function (err) { + t.notOk(err) + }) + }) + } +) + +test( + 'teardown', + function (t) { + return dbConn.then(function(db) { + return db.close() + }).then(function() { + return dbServer.stop() + }).then(function () { + process.env.VERIFICATION_REMINDER_RATE = reminderConfig + t.end() + }) + } +) diff --git a/test/local/verification_reminder_tests.js b/test/local/verification_reminder_tests.js new file mode 100644 index 000000000..7a7975c2d --- /dev/null +++ b/test/local/verification_reminder_tests.js @@ -0,0 +1,137 @@ +/* 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 http://mozilla.org/MPL/2.0/. */ + +var tap = require('tap') +var proxyquire = require('proxyquire') +var uuid = require('uuid') + +var test = tap.test +var P = require('../../lib/promise') +var mockLog = require('../mocks').mockLog + +var zeroBuffer16 = Buffer('00000000000000000000000000000000', 'hex') + +var ACCOUNT = { + uid: uuid.v4('binary'), + email: 'reminder' + Math.random() + '@bar.com', + emailCode: zeroBuffer16, + acceptLanguage: 'bg-BG,en-US;q=0.7,ar-BH;q=0.3' +} + +var reminderData = { + email: ACCOUNT.email +} + +var mockDb = { + createVerificationReminder: function () { + return P.resolve() + } +} + +test( + 'creates reminders with valid options and rate', + function (t) { + var moduleMocks = { + '../config': { + 'get': function (item) { + if (item === 'verificationReminders') { + return { + rate: 1 + } + } + } + } + } + + var addedTimes = 0 + var thisMockLog = mockLog({ + increment: function (name) { + if (name === 'verification-reminders.created') { + addedTimes++ + if (addedTimes === 5) { + t.end() + } + } + } + }) + + var verificationReminder = proxyquire('../../lib/verification-reminders', moduleMocks)(thisMockLog, mockDb) + + verificationReminder.create(reminderData) + verificationReminder.create(reminderData) + verificationReminder.create(reminderData) + verificationReminder.create(reminderData) + verificationReminder.create(reminderData) + } +) + +test( + 'does not create reminders when rate is 0', + function (t) { + var moduleMocks = { + '../config': { + 'get': function (item) { + if (item === 'verificationReminders') { + return { + rate: 0 + } + } + } + } + } + + var verificationReminder = proxyquire('../../lib/verification-reminders', moduleMocks)(mockLog, mockDb) + verificationReminder.create(reminderData) + .then(function (result) { + if (result === false) { + t.end() + } + }) + } +) + +test( + 'deletes reminders', + function (t) { + var thisMockLog = mockLog({ + increment: function (name) { + if (name === 'verification-reminders.deleted') { + t.end() + } + } + }) + var thisMockDb = { + deleteVerificationReminder: function (reminderData) { + t.ok(reminderData.email) + t.ok(reminderData.type) + return P.resolve() + } + } + + var verificationReminder = proxyquire('../../lib/verification-reminders', {})(thisMockLog, thisMockDb) + verificationReminder.delete(reminderData) + } +) + +test( + 'deletes reminders can catch errors', + function (t) { + var thisMockLog = mockLog({ + error: function (logErr) { + t.equal(logErr.op, 'verification-reminder.delete') + t.ok(logErr.err.message) + t.end() + } + }) + var thisMockDb = { + deleteVerificationReminder: function () { + return P.reject(new Error('Something is wrong')) + } + } + + var verificationReminder = proxyquire('../../lib/verification-reminders', {})(thisMockLog, thisMockDb) + verificationReminder.delete(reminderData) + } +) +