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

Commit

Permalink
feat(email): When primary email gated, send to secondary email if ava…
Browse files Browse the repository at this point in the history 
…lible (#1954), r=@seanmonstar
  • Loading branch information
@vbudhram
vbudhram authored Jul 5, 2017
1 parent 7d59790 commit 979968a
Show file tree
Hide file tree
Showing 4 changed files with 122 additions and 45 deletions.
9 changes: 3 additions & 6 deletions lib/routes/account.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -908,8 +908,7 @@ module.exports = (
&& ! doSigninConfirmation
&& emailRecord.emailVerified
if (shouldSendNewDeviceLoginEmail) {
const secondaryEmails = features.isSecondaryEmailEnabled(emailRecord.email) ? db.accountEmails(sessionToken.uid) : P.resolve([])
return P.all([getGeoData(ip), secondaryEmails])
return P.all([getGeoData(ip), db.accountEmails(sessionToken.uid)])
.spread((geoData, emails) => {
// New device notifications are always sent to the primary account email (emailRecord.email)
// and CCed to all secondary email if enabled.
Expand Down Expand Up @@ -950,8 +949,7 @@ module.exports = (
tokenVerificationId: tokenVerificationId
})

const secondaryEmails = features.isSecondaryEmailEnabled(emailRecord.email) ? db.accountEmails(sessionToken.uid) : P.resolve([])
return P.all([getGeoData(ip), secondaryEmails])
return P.all([getGeoData(ip), db.accountEmails(sessionToken.uid)])
.spread((geoData, emails) => {
return mailer.sendVerifyLoginEmail(
emails,
Expand Down Expand Up @@ -1362,8 +1360,7 @@ module.exports = (
)

function setVerifyCode() {
const secondaryEmails = features.isSecondaryEmailEnabled(sessionToken.email) ? db.accountEmails(sessionToken.uid) : P.resolve([])
return secondaryEmails
return db.accountEmails(sessionToken.uid)
.then((emailData) => {
if (email) {
// If an email address is specified in payload, this is a request to verify
Expand Down
11 changes: 4 additions & 7 deletions lib/routes/password.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -259,7 +259,7 @@ module.exports = function (
.then(
function (accountData) {
account = accountData
return features.isSecondaryEmailEnabled(account.email) ? db.accountEmails(passwordChangeToken.uid) : P.resolve([])
return db.accountEmails(passwordChangeToken.uid)
}
)
.then(
Expand Down Expand Up @@ -434,8 +434,7 @@ module.exports = function (
)
.then(
function (passwordForgotToken) {
const secondaryEmails = features.isSecondaryEmailEnabled(email) ? db.accountEmails(passwordForgotToken.uid) : P.resolve([])
return P.all([getGeoData(ip), secondaryEmails])
return P.all([getGeoData(ip), db.accountEmails(passwordForgotToken.uid)])
.spread((geoData, emails) => {
return mailer.sendRecoveryCode(
emails,
Expand Down Expand Up @@ -531,8 +530,7 @@ module.exports = function (
])
.then(
function () {
const secondaryEmails = features.isSecondaryEmailEnabled(passwordForgotToken.email) ? db.accountEmails(passwordForgotToken.uid) : P.resolve([])
return P.all([getGeoData(ip), secondaryEmails])
return P.all([getGeoData(ip), db.accountEmails(passwordForgotToken.uid)])
.spread((geoData, emails) => {
return mailer.sendRecoveryCode(
emails,
Expand Down Expand Up @@ -618,8 +616,7 @@ module.exports = function (
db.forgotPasswordVerified(passwordForgotToken)
.then(
function (accountResetToken) {
const secondaryEmails = features.isSecondaryEmailEnabled(passwordForgotToken.email) ? db.accountEmails(passwordForgotToken.uid) : P.resolve([])
return secondaryEmails
return db.accountEmails(passwordForgotToken.uid)
.then((emails) => {
return mailer.sendPasswordResetNotification(
emails,
Expand Down
104 changes: 80 additions & 24 deletions lib/senders/index.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@

var createMailer = require('./email')
var createSms = require('./sms')
var P = require('../promise')

module.exports = function (log, config, error, bounces, translator, sender) {
var defaultLanguage = config.i18n.defaultLanguage
Expand Down Expand Up @@ -42,6 +43,43 @@ module.exports = function (log, config, error, bounces, translator, sender) {
})
}

function getSafeMailerWithEmails(emails) {
var ungatedEmails = []
var gatedEmailErrors = []

// Filter down to emails to only ones that are not gated
return P.map(emails, function (email) {
return getSafeMailer(email.email)
.then(function () {
ungatedEmails.push(email)
}, function (err) {
gatedEmailErrors.push(err)
})
})
.then(function () {
// There are no ungated emails, lets throw the first bounce error so that
// we don't hurt our email scores.
if (ungatedEmails.length === 0 && gatedEmailErrors.length > 0) {
throw gatedEmailErrors[0]
}

var ungatedPrimaryEmail = getPrimaryEmail(ungatedEmails)
var ungatedCcEmails = getVerifiedSecondaryEmails(ungatedEmails)

// This user is having a bad time, their primary email is bouncing.
// Send emails to ungated secondary emails
if (! ungatedPrimaryEmail) {
ungatedPrimaryEmail = ungatedCcEmails[0]
}

return {
ungatedMailer: ungatedMailer,
ungatedPrimaryEmail: ungatedPrimaryEmail,
ungatedCcEmails: ungatedCcEmails
}
})
}

// Returns an array of only emails that are verified.
// This returns only the email and not the email object.
function getVerifiedSecondaryEmails(emails) {
Expand All @@ -52,6 +90,18 @@ module.exports = function (log, config, error, bounces, translator, sender) {
})
}

function getPrimaryEmail(emails) {
var primaryEmail
for (var i=0; i<emails.length; i++) {
if (emails[i].isPrimary) {
primaryEmail = emails[i]
break
}
}

return primaryEmail && primaryEmail.email
}

senders.email = {
sendVerifyCode: function (emails, account, opts) {
var primaryEmail = account.email
Expand All @@ -77,11 +127,12 @@ module.exports = function (log, config, error, bounces, translator, sender) {
})
},
sendVerifyLoginEmail: function (emails, account, opts) {
var primaryEmail = account.email
var ccEmails = getVerifiedSecondaryEmails(emails)
return getSafeMailerWithEmails(emails)
.then(function (result) {
var mailer = result.ungatedMailer
var primaryEmail = result.ungatedPrimaryEmail
var ccEmails = result.ungatedCcEmails

return getSafeMailer(primaryEmail)
.then(function (mailer) {
return mailer.verifyLoginEmail({
acceptLanguage: opts.acceptLanguage || defaultLanguage,
code: opts.code,
Expand Down Expand Up @@ -129,11 +180,12 @@ module.exports = function (log, config, error, bounces, translator, sender) {
})
},
sendRecoveryCode: function (emails, account, opts) {
var primaryEmail = account.email
var ccEmails = getVerifiedSecondaryEmails(emails)
return getSafeMailerWithEmails(emails)
.then(function (result) {
var mailer = result.ungatedMailer
var primaryEmail = result.ungatedPrimaryEmail
var ccEmails = result.ungatedCcEmails

return getSafeMailer(primaryEmail)
.then(function (mailer) {
return mailer.recoveryEmail({
ccEmails: ccEmails,
email: primaryEmail,
Expand All @@ -156,11 +208,12 @@ module.exports = function (log, config, error, bounces, translator, sender) {
})
},
sendPasswordChangedNotification: function (emails, account, opts) {
var primaryEmail = account.email
var ccEmails = getVerifiedSecondaryEmails(emails)
return getSafeMailerWithEmails(emails)
.then(function (result) {
var mailer = result.ungatedMailer
var primaryEmail = result.ungatedPrimaryEmail
var ccEmails = result.ungatedCcEmails

return getSafeMailer(primaryEmail)
.then(function (mailer) {
return mailer.passwordChangedEmail({
email: primaryEmail,
ccEmails: ccEmails,
Expand All @@ -175,11 +228,12 @@ module.exports = function (log, config, error, bounces, translator, sender) {
})
},
sendPasswordResetNotification: function (emails, account, opts) {
var primaryEmail = account.email
var ccEmails = getVerifiedSecondaryEmails(emails)
return getSafeMailerWithEmails(emails)
.then(function (result) {
var mailer = result.ungatedMailer
var primaryEmail = result.ungatedPrimaryEmail
var ccEmails = result.ungatedCcEmails

return getSafeMailer(primaryEmail)
.then(function (mailer) {
return mailer.passwordResetEmail({
ccEmails: ccEmails,
email: primaryEmail,
Expand All @@ -190,11 +244,12 @@ module.exports = function (log, config, error, bounces, translator, sender) {
})
},
sendNewDeviceLoginNotification: function (emails, account, opts) {
var primaryEmail = account.email
var ccEmails = getVerifiedSecondaryEmails(emails)
return getSafeMailerWithEmails(emails)
.then(function (result) {
var mailer = result.ungatedMailer
var primaryEmail = result.ungatedPrimaryEmail
var ccEmails = result.ungatedCcEmails

return getSafeMailer(primaryEmail)
.then(function (mailer) {
return mailer.newDeviceLoginEmail({
acceptLanguage: opts.acceptLanguage || defaultLanguage,
flowId: opts.flowId,
Expand Down Expand Up @@ -235,11 +290,12 @@ module.exports = function (log, config, error, bounces, translator, sender) {
})
},
sendUnblockCode: function (emails, account, opts) {
var primaryEmail = account.email
var ccEmails = getVerifiedSecondaryEmails(emails)
return getSafeMailerWithEmails(emails)
.then(function (result) {
var mailer = result.ungatedMailer
var primaryEmail = result.ungatedPrimaryEmail
var ccEmails = result.ungatedCcEmails

return getSafeMailer(primaryEmail)
.then(function (mailer) {
return mailer.unblockCodeEmail({
acceptLanguage: opts.acceptLanguage || defaultLanguage,
flowId: opts.flowId,
Expand Down
43 changes: 35 additions & 8 deletions test/local/senders/index.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,7 @@ describe('lib/senders/index', () => {
return email.sendVerifyLoginEmail(EMAILS, acct, {code: code})
})
.then(() => {
assert.equal(bounces.check.callCount, 1)
assert.equal(bounces.check.callCount, 3)
assert.equal(email._ungatedMailer.verifyLoginEmail.callCount, 1)

const args = email._ungatedMailer.verifyLoginEmail.getCall(0).args
Expand Down Expand Up @@ -120,7 +120,7 @@ describe('lib/senders/index', () => {
return email.sendRecoveryCode(EMAILS, acct, {code: code, token: token})
})
.then(() => {
assert.equal(bounces.check.callCount, 1)
assert.equal(bounces.check.callCount, 3)
assert.equal(email._ungatedMailer.recoveryEmail.callCount, 1)

const args = email._ungatedMailer.recoveryEmail.getCall(0).args
Expand All @@ -147,7 +147,7 @@ describe('lib/senders/index', () => {
assert.equal(args[0].email, EMAIL, 'email correctly set')
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(bounces.check.callCount, 1)
assert.equal(bounces.check.callCount, 3)
})
})
})
Expand All @@ -168,7 +168,7 @@ describe('lib/senders/index', () => {
assert.equal(args[0].email, EMAIL, 'email correctly set')
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(bounces.check.callCount, 1)
assert.equal(bounces.check.callCount, 3)
})
})
})
Expand All @@ -189,7 +189,7 @@ describe('lib/senders/index', () => {
assert.equal(args[0].email, EMAIL, 'email correctly set')
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(bounces.check.callCount, 1)
assert.equal(bounces.check.callCount, 3)
})
})
})
Expand Down Expand Up @@ -233,7 +233,7 @@ 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(bounces.check.callCount, 1)
assert.equal(bounces.check.callCount, 3)
})
})
})
Expand All @@ -253,16 +253,43 @@ describe('lib/senders/index', () => {
return email.sendUnblockCode(EMAILS, acct, {code: code})
})
.catch(e => {
assert.equal(errorBounces.check.callCount, 1)
assert.equal(errorBounces.check.callCount, 3)
assert.equal(e.errno, error.ERRNO.BOUNCE_COMPLAINT)

assert.equal(log.info.callCount, 1)
assert.equal(log.info.callCount, 3)
const msg = log.info.args[0][0]
assert.equal(msg.op, 'mailer.blocked')
assert.equal(msg.errno, e.errno)
assert.equal(msg.bouncedAt, DATE)
})
})

it('on gated primary email, sends to secondary', () => {
const log = mocks.spyLog()
const DATE = Date.now() - 10000
let email
const errorBounces = {
check: sinon.spy((email) => {
if (email === EMAIL) {
return P.reject(error.emailComplaint(DATE))
}
return P.resolve({})
})
}
return createSender(config, errorBounces, log)
.then(e => {
email = e
email._ungatedMailer.unblockCodeEmail = sinon.spy(() => P.resolve({}))
return email.sendUnblockCode(EMAILS, acct, {code: code})
})
.then(() => {
const args = email._ungatedMailer.unblockCodeEmail.getCall(0).args
assert.equal(args[0].email, EMAILS[1].email, 'email correctly set')
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(errorBounces.check.callCount, 3)
})
})
})
})
})

0 comments on commit 979968a

Please sign in to comment.