Merge pull request #15515 from overleaf/em-promisify-user-controller

Promisify UserController

GitOrigin-RevId: d56ddb32abff2c33f45efa58285c7bf02b578cd2

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

@@ -16,7 +16,7 @@ const HttpErrorHandler = require('../Errors/HttpErrorHandler')
 const OError = require('@overleaf/o-error')
 const EmailHandler = require('../Email/EmailHandler')
 const UrlHelper = require('../Helpers/UrlHelper')
-const { promisify } = require('util')
+const { promisify, callbackify } = require('util')
 const { expressify } = require('@overleaf/promise-utils')
 const {
   acceptsJson,
@@ -46,15 +46,15 @@ function _sendSecurityAlertPasswordChanged(user) {
     actionDescribed: `your password has been changed on your account ${user.email}`,
     action: 'password changed',
   }
-  EmailHandler.sendEmail('securityAlert', emailOptions, error => {
-    if (error) {
+  EmailHandler.promises
+    .sendEmail('securityAlert', emailOptions)
+    .catch(error => {
       // log error when sending security alert email but do not pass back
       logger.error(
         { error, userId: user._id },
         'could not send security alert email when password changed'
       )
-    }
-  })
+    })
 }
 
 async function _ensureAffiliation(userId, emailData) {
@@ -210,326 +210,276 @@ async function ensureAffiliationMiddleware(req, res, next) {
   return next()
 }
 
-const UserController = {
-  clearSessions: expressify(clearSessions),
+async function tryDeleteUser(req, res, next) {
+  const userId = SessionManager.getLoggedInUserId(req.session)
+  const { password } = req.body
+  req.logger.addFields({ userId })
 
-  tryDeleteUser(req, res, next) {
-    const userId = SessionManager.getLoggedInUserId(req.session)
-    const { password } = req.body
+  if (password == null || password === '') {
+    logger.err({ userId }, 'no password supplied for attempt to delete account')
+    return res.sendStatus(403)
+  }
 
-    if (password == null || password === '') {
-      logger.err(
-        { userId },
-        'no password supplied for attempt to delete account'
-      )
-      return res.sendStatus(403)
+  const user = await AuthenticationManager.promises.authenticate(
+    { _id: userId },
+    password
+  )
+  if (!user) {
+    logger.err({ userId }, 'auth failed during attempt to delete account')
+    return res.sendStatus(403)
+  }
+
+  try {
+    await UserDeleter.promises.deleteUser(userId, {
+      deleterUser: user,
+      ipAddress: req.ip,
+    })
+  } catch (err) {
+    const errorData = {
+      message: 'error while deleting user account',
+      info: { userId },
     }
-    AuthenticationManager.authenticate(
-      { _id: userId },
-      password,
-      (err, user) => {
-        if (err != null) {
-          OError.tag(
-            err,
-            'error authenticating during attempt to delete account',
-            {
-              userId,
-            }
-          )
-          return next(err)
-        }
-        if (!user) {
-          logger.err({ userId }, 'auth failed during attempt to delete account')
-          return res.sendStatus(403)
-        }
-        UserDeleter.deleteUser(
-          userId,
-          { deleterUser: user, ipAddress: req.ip },
-          err => {
-            if (err) {
-              const errorData = {
-                message: 'error while deleting user account',
-                info: { userId },
-              }
-              if (err instanceof Errors.SubscriptionAdminDeletionError) {
-                // set info.public.error for JSON response so frontend can display
-                // a specific message
-                errorData.info.public = {
-                  error: 'SubscriptionAdminDeletionError',
-                }
-                logger.warn(OError.tag(err, errorData.message, errorData.info))
-                return HttpErrorHandler.unprocessableEntity(
-                  req,
-                  res,
-                  errorData.message,
-                  errorData.info.public
-                )
-              } else {
-                return next(OError.tag(err, errorData.message, errorData.info))
-              }
-            }
-            const sessionId = req.sessionID
-            if (typeof req.logout === 'function') {
-              req.logout()
-            }
-            req.session.destroy(err => {
-              if (err != null) {
-                OError.tag(err, 'error destroying session')
-                return next(err)
-              }
-              UserSessionsManager.untrackSession(user, sessionId, () => {})
-              res.sendStatus(200)
-            })
-          }
-        )
+    if (err instanceof Errors.SubscriptionAdminDeletionError) {
+      // set info.public.error for JSON response so frontend can display
+      // a specific message
+      errorData.info.public = {
+        error: 'SubscriptionAdminDeletionError',
       }
-    )
-  },
+      logger.warn(OError.tag(err, errorData.message, errorData.info))
+      return HttpErrorHandler.unprocessableEntity(
+        req,
+        res,
+        errorData.message,
+        errorData.info.public
+      )
+    } else {
+      throw err
+    }
+  }
 
-  subscribe(req, res, next) {
-    const userId = SessionManager.getLoggedInUserId(req.session)
-    UserGetter.getUser(
-      userId,
-      { _id: 1, email: 1, first_name: 1, last_name: 1 },
-      (err, user) => {
-        if (err != null) {
-          return next(err)
-        }
-        NewsletterManager.subscribe(user, err => {
-          if (err != null) {
-            OError.tag(err, 'error subscribing to newsletter')
-            return next(err)
-          }
-          return res.json({
-            message: req.i18n.translate('thanks_settings_updated'),
-          })
-        })
-      }
-    )
-  },
+  const sessionId = req.sessionID
 
-  unsubscribe(req, res, next) {
-    const userId = SessionManager.getLoggedInUserId(req.session)
-    UserGetter.getUser(
-      userId,
-      { _id: 1, email: 1, first_name: 1, last_name: 1 },
-      (err, user) => {
-        if (err != null) {
-          return next(err)
-        }
-        NewsletterManager.unsubscribe(user, err => {
-          if (err != null) {
-            OError.tag(err, 'error unsubscribing to newsletter')
-            return next(err)
-          }
-          Modules.hooks.fire('newsletterUnsubscribed', user, err => {
-            if (err) {
-              OError.tag(err, 'error firing "newsletterUnsubscribed" hook')
-              return next(err)
-            }
-            return res.json({
-              message: req.i18n.translate('thanks_settings_updated'),
-            })
-          })
-        })
-      }
-    )
-  },
+  if (typeof req.logout === 'function') {
+    req.logout()
+  }
 
-  updateUserSettings(req, res, next) {
-    const userId = SessionManager.getLoggedInUserId(req.session)
-    User.findById(userId, (err, user) => {
-      if (err != null || user == null) {
-        logger.err({ err, userId }, 'problem updaing user settings')
-        return res.sendStatus(500)
-      }
+  const destroySession = promisify(req.session.destroy.bind(req.session))
+  await destroySession()
 
-      if (req.body.first_name != null) {
-        user.first_name = req.body.first_name.trim()
-      }
-      if (req.body.last_name != null) {
-        user.last_name = req.body.last_name.trim()
-      }
-      if (req.body.role != null) {
-        user.role = req.body.role.trim()
-      }
-      if (req.body.institution != null) {
-        user.institution = req.body.institution.trim()
-      }
-      if (req.body.mode != null) {
-        user.ace.mode = req.body.mode
-      }
-      if (req.body.editorTheme != null) {
-        user.ace.theme = req.body.editorTheme
-      }
-      if (req.body.overallTheme != null) {
-        user.ace.overallTheme = req.body.overallTheme
-      }
-      if (req.body.fontSize != null) {
-        user.ace.fontSize = req.body.fontSize
-      }
-      if (req.body.autoComplete != null) {
-        user.ace.autoComplete = req.body.autoComplete
-      }
-      if (req.body.autoPairDelimiters != null) {
-        user.ace.autoPairDelimiters = req.body.autoPairDelimiters
-      }
-      if (req.body.spellCheckLanguage != null) {
-        user.ace.spellCheckLanguage = req.body.spellCheckLanguage
-      }
-      if (req.body.pdfViewer != null) {
-        user.ace.pdfViewer = req.body.pdfViewer
-      }
-      if (req.body.syntaxValidation != null) {
-        user.ace.syntaxValidation = req.body.syntaxValidation
-      }
-      if (req.body.fontFamily != null) {
-        user.ace.fontFamily = req.body.fontFamily
-      }
-      if (req.body.lineHeight != null) {
-        user.ace.lineHeight = req.body.lineHeight
-      }
+  UserSessionsManager.promises.untrackSession(user, sessionId).catch(err => {
+    logger.warn({ err, userId: user._id }, 'failed to untrack session')
+  })
+  res.sendStatus(200)
+}
 
-      user.save(err => {
-        if (err != null) {
-          return next(err)
-        }
-        const newEmail =
-          req.body.email != null
-            ? req.body.email.trim().toLowerCase()
-            : undefined
-        if (
-          newEmail == null ||
-          newEmail === user.email ||
-          req.externalAuthenticationSystemUsed()
-        ) {
-          // end here, don't update email
-          SessionManager.setInSessionUser(req.session, {
-            first_name: user.first_name,
-            last_name: user.last_name,
-          })
-          res.sendStatus(200)
-        } else if (newEmail.indexOf('@') === -1) {
-          // email invalid
-          res.sendStatus(400)
-        } else {
-          // update the user email
-          const auditLog = {
-            initiatorId: userId,
-            ipAddress: req.ip,
-          }
-          UserUpdater.changeEmailAddress(userId, newEmail, auditLog, err => {
-            if (err) {
-              if (err instanceof Errors.EmailExistsError) {
-                const translation = req.i18n.translate(
-                  'email_already_registered'
-                )
-                return HttpErrorHandler.conflict(req, res, translation)
-              } else {
-                return HttpErrorHandler.legacyInternal(
-                  req,
-                  res,
-                  req.i18n.translate('problem_changing_email_address'),
-                  OError.tag(err, 'problem_changing_email_address', {
-                    userId,
-                    newEmail,
-                  })
-                )
-              }
-            }
-            User.findById(userId, (err, user) => {
-              if (err != null) {
-                logger.err(
-                  { err, userId },
-                  'error getting user for email update'
-                )
-                return res.sendStatus(500)
-              }
-              SessionManager.setInSessionUser(req.session, {
-                email: user.email,
-                first_name: user.first_name,
-                last_name: user.last_name,
-              })
-              UserHandler.populateTeamInvites(user, err => {
-                // need to refresh this in the background
-                if (err != null) {
-                  logger.err({ err }, 'error populateTeamInvites')
-                }
-                res.sendStatus(200)
-              })
-            })
-          })
-        }
-      })
-    })
-  },
+async function subscribe(req, res, next) {
+  const userId = SessionManager.getLoggedInUserId(req.session)
+  req.logger.addFields({ userId })
 
-  doLogout(req, cb) {
-    metrics.inc('user.logout')
-    const user = SessionManager.getSessionUser(req.session)
-    logger.debug({ user }, 'logging out')
-    const sessionId = req.sessionID
-    if (typeof req.logout === 'function') {
-      req.logout()
-    } // passport logout
-    req.session.destroy(err => {
-      if (err) {
-        OError.tag(err, 'error destroying session')
-        return cb(err)
-      }
-      if (user != null) {
-        UserSessionsManager.untrackSession(user, sessionId, () => {})
-      }
-      cb()
-    })
-  },
+  const user = await UserGetter.promises.getUser(userId, {
+    _id: 1,
+    email: 1,
+    first_name: 1,
+    last_name: 1,
+  })
+  await NewsletterManager.promises.subscribe(user)
+  res.json({
+    message: req.i18n.translate('thanks_settings_updated'),
+  })
+}
 
-  logout(req, res, next) {
-    const requestedRedirect = req.body.redirect
-      ? UrlHelper.getSafeRedirectPath(req.body.redirect)
-      : undefined
-    const redirectUrl = requestedRedirect || '/login'
+async function unsubscribe(req, res, next) {
+  const userId = SessionManager.getLoggedInUserId(req.session)
+  req.logger.addFields({ userId })
 
-    UserController.doLogout(req, err => {
-      if (err != null) {
-        return next(err)
-      }
-      if (acceptsJson(req)) {
-        res.status(200).json({ redir: redirectUrl })
-      } else {
-        res.redirect(redirectUrl)
-      }
+  const user = await UserGetter.promises.getUser(userId, {
+    _id: 1,
+    email: 1,
+    first_name: 1,
+    last_name: 1,
+  })
+  await NewsletterManager.promises.unsubscribe(user)
+  await Modules.promises.hooks.fire('newsletterUnsubscribed', user)
+  res.json({
+    message: req.i18n.translate('thanks_settings_updated'),
+  })
+}
+
+async function updateUserSettings(req, res, next) {
+  const userId = SessionManager.getLoggedInUserId(req.session)
+  req.logger.addFields({ userId })
+
+  const user = await User.findById(userId).exec()
+  if (user == null) {
+    throw new OError('problem updating user settings', { userId })
+  }
+
+  if (req.body.first_name != null) {
+    user.first_name = req.body.first_name.trim()
+  }
+  if (req.body.last_name != null) {
+    user.last_name = req.body.last_name.trim()
+  }
+  if (req.body.role != null) {
+    user.role = req.body.role.trim()
+  }
+  if (req.body.institution != null) {
+    user.institution = req.body.institution.trim()
+  }
+  if (req.body.mode != null) {
+    user.ace.mode = req.body.mode
+  }
+  if (req.body.editorTheme != null) {
+    user.ace.theme = req.body.editorTheme
+  }
+  if (req.body.overallTheme != null) {
+    user.ace.overallTheme = req.body.overallTheme
+  }
+  if (req.body.fontSize != null) {
+    user.ace.fontSize = req.body.fontSize
+  }
+  if (req.body.autoComplete != null) {
+    user.ace.autoComplete = req.body.autoComplete
+  }
+  if (req.body.autoPairDelimiters != null) {
+    user.ace.autoPairDelimiters = req.body.autoPairDelimiters
+  }
+  if (req.body.spellCheckLanguage != null) {
+    user.ace.spellCheckLanguage = req.body.spellCheckLanguage
+  }
+  if (req.body.pdfViewer != null) {
+    user.ace.pdfViewer = req.body.pdfViewer
+  }
+  if (req.body.syntaxValidation != null) {
+    user.ace.syntaxValidation = req.body.syntaxValidation
+  }
+  if (req.body.fontFamily != null) {
+    user.ace.fontFamily = req.body.fontFamily
+  }
+  if (req.body.lineHeight != null) {
+    user.ace.lineHeight = req.body.lineHeight
+  }
+  await user.save()
+
+  const newEmail = req.body.email?.trim().toLowerCase()
+  if (
+    newEmail == null ||
+    newEmail === user.email ||
+    req.externalAuthenticationSystemUsed()
+  ) {
+    // end here, don't update email
+    SessionManager.setInSessionUser(req.session, {
+      first_name: user.first_name,
+      last_name: user.last_name,
     })
-  },
+    res.sendStatus(200)
+  } else if (newEmail.indexOf('@') === -1) {
+    // email invalid
+    res.sendStatus(400)
+  } else {
+    // update the user email
+    const auditLog = {
+      initiatorId: userId,
+      ipAddress: req.ip,
+    }
 
-  expireDeletedUser(req, res, next) {
-    const userId = req.params.userId
-    UserDeleter.expireDeletedUser(userId, error => {
-      if (error) {
-        return next(error)
+    try {
+      await UserUpdater.promises.changeEmailAddress(userId, newEmail, auditLog)
+    } catch (err) {
+      if (err instanceof Errors.EmailExistsError) {
+        const translation = req.i18n.translate('email_already_registered')
+        return HttpErrorHandler.conflict(req, res, translation)
+      } else {
+        return HttpErrorHandler.legacyInternal(
+          req,
+          res,
+          req.i18n.translate('problem_changing_email_address'),
+          OError.tag(err, 'problem_changing_email_address', {
+            userId,
+            newEmail,
+          })
+        )
       }
+    }
 
-      res.sendStatus(204)
+    const user = await User.findById(userId).exec()
+    SessionManager.setInSessionUser(req.session, {
+      email: user.email,
+      first_name: user.first_name,
+      last_name: user.last_name,
     })
-  },
 
-  expireDeletedUsersAfterDuration(req, res, next) {
-    UserDeleter.expireDeletedUsersAfterDuration(error => {
-      if (error) {
-        return next(error)
-      }
+    try {
+      await UserHandler.promises.populateTeamInvites(user)
+    } catch (err) {
+      logger.error({ err }, 'error populateTeamInvites')
+    }
+
+    res.sendStatus(200)
+  }
+}
+
+async function doLogout(req) {
+  metrics.inc('user.logout')
+  const user = SessionManager.getSessionUser(req.session)
+  logger.debug({ user }, 'logging out')
+  const sessionId = req.sessionID
+
+  // passport logout
+  if (typeof req.logout === 'function') {
+    req.logout()
+  }
 
-      res.sendStatus(204)
+  const destroySession = promisify(req.session.destroy.bind(req.session))
+  await destroySession()
+
+  if (user != null) {
+    UserSessionsManager.promises.untrackSession(user, sessionId).catch(err => {
+      logger.warn({ err, userId: user._id }, 'failed to untrack session')
     })
-  },
+  }
+}
 
-  changePassword: expressify(changePassword),
+async function logout(req, res, next) {
+  const requestedRedirect = req.body.redirect
+    ? UrlHelper.getSafeRedirectPath(req.body.redirect)
+    : undefined
+  const redirectUrl = requestedRedirect || '/login'
+
+  await doLogout(req)
+
+  if (acceptsJson(req)) {
+    res.status(200).json({ redir: redirectUrl })
+  } else {
+    res.redirect(redirectUrl)
+  }
+}
+
+async function expireDeletedUser(req, res, next) {
+  const userId = req.params.userId
+  await UserDeleter.promises.expireDeletedUser(userId)
+  res.sendStatus(204)
 }
 
-UserController.promises = {
-  doLogout: promisify(UserController.doLogout),
-  ensureAffiliation,
-  ensureAffiliationMiddleware,
+async function expireDeletedUsersAfterDuration(req, res, next) {
+  await UserDeleter.promises.expireDeletedUsersAfterDuration()
+  res.sendStatus(204)
 }
 
-module.exports = UserController
+module.exports = {
+  clearSessions: expressify(clearSessions),
+  changePassword: expressify(changePassword),
+  tryDeleteUser: expressify(tryDeleteUser),
+  subscribe: expressify(subscribe),
+  unsubscribe: expressify(unsubscribe),
+  updateUserSettings: expressify(updateUserSettings),
+  doLogout: callbackify(doLogout),
+  logout: expressify(logout),
+  expireDeletedUser: expressify(expireDeletedUser),
+  expireDeletedUsersAfterDuration: expressify(expireDeletedUsersAfterDuration),
+  promises: {
+    doLogout,
+    ensureAffiliation,
+    ensureAffiliationMiddleware,
+  },
+}

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

@@ -1,3 +1,4 @@
+const { promisifyAll } = require('@overleaf/promise-utils')
 const TeamInvitesHandler = require('../Subscription/TeamInvitesHandler')
 
 const UserHandler = {
@@ -13,3 +14,4 @@ const UserHandler = {
   },
 }
 module.exports = UserHandler
+module.exports.promises = promisifyAll(UserHandler)

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

@@ -243,6 +243,7 @@ const UserSessionsManager = {
 UserSessionsManager.promises = {
   getAllUserSessions: promisify(UserSessionsManager.getAllUserSessions),
   revokeAllUserSessions: promisify(UserSessionsManager.revokeAllUserSessions),
+  untrackSession: promisify(UserSessionsManager.untrackSession),
 }
 
 module.exports = UserSessionsManager

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

@@ -12,7 +12,7 @@ describe('UserController', function () {
     this.user = {
       _id: this.user_id,
       email: 'email@overleaf.com',
-      save: sinon.stub().callsArgWith(0),
+      save: sinon.stub().resolves(),
       ace: {},
     }
 
@@ -34,29 +34,37 @@ describe('UserController', function () {
       ip: '0:0:0:0',
       query: {},
       headers: {},
+      logger: {
+        addFields: sinon.stub(),
+      },
     }
 
-    this.UserDeleter = { deleteUser: sinon.stub().yields() }
+    this.UserDeleter = { promises: { deleteUser: sinon.stub().resolves() } }
+
     this.UserGetter = {
-      getUser: sinon.stub().yields(null, this.user),
       promises: { getUser: sinon.stub().resolves(this.user) },
     }
-    this.User = { findById: sinon.stub().yields(null, this.user) }
-    this.NewsLetterManager = {
-      subscribe: sinon.stub().yields(),
-      unsubscribe: sinon.stub().yields(),
+
+    this.User = {
+      findById: sinon
+        .stub()
+        .returns({ exec: sinon.stub().resolves(this.user) }),
     }
-    this.AuthenticationController = {
-      establishUserSession: sinon.stub().callsArg(2),
+
+    this.NewsLetterManager = {
+      promises: {
+        subscribe: sinon.stub().resolves(),
+        unsubscribe: sinon.stub().resolves(),
+      },
     }
+
     this.SessionManager = {
       getLoggedInUserId: sinon.stub().returns(this.user._id),
       getSessionUser: sinon.stub().returns(this.req.session.user),
       setInSessionUser: sinon.stub(),
     }
+
     this.AuthenticationManager = {
-      authenticate: sinon.stub(),
-      validatePassword: sinon.stub(),
       promises: {
         authenticate: sinon.stub(),
         setUserPassword: sinon.stub(),
@@ -65,24 +73,29 @@ describe('UserController', function () {
         .stub()
         .returns({ type: 'error', key: 'some-key' }),
     }
+
     this.UserUpdater = {
-      changeEmailAddress: sinon.stub(),
       promises: {
+        changeEmailAddress: sinon.stub().resolves(),
         confirmEmail: sinon.stub().resolves(),
         addAffiliationForNewUser: sinon.stub().resolves(),
       },
     }
+
     this.settings = { siteUrl: 'sharelatex.example.com' }
-    this.UserHandler = { populateTeamInvites: sinon.stub().callsArgWith(1) }
+
+    this.UserHandler = {
+      promises: { populateTeamInvites: sinon.stub().resolves() },
+    }
+
     this.UserSessionsManager = {
-      trackSession: sinon.stub(),
-      untrackSession: sinon.stub(),
-      revokeAllUserSessions: sinon.stub().callsArgWith(2, null),
       promises: {
         getAllUserSessions: sinon.stub().resolves(),
         revokeAllUserSessions: sinon.stub().resolves(),
+        untrackSession: sinon.stub().resolves(),
       },
     }
+
     this.HttpErrorHandler = {
       badRequest: sinon.stub(),
       conflict: sinon.stub(),
@@ -97,7 +110,24 @@ describe('UserController', function () {
       .withArgs('https://evil.com')
       .returns(undefined)
     this.UrlHelper.getSafeRedirectPath.returnsArg(0)
-    this.acceptsJson = sinon.stub().returns(false)
+
+    this.Features = {
+      hasFeature: sinon.stub(),
+    }
+
+    this.UserAuditLogHandler = {
+      promises: {
+        addEntry: sinon.stub().resolves(),
+      },
+    }
+
+    this.RequestContentTypeDetection = {
+      acceptsJson: sinon.stub().returns(false),
+    }
+
+    this.EmailHandler = {
+      promises: { sendEmail: sinon.stub().resolves() },
+    }
 
     this.UserController = SandboxedModule.require(modulePath, {
       requires: {
@@ -105,37 +135,22 @@ describe('UserController', function () {
         './UserGetter': this.UserGetter,
         './UserDeleter': this.UserDeleter,
         './UserUpdater': this.UserUpdater,
-        '../../models/User': {
-          User: this.User,
-        },
+        '../../models/User': { User: this.User },
         '../Newsletter/NewsletterManager': this.NewsLetterManager,
         '../Authentication/AuthenticationController':
           this.AuthenticationController,
         '../Authentication/SessionManager': this.SessionManager,
         '../Authentication/AuthenticationManager': this.AuthenticationManager,
-        '../../infrastructure/Features': (this.Features = {
-          hasFeature: sinon.stub(),
-        }),
-        './UserAuditLogHandler': (this.UserAuditLogHandler = {
-          promises: {
-            addEntry: sinon.stub().resolves(),
-          },
-        }),
+        '../../infrastructure/Features': this.Features,
+        './UserAuditLogHandler': this.UserAuditLogHandler,
         './UserHandler': this.UserHandler,
         './UserSessionsManager': this.UserSessionsManager,
         '../Errors/HttpErrorHandler': this.HttpErrorHandler,
         '@overleaf/settings': this.settings,
-        '@overleaf/metrics': {
-          inc() {},
-        },
         '@overleaf/o-error': OError,
-        '../Email/EmailHandler': (this.EmailHandler = {
-          sendEmail: sinon.stub(),
-          promises: { sendEmail: sinon.stub().resolves() },
-        }),
-        '../../infrastructure/RequestContentTypeDetection': {
-          acceptsJson: this.acceptsJson,
-        },
+        '../Email/EmailHandler': this.EmailHandler,
+        '../../infrastructure/RequestContentTypeDetection':
+          this.RequestContentTypeDetection,
       },
     })
 
@@ -154,13 +169,11 @@ describe('UserController', function () {
     beforeEach(function () {
       this.req.body.password = 'wat'
       this.req.logout = sinon.stub()
-      this.req.session.destroy = sinon.stub().callsArgWith(0, null)
+      this.req.session.destroy = sinon.stub().yields()
       this.SessionManager.getLoggedInUserId = sinon
         .stub()
         .returns(this.user._id)
-      this.AuthenticationManager.authenticate = sinon
-        .stub()
-        .callsArgWith(2, null, this.user)
+      this.AuthenticationManager.promises.authenticate.resolves(this.user)
     })
 
     it('should send 200', function (done) {
@@ -173,10 +186,12 @@ describe('UserController', function () {
 
     it('should try to authenticate user', function (done) {
       this.res.sendStatus = code => {
-        this.AuthenticationManager.authenticate.callCount.should.equal(1)
-        this.AuthenticationManager.authenticate
-          .calledWith({ _id: this.user._id }, this.req.body.password)
-          .should.equal(true)
+        this.AuthenticationManager.promises.authenticate.should.have.been
+          .calledOnce
+        this.AuthenticationManager.promises.authenticate.should.have.been.calledWith(
+          { _id: this.user._id },
+          this.req.body.password
+        )
         done()
       }
       this.UserController.tryDeleteUser(this.req, this.res, this.next)
@@ -184,8 +199,10 @@ describe('UserController', function () {
 
     it('should delete the user', function (done) {
       this.res.sendStatus = code => {
-        this.UserDeleter.deleteUser.callCount.should.equal(1)
-        this.UserDeleter.deleteUser.calledWith(this.user._id).should.equal(true)
+        this.UserDeleter.promises.deleteUser.should.have.been.calledOnce
+        this.UserDeleter.promises.deleteUser.should.have.been.calledWith(
+          this.user._id
+        )
         done()
       }
       this.UserController.tryDeleteUser(this.req, this.res, this.next)
@@ -207,9 +224,9 @@ describe('UserController', function () {
 
     describe('when authenticate produces an error', function () {
       beforeEach(function () {
-        this.AuthenticationManager.authenticate = sinon
-          .stub()
-          .callsArgWith(2, new Error('woops'))
+        this.AuthenticationManager.promises.authenticate.rejects(
+          new Error('woops')
+        )
       })
 
       it('should call next with an error', function (done) {
@@ -224,9 +241,7 @@ describe('UserController', function () {
 
     describe('when authenticate does not produce a user', function () {
       beforeEach(function () {
-        this.AuthenticationManager.authenticate = sinon
-          .stub()
-          .callsArgWith(2, null, null)
+        this.AuthenticationManager.promises.authenticate.resolves(null)
       })
 
       it('should return 403', function (done) {
@@ -240,7 +255,7 @@ describe('UserController', function () {
 
     describe('when deleteUser produces an error', function () {
       beforeEach(function () {
-        this.UserDeleter.deleteUser = sinon.stub().yields(new Error('woops'))
+        this.UserDeleter.promises.deleteUser.rejects(new Error('woops'))
       })
 
       it('should call next with an error', function (done) {
@@ -255,9 +270,9 @@ describe('UserController', function () {
 
     describe('when deleteUser produces a known error', function () {
       beforeEach(function () {
-        this.UserDeleter.deleteUser = sinon
-          .stub()
-          .yields(new Errors.SubscriptionAdminDeletionError())
+        this.UserDeleter.promises.deleteUser.rejects(
+          new Errors.SubscriptionAdminDeletionError()
+        )
       })
 
       it('should return a HTTP Unprocessable Entity error', function (done) {
@@ -298,9 +313,9 @@ describe('UserController', function () {
     it('should send the user to subscribe', function (done) {
       this.res.json = data => {
         expect(data.message).to.equal('thanks_settings_updated')
-        this.NewsLetterManager.subscribe
-          .calledWith(this.user)
-          .should.equal(true)
+        this.NewsLetterManager.promises.subscribe.should.have.been.calledWith(
+          this.user
+        )
         done()
       }
       this.UserController.subscribe(this.req, this.res)
@@ -311,12 +326,12 @@ describe('UserController', function () {
     it('should send the user to unsubscribe', function (done) {
       this.res.json = data => {
         expect(data.message).to.equal('thanks_settings_updated')
-        this.NewsLetterManager.unsubscribe
-          .calledWith(this.user)
-          .should.equal(true)
+        this.NewsLetterManager.promises.unsubscribe.should.have.been.calledWith(
+          this.user
+        )
         done()
       }
-      this.UserController.unsubscribe(this.req, this.res)
+      this.UserController.unsubscribe(this.req, this.res, this.next)
     })
   })
 
@@ -333,7 +348,7 @@ describe('UserController', function () {
         this.user.save.called.should.equal(true)
         done()
       }
-      this.UserController.updateUserSettings(this.req, this.res)
+      this.UserController.updateUserSettings(this.req, this.res, this.next)
     })
 
     it('should set the first name', function (done) {
@@ -401,12 +416,13 @@ describe('UserController', function () {
 
     it('should call the user updater with the new email and user _id', function (done) {
       this.req.body.email = this.newEmail.toUpperCase()
-      this.UserUpdater.changeEmailAddress.callsArgWith(3)
       this.res.sendStatus = code => {
         code.should.equal(200)
-        this.UserUpdater.changeEmailAddress
-          .calledWith(this.user_id, this.newEmail, this.auditLog)
-          .should.equal(true)
+        this.UserUpdater.promises.changeEmailAddress.should.have.been.calledWith(
+          this.user_id,
+          this.newEmail,
+          this.auditLog
+        )
         done()
       }
       this.UserController.updateUserSettings(this.req, this.res)
@@ -414,14 +430,15 @@ describe('UserController', function () {
 
     it('should update the email on the session', function (done) {
       this.req.body.email = this.newEmail.toUpperCase()
-      this.UserUpdater.changeEmailAddress.callsArgWith(3)
       let callcount = 0
-      this.User.findById = (id, cb) => {
-        if (++callcount === 2) {
-          this.user.email = this.newEmail
-        }
-        cb(null, this.user)
-      }
+      this.User.findById = id => ({
+        exec: async () => {
+          if (++callcount === 2) {
+            this.user.email = this.newEmail
+          }
+          return this.user
+        },
+      })
       this.res.sendStatus = code => {
         code.should.equal(200)
         this.SessionManager.setInSessionUser
@@ -438,12 +455,11 @@ describe('UserController', function () {
 
     it('should call populateTeamInvites', function (done) {
       this.req.body.email = this.newEmail.toUpperCase()
-      this.UserUpdater.changeEmailAddress.callsArgWith(3)
       this.res.sendStatus = code => {
         code.should.equal(200)
-        this.UserHandler.populateTeamInvites
-          .calledWith(this.user)
-          .should.equal(true)
+        this.UserHandler.promises.populateTeamInvites.should.have.been.calledWith(
+          this.user
+        )
         done()
       }
       this.UserController.updateUserSettings(this.req, this.res)
@@ -452,7 +468,7 @@ describe('UserController', function () {
     describe('when changeEmailAddress yields an error', function () {
       it('should pass on an error and not send a success status', function (done) {
         this.req.body.email = this.newEmail.toUpperCase()
-        this.UserUpdater.changeEmailAddress.callsArgWith(3, new OError())
+        this.UserUpdater.promises.changeEmailAddress.rejects(new OError())
         this.HttpErrorHandler.legacyInternal = sinon.spy(
           (req, res, message, error) => {
             expect(req).to.exist
@@ -473,8 +489,7 @@ describe('UserController', function () {
           done()
         })
         this.req.body.email = this.newEmail.toUpperCase()
-        this.UserUpdater.changeEmailAddress.callsArgWith(
-          3,
+        this.UserUpdater.promises.changeEmailAddress.rejects(
           new Errors.EmailExistsError()
         )
         this.UserController.updateUserSettings(this.req, this.res)
@@ -483,7 +498,6 @@ describe('UserController', function () {
 
     describe('when using an external auth source', function () {
       beforeEach(function () {
-        this.UserUpdater.changeEmailAddress.callsArgWith(2)
         this.newEmail = 'someone23@example.com'
         this.req.externalAuthenticationSystemUsed = sinon.stub().returns(true)
       })
@@ -492,7 +506,7 @@ describe('UserController', function () {
         this.req.body.email = this.newEmail
         this.res.sendStatus = code => {
           code.should.equal(200)
-          this.UserUpdater.changeEmailAddress
+          this.UserUpdater.promises.changeEmailAddress
             .calledWith(this.user_id, this.newEmail)
             .should.equal(false)
           done()
@@ -504,7 +518,7 @@ describe('UserController', function () {
 
   describe('logout', function () {
     beforeEach(function () {
-      this.acceptsJson.returns(false)
+      this.RequestContentTypeDetection.acceptsJson.returns(false)
     })
 
     it('should destroy the session', function (done) {
@@ -522,10 +536,12 @@ describe('UserController', function () {
       this.req.session.destroy = sinon.stub().callsArgWith(0)
       this.res.redirect = url => {
         url.should.equal('/login')
-        this.UserSessionsManager.untrackSession.callCount.should.equal(1)
-        this.UserSessionsManager.untrackSession
-          .calledWith(sinon.match(this.req.user), this.req.sessionID)
-          .should.equal(true)
+        this.UserSessionsManager.promises.untrackSession.should.have.been
+          .calledOnce
+        this.UserSessionsManager.promises.untrackSession.should.have.been.calledWith(
+          sinon.match(this.req.user),
+          this.req.sessionID
+        )
         done()
       }
 
@@ -562,7 +578,7 @@ describe('UserController', function () {
     })
 
     it('should send json with redir property for json request', function (done) {
-      this.acceptsJson.returns(true)
+      this.RequestContentTypeDetection.acceptsJson.returns(true)
       this.req.session.destroy = sinon.stub().callsArgWith(0)
       this.res.status = code => {
         code.should.equal(200)
@@ -580,9 +596,8 @@ describe('UserController', function () {
     describe('success', function () {
       it('should call revokeAllUserSessions', function (done) {
         this.res.sendStatus.callsFake(() => {
-          this.UserSessionsManager.promises.revokeAllUserSessions.callCount.should.equal(
-            1
-          )
+          this.UserSessionsManager.promises.revokeAllUserSessions.should.have
+            .been.calledOnce
           done()
         })
         this.UserController.clearSessions(this.req, this.res)
@@ -717,7 +732,7 @@ describe('UserController', function () {
             actionDescribed: `your password has been changed on your account ${this.user.email}`,
             action: 'password changed',
           }
-          const emailCall = this.EmailHandler.sendEmail.lastCall
+          const emailCall = this.EmailHandler.promises.sendEmail.lastCall
           expect(emailCall.args[0]).to.equal('securityAlert')
           expect(emailCall.args[1]).to.deep.equal(expectedArg)
           done()
@@ -826,7 +841,7 @@ describe('UserController', function () {
             newPassword1: 'newpass',
             newPassword2: 'newpass',
           }
-          this.EmailHandler.sendEmail.yields(anError)
+          this.EmailHandler.promises.sendEmail.rejects(anError)
         })
         it('should not return error but should log it', function (done) {
           this.res.json.callsFake(result => {