refactor(bounces): pull bounce logic into separate module

This commit is contained in:
Sean McArthur 2017-03-28 10:06:29 -07:00
Родитель b06b0da0e4
Коммит 48d7625d65
15 изменённых файлов: 270 добавлений и 203 удалений

Просмотреть файл

@ -95,8 +95,9 @@ function run(config) {
.spread(
(db, translator) => {
database = db
const bounces = require('../lib/bounces')(config, db)
return require('../lib/senders')(log, config, error, db, translator)
return require('../lib/senders')(log, config, error, bounces, translator)
.then(result => {
senders = result
customs = new Customs(config.customsUrl)
@ -106,6 +107,7 @@ function run(config) {
serverPublicKeys,
signer,
db,
bounces,
senders.email,
senders.sms,
Password,

73
lib/bounces.js Normal file
Просмотреть файл

@ -0,0 +1,73 @@
/* 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/. */
'use strict'
const error = require('./error')
const P = require('./promise')
module.exports = (config, db) => {
const configBounces = config.smtp && config.smtp.bounces || {}
const BOUNCES_ENABLED = !! configBounces.enabled
const MAX_HARD = configBounces.hard && configBounces.hard.max || 0
const MAX_SOFT = configBounces.soft && configBounces.soft.max || 0
const MAX_COMPLAINT = configBounces.complaint && configBounces.complaint.max || 0
const DURATION_HARD = configBounces.hard && configBounces.hard.duration || Infinity
const DURATION_SOFT = configBounces.soft && configBounces.soft.duration || Infinity
const DURATION_COMPLAINT = configBounces.complaint && configBounces.complaint.duration || Infinity
const BOUNCE_TYPE_HARD = 1
const BOUNCE_TYPE_SOFT = 2
const BOUNCE_TYPE_COMPLAINT = 3
const freeze = Object.freeze
const BOUNCE_RULES = freeze({
[BOUNCE_TYPE_HARD]: freeze({
duration: DURATION_HARD,
error: error.emailBouncedHard,
max: MAX_HARD
}),
[BOUNCE_TYPE_COMPLAINT]: freeze({
duration: DURATION_COMPLAINT,
error: error.emailComplaint,
max: MAX_COMPLAINT
}),
[BOUNCE_TYPE_SOFT]: freeze({
duration: DURATION_SOFT,
error: error.emailBouncedSoft,
max: MAX_SOFT
})
})
function checkBounces(email) {
return db.emailBounces(email)
.then(bounces => {
const counts = {
[BOUNCE_TYPE_HARD]: 0,
[BOUNCE_TYPE_COMPLAINT]: 0,
[BOUNCE_TYPE_SOFT]: 0
}
const now = Date.now()
bounces.forEach(bounce => {
const type = bounce.bounceType
const ruleSet = BOUNCE_RULES[type]
if (ruleSet) {
if (bounce.createdAt > now - ruleSet.duration) {
counts[type]++
if (counts[type] > ruleSet.max) {
throw ruleSet.error()
}
}
}
})
})
}
function disabled() {
return P.resolve()
}
return {
check: BOUNCES_ENABLED ? checkBounces : disabled
}
}

Просмотреть файл

@ -37,6 +37,7 @@ module.exports = function (
isA,
error,
db,
bounces,
mailer,
Password,
config,
@ -1619,6 +1620,12 @@ module.exports = function (
return P.resolve()
}
/*
function checkBounces() {
}
*/
function createResponse() {
var sessionVerified = sessionToken.tokenVerified

Просмотреть файл

@ -16,6 +16,7 @@ module.exports = function (
serverPublicKeys,
signer,
db,
bounces,
mailer,
smsImpl,
Password,
@ -35,6 +36,7 @@ module.exports = function (
isA,
error,
db,
bounces,
mailer,
Password,
config,

Просмотреть файл

@ -9,9 +9,8 @@
// there's nothing left that imports mailer/config, it is safe to merge
// legacy_index.js and this file into one.
var createSenders = require('./legacy_index')
var P = require('../promise')
module.exports = function (log, config, error, db, translator, sender) {
module.exports = function (log, config, error, bounces, translator, sender) {
var defaultLanguage = config.i18n.defaultLanguage
return createSenders(
@ -25,62 +24,10 @@ module.exports = function (log, config, error, db, translator, sender) {
)
.then(function (senders) {
var ungatedMailer = senders.email
var configBounces = config.smtp && config.smtp.bounces || {}
var BOUNCES_ENABLED = !! configBounces.enabled
var MAX_HARD = configBounces.hard && configBounces.hard.max || 0
var MAX_SOFT = configBounces.soft && configBounces.soft.max || 0
var MAX_COMPLAINT = configBounces.complaint && configBounces.complaint.max || 0
var DURATION_HARD = configBounces.hard && configBounces.hard.duration || Infinity
var DURATION_SOFT = configBounces.soft && configBounces.soft.duration || Infinity
var DURATION_COMPLAINT = configBounces.complaint && configBounces.complaint.duration || Infinity
var BOUNCE_TYPE_HARD = 1
var BOUNCE_TYPE_SOFT = 2
var BOUNCE_TYPE_COMPLAINT = 3
// I really wanted to use Computer property names here, but thats
// an ES2015 feature, and this directory (senders) is stuck in
// ES5-land.
var freeze = Object.freeze
var BOUNCE_RULES = {}
BOUNCE_RULES[BOUNCE_TYPE_HARD] = freeze({
duration: DURATION_HARD,
error: error.emailBouncedHard,
max: MAX_HARD
})
BOUNCE_RULES[BOUNCE_TYPE_COMPLAINT] = freeze({
duration: DURATION_COMPLAINT,
error: error.emailComplaint,
max: MAX_COMPLAINT
})
BOUNCE_RULES[BOUNCE_TYPE_SOFT] = freeze({
duration: DURATION_SOFT,
error: error.emailBouncedSoft,
max: MAX_SOFT
})
BOUNCE_RULES = freeze(BOUNCE_RULES)
function bounceGatedMailer(email) {
return db.emailBounces(email)
.then(function (bounces) {
var counts = {}
counts[BOUNCE_TYPE_HARD] = 0
counts[BOUNCE_TYPE_COMPLAINT] = 0
counts[BOUNCE_TYPE_SOFT] = 0
var now = Date.now()
bounces.forEach(function (bounce) {
var type = bounce.bounceType
var ruleSet = BOUNCE_RULES[type]
if (ruleSet) {
if (bounce.createdAt > now - ruleSet.duration) {
counts[type]++
if (counts[type] > ruleSet.max) {
throw ruleSet.error()
}
}
}
})
return ungatedMailer
})
function getSafeMailer(email) {
return bounces.check(email)
.return(ungatedMailer)
.catch(function (err) {
log.info({
op: 'mailer.blocked',
@ -90,12 +37,6 @@ module.exports = function (log, config, error, db, translator, sender) {
})
}
function noopMailer() {
return P.resolve(ungatedMailer)
}
var getSafeMailer = BOUNCES_ENABLED ? bounceGatedMailer : noopMailer
function getVerifiedSecondaryEmails(emais) {
return emais.reduce(function(list, email) {
if (! email.isPrimary && email.isVerified) {

Просмотреть файл

@ -53,13 +53,14 @@ var mailSender = {
close: function () {}
}
const db = {
emailBounces: () => P.resolve([])
const bounces = {
// this is for dev purposes, no need to check db
check: () => P.resolve()
}
require('../lib/senders/translator')(config.i18n.supportedLanguages, config.i18n.defaultLanguage)
.then(translator => {
return createSenders(log, config, error, db, translator, mailSender)
return createSenders(log, config, error, bounces, translator, mailSender)
})
.then((senders) => {
const mailer = senders.email._ungatedMailer

Просмотреть файл

@ -61,6 +61,7 @@ var makeRoutes = function (options, requireMocks) {
isA,
error,
db,
mocks.mockBounces(),
options.mailer || {},
Password,
config,

139
test/local/lib/bounces.js Normal file
Просмотреть файл

@ -0,0 +1,139 @@
/* 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/. */
'use strict'
const ROOT_DIR = '../../..'
const assert = require('insist')
const config = require(`${ROOT_DIR}/config`).getProperties()
const createBounces = require(`${ROOT_DIR}/lib/bounces`)
const error = require(`${ROOT_DIR}/lib/error`)
const P = require('bluebird')
const sinon = require('sinon')
const EMAIL = Math.random() + '@example.test'
const BOUNCE_TYPE_HARD = 1
const BOUNCE_TYPE_COMPLAINT = 3
const NOW = Date.now()
describe('bounces', () => {
it('succeeds if bounces not over limit', () => {
const db = {
emailBounces: sinon.spy(() => P.resolve([]))
}
return createBounces(config, db).check(EMAIL)
.then(() => {
assert.equal(db.emailBounces.callCount, 1)
})
})
it('error if complaints over limit', () => {
const conf = Object.assign({}, config)
conf.smtp = {
bounces: {
enabled: true,
complaint: {
max: 0
}
}
}
const db = {
emailBounces: sinon.spy(() => P.resolve([
{
bounceType: BOUNCE_TYPE_COMPLAINT,
createdAt: NOW
}
]))
}
return createBounces(conf, db).check(EMAIL)
.then(
() => assert(false),
e => {
assert.equal(db.emailBounces.callCount, 1)
assert.equal(e.errno, error.ERRNO.BOUNCE_COMPLAINT)
}
)
})
it('error if hard bounces over limit', () => {
const conf = Object.assign({}, config)
conf.smtp = {
bounces: {
enabled: true,
hard: {
max: 0
}
}
}
const db = {
emailBounces: sinon.spy(() => P.resolve([
{
bounceType: BOUNCE_TYPE_HARD,
createdAt: NOW
}
]))
}
return createBounces(conf, db).check(EMAIL)
.then(
() => assert(false),
e => {
assert.equal(db.emailBounces.callCount, 1)
assert.equal(e.errno, error.ERRNO.BOUNCE_HARD)
}
)
})
it('does not error if not enough bounces in duration', () => {
const conf = Object.assign({}, config)
conf.smtp = {
bounces: {
enabled: true,
hard: {
max: 0,
duration: 5000
}
}
}
const db = {
emailBounces: sinon.spy(() => P.resolve([
{
bounceType: BOUNCE_TYPE_HARD,
createdAt: Date.now() - 20000
}
]))
}
return createBounces(conf, db).check(EMAIL)
.then(() => {
assert.equal(db.emailBounces.callCount, 1)
})
})
describe('disabled', () => {
it('does not call bounces.check if disabled', () => {
const conf = Object.assign({}, config)
conf.smtp = {
bounces: {
enabled: false
}
}
const db = {
emailBounces: sinon.spy(() => P.resolve([
{
bounceType: BOUNCE_TYPE_HARD,
createdAt: Date.now() - 20000
}
]))
}
assert.equal(db.emailBounces.callCount, 0)
return createBounces(conf, db).check(EMAIL)
.then(() => {
assert.equal(db.emailBounces.callCount, 0)
})
})
})
})

Просмотреть файл

@ -8,7 +8,11 @@ const ROOT_DIR = '../../..'
const assert = require('insist')
const config = require(`${ROOT_DIR}/config/index`).getProperties()
const db = {}
const bounces = {
check() {
return require(`${ROOT_DIR}/lib/promise`).resolve()
}
}
const error = require(`${ROOT_DIR}/lib/error`)
const log = {}
@ -18,7 +22,7 @@ describe('mailer locales', () => {
before(() => {
return require(`${ROOT_DIR}/lib/senders/translator`)(config.i18n.supportedLanguages, config.i18n.defaultLanguage)
.then(translator => {
return require(`${ROOT_DIR}/lib/senders`)(log, config, error, db, translator)
return require(`${ROOT_DIR}/lib/senders`)(log, config, error, bounces, translator)
})
.then(result => {
mailer = result.email

Просмотреть файл

@ -36,16 +36,16 @@ describe('lib/senders/index', () => {
isPrimary: false,
isVerified: false
}]
const db = {
emailBounces: sinon.spy(() => P.resolve([]))
const bounces = {
check: sinon.spy(() => P.resolve([]))
}
const acct = {
email: EMAIL,
uid: UID
}
function createSender(config, db) {
return senders(nullLog, config, error, db, {})
function createSender(config, bounces) {
return senders(nullLog, config, error, bounces, {})
.then(sndrs => {
const email = sndrs.email
email._ungatedMailer.mailer.sendMail = sinon.spy((opts, cb) => {
@ -56,7 +56,7 @@ describe('lib/senders/index', () => {
}
beforeEach(() => {
db.emailBounces.reset()
bounces.check.reset()
})
describe('.sendVerifyCode()', () => {
@ -64,14 +64,14 @@ describe('lib/senders/index', () => {
it('should call mailer.verifyEmail()', () => {
let email
return createSender(config, db)
return createSender(config, bounces)
.then(e => {
email = e
email._ungatedMailer.verifyEmail = sinon.spy(() => P.resolve({}))
return email.sendVerifyCode(EMAILS, acct, {code: code})
})
.then(() => {
assert.equal(db.emailBounces.callCount, 1)
assert.equal(bounces.check.callCount, 1)
assert.equal(email._ungatedMailer.verifyEmail.callCount, 1)
const args = email._ungatedMailer.verifyEmail.getCall(0).args
@ -85,14 +85,14 @@ describe('lib/senders/index', () => {
it('should call mailer.verifyLoginEmail()', () => {
let email
return createSender(config, db)
return createSender(config, bounces)
.then(e => {
email = e
email._ungatedMailer.verifyLoginEmail = sinon.spy(() => P.resolve({}))
return email.sendVerifyLoginEmail(EMAILS, acct, {code: code})
})
.then(() => {
assert.equal(db.emailBounces.callCount, 1)
assert.equal(bounces.check.callCount, 1)
assert.equal(email._ungatedMailer.verifyLoginEmail.callCount, 1)
const args = email._ungatedMailer.verifyLoginEmail.getCall(0).args
@ -113,14 +113,14 @@ describe('lib/senders/index', () => {
it('should call mailer.recoveryEmail()', () => {
let email
return createSender(config, db)
return createSender(config, bounces)
.then(e => {
email = e
email._ungatedMailer.recoveryEmail = sinon.spy(() => P.resolve({}))
return email.sendRecoveryCode(EMAILS, acct, {code: code, token: token})
})
.then(() => {
assert.equal(db.emailBounces.callCount, 1)
assert.equal(bounces.check.callCount, 1)
assert.equal(email._ungatedMailer.recoveryEmail.callCount, 1)
const args = email._ungatedMailer.recoveryEmail.getCall(0).args
@ -134,7 +134,7 @@ describe('lib/senders/index', () => {
describe('.sendPasswordChangedNotification()', () => {
it('should call mailer.passwordChangedEmail()', () => {
let email
return createSender(config, db)
return createSender(config, bounces)
.then(e => {
email = e
email._ungatedMailer.passwordChangedEmail = sinon.spy(() => P.resolve({}))
@ -149,7 +149,7 @@ describe('lib/senders/index', () => {
assert.equal(args[0].ccEmails[0], EMAILS[1].email, 'cc email correctly set')
assert.equal(args[0].ccEmails[1], EMAILS[2].email, 'cc email correctly set')
assert.equal(db.emailBounces.callCount, 1)
assert.equal(bounces.check.callCount, 1)
})
})
})
@ -157,7 +157,7 @@ describe('lib/senders/index', () => {
describe('.sendPasswordResetNotification()', () => {
it('should call mailer.passwordResetEmail()', () => {
let email
return createSender(config, db)
return createSender(config, bounces)
.then(e => {
email = e
email._ungatedMailer.passwordResetEmail = sinon.spy(() => P.resolve({}))
@ -172,7 +172,7 @@ describe('lib/senders/index', () => {
assert.equal(args[0].ccEmails[0], EMAILS[1].email, 'cc email correctly set')
assert.equal(args[0].ccEmails[1], EMAILS[2].email, 'cc email correctly set')
assert.equal(db.emailBounces.callCount, 1)
assert.equal(bounces.check.callCount, 1)
})
})
})
@ -180,7 +180,7 @@ describe('lib/senders/index', () => {
describe('.sendNewDeviceLoginNotification()', () => {
it('should call mailer.newDeviceLoginEmail()', () => {
let email
return createSender(config, db)
return createSender(config, bounces)
.then(e => {
email = e
email._ungatedMailer.newDeviceLoginEmail = sinon.spy(() => P.resolve({}))
@ -195,7 +195,7 @@ describe('lib/senders/index', () => {
assert.equal(args[0].ccEmails[0], EMAILS[1].email, 'cc email correctly set')
assert.equal(args[0].ccEmails[1], EMAILS[2].email, 'cc email correctly set')
assert.equal(db.emailBounces.callCount, 1)
assert.equal(bounces.check.callCount, 1)
})
})
})
@ -203,7 +203,7 @@ describe('lib/senders/index', () => {
describe('.sendPostVerifyEmail()', () => {
it('should call mailer.postVerifyEmail()', () => {
let email
return createSender(config, db)
return createSender(config, bounces)
.then(e => {
email = e
email._ungatedMailer.postVerifyEmail = sinon.spy(() => P.resolve({}))
@ -216,7 +216,7 @@ describe('lib/senders/index', () => {
assert.equal(args[0].email, EMAIL, 'email correctly set')
assert.equal(args[0].ccEmails, undefined, 'no cc emails set')
assert.equal(db.emailBounces.callCount, 1)
assert.equal(bounces.check.callCount, 1)
})
})
})
@ -226,7 +226,7 @@ describe('lib/senders/index', () => {
it('should call mailer.unblockCodeEmail()', () => {
let email
return createSender(config, db)
return createSender(config, bounces)
.then(e => {
email = e
email._ungatedMailer.unblockCodeEmail = sinon.spy(() => P.resolve({}))
@ -240,136 +240,28 @@ describe('lib/senders/index', () => {
assert.equal(args[0].ccEmails.length, 1, 'email correctly set')
assert.equal(args[0].ccEmails[0], EMAILS[1].email, 'cc email correctly set')
assert.equal(db.emailBounces.callCount, 1)
assert.equal(bounces.check.callCount, 1)
})
})
})
describe('gated on bounces', () => {
const code = crypto.randomBytes(8).toString('hex')
const BOUNCE_TYPE_HARD = 1
const BOUNCE_TYPE_COMPLAINT = 3
it('succeeds if bounces not over limit', () => {
const db = {
emailBounces: sinon.spy(() => P.resolve([]))
it('errors if bounce check fails', () => {
const errorBounces = {
check: sinon.spy(() => P.reject(error.emailComplaint()))
}
return createSender(config, db)
.then(email => {
email._ungatedMailer.unblockCodeEmail = sinon.spy(() => P.resolve({}))
return email.sendUnblockCode(EMAILS, acct, {code: code})
})
.then(() => {
assert.equal(db.emailBounces.callCount, 1)
})
})
it('error if complaints over limit', () => {
const conf = Object.assign({}, config)
conf.smtp = {
bounces: {
enabled: true,
complaint: {
max: 0
}
}
}
const db = {
emailBounces: sinon.spy(() => P.resolve([
{
bounceType: BOUNCE_TYPE_COMPLAINT
}
]))
}
return createSender(conf, db)
return createSender(config, errorBounces)
.then(email => {
email._ungatedMailer.unblockCodeEmail = sinon.spy(() => P.resolve({}))
return email.sendUnblockCode(EMAILS, acct, {code: code})
})
.catch(e => {
assert.equal(db.emailBounces.callCount, 1)
assert.equal(errorBounces.check.callCount, 1)
assert.equal(e.errno, error.ERRNO.BOUNCE_COMPLAINT)
})
})
it('error if hard bounces over limit', () => {
const conf = Object.assign({}, config)
conf.smtp = {
bounces: {
enabled: true,
hard: {
max: 0
}
}
}
const db = {
emailBounces: sinon.spy(() => P.resolve([
{
bounceType: BOUNCE_TYPE_HARD
}
]))
}
return createSender(conf, db)
.then(email => {
email._ungatedMailer.unblockCodeEmail = sinon.spy(() => P.resolve({}))
return email.sendUnblockCode(EMAILS, acct, {code: code})
})
.catch(e => {
assert.equal(db.emailBounces.callCount, 1)
assert.equal(e.errno, error.ERRNO.BOUNCE_HARD)
})
})
it('does not error if not enough bounces in duration', () => {
const conf = Object.assign({}, config)
conf.smtp = {
bounces: {
enabled: true,
hard: {
max: 0,
duration: 5000
}
}
}
const db = {
emailBounces: sinon.spy(() => P.resolve([
{
bounceType: BOUNCE_TYPE_HARD,
createdAt: Date.now() - 20000
}
]))
}
return createSender(conf, db)
.then(email => {
email._ungatedMailer.unblockCodeEmail = sinon.spy(() => P.resolve({}))
return email.sendUnblockCode(EMAILS, acct, {code: code})
})
.then(() => {
assert.equal(db.emailBounces.callCount, 1)
})
})
it('does not call db.emailBounces if disabled', () => {
const conf = Object.assign({}, config)
conf.smtp = {
bounces: {
enabled: false
}
}
assert.equal(db.emailBounces.callCount, 0)
const spy = sinon.spy(() => P.resolve({}))
return createSender(conf, db)
.then(email => {
email._ungatedMailer.unblockCodeEmail = spy
return email.sendUnblockCode(EMAILS, acct, {code: code})
})
.then(() => {
assert.equal(db.emailBounces.callCount, 0)
assert.equal(spy.callCount, 1)
})
})
})
})
})

Просмотреть файл

@ -58,6 +58,7 @@ var makeRoutes = function (options, requireMocks) {
isA,
error,
db,
mocks.mockBounces(),
options.mailer || {},
Password,
config,

Просмотреть файл

@ -55,6 +55,7 @@ var makeRoutes = function (options, requireMocks) {
isA,
error,
db,
mocks.mockBounces(),
options.mailer || {},
Password,
config,

Просмотреть файл

@ -52,6 +52,7 @@ var makeRoutes = function (options, requireMocks) {
isA,
error,
db,
mocks.mockBounces(),
options.mailer || {},
Password,
config,

Просмотреть файл

@ -58,6 +58,7 @@ var makeRoutes = function (options, requireMocks) {
isA,
error,
db,
mocks.mockBounces(),
options.mailer || {},
Password,
config,

Просмотреть файл

@ -105,6 +105,7 @@ const PUSH_METHOD_NAMES = [
module.exports = {
generateMetricsContext: generateMetricsContext,
mockBounces: mockObject(['check']),
mockCustoms: mockObject(CUSTOMS_METHOD_NAMES),
mockDB: mockDB,
mockDevices: mockDevices,