Merge pull request #17408 from overleaf/dp-mongoose-callback-autherntication-manager

Promisify AuthenticationManager and AuthenticationManagerTests

GitOrigin-RevId: 8120bf55d19380a6ecf5241ffab8722eff2d4fe3

services/web/app/src/Features/Authentication/AuthenticationManager.js

@@ -10,7 +10,7 @@ const {
   PasswordMustBeDifferentError,
   PasswordReusedError,
 } = require('./AuthenticationErrors')
-const util = require('util')
+const { callbackify } = require('util')
 const HaveIBeenPwned = require('./HaveIBeenPwned')
 const UserAuditLogHandler = require('../User/UserAuditLogHandler')
 const logger = require('@overleaf/logger')
@@ -30,13 +30,9 @@ function _exceedsMaximumLengthRatio(password, maxSimilarity, value) {
   )
 }
 
-const _checkWriteResult = function (result, callback) {
+const _checkWriteResult = function (result) {
   // for MongoDB
-  if (result && result.modifiedCount === 1) {
-    callback(null, true)
-  } else {
-    callback(null, false)
-  }
+  return !!(result && result.modifiedCount === 1)
 }
 
 function _validatePasswordNotTooLong(password) {
@@ -59,144 +55,129 @@ function _metricsForSuccessfulPasswordMatch(password) {
 }
 
 const AuthenticationManager = {
-  _checkUserPassword(query, password, callback) {
+  async _checkUserPassword(query, password) {
     // Using Mongoose for legacy reasons here. The returned User instance
     // gets serialized into the session and there may be subtle differences
     // between the user returned by Mongoose vs mongodb (such as default values)
-    User.findOne(query, (error, user) => {
-      if (error) {
-        return callback(error)
-      }
-      if (!user || !user.hashedPassword) {
-        return callback(null, null, null)
-      }
-      let rounds = 0
-      try {
-        rounds = bcrypt.getRounds(user.hashedPassword)
-      } catch (err) {
-        let prefix, suffix, length
-        if (typeof user.hashedPassword === 'string') {
-          length = user.hashedPassword.length
-          if (user.hashedPassword.length > 50) {
-            // A full bcrypt hash is 60 characters long.
-            prefix = user.hashedPassword.slice(0, '$2a$12$x'.length)
-            suffix = user.hashedPassword.slice(-4)
-          } else if (user.hashedPassword.length > 20) {
-            prefix = user.hashedPassword.slice(0, 4)
-            suffix = user.hashedPassword.slice(-4)
-          } else {
-            prefix = user.hashedPassword.slice(0, 4)
-          }
+    const user = await User.findOne(query).exec()
+
+    if (!user || !user.hashedPassword) {
+      return { user: null, match: null }
+    }
+
+    let rounds = 0
+    try {
+      rounds = bcrypt.getRounds(user.hashedPassword)
+    } catch (err) {
+      let prefix, suffix, length
+      if (typeof user.hashedPassword === 'string') {
+        length = user.hashedPassword.length
+        if (user.hashedPassword.length > 50) {
+          // A full bcrypt hash is 60 characters long.
+          prefix = user.hashedPassword.slice(0, '$2a$12$x'.length)
+          suffix = user.hashedPassword.slice(-4)
+        } else if (user.hashedPassword.length > 20) {
+          prefix = user.hashedPassword.slice(0, 4)
+          suffix = user.hashedPassword.slice(-4)
+        } else {
+          prefix = user.hashedPassword.slice(0, 4)
         }
-        logger.warn(
-          {
-            err,
-            userId: user._id,
-            hashedPassword: {
-              type: typeof user.hashedPassword,
-              length,
-              prefix,
-              suffix,
-            },
-          },
-          'unexpected user.hashedPassword value'
-        )
       }
-      Metrics.inc('bcrypt', 1, {
-        method: 'compare',
-        path: rounds,
-      })
-      bcrypt.compare(password, user.hashedPassword, function (error, match) {
-        if (error) {
-          return callback(error)
-        }
-        if (match) {
-          _metricsForSuccessfulPasswordMatch(password)
-        }
-        callback(null, user, match)
-      })
+      logger.warn(
+        {
+          err,
+          userId: user._id,
+          hashedPassword: {
+            type: typeof user.hashedPassword,
+            length,
+            prefix,
+            suffix,
+          },
+        },
+        'unexpected user.hashedPassword value'
+      )
+    }
+    Metrics.inc('bcrypt', 1, {
+      method: 'compare',
+      path: rounds,
     })
+
+    const match = await bcrypt.compare(password, user.hashedPassword)
+
+    if (match) {
+      _metricsForSuccessfulPasswordMatch(password)
+    }
+
+    return { user, match }
   },
 
-  authenticate(query, password, auditLog, { skipHIBPCheck = false }, callback) {
-    AuthenticationManager._checkUserPassword(
+  async authenticate(query, password, auditLog, { skipHIBPCheck = false }) {
+    const { user, match } = await AuthenticationManager._checkUserPassword(
       query,
-      password,
-      (error, user, match) => {
-        if (error) {
-          return callback(error)
-        }
-        if (!user) {
-          return callback(null, null)
-        }
-        const update = { $inc: { loginEpoch: 1 } }
-        if (!match) {
-          update.$set = { lastFailedLogin: new Date() }
+      password
+    )
+
+    if (!user) {
+      return null
+    }
+
+    const update = { $inc: { loginEpoch: 1 } }
+    if (!match) {
+      update.$set = { lastFailedLogin: new Date() }
+    }
+
+    const result = await User.updateOne(
+      { _id: user._id, loginEpoch: user.loginEpoch },
+      update,
+      {}
+    ).exec()
+
+    if (result.modifiedCount !== 1) {
+      throw new ParallelLoginError()
+    }
+
+    if (!match) {
+      if (!auditLog) {
+        return null
+      } else {
+        try {
+          await UserAuditLogHandler.promises.addEntry(
+            user._id,
+            'failed-password-match',
+            user._id,
+            auditLog.ipAddress,
+            auditLog.info
+          )
+        } catch (err) {
+          logger.error(
+            { userId: user._id, err, info: auditLog.info },
+            'Error while adding AuditLog entry for failed-password-match'
+          )
         }
-        User.updateOne(
-          { _id: user._id, loginEpoch: user.loginEpoch },
-          update,
-          {},
-          (err, result) => {
-            if (err) {
-              return callback(err)
-            }
-            if (result.modifiedCount !== 1) {
-              return callback(new ParallelLoginError())
-            }
-            if (!match) {
-              if (!auditLog) {
-                return callback(null, null)
-              } else {
-                return UserAuditLogHandler.addEntry(
-                  user._id,
-                  'failed-password-match',
-                  user._id,
-                  auditLog.ipAddress,
-                  auditLog.info,
-                  err => {
-                    if (err) {
-                      logger.error(
-                        { userId: user._id, err, info: auditLog.info },
-                        'Error while adding AuditLog entry for failed-password-match'
-                      )
-                    }
-                    callback(null, null)
-                  }
-                )
-              }
-            }
-            AuthenticationManager.checkRounds(
-              user,
-              user.hashedPassword,
-              password,
-              function (err) {
-                if (err) {
-                  return callback(err)
-                }
-                if (skipHIBPCheck) {
-                  callback(null, user)
-                  HaveIBeenPwned.checkPasswordForReuseInBackground(password)
-                  return
-                }
-                HaveIBeenPwned.checkPasswordForReuse(
-                  password,
-                  (err, isPasswordReused) => {
-                    if (err) {
-                      logger.err({ err }, 'cannot check password for re-use')
-                    }
-                    if (isPasswordReused) {
-                      return callback(new PasswordReusedError())
-                    }
-                    callback(null, user)
-                  }
-                )
-              }
-            )
-          }
-        )
+        return null
       }
-    )
+    }
+    await AuthenticationManager.checkRounds(user, user.hashedPassword, password)
+
+    if (skipHIBPCheck) {
+      HaveIBeenPwned.checkPasswordForReuseInBackground(password)
+      return user
+    }
+
+    let isPasswordReused
+    try {
+      isPasswordReused = await HaveIBeenPwned.promises.checkPasswordForReuse(
+        password
+      )
+    } catch (err) {
+      logger.err({ err }, 'cannot check password for re-use')
+    }
+
+    if (isPasswordReused) {
+      throw new PasswordReusedError()
+    }
+
+    return user
   },
 
   validateEmail(email) {
@@ -295,108 +276,93 @@ const AuthenticationManager = {
     return null
   },
 
-  setUserPassword(user, password, callback) {
-    AuthenticationManager.setUserPasswordInV2(user, password, callback)
+  async setUserPassword(user, password) {
+    return await AuthenticationManager.setUserPasswordInV2(user, password)
   },
 
-  checkRounds(user, hashedPassword, password, callback) {
+  async checkRounds(user, hashedPassword, password) {
     // Temporarily disable this function, TODO: re-enable this
     if (Settings.security.disableBcryptRoundsUpgrades) {
       Metrics.inc('bcrypt_check_rounds', 1, { status: 'disabled' })
-      return callback()
+      return
     }
     // check current number of rounds and rehash if necessary
     const currentRounds = bcrypt.getRounds(hashedPassword)
     if (currentRounds < BCRYPT_ROUNDS) {
       Metrics.inc('bcrypt_check_rounds', 1, { status: 'upgrade' })
-      AuthenticationManager._setUserPasswordInMongo(user, password, callback)
+      return await AuthenticationManager._setUserPasswordInMongo(user, password)
     } else {
       Metrics.inc('bcrypt_check_rounds', 1, { status: 'success' })
-      callback()
     }
   },
 
-  hashPassword(password, callback) {
+  async hashPassword(password) {
     // Double-check the size to avoid truncating in bcrypt.
     const error = _validatePasswordNotTooLong(password)
     if (error) {
-      return callback(error)
+      throw error
     }
-    bcrypt.genSalt(BCRYPT_ROUNDS, BCRYPT_MINOR_VERSION, function (error, salt) {
-      if (error) {
-        return callback(error)
-      }
-      Metrics.inc('bcrypt', 1, {
-        method: 'hash',
-        path: BCRYPT_ROUNDS,
-      })
-      bcrypt.hash(password, salt, callback)
+
+    const salt = await bcrypt.genSalt(BCRYPT_ROUNDS, BCRYPT_MINOR_VERSION)
+
+    Metrics.inc('bcrypt', 1, {
+      method: 'hash',
+      path: BCRYPT_ROUNDS,
     })
+    return await bcrypt.hash(password, salt)
   },
 
-  setUserPasswordInV2(user, password, callback) {
+  async setUserPasswordInV2(user, password) {
     if (!user || !user.email || !user._id) {
-      return callback(new Error('invalid user object'))
+      throw new Error('invalid user object')
     }
     const validationError = this.validatePassword(password, user.email)
     if (validationError) {
-      return callback(validationError)
+      throw validationError
     }
     // check if we can log in with this password. In which case we should reject it,
     // because it is the same as the existing password.
-    AuthenticationManager._checkUserPassword(
+    const { match } = await AuthenticationManager._checkUserPassword(
       { _id: user._id },
-      password,
-      (err, _user, match) => {
-        if (err) {
-          return callback(err)
-        }
-        if (match) {
-          return callback(new PasswordMustBeDifferentError())
-        }
+      password
+    )
+
+    if (match) {
+      throw new PasswordMustBeDifferentError()
+    }
 
-        HaveIBeenPwned.checkPasswordForReuse(
-          password,
-          (error, isPasswordReused) => {
-            if (error) {
-              logger.err({ error }, 'cannot check password for re-use')
-            }
+    let isPasswordReused
+    try {
+      isPasswordReused = await HaveIBeenPwned.promises.checkPasswordForReuse(
+        password
+      )
+    } catch (error) {
+      logger.err({ error }, 'cannot check password for re-use')
+    }
 
-            if (!error && isPasswordReused) {
-              return callback(new PasswordReusedError())
-            }
+    if (isPasswordReused) {
+      throw new PasswordReusedError()
+    }
 
-            // password is strong enough or the validation with the service did not happen
-            this._setUserPasswordInMongo(user, password, callback)
-          }
-        )
-      }
-    )
+    // password is strong enough or the validation with the service did not happen
+    return await this._setUserPasswordInMongo(user, password)
   },
 
-  _setUserPasswordInMongo(user, password, callback) {
-    this.hashPassword(password, function (error, hash) {
-      if (error) {
-        return callback(error)
-      }
-      db.users.updateOne(
-        { _id: new ObjectId(user._id.toString()) },
-        {
-          $set: {
-            hashedPassword: hash,
-          },
-          $unset: {
-            password: true,
-          },
+  async _setUserPasswordInMongo(user, password) {
+    const hash = await this.hashPassword(password)
+    const result = await db.users.updateOne(
+      { _id: new ObjectId(user._id.toString()) },
+      {
+        $set: {
+          hashedPassword: hash,
         },
-        function (updateError, result) {
-          if (updateError) {
-            return callback(updateError)
-          }
-          _checkWriteResult(result, callback)
-        }
-      )
-    })
+        $unset: {
+          password: true,
+        },
+      }
+    )
+
+    return _checkWriteResult(result)
   },
 
   _passwordCharactersAreValid(password) {
@@ -500,10 +466,17 @@ const AuthenticationManager = {
   },
 }
 
-AuthenticationManager.promises = {
-  authenticate: util.promisify(AuthenticationManager.authenticate),
-  hashPassword: util.promisify(AuthenticationManager.hashPassword),
-  setUserPassword: util.promisify(AuthenticationManager.setUserPassword),
+module.exports = {
+  _validatePasswordNotTooSimilar:
+    AuthenticationManager._validatePasswordNotTooSimilar, // Private function exported for tests
+  validateEmail: AuthenticationManager.validateEmail,
+  validatePassword: AuthenticationManager.validatePassword,
+  getMessageForInvalidPasswordError:
+    AuthenticationManager.getMessageForInvalidPasswordError,
+  authenticate: callbackify(AuthenticationManager.authenticate),
+  setUserPassword: callbackify(AuthenticationManager.setUserPassword),
+  checkRounds: callbackify(AuthenticationManager.checkRounds),
+  hashPassword: callbackify(AuthenticationManager.hashPassword),
+  setUserPasswordInV2: callbackify(AuthenticationManager.setUserPasswordInV2),
+  promises: AuthenticationManager,
 }
-
-module.exports = AuthenticationManager

services/web/test/unit/src/Authentication/AuthenticationManagerTests.js

@@ -14,14 +14,18 @@ describe('AuthenticationManager', function () {
     this.settings = { security: { bcryptRounds: 4 } }
     this.metrics = { inc: sinon.stub().returns() }
     this.HaveIBeenPwned = {
-      checkPasswordForReuse: sinon.stub().yields(null, false),
+      promises: {
+        checkPasswordForReuse: sinon.stub().resolves(false),
+      },
       checkPasswordForReuseInBackground: sinon.stub(),
     }
     this.AuthenticationManager = SandboxedModule.require(modulePath, {
       requires: {
         '../../models/User': {
           User: (this.User = {
-            updateOne: sinon.stub().callsArgWith(3, null, { modifiedCount: 1 }),
+            updateOne: sinon
+              .stub()
+              .returns({ exec: sinon.stub().resolves({ modifiedCount: 1 }) }),
           }),
         },
         '../../infrastructure/mongodb': {
@@ -32,16 +36,17 @@ describe('AuthenticationManager', function () {
           getRounds: sinon.stub().returns(4),
         }),
         '@overleaf/settings': this.settings,
-        '../User/UserGetter': (this.UserGetter = {}),
+        '../User/UserGetter': (this.UserGetter = { promises: {} }),
         './AuthenticationErrors': AuthenticationErrors,
         './HaveIBeenPwned': this.HaveIBeenPwned,
         '../User/UserAuditLogHandler': (this.UserAuditLogHandler = {
-          addEntry: sinon.stub().callsArgWith(5, null),
+          promises: {
+            addEntry: sinon.stub().resolves(null),
+          },
         }),
         '@overleaf/metrics': this.metrics,
       },
     })
-    this.callback = sinon.stub()
   })
 
   afterEach(function () {
@@ -67,22 +72,20 @@ describe('AuthenticationManager', function () {
           email: (this.email = 'USER@overleaf.com'),
         }
         this.user.hashedPassword = this.testPassword
-        this.User.findOne = sinon.stub().callsArgWith(1, null, this.user)
+        this.User.findOne = sinon
+          .stub()
+          .returns({ exec: sinon.stub().resolves(this.user) })
         this.metrics.inc.reset()
       })
 
       describe('when the hashed password matches', function () {
-        beforeEach(function (done) {
+        beforeEach(async function () {
           this.unencryptedPassword = 'testpassword'
-          this.AuthenticationManager.authenticate(
+          this.result = await this.AuthenticationManager.promises.authenticate(
             { email: this.email },
             this.unencryptedPassword,
             null,
-            { skipHIBPCheck: true },
-            (error, user) => {
-              this.callback(error, user)
-              done()
-            }
+            { skipHIBPCheck: true }
           )
         })
 
@@ -104,7 +107,7 @@ describe('AuthenticationManager', function () {
         })
 
         it('should return the user', function () {
-          this.callback.should.have.been.calledWith(null, this.user)
+          this.result.should.equal(this.user)
         })
 
         it('should send metrics', function () {
@@ -115,16 +118,12 @@ describe('AuthenticationManager', function () {
       })
 
       describe('when the encrypted passwords do not match', function () {
-        beforeEach(function (done) {
-          this.AuthenticationManager.authenticate(
+        beforeEach(async function () {
+          this.result = await this.AuthenticationManager.promises.authenticate(
             { email: this.email },
             'notthecorrectpassword',
             null,
-            { skipHIBPCheck: true },
-            (...args) => {
-              this.callback(...args)
-              done()
-            }
+            { skipHIBPCheck: true }
           )
         })
 
@@ -142,7 +141,7 @@ describe('AuthenticationManager', function () {
         })
 
         it('should not return the user', function () {
-          this.callback.calledWith(null, null).should.equal(true)
+          expect(this.result).to.equal(null)
         })
       })
 
@@ -150,50 +149,37 @@ describe('AuthenticationManager', function () {
         beforeEach(function () {
           this.User.updateOne = sinon
             .stub()
-            .callsArgWith(3, null, { modifiedCount: 0 })
+            .returns({ exec: sinon.stub().resolves({ modifiedCount: 0 }) })
         })
 
         describe('correct password', function () {
-          beforeEach(function (done) {
-            this.AuthenticationManager.authenticate(
-              { email: this.email },
-              'testpassword',
-              null,
-              { skipHIBPCheck: true },
-              (...args) => {
-                this.callback(...args)
-                done()
-              }
-            )
-          })
-
-          it('should return an error', function () {
-            this.callback.should.have.been.calledWith(
-              sinon.match.instanceOf(AuthenticationErrors.ParallelLoginError)
-            )
+          it('should return an error', async function () {
+            await expect(
+              this.AuthenticationManager.promises.authenticate(
+                { email: this.email },
+                'testpassword',
+                null,
+                { skipHIBPCheck: true }
+              )
+            ).to.be.rejectedWith(AuthenticationErrors.ParallelLoginError)
           })
         })
 
         describe('bad password', function () {
-          beforeEach(function (done) {
+          beforeEach(function () {
             this.User.updateOne = sinon
               .stub()
-              .yields(null, { modifiedCount: 0 })
-            this.AuthenticationManager.authenticate(
-              { email: this.email },
-              'notthecorrectpassword',
-              null,
-              { skipHIBPCheck: true },
-              (...args) => {
-                this.callback(...args)
-                done()
-              }
-            )
+              .returns({ exec: sinon.stub().resolves({ modifiedCount: 0 }) })
           })
-          it('should return an error', function () {
-            this.callback.should.have.been.calledWith(
-              sinon.match.instanceOf(AuthenticationErrors.ParallelLoginError)
-            )
+          it('should return an error', async function () {
+            await expect(
+              this.AuthenticationManager.promises.authenticate(
+                { email: this.email },
+                'notthecorrectpassword',
+                null,
+                { skipHIBPCheck: true }
+              )
+            ).to.be.rejectedWith(AuthenticationErrors.ParallelLoginError)
           })
         })
       })
@@ -206,50 +192,41 @@ describe('AuthenticationManager', function () {
           email: (this.email = 'USER@overleaf.com'),
         }
         this.db.users.updateOne = sinon
-        this.User.findOne = sinon.stub().callsArgWith(1, null, this.user)
-        this.bcrypt.compare = sinon.stub().callsArgWith(2, null, false)
-        this.db.users.updateOne = sinon
+        this.User.findOne = sinon
           .stub()
-          .callsArgWith(2, null, { modifiedCount: 1 })
+          .returns({ exec: sinon.stub().resolves(this.user) })
+        this.bcrypt.compare = sinon.stub().resolves(false)
+        this.db.users.updateOne = sinon.stub().resolves({ modifiedCount: 1 })
       })
 
-      it('should not produce an error', function (done) {
-        this.AuthenticationManager.setUserPasswordInV2(
-          this.user,
-          'testpassword',
-          (err, updated) => {
-            expect(err).to.not.exist
-            expect(updated).to.equal(true)
-            done()
-          }
-        )
+      it('should not produce an error', async function () {
+        const updated =
+          await this.AuthenticationManager.promises.setUserPasswordInV2(
+            this.user,
+            'testpassword'
+          )
+        expect(updated).to.equal(true)
       })
 
-      it('should set the hashed password', function (done) {
-        this.AuthenticationManager.setUserPasswordInV2(
+      it('should set the hashed password', async function () {
+        await this.AuthenticationManager.promises.setUserPasswordInV2(
           this.user,
-          'testpassword',
-          err => {
-            expect(err).to.not.exist
-            const { hashedPassword } =
-              this.db.users.updateOne.lastCall.args[1].$set
-            expect(hashedPassword).to.exist
-            expect(hashedPassword.length).to.equal(60)
-            expect(hashedPassword).to.match(/^\$2a\$04\$[a-zA-Z0-9/.]{53}$/)
-            done()
-          }
+          'testpassword'
         )
+
+        const { hashedPassword } = this.db.users.updateOne.lastCall.args[1].$set
+        expect(hashedPassword).to.exist
+        expect(hashedPassword.length).to.equal(60)
+        expect(hashedPassword).to.match(/^\$2a\$04\$[a-zA-Z0-9/.]{53}$/)
       })
     })
   })
 
   describe('hashPassword', function () {
-    it('should block too long passwords', function (done) {
-      this.AuthenticationManager.hashPassword('x'.repeat(100), err => {
-        expect(err).to.exist
-        expect(err.message).to.equal('password is too long')
-        done()
-      })
+    it('should block too long passwords', async function () {
+      await expect(
+        this.AuthenticationManager.promises.hashPassword('x'.repeat(100))
+      ).to.be.rejectedWith('password is too long')
     })
   })
 
@@ -261,24 +238,22 @@ describe('AuthenticationManager', function () {
           email: (this.email = 'USER@overleaf.com'),
         }
         this.unencryptedPassword = 'banana'
-        this.User.findOne = sinon.stub().callsArgWith(1, null, this.user)
+        this.User.findOne = sinon
+          .stub()
+          .returns({ exec: sinon.stub().resolves(this.user) })
         this.metrics.inc.reset()
       })
 
       describe('when the hashed password matches', function () {
-        beforeEach(function (done) {
+        beforeEach(async function () {
           this.user.hashedPassword = this.hashedPassword = 'asdfjadflasdf'
-          this.bcrypt.compare = sinon.stub().callsArgWith(2, null, true)
+          this.bcrypt.compare = sinon.stub().resolves(true)
           this.bcrypt.getRounds = sinon.stub().returns(4)
-          this.AuthenticationManager.authenticate(
+          this.result = await this.AuthenticationManager.promises.authenticate(
             { email: this.email },
             this.unencryptedPassword,
             null,
-            { skipHIBPCheck: true },
-            (error, user) => {
-              this.callback(error, user)
-              done()
-            }
+            { skipHIBPCheck: true }
           )
         })
 
@@ -301,70 +276,65 @@ describe('AuthenticationManager', function () {
         })
 
         it('should return the user', function () {
-          this.callback.calledWith(null, this.user).should.equal(true)
+          this.result.should.equal(this.user)
         })
 
         describe('HIBP', function () {
           it('should not check HIBP if not requested', function () {
-            this.HaveIBeenPwned.checkPasswordForReuse.should.not.have.been
-              .called
+            this.HaveIBeenPwned.promises.checkPasswordForReuse.should.not.have
+              .been.called
           })
 
-          it('should check HIBP if requested', function (done) {
-            this.AuthenticationManager.authenticate(
+          it('should check HIBP if requested', async function () {
+            await this.AuthenticationManager.promises.authenticate(
               { email: this.email },
               this.unencryptedPassword,
               null,
-              { skipHIBPCheck: false },
-              error => {
-                if (error) return done(error)
-                this.HaveIBeenPwned.checkPasswordForReuse.should.have.been.calledWith(
-                  this.unencryptedPassword
-                )
-                done()
-              }
+              { skipHIBPCheck: false }
+            )
+
+            this.HaveIBeenPwned.promises.checkPasswordForReuse.should.have.been.calledWith(
+              this.unencryptedPassword
             )
           })
         })
       })
 
       describe('when the encrypted passwords do not match', function () {
-        beforeEach(function () {
+        beforeEach(async function () {
           this.user.hashedPassword = this.hashedPassword = 'asdfjadflasdf'
-          this.bcrypt.compare = sinon.stub().callsArgWith(2, null, false)
-          this.AuthenticationManager.authenticate(
+          this.bcrypt.compare = sinon.stub().resolves(false)
+          this.result = await this.AuthenticationManager.promises.authenticate(
             { email: this.email },
             this.unencryptedPassword,
             null,
-            { skipHIBPCheck: true },
-            this.callback
+            { skipHIBPCheck: true }
           )
         })
 
         it('should not return the user', function () {
-          this.callback.calledWith(null, null).should.equal(true)
-          this.UserAuditLogHandler.addEntry.callCount.should.equal(0)
+          expect(this.result).to.equal(null)
+          this.UserAuditLogHandler.promises.addEntry.callCount.should.equal(0)
         })
       })
 
       describe('when the encrypted passwords do not match, with auditLog', function () {
-        beforeEach(function () {
+        beforeEach(async function () {
           this.user.hashedPassword = this.hashedPassword = 'asdfjadflasdf'
-          this.bcrypt.compare = sinon.stub().callsArgWith(2, null, false)
+          this.bcrypt.compare = sinon.stub().resolves(false)
           this.auditLog = { ipAddress: 'ip', info: { method: 'foo' } }
-          this.AuthenticationManager.authenticate(
+          this.result = await this.AuthenticationManager.promises.authenticate(
             { email: this.email },
             this.unencryptedPassword,
             this.auditLog,
-            { skipHIBPCheck: true },
-            this.callback
+            { skipHIBPCheck: true }
           )
         })
 
         it('should not return the user, but add entry to audit log', function () {
-          this.callback.calledWith(null, null).should.equal(true)
-          this.UserAuditLogHandler.addEntry.callCount.should.equal(1)
-          this.UserAuditLogHandler.addEntry
+          expect(this.result).to.equal(null)
+          this.UserAuditLogHandler.promises.addEntry.callCount.should.equal(1)
+          this.UserAuditLogHandler.promises.addEntry
             .calledWith(
               this.user._id,
               'failed-password-match',
@@ -377,22 +347,18 @@ describe('AuthenticationManager', function () {
       })
 
       describe('when the hashed password matches but the number of rounds is too low', function () {
-        beforeEach(function (done) {
+        beforeEach(async function () {
           this.user.hashedPassword = this.hashedPassword = 'asdfjadflasdf'
-          this.bcrypt.compare = sinon.stub().callsArgWith(2, null, true)
+          this.bcrypt.compare = sinon.stub().resolves(true)
           this.bcrypt.getRounds = sinon.stub().returns(1)
-          this.AuthenticationManager._setUserPasswordInMongo = sinon
+          this.AuthenticationManager.promises._setUserPasswordInMongo = sinon
             .stub()
-            .callsArgWith(2, null)
-          this.AuthenticationManager.authenticate(
+            .resolves()
+          this.result = await this.AuthenticationManager.promises.authenticate(
             { email: this.email },
             this.unencryptedPassword,
             null,
-            { skipHIBPCheck: true },
-            (error, user) => {
-              this.callback(error, user)
-              done()
-            }
+            { skipHIBPCheck: true }
           )
         })
 
@@ -415,34 +381,30 @@ describe('AuthenticationManager', function () {
         })
 
         it('should set the users password (with a higher number of rounds)', function () {
-          this.AuthenticationManager._setUserPasswordInMongo
+          this.AuthenticationManager.promises._setUserPasswordInMongo
             .calledWith(this.user, this.unencryptedPassword)
             .should.equal(true)
         })
 
         it('should return the user', function () {
-          this.callback.calledWith(null, this.user).should.equal(true)
+          this.result.should.equal(this.user)
         })
       })
 
       describe('when the hashed password matches but the number of rounds is too low, but upgrades disabled', function () {
-        beforeEach(function (done) {
+        beforeEach(async function () {
           this.settings.security.disableBcryptRoundsUpgrades = true
           this.user.hashedPassword = this.hashedPassword = 'asdfjadflasdf'
-          this.bcrypt.compare = sinon.stub().callsArgWith(2, null, true)
+          this.bcrypt.compare = sinon.stub().resolves(true)
           this.bcrypt.getRounds = sinon.stub().returns(1)
-          this.AuthenticationManager.setUserPassword = sinon
+          this.AuthenticationManager.promises.setUserPassword = sinon
             .stub()
-            .callsArgWith(2, null)
-          this.AuthenticationManager.authenticate(
+            .resolves()
+          this.result = await this.AuthenticationManager.promises.authenticate(
             { email: this.email },
             this.unencryptedPassword,
             null,
-            { skipHIBPCheck: true },
-            (error, user) => {
-              this.callback(error, user)
-              done()
-            }
+            { skipHIBPCheck: true }
           )
         })
 
@@ -455,31 +417,32 @@ describe('AuthenticationManager', function () {
         })
 
         it('should not set the users password (with a higher number of rounds)', function () {
-          this.AuthenticationManager.setUserPassword
+          this.AuthenticationManager.promises.setUserPassword
             .calledWith(this.user, this.unencryptedPassword)
             .should.equal(false)
         })
 
         it('should return the user', function () {
-          this.callback.calledWith(null, this.user).should.equal(true)
+          this.result.should.equal(this.user)
         })
       })
     })
 
     describe('when the user does not exist in the database', function () {
-      beforeEach(function () {
-        this.User.findOne = sinon.stub().callsArgWith(1, null, null)
-        this.AuthenticationManager.authenticate(
+      beforeEach(async function () {
+        this.User.findOne = sinon
+          .stub()
+          .returns({ exec: sinon.stub().resolves(null) })
+        this.result = await this.AuthenticationManager.promises.authenticate(
           { email: this.email },
           this.unencrpytedPassword,
           null,
-          { skipHIBPCheck: true },
-          this.callback
+          { skipHIBPCheck: true }
         )
       })
 
       it('should not return a user', function () {
-        this.callback.calledWith(null, null).should.equal(true)
+        expect(this.result).to.equal(null)
       })
     })
   })
@@ -799,28 +762,27 @@ describe('AuthenticationManager', function () {
         email: 'user@example.com',
         hashedPassword: this.hashedPassword,
       }
-      this.bcrypt.compare = sinon.stub().callsArgWith(2, null, false)
-      this.bcrypt.genSalt = sinon.stub().callsArgWith(2, null, this.salt)
-      this.bcrypt.hash = sinon.stub().callsArgWith(2, null, this.hashedPassword)
-      this.User.findOne = sinon.stub().callsArgWith(1, null, this.user)
-      this.db.users.updateOne = sinon.stub().callsArg(2)
+      this.bcrypt.compare = sinon.stub().resolves(false)
+      this.bcrypt.genSalt = sinon.stub().resolves(this.salt)
+      this.bcrypt.hash = sinon.stub().resolves(this.hashedPassword)
+      this.User.findOne = sinon
+        .stub()
+        .returns({ exec: sinon.stub().resolves(this.user) })
+      this.db.users.updateOne = sinon.stub().resolves()
     })
 
     describe('same as previous password', function () {
       beforeEach(function () {
-        this.bcrypt.compare.callsArgWith(2, null, true)
+        this.bcrypt.compare.resolves(true)
       })
 
-      it('should return an error', function (done) {
-        this.AuthenticationManager.setUserPassword(
-          this.user,
-          this.password,
-          err => {
-            expect(err).to.exist
-            expect(err.name).to.equal('PasswordMustBeDifferentError')
-            done()
-          }
-        )
+      it('should be rejected', async function () {
+        await expect(
+          this.AuthenticationManager.promises.setUserPassword(
+            this.user,
+            this.password
+          )
+        ).to.be.rejectedWith(AuthenticationErrors.PasswordMustBeDifferentError)
       })
     })
 
@@ -834,27 +796,25 @@ describe('AuthenticationManager', function () {
         this.password = 'dsdsadsadsadsadsadkjsadjsadjsadljs'
       })
 
-      it('should return and error', function (done) {
-        this.AuthenticationManager.setUserPassword(
-          this.user,
-          this.password,
-          err => {
-            expect(err).to.exist
-            done()
-          }
-        )
+      it('should return and error', async function () {
+        await expect(
+          this.AuthenticationManager.promises.setUserPassword(
+            this.user,
+            this.password
+          )
+        ).to.be.rejectedWith('password is too long')
       })
 
-      it('should not start the bcrypt process', function (done) {
-        this.AuthenticationManager.setUserPassword(
-          this.user,
-          this.password,
-          () => {
-            this.bcrypt.genSalt.called.should.equal(false)
-            this.bcrypt.hash.called.should.equal(false)
-            done()
-          }
-        )
+      it('should not start the bcrypt process', async function () {
+        await expect(
+          this.AuthenticationManager.promises.setUserPassword(
+            this.user,
+            this.password
+          )
+        ).to.be.rejected
+
+        this.bcrypt.genSalt.called.should.equal(false)
+        this.bcrypt.hash.called.should.equal(false)
       })
     })
 
@@ -863,16 +823,13 @@ describe('AuthenticationManager', function () {
         this.password = `some${this.user.email}password`
       })
 
-      it('should reject the password', function (done) {
-        this.AuthenticationManager.setUserPassword(
-          this.user,
-          this.password,
-          err => {
-            expect(err).to.exist
-            expect(err.name).to.equal('InvalidPasswordError')
-            done()
-          }
-        )
+      it('should reject the password', async function () {
+        await expect(
+          this.AuthenticationManager.promises.setUserPassword(
+            this.user,
+            this.password
+          )
+        ).to.be.rejectedWith(AuthenticationErrors.InvalidPasswordError)
       })
     })
 
@@ -881,16 +838,13 @@ describe('AuthenticationManager', function () {
         this.password = `some${this.user.email.split('@')[0]}password`
       })
 
-      it('should reject the password', function (done) {
-        this.AuthenticationManager.setUserPassword(
-          this.user,
-          this.password,
-          err => {
-            expect(err).to.exist
-            expect(err.name).to.equal('InvalidPasswordError')
-            done()
-          }
-        )
+      it('should reject the password', async function () {
+        await expect(
+          this.AuthenticationManager.promises.setUserPassword(
+            this.user,
+            this.password
+          )
+        ).to.be.rejectedWith(AuthenticationErrors.InvalidPasswordError)
       })
     })
 
@@ -901,13 +855,17 @@ describe('AuthenticationManager', function () {
         user = { _id: 'some-user-id', email: 'someuser@somedomain.com' }
       })
 
-      it('should reject the password', function (done) {
-        this.AuthenticationManager.setUserPassword(user, password, err => {
-          expect(err).to.exist
+      it('should reject the password', async function () {
+        try {
+          await this.AuthenticationManager.promises.setUserPassword(
+            user,
+            password
+          )
+          expect.fail('should have thrown')
+        } catch (err) {
           expect(err.name).to.equal('InvalidPasswordError')
           expect(err?.info?.code).to.equal('contains_email')
-          done()
-        })
+        }
       })
     })
 
@@ -922,27 +880,25 @@ describe('AuthenticationManager', function () {
         this.password = 'dsd'
       })
 
-      it('should return and error', function (done) {
-        this.AuthenticationManager.setUserPassword(
-          this.user,
-          this.password,
-          err => {
-            expect(err).to.exist
-            done()
-          }
-        )
+      it('should return and error', async function () {
+        await expect(
+          this.AuthenticationManager.promises.setUserPassword(
+            this.user,
+            this.password
+          )
+        ).to.be.rejectedWith('password is too short')
       })
 
-      it('should not start the bcrypt process', function (done) {
-        this.AuthenticationManager.setUserPassword(
-          this.user,
-          this.password,
-          () => {
-            this.bcrypt.genSalt.called.should.equal(false)
-            this.bcrypt.hash.called.should.equal(false)
-            done()
-          }
-        )
+      it('should not start the bcrypt process', async function () {
+        await expect(
+          this.AuthenticationManager.promises.setUserPassword(
+            this.user,
+            this.password
+          )
+        ).to.be.rejected
+
+        this.bcrypt.genSalt.called.should.equal(false)
+        this.bcrypt.hash.called.should.equal(false)
       })
     })
 
@@ -953,45 +909,54 @@ describe('AuthenticationManager', function () {
         this.metrics.inc.reset()
       })
 
-      it('should produce an error when the password is too similar to the email', function (done) {
-        this.AuthenticationManager.setUserPassword(
-          this.user,
-          this.password,
-          err => {
-            expect(err).to.exist
-            expect(err?.info?.code).to.equal('too_similar')
-            expect(
-              this.metrics.inc.calledWith('password-too-similar-to-email')
-            ).to.equal(true)
-            done()
-          }
-        )
+      it('should produce an error when the password is too similar to the email', async function () {
+        try {
+          await this.AuthenticationManager.promises.setUserPassword(
+            this.user,
+            this.password
+          )
+          expect.fail('should have thrown')
+        } catch (err) {
+          expect(err.message).to.equal(
+            'password is too similar to email address'
+          )
+          expect(err?.info?.code).to.equal('too_similar')
+        }
+
+        expect(
+          this.metrics.inc.calledWith('password-too-similar-to-email')
+        ).to.equal(true)
       })
 
-      it('should produce an error when the password is too similar to the email, regardless of case', function (done) {
-        this.AuthenticationManager.setUserPassword(
-          this.user,
-          this.password.toUpperCase(),
-          err => {
-            expect(err).to.exist
-            expect(err?.info?.code).to.equal('too_similar')
-            expect(
-              this.metrics.inc.calledWith('password-too-similar-to-email')
-            ).to.equal(true)
-            done()
-          }
-        )
+      it('should produce an error when the password is too similar to the email, regardless of case', async function () {
+        try {
+          await this.AuthenticationManager.promises.setUserPassword(
+            this.user,
+            this.password.toUpperCase()
+          )
+          expect.fail('should have thrown')
+        } catch (err) {
+          expect(err.message).to.equal(
+            'password is too similar to email address'
+          )
+          expect(err?.info?.code).to.equal('too_similar')
+        }
+
+        expect(
+          this.metrics.inc.calledWith('password-too-similar-to-email')
+        ).to.equal(true)
       })
     })
 
     describe('successful password set attempt', function () {
-      beforeEach(function () {
+      beforeEach(async function () {
         this.metrics.inc.reset()
-        this.UserGetter.getUser = sinon.stub().yields(null, { overleaf: null })
-        this.AuthenticationManager.setUserPassword(
+        this.UserGetter.promises.getUser = sinon
+          .stub()
+          .resolves({ overleaf: null })
+        await this.AuthenticationManager.promises.setUserPassword(
           this.user,
-          this.password,
-          this.callback
+          this.password
         )
       })
 
@@ -1020,11 +985,6 @@ describe('AuthenticationManager', function () {
           this.metrics.inc.calledWith('password-too-similar-to-email')
         ).to.equal(false)
       })
-
-      it('should call the callback', function () {
-        this.callback.called.should.equal(true)
-        expect(this.callback.lastCall.args[0]).to.not.be.instanceOf(Error)
-      })
     })
   })
 })