Skip to content
This repository has been archived by the owner on Apr 3, 2019. It is now read-only.

Commit

Permalink
feat(email): add verification reminders
Browse files Browse the repository at this point in the history
Fixes #1081
  • Loading branch information
vladikoff committed Jun 9, 2016
1 parent 42dc3d1 commit 5007b4d
Show file tree
Hide file tree
Showing 10 changed files with 390 additions and 6 deletions.
1 change: 1 addition & 0 deletions .env.dev
Original file line number Diff line number Diff line change
Expand Up @@ -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
7 changes: 7 additions & 0 deletions config/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -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',
Expand Down
20 changes: 20 additions & 0 deletions lib/db.js
Original file line number Diff line number Diff line change
Expand Up @@ -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
}

Expand Down
4 changes: 2 additions & 2 deletions lib/pool.js
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
19 changes: 18 additions & 1 deletion lib/routes/account.js
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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 })
})
}
Expand Down Expand Up @@ -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(
Expand Down
78 changes: 78 additions & 0 deletions lib/verification-reminders.js
Original file line number Diff line number Diff line change
@@ -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)

This comment has been minimized.

Copy link
@l-hedgehog

l-hedgehog Jul 11, 2016

Contributor

This doesn't seem right.

If Math.random() is 0.991, rand will be 100, rand < (reminderConfig.rate * 100) will be false even with 1 as reminderConfig.rate.

This comment has been minimized.

Copy link
@vladikoff

vladikoff Jul 11, 2016

Author Contributor

Thanks for the note @l-hedgehog! We had this code somewhere else before as well, I'll check it out.

}

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 })
}
)
}
}
}
2 changes: 1 addition & 1 deletion npm-shrinkwrap.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

5 changes: 3 additions & 2 deletions test/local/pool_tests.js
Original file line number Diff line number Diff line change
Expand Up @@ -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()
}
Expand Down
123 changes: 123 additions & 0 deletions test/local/verification_reminder_db_tests.js
Original file line number Diff line number Diff line change
@@ -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()
})
}
)
Loading

0 comments on commit 5007b4d

Please sign in to comment.