Merge pull request #3051 from overleaf/jel-log-when-email-added

Add entry to auditLog when user adds an email

GitOrigin-RevId: 797c85e18cb2b201e09fd2631b1e5ea066adfc37

services/web/app/src/Features/User/UserEmailsController.js

@@ -3,6 +3,7 @@ const UserGetter = require('./UserGetter')
 const UserUpdater = require('./UserUpdater')
 const EmailHandler = require('../Email/EmailHandler')
 const EmailHelper = require('../Helpers/EmailHelper')
+const UserAuditLogHandler = require('./UserAuditLogHandler')
 const UserEmailsConfirmationHandler = require('./UserEmailsConfirmationHandler')
 const { endorseAffiliation } = require('../Institutions/InstitutionsAPI')
 const Errors = require('../Errors/Errors')
@@ -23,6 +24,16 @@ async function add(req, res, next) {
     department: req.body.department
   }
 
+  await UserAuditLogHandler.promises.addEntry(
+    user._id,
+    'add-email',
+    user._id,
+    req.ip,
+    {
+      newSecondaryEmail: email
+    }
+  )
+
   try {
     await UserUpdater.promises.addEmailAddress(
       userId,
@@ -185,8 +196,4 @@ const UserEmailsController = {
   }
 }
 
-UserEmailsController.promises = {
-  add
-}
-
 module.exports = UserEmailsController

services/web/test/acceptance/src/UserEmailsTests.js

@@ -1,21 +1,7 @@
-/* eslint-disable
-    handle-callback-err,
-    max-len,
-    no-unused-vars,
-*/
-// TODO: This file was created by bulk-decaffeinate.
-// Fix any style issues and re-enable lint.
-/*
- * decaffeinate suggestions:
- * DS102: Remove unnecessary code created because of implicit returns
- * DS207: Consider shorter variations of null checks
- * Full docs: https://github.com/decaffeinate/decaffeinate/blob/master/docs/suggestions.md
- */
 const { expect } = require('chai')
 const async = require('async')
 const User = require('./helpers/User')
-const request = require('./helpers/request')
-const settings = require('settings-sharelatex')
+const UserHelper = require('./helpers/UserHelper')
 const { db, ObjectId } = require('../../../app/src/infrastructure/mongojs')
 const MockV1Api = require('./helpers/MockV1Api')
 const expectErrorResponse = require('./helpers/expectErrorResponse')
@@ -24,16 +10,16 @@ describe('UserEmails', function() {
   beforeEach(function(done) {
     this.timeout(20000)
     this.user = new User()
-    return this.user.login(done)
+    this.user.login(done)
   })
 
   describe('confirming an email', function() {
     it('should confirm the email', function(done) {
       let token = null
-      return async.series(
+      async.series(
         [
           cb => {
-            return this.user.request(
+            this.user.request(
               {
                 method: 'POST',
                 url: '/user/emails',
@@ -42,33 +28,33 @@ describe('UserEmails', function() {
                 }
               },
               (error, response, body) => {
-                if (error != null) {
-                  return done(error)
-                }
+                expect(error).to.not.exist
                 expect(response.statusCode).to.equal(204)
-                return cb()
+                cb()
               }
             )
           },
           cb => {
-            return this.user.request(
+            this.user.request(
               { url: '/user/emails', json: true },
               (error, response, body) => {
+                expect(error).to.not.exist
                 expect(response.statusCode).to.equal(200)
                 expect(body[0].confirmedAt).to.not.exist
                 expect(body[1].confirmedAt).to.not.exist
-                return cb()
+                cb()
               }
             )
           },
           cb => {
-            return db.tokens.find(
+            db.tokens.find(
               {
                 use: 'email_confirmation',
                 'data.user_id': this.user._id,
                 usedAt: { $exists: false }
               },
               (error, tokens) => {
+                expect(error).to.not.exist
                 // There should only be one confirmation token at the moment
                 expect(tokens.length).to.equal(1)
                 expect(tokens[0].data.email).to.equal(
@@ -76,12 +62,12 @@ describe('UserEmails', function() {
                 )
                 expect(tokens[0].data.user_id).to.equal(this.user._id)
                 ;({ token } = tokens[0])
-                return cb()
+                cb()
               }
             )
           },
           cb => {
-            return this.user.request(
+            this.user.request(
               {
                 method: 'POST',
                 url: '/user/emails/confirm',
@@ -90,36 +76,36 @@ describe('UserEmails', function() {
                 }
               },
               (error, response, body) => {
-                if (error != null) {
-                  return done(error)
-                }
+                expect(error).to.not.exist
                 expect(response.statusCode).to.equal(200)
-                return cb()
+                cb()
               }
             )
           },
           cb => {
-            return this.user.request(
+            this.user.request(
               { url: '/user/emails', json: true },
               (error, response, body) => {
+                expect(error).to.not.exist
                 expect(response.statusCode).to.equal(200)
                 expect(body[0].confirmedAt).to.not.exist
                 expect(body[1].confirmedAt).to.exist
-                return cb()
+                cb()
               }
             )
           },
           cb => {
-            return db.tokens.find(
+            db.tokens.find(
               {
                 use: 'email_confirmation',
                 'data.user_id': this.user._id,
                 usedAt: { $exists: false }
               },
               (error, tokens) => {
+                expect(error).to.not.exist
                 // Token should be deleted after use
                 expect(tokens.length).to.equal(0)
-                return cb()
+                cb()
               }
             )
           }
@@ -133,12 +119,12 @@ describe('UserEmails', function() {
       let token2 = null
       this.user2 = new User()
       this.email = 'duplicate-email@example.com'
-      return async.series(
+      async.series(
         [
           cb => this.user2.login(cb),
           cb => {
             // Create email for first user
-            return this.user.request(
+            this.user.request(
               {
                 method: 'POST',
                 url: '/user/emails',
@@ -148,25 +134,26 @@ describe('UserEmails', function() {
             )
           },
           cb => {
-            return db.tokens.find(
+            db.tokens.find(
               {
                 use: 'email_confirmation',
                 'data.user_id': this.user._id,
                 usedAt: { $exists: false }
               },
               (error, tokens) => {
+                expect(error).to.not.exist
                 // There should only be one confirmation token at the moment
                 expect(tokens.length).to.equal(1)
                 expect(tokens[0].data.email).to.equal(this.email)
                 expect(tokens[0].data.user_id).to.equal(this.user._id)
                 token1 = tokens[0].token
-                return cb()
+                cb()
               }
             )
           },
           cb => {
             // Delete the email from the first user
-            return this.user.request(
+            this.user.request(
               {
                 method: 'POST',
                 url: '/user/emails/delete',
@@ -177,7 +164,7 @@ describe('UserEmails', function() {
           },
           cb => {
             // Create email for second user
-            return this.user2.request(
+            this.user2.request(
               {
                 method: 'POST',
                 url: '/user/emails',
@@ -188,7 +175,7 @@ describe('UserEmails', function() {
           },
           cb => {
             // Original confirmation token should no longer work
-            return this.user.request(
+            this.user.request(
               {
                 method: 'POST',
                 url: '/user/emails/confirm',
@@ -197,34 +184,33 @@ describe('UserEmails', function() {
                 }
               },
               (error, response, body) => {
-                if (error != null) {
-                  return done(error)
-                }
+                expect(error).to.not.exist
                 expect(response.statusCode).to.equal(404)
-                return cb()
+                cb()
               }
             )
           },
           cb => {
-            return db.tokens.find(
+            db.tokens.find(
               {
                 use: 'email_confirmation',
                 'data.user_id': this.user2._id,
                 usedAt: { $exists: false }
               },
               (error, tokens) => {
+                expect(error).to.not.exist
                 // The first token has been used, so this should be token2 now
                 expect(tokens.length).to.equal(1)
                 expect(tokens[0].data.email).to.equal(this.email)
                 expect(tokens[0].data.user_id).to.equal(this.user2._id)
                 token2 = tokens[0].token
-                return cb()
+                cb()
               }
             )
           },
           cb => {
             // Second user should be able to confirm the email
-            return this.user2.request(
+            this.user2.request(
               {
                 method: 'POST',
                 url: '/user/emails/confirm',
@@ -233,22 +219,21 @@ describe('UserEmails', function() {
                 }
               },
               (error, response, body) => {
-                if (error != null) {
-                  return done(error)
-                }
+                expect(error).to.not.exist
                 expect(response.statusCode).to.equal(200)
-                return cb()
+                cb()
               }
             )
           },
           cb => {
-            return this.user2.request(
+            this.user2.request(
               { url: '/user/emails', json: true },
               (error, response, body) => {
+                expect(error).to.not.exist
                 expect(response.statusCode).to.equal(200)
                 expect(body[0].confirmedAt).to.not.exist
                 expect(body[1].confirmedAt).to.exist
-                return cb()
+                cb()
               }
             )
           }
@@ -261,10 +246,10 @@ describe('UserEmails', function() {
   describe('with an expired token', function() {
     it('should not confirm the email', function(done) {
       let token = null
-      return async.series(
+      async.series(
         [
           cb => {
-            return this.user.request(
+            this.user.request(
               {
                 method: 'POST',
                 url: '/user/emails',
@@ -273,33 +258,32 @@ describe('UserEmails', function() {
                 }
               },
               (error, response, body) => {
-                if (error != null) {
-                  return done(error)
-                }
+                expect(error).to.not.exist
                 expect(response.statusCode).to.equal(204)
-                return cb()
+                cb()
               }
             )
           },
           cb => {
-            return db.tokens.find(
+            db.tokens.find(
               {
                 use: 'email_confirmation',
                 'data.user_id': this.user._id,
                 usedAt: { $exists: false }
               },
               (error, tokens) => {
+                expect(error).to.not.exist
                 // There should only be one confirmation token at the moment
                 expect(tokens.length).to.equal(1)
                 expect(tokens[0].data.email).to.equal(this.email)
                 expect(tokens[0].data.user_id).to.equal(this.user._id)
                 ;({ token } = tokens[0])
-                return cb()
+                cb()
               }
             )
           },
           cb => {
-            return db.tokens.update(
+            db.tokens.update(
               {
                 token
               },
@@ -312,7 +296,7 @@ describe('UserEmails', function() {
             )
           },
           cb => {
-            return this.user.request(
+            this.user.request(
               {
                 method: 'POST',
                 url: '/user/emails/confirm',
@@ -321,11 +305,9 @@ describe('UserEmails', function() {
                 }
               },
               (error, response, body) => {
-                if (error != null) {
-                  return done(error)
-                }
+                expect(error).to.not.exist
                 expect(response.statusCode).to.equal(404)
-                return cb()
+                cb()
               }
             )
           }
@@ -337,10 +319,10 @@ describe('UserEmails', function() {
 
   describe('resending the confirmation', function() {
     it('should generate a new token', function(done) {
-      return async.series(
+      async.series(
         [
           cb => {
-            return this.user.request(
+            this.user.request(
               {
                 method: 'POST',
                 url: '/user/emails',
@@ -349,34 +331,33 @@ describe('UserEmails', function() {
                 }
               },
               (error, response, body) => {
-                if (error != null) {
-                  return done(error)
-                }
+                expect(error).to.not.exist
                 expect(response.statusCode).to.equal(204)
-                return cb()
+                cb()
               }
             )
           },
           cb => {
-            return db.tokens.find(
+            db.tokens.find(
               {
                 use: 'email_confirmation',
                 'data.user_id': this.user._id,
                 usedAt: { $exists: false }
               },
               (error, tokens) => {
+                expect(error).to.not.exist
                 // There should only be one confirmation token at the moment
                 expect(tokens.length).to.equal(1)
                 expect(tokens[0].data.email).to.equal(
                   'reconfirmation-email@example.com'
                 )
                 expect(tokens[0].data.user_id).to.equal(this.user._id)
-                return cb()
+                cb()
               }
             )
           },
           cb => {
-            return this.user.request(
+            this.user.request(
               {
                 method: 'POST',
                 url: '/user/emails/resend_confirmation',
@@ -385,22 +366,21 @@ describe('UserEmails', function() {
                 }
               },
               (error, response, body) => {
-                if (error != null) {
-                  return done(error)
-                }
+                expect(error).to.not.exist
                 expect(response.statusCode).to.equal(200)
-                return cb()
+                cb()
               }
             )
           },
           cb => {
-            return db.tokens.find(
+            db.tokens.find(
               {
                 use: 'email_confirmation',
                 'data.user_id': this.user._id,
                 usedAt: { $exists: false }
               },
               (error, tokens) => {
+                expect(error).to.not.exist
                 // There should be two tokens now
                 expect(tokens.length).to.equal(2)
                 expect(tokens[0].data.email).to.equal(
@@ -411,7 +391,7 @@ describe('UserEmails', function() {
                   'reconfirmation-email@example.com'
                 )
                 expect(tokens[1].data.user_id).to.equal(this.user._id)
-                return cb()
+                cb()
               }
             )
           }
@@ -423,10 +403,10 @@ describe('UserEmails', function() {
     it('should create a new token if none exists', function(done) {
       // This should only be for users that have sign up with their main
       // emails before the confirmation system existed
-      return async.series(
+      async.series(
         [
           cb => {
-            return db.tokens.remove(
+            db.tokens.remove(
               {
                 use: 'email_confirmation',
                 'data.user_id': this.user._id,
@@ -436,7 +416,7 @@ describe('UserEmails', function() {
             )
           },
           cb => {
-            return this.user.request(
+            this.user.request(
               {
                 method: 'POST',
                 url: '/user/emails/resend_confirmation',
@@ -445,27 +425,26 @@ describe('UserEmails', function() {
                 }
               },
               (error, response, body) => {
-                if (error != null) {
-                  return done(error)
-                }
+                expect(error).to.not.exist
                 expect(response.statusCode).to.equal(200)
-                return cb()
+                cb()
               }
             )
           },
           cb => {
-            return db.tokens.find(
+            db.tokens.find(
               {
                 use: 'email_confirmation',
                 'data.user_id': this.user._id,
                 usedAt: { $exists: false }
               },
               (error, tokens) => {
+                expect(error).to.not.exist
                 // There should still only be one confirmation token
                 expect(tokens.length).to.equal(1)
                 expect(tokens[0].data.email).to.equal(this.user.email)
                 expect(tokens[0].data.user_id).to.equal(this.user._id)
-                return cb()
+                cb()
               }
             )
           }
@@ -475,10 +454,10 @@ describe('UserEmails', function() {
     })
 
     it("should not allow reconfirmation if the email doesn't match the user", function(done) {
-      return async.series(
+      async.series(
         [
           cb => {
-            return this.user.request(
+            this.user.request(
               {
                 method: 'POST',
                 url: '/user/emails/resend_confirmation',
@@ -487,24 +466,23 @@ describe('UserEmails', function() {
                 }
               },
               (error, response, body) => {
-                if (error != null) {
-                  return done(error)
-                }
+                expect(error).to.not.exist
                 expect(response.statusCode).to.equal(422)
-                return cb()
+                cb()
               }
             )
           },
           cb => {
-            return db.tokens.find(
+            db.tokens.find(
               {
                 use: 'email_confirmation',
                 'data.user_id': this.user._id,
                 usedAt: { $exists: false }
               },
               (error, tokens) => {
+                expect(error).to.not.exist
                 expect(tokens.length).to.equal(0)
-                return cb()
+                cb()
               }
             )
           }
@@ -516,11 +494,10 @@ describe('UserEmails', function() {
 
   describe('setting a default email', function() {
     it('should update confirmed emails for users not in v1', function(done) {
-      const token = null
-      return async.series(
+      async.series(
         [
           cb => {
-            return this.user.request(
+            this.user.request(
               {
                 method: 'POST',
                 url: '/user/emails',
@@ -529,17 +506,15 @@ describe('UserEmails', function() {
                 }
               },
               (error, response, body) => {
-                if (error != null) {
-                  return done(error)
-                }
+                expect(error).to.not.exist
                 expect(response.statusCode).to.equal(204)
-                return cb()
+                cb()
               }
             )
           },
           cb => {
             // Mark the email as confirmed
-            return db.users.update(
+            db.users.update(
               {
                 'emails.email': 'new-confirmed-default@example.com'
               },
@@ -552,7 +527,7 @@ describe('UserEmails', function() {
             )
           },
           cb => {
-            return this.user.request(
+            this.user.request(
               {
                 method: 'POST',
                 url: '/user/emails/default',
@@ -561,24 +536,23 @@ describe('UserEmails', function() {
                 }
               },
               (error, response, body) => {
-                if (error != null) {
-                  return done(error)
-                }
+                expect(error).to.not.exist
                 expect(response.statusCode).to.equal(200)
-                return cb()
+                cb()
               }
             )
           },
           cb => {
-            return this.user.request(
+            this.user.request(
               { url: '/user/emails', json: true },
               (error, response, body) => {
+                expect(error).to.not.exist
                 expect(response.statusCode).to.equal(200)
                 expect(body[0].confirmedAt).to.not.exist
                 expect(body[0].default).to.equal(false)
                 expect(body[1].confirmedAt).to.exist
                 expect(body[1].default).to.equal(true)
-                return cb()
+                cb()
               }
             )
           }
@@ -588,11 +562,10 @@ describe('UserEmails', function() {
     })
 
     it('should not allow changing unconfirmed emails in v1', function(done) {
-      const token = null
-      return async.series(
+      async.series(
         [
           cb => {
-            return db.users.update(
+            db.users.update(
               {
                 _id: ObjectId(this.user._id)
               },
@@ -605,7 +578,7 @@ describe('UserEmails', function() {
             )
           },
           cb => {
-            return this.user.request(
+            this.user.request(
               {
                 method: 'POST',
                 url: '/user/emails',
@@ -614,16 +587,14 @@ describe('UserEmails', function() {
                 }
               },
               (error, response, body) => {
-                if (error != null) {
-                  return done(error)
-                }
+                expect(error).to.not.exist
                 expect(response.statusCode).to.equal(204)
-                return cb()
+                cb()
               }
             )
           },
           cb => {
-            return this.user.request(
+            this.user.request(
               {
                 method: 'POST',
                 url: '/user/emails/default',
@@ -632,21 +603,20 @@ describe('UserEmails', function() {
                 }
               },
               (error, response, body) => {
-                if (error != null) {
-                  return done(error)
-                }
+                expect(error).to.not.exist
                 expect(response.statusCode).to.equal(409)
-                return cb()
+                cb()
               }
             )
           },
           cb => {
-            return this.user.request(
+            this.user.request(
               { url: '/user/emails', json: true },
               (error, response, body) => {
+                expect(error).to.not.exist
                 expect(body[0].default).to.equal(true)
                 expect(body[1].default).to.equal(false)
-                return cb()
+                cb()
               }
             )
           }
@@ -656,11 +626,10 @@ describe('UserEmails', function() {
     })
 
     it('should not update the email in v1', function(done) {
-      const token = null
-      return async.series(
+      async.series(
         [
           cb => {
-            return db.users.update(
+            db.users.update(
               {
                 _id: ObjectId(this.user._id)
               },
@@ -673,7 +642,7 @@ describe('UserEmails', function() {
             )
           },
           cb => {
-            return this.user.request(
+            this.user.request(
               {
                 method: 'POST',
                 url: '/user/emails',
@@ -682,17 +651,15 @@ describe('UserEmails', function() {
                 }
               },
               (error, response, body) => {
-                if (error != null) {
-                  return done(error)
-                }
+                expect(error).to.not.exist
                 expect(response.statusCode).to.equal(204)
-                return cb()
+                cb()
               }
             )
           },
           cb => {
             // Mark the email as confirmed
-            return db.users.update(
+            db.users.update(
               {
                 'emails.email': 'new-confirmed-default-in-v1@example.com'
               },
@@ -705,7 +672,7 @@ describe('UserEmails', function() {
             )
           },
           cb => {
-            return this.user.request(
+            this.user.request(
               {
                 method: 'POST',
                 url: '/user/emails/default',
@@ -714,19 +681,15 @@ describe('UserEmails', function() {
                 }
               },
               (error, response, body) => {
-                if (error != null) {
-                  return done(error)
-                }
+                expect(error).to.not.exist
                 expect(response.statusCode).to.equal(200)
-                return cb()
+                cb()
               }
             )
           }
         ],
         error => {
-          if (error != null) {
-            return done(error)
-          }
+          expect(error).to.not.exist
           expect(MockV1Api.updateEmail.callCount).to.equal(0)
           done()
         }
@@ -735,10 +698,10 @@ describe('UserEmails', function() {
 
     it('should not return an error if the email exists in v1', function(done) {
       MockV1Api.existingEmails.push('exists-in-v1@example.com')
-      return async.series(
+      async.series(
         [
           cb => {
-            return db.users.update(
+            db.users.update(
               {
                 _id: ObjectId(this.user._id)
               },
@@ -751,7 +714,7 @@ describe('UserEmails', function() {
             )
           },
           cb => {
-            return this.user.request(
+            this.user.request(
               {
                 method: 'POST',
                 url: '/user/emails',
@@ -760,17 +723,15 @@ describe('UserEmails', function() {
                 }
               },
               (error, response, body) => {
-                if (error != null) {
-                  return done(error)
-                }
+                expect(error).to.not.exist
                 expect(response.statusCode).to.equal(204)
-                return cb()
+                cb()
               }
             )
           },
           cb => {
             // Mark the email as confirmed
-            return db.users.update(
+            db.users.update(
               {
                 'emails.email': 'exists-in-v1@example.com'
               },
@@ -783,7 +744,7 @@ describe('UserEmails', function() {
             )
           },
           cb => {
-            return this.user.request(
+            this.user.request(
               {
                 method: 'POST',
                 url: '/user/emails/default',
@@ -792,21 +753,20 @@ describe('UserEmails', function() {
                 }
               },
               (error, response, body) => {
-                if (error != null) {
-                  return done(error)
-                }
+                expect(error).to.not.exist
                 expect(response.statusCode).to.equal(200)
                 cb()
               }
             )
           },
           cb => {
-            return this.user.request(
+            this.user.request(
               { url: '/user/emails', json: true },
               (error, response, body) => {
+                expect(error).to.not.exist
                 expect(body[0].default).to.equal(false)
                 expect(body[1].default).to.equal(true)
-                return cb()
+                cb()
               }
             )
           }
@@ -840,4 +800,36 @@ describe('UserEmails', function() {
       )
     })
   })
+
+  describe('secondary email', function() {
+    let newEmail, userHelper, userId, user
+    beforeEach(async function() {
+      newEmail = 'a-new-email@overleaf.com'
+      userHelper = new UserHelper()
+      userHelper = await UserHelper.createUser()
+      userHelper = await UserHelper.loginUser({
+        email: userHelper.getDefaultEmail(),
+        password: userHelper.getDefaultPassword()
+      })
+      userId = userHelper.user._id
+      await userHelper.request.post({
+        form: {
+          email: newEmail
+        },
+        simple: false,
+        uri: '/user/emails'
+      })
+      userHelper = await UserHelper.getUser(userId)
+      user = userHelper.user
+    })
+    it('should add the email', async function() {
+      expect(user.emails[1].email).to.equal(newEmail)
+    })
+    it('should add to the user audit log', async function() {
+      expect(typeof user.auditLog[0].initiatorId).to.equal('object')
+      expect(user.auditLog[0].initiatorId).to.deep.equal(user._id)
+      expect(user.auditLog[0].info.newSecondaryEmail).to.equal(newEmail)
+      expect(user.auditLog[0].ip).to.equal(this.user.request.ip)
+    })
+  })
 })

services/web/test/unit/src/User/UserEmailsControllerTests.js

@@ -66,6 +66,11 @@ describe('UserEmailsController', function() {
           }
         }),
         '../Helpers/EmailHelper': this.EmailHelper,
+        './UserAuditLogHandler': (this.UserAuditLogHandler = {
+          promises: {
+            addEntry: sinon.stub().resolves()
+          }
+        }),
         './UserEmailsConfirmationHandler': (this.UserEmailsConfirmationHandler = {
           promises: {
             sendConfirmationEmail: sinon.stub().resolves()
@@ -116,6 +121,21 @@ describe('UserEmailsController', function() {
         .yields()
     })
 
+    it('adds an entry to user audit log', function(done) {
+      this.res.sendStatus = sinon.stub()
+      this.res.sendStatus.callsFake(() => {
+        this.UserAuditLogHandler.promises.addEntry.should.have.been.calledWith(
+          this.user._id,
+          'add-email',
+          this.user._id,
+          this.req.ip,
+          { newSecondaryEmail: this.newEmail }
+        )
+        done()
+      })
+      this.UserEmailsController.add(this.req, this.res)
+    })
+
     it('adds new email', function(done) {
       this.UserEmailsController.add(
         this.req,
@@ -143,20 +163,21 @@ describe('UserEmailsController', function() {
       )
     })
 
-    it('sends a security alert email', async function() {
-      await this.UserEmailsController.promises.add(
-        this.req,
-        this.res,
-        this.next
-      )
-      const emailCall = this.EmailHandler.promises.sendEmail.getCall(0)
-      emailCall.args[0].should.to.equal('securityAlert')
-      emailCall.args[1].to.should.equal(this.user.email)
-      emailCall.args[1].actionDescribed.should.contain(
-        'a secondary email address'
-      )
-      emailCall.args[1].to.should.equal(this.user.email)
-      emailCall.args[1].message[0].should.contain(this.newEmail)
+    it('sends a security alert email', function(done) {
+      this.res.sendStatus = sinon.stub()
+      this.res.sendStatus.callsFake(() => {
+        const emailCall = this.EmailHandler.promises.sendEmail.getCall(0)
+        emailCall.args[0].should.to.equal('securityAlert')
+        emailCall.args[1].to.should.equal(this.user.email)
+        emailCall.args[1].actionDescribed.should.contain(
+          'a secondary email address'
+        )
+        emailCall.args[1].to.should.equal(this.user.email)
+        emailCall.args[1].message[0].should.contain(this.newEmail)
+        done()
+      })
+
+      this.UserEmailsController.add(this.req, this.res)
     })
 
     it('sends an email confirmation', function(done) {
@@ -225,6 +246,22 @@ describe('UserEmailsController', function() {
       })
       this.UserEmailsController.add(this.req, this.res, this.next)
     })
+
+    describe('errors', function() {
+      describe('via UserAuditLogHandler', function() {
+        beforeEach(function() {
+          this.UserAuditLogHandler.promises.addEntry.throws('oops')
+        })
+        it('should not add email and should return error', function(done) {
+          this.UserEmailsController.add(this.req, this.res, error => {
+            expect(error).to.exist
+            this.UserUpdater.promises.addEmailAddress.should.not.have.been
+              .called
+            done()
+          })
+        })
+      })
+    })
   })
 
   describe('remove', function() {