remove v1 deps for password change/reset

GitOrigin-RevId: be25f19ae589c50bfde0b170860127fa8d6f63b7

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

@@ -134,48 +134,7 @@ module.exports = AuthenticationManager = {
   },
 
   setUserPassword(user_id, password, callback) {
-    if (callback == null) {
+    AuthenticationManager.setUserPasswordInV2(user_id, password, callback)
-      callback = function(error, changed) {}
-    }
-    const validation = this.validatePassword(password)
-    if (validation != null) {
-      return callback(validation.message)
-    }
-
-    return UserGetter.getUser(user_id, { email: 1, overleaf: 1 }, function(
-      error,
-      user
-    ) {
-      if (error != null) {
-        return callback(error)
-      }
-      const v1IdExists =
-        (user.overleaf != null ? user.overleaf.id : undefined) != null
-      if (v1IdExists && Settings.overleaf != null) {
-        // v2 user in v2
-        // v2 user in v2, change password in v1
-        return AuthenticationManager.setUserPasswordInV1(
-          user.overleaf.id,
-          password,
-          callback
-        )
-      } else if (v1IdExists && Settings.overleaf == null) {
-        // v2 user in SL
-        return callback(new Errors.NotInV2Error('Password Reset Attempt'))
-      } else if (!v1IdExists && Settings.overleaf == null) {
-        // SL user in SL, change password in SL
-        return AuthenticationManager.setUserPasswordInV2(
-          user_id,
-          password,
-          callback
-        )
-      } else if (!v1IdExists && Settings.overleaf != null) {
-        // SL user in v2, should not happen
-        return callback(new Errors.SLInV2Error('Password Reset Attempt'))
-      } else {
-        return callback(new Error('Password Reset Attempt Failed'))
-      }
-    })
   },
 
   checkRounds(user, hashedPassword, password, callback) {

services/web/app/src/Features/PasswordReset/PasswordResetController.js

@@ -6,7 +6,6 @@ const UserGetter = require('../User/UserGetter')
 const UserUpdater = require('../User/UserUpdater')
 const UserSessionsManager = require('../User/UserSessionsManager')
 const logger = require('logger-sharelatex')
-const Settings = require('settings-sharelatex')
 
 module.exports = {
   renderRequestResetForm(req, res) {
@@ -22,23 +21,18 @@ module.exports = {
       subjectName: req.ip,
       throttle: 6
     }
-    RateLimiter.addCount(opts, function(err, canContinue) {
+    RateLimiter.addCount(opts, (err, canContinue) => {
       if (err != null) {
-        return next(err)
+        res.send(500, { message: err.message })
       }
       if (!canContinue) {
         return res.send(429, {
           message: req.i18n.translate('rate_limit_hit_wait')
         })
       }
-      PasswordResetHandler.generateAndEmailResetToken(email, function(
+      PasswordResetHandler.generateAndEmailResetToken(email, (err, status) => {
-        err,
-        status
-      ) {
         if (err != null) {
-          res.send(500, {
+          res.send(500, { message: err.message })
-            message: err.message
-          })
         } else if (status === 'primary') {
           res.send(200, {
             message: { text: req.i18n.translate('password_reset_email_sent') }
@@ -47,12 +41,6 @@ module.exports = {
           res.send(404, {
             message: req.i18n.translate('secondary_email_password_reset')
           })
-        } else if (status === 'sharelatex') {
-          res.send(404, {
-            message: `<a href="${
-              Settings.accountMerge.sharelatexHost
-            }/user/password/reset">${req.i18n.translate('reset_from_sl')}</a>`
-          })
         } else {
           res.send(404, {
             message: req.i18n.translate('cant_find_email')
@@ -77,78 +65,61 @@ module.exports = {
   },
 
   setNewUserPassword(req, res, next) {
-    const { passwordResetToken, password } = req.body
+    let { passwordResetToken, password } = req.body
-    if (
+    if (!passwordResetToken || !password) {
-      !password ||
+      return res.sendStatus(400)
-      !passwordResetToken ||
+    }
-      AuthenticationManager.validatePassword(password.trim()) != null
+    password = password.trim()
-    ) {
+    passwordResetToken = passwordResetToken.trim()
+    if (AuthenticationManager.validatePassword(password) != null) {
       return res.sendStatus(400)
     }
     delete req.session.resetToken
     PasswordResetHandler.setNewUserPassword(
-      passwordResetToken.trim(),
+      passwordResetToken,
-      password.trim(),
+      password,
-      function(err, found, userId) {
+      (err, found, userId) => {
-        if (err && err.name && err.name === 'NotFoundError') {
+        if ((err && err.name === 'NotFoundError') || !found) {
-          res.status(404).send('NotFoundError')
+          return res.status(404).send('NotFoundError')
-        } else if (err && err.name && err.name === 'NotInV2Error') {
+        } else if (err) {
-          res.status(403).send('NotInV2Error')
+          return res.status(500)
-        } else if (err && err.name && err.name === 'SLInV2Error') {
+        }
-          res.status(403).send('SLInV2Error')
+        UserSessionsManager.revokeAllUserSessions({ _id: userId }, [], err => {
-        } else if (err && err.statusCode && err.statusCode === 500) {
+          if (err != null) {
-          res.status(500)
+            return next(err)
-        } else if (err && !err.statusCode) {
+          }
-          res.status(500)
+          UserUpdater.removeReconfirmFlag(userId, err => {
-        } else if (found) {
+            if (err != null) {
-          if (userId == null) {
+              return next(err)
-            return res.sendStatus(200)
+            }
-          } // will not exist for v1-only users
+            if (!req.body.login_after) {
-          UserSessionsManager.revokeAllUserSessions(
+              return res.sendStatus(200)
-            { _id: userId },
+            }
-            [],
+            UserGetter.getUser(userId, { email: 1 }, (err, user) => {
-            function(err) {
               if (err != null) {
                 return next(err)
               }
-              UserUpdater.removeReconfirmFlag(userId, function(err) {
+              AuthenticationController.afterLoginSessionSetup(
-                if (err != null) {
+                req,
-                  return next(err)
+                user,
-                }
+                err => {
-                if (req.body.login_after) {
+                  if (err != null) {
-                  UserGetter.getUser(userId, { email: 1 }, function(err, user) {
+                    logger.err(
-                    if (err != null) {
+                      { err, email: user.email },
-                      return next(err)
+                      'Error setting up session after setting password'
-                    }
-                    AuthenticationController.afterLoginSessionSetup(
-                      req,
-                      user,
-                      function(err) {
-                        if (err != null) {
-                          logger.warn(
-                            { err, email: user.email },
-                            'Error setting up session after setting password'
-                          )
-                          return next(err)
-                        }
-                        res.json({
-                          redir:
-                            AuthenticationController._getRedirectFromSession(
-                              req
-                            ) || '/project'
-                        })
-                      }
                     )
+                    return next(err)
+                  }
+                  res.json({
+                    redir:
+                      AuthenticationController._getRedirectFromSession(req) ||
+                      '/project'
                   })
-                } else {
-                  res.sendStatus(200)
                 }
-              })
+              )
-            }
+            })
-          )
+          })
-        } else {
+        })
-          res.sendStatus(404)
-        }
       }
     )
   }

services/web/app/src/Features/PasswordReset/PasswordResetHandler.js

@@ -3,81 +3,58 @@ const UserGetter = require('../User/UserGetter')
 const OneTimeTokenHandler = require('../Security/OneTimeTokenHandler')
 const EmailHandler = require('../Email/EmailHandler')
 const AuthenticationManager = require('../Authentication/AuthenticationManager')
-const logger = require('logger-sharelatex')
-const V1Api = require('../V1/V1Api')
 
 const PasswordResetHandler = {
   generateAndEmailResetToken(email, callback) {
-    PasswordResetHandler._getPasswordResetData(email, function(
+    UserGetter.getUserByAnyEmail(email, (err, user) => {
-      error,
+      if (err || !user) {
-      exists,
+        return callback(err, null)
-      data
+      }
-    ) {
+      if (user.email !== email) {
-      if (error != null) {
+        return callback(null, 'secondary')
-        callback(error)
+      }
-      } else if (exists) {
+      const data = { user_id: user._id.toString(), email: email }
-        OneTimeTokenHandler.getNewToken('password', data, function(err, token) {
+      OneTimeTokenHandler.getNewToken('password', data, (err, token) => {
+        if (err) {
+          return callback(err)
+        }
+        const emailOptions = {
+          to: email,
+          setNewPasswordUrl: `${
+            settings.siteUrl
+          }/user/password/set?passwordResetToken=${token}&email=${encodeURIComponent(
+            email
+          )}`
+        }
+        EmailHandler.sendEmail('passwordResetRequested', emailOptions, err => {
           if (err) {
             return callback(err)
           }
-          const emailOptions = {
+          callback(null, 'primary')
-            to: email,
-            setNewPasswordUrl: `${
-              settings.siteUrl
-            }/user/password/set?passwordResetToken=${token}&email=${encodeURIComponent(
-              email
-            )}`
-          }
-          EmailHandler.sendEmail(
-            'passwordResetRequested',
-            emailOptions,
-            function(error) {
-              if (error != null) {
-                return callback(error)
-              }
-              callback(null, 'primary')
-            }
-          )
         })
-      } else {
+      })
-        UserGetter.getUserByAnyEmail(email, function(err, user) {
-          if (err != null) {
-            return callback(err)
-          }
-          if (!user) {
-            callback(null, null)
-          } else if (user.overleaf == null || user.overleaf.id == null) {
-            callback(null, 'sharelatex')
-          } else {
-            callback(null, 'secondary')
-          }
-        })
-      }
     })
   },
 
   setNewUserPassword(token, password, callback) {
-    PasswordResetHandler.getUserForPasswordResetToken(
+    PasswordResetHandler.getUserForPasswordResetToken(token, (err, user) => {
-      token,
+      if (err) {
-      (err, user, version) => {
+        return callback(err)
-        if (err != null) {
-          return callback(err)
-        }
-        if (user == null) {
-          return callback(null, false, null)
-        }
-        AuthenticationManager.setUserPassword(
-          user._id,
-          password,
-          (err, reset) => {
-            if (err) {
-              return callback(err)
-            }
-            callback(null, reset, user._id)
-          }
-        )
       }
-    )
+      if (!user) {
+        return callback(null, false, null)
+      }
+      AuthenticationManager.setUserPassword(
+        user._id,
+        password,
+        (err, reset) => {
+          if (err) {
+            return callback(err)
+          }
+          callback(null, reset, user._id)
+        }
+      )
+    })
   },
 
   getUserForPasswordResetToken(token, callback) {
@@ -107,13 +84,13 @@ const PasswordResetHandler = {
               data.user_id != null &&
               data.user_id === user._id.toString()
             ) {
-              callback(null, user, 'v2')
+              callback(null, user)
             } else if (
               data.v1_user_id != null &&
               user.overleaf != null &&
               data.v1_user_id === user.overleaf.id
             ) {
-              callback(null, user, 'v1')
+              callback(null, user)
             } else {
               callback(null, null)
             }
@@ -121,43 +98,6 @@ const PasswordResetHandler = {
         )
       }
     )
-  },
-
-  _getPasswordResetData(email, callback) {
-    if (settings.overleaf != null) {
-      // Overleaf v2
-      V1Api.request(
-        {
-          url: '/api/v1/sharelatex/user_emails',
-          qs: {
-            email
-          },
-          expectedStatusCodes: [404]
-        },
-        function(error, response, body) {
-          if (error != null) {
-            return callback(error)
-          }
-          if (response.statusCode === 404) {
-            callback(null, false)
-          } else {
-            callback(null, true, { v1_user_id: body.user_id, email: email })
-          }
-        }
-      )
-    } else {
-      // ShareLaTeX
-      UserGetter.getUserByMainEmail(email, function(err, user) {
-        if (err) {
-          return callback(err)
-        }
-        if (user == null || user.holdingAccount || user.overleaf != null) {
-          logger.err({ email }, 'user could not be found for password reset')
-          return callback(null, false)
-        }
-        callback(null, true, { user_id: user._id, email: email })
-      })
-    }
   }
 }
 

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

@@ -29,6 +29,7 @@ const UserUpdater = require('./UserUpdater')
 const SudoModeHandler = require('../SudoMode/SudoModeHandler')
 const settings = require('settings-sharelatex')
 const Errors = require('../Errors/Errors')
+const EmailHandler = require('../Email/EmailHandler')
 
 module.exports = UserController = {
   tryDeleteUser(req, res, next) {
@@ -302,78 +303,80 @@ module.exports = UserController = {
   },
 
   changePassword(req, res, next) {
-    if (next == null) {
-      next = function(error) {}
-    }
     metrics.inc('user.password-change')
-    const oldPass = req.body.currentPassword
+    const internalError = {
+      message: { type: 'error', text: req.i18n.translate('internal_error') }
+    }
     const user_id = AuthenticationController.getLoggedInUserId(req)
-    return AuthenticationManager.authenticate(
+    AuthenticationManager.authenticate(
       { _id: user_id },
-      oldPass,
+      req.body.currentPassword,
-      function(err, user) {
+      (err, user) => {
-        if (err != null) {
+        if (err) {
-          return next(err)
+          return res.status(500).json(internalError)
         }
-        if (user) {
+        if (!user) {
-          logger.log({ user: user._id }, 'changing password')
+          return res.status(400).json({
-          const { newPassword1 } = req.body
-          const { newPassword2 } = req.body
-          const validationError = AuthenticationManager.validatePassword(
-            newPassword1
-          )
-          if (newPassword1 !== newPassword2) {
-            logger.log({ user }, 'passwords do not match')
-            return res.send({
-              message: {
-                type: 'error',
-                text: 'Your passwords do not match'
-              }
-            })
-          } else if (validationError != null) {
-            logger.log({ user }, validationError.message)
-            return res.send({
-              message: {
-                type: 'error',
-                text: validationError.message
-              }
-            })
-          } else {
-            logger.log({ user }, 'password changed')
-            return AuthenticationManager.setUserPassword(
-              user._id,
-              newPassword1,
-              function(error) {
-                if (error != null) {
-                  return next(error)
-                }
-                return UserSessionsManager.revokeAllUserSessions(
-                  user,
-                  [req.sessionID],
-                  function(err) {
-                    if (err != null) {
-                      return next(err)
-                    }
-                    return res.send({
-                      message: {
-                        type: 'success',
-                        text: 'Your password has been changed'
-                      }
-                    })
-                  }
-                )
-              }
-            )
-          }
-        } else {
-          logger.log({ user_id }, 'current password wrong')
-          return res.send({
             message: {
               type: 'error',
               text: 'Your old password is wrong'
             }
           })
         }
+        if (req.body.newPassword1 !== req.body.newPassword2) {
+          return res.status(400).json({
+            message: {
+              type: 'error',
+              text: req.i18n.translate('password_change_passwords_do_not_match')
+            }
+          })
+        }
+        const validationError = AuthenticationManager.validatePassword(
+          req.body.newPassword1
+        )
+        if (validationError != null) {
+          return res.status(400).json({
+            message: {
+              type: 'error',
+              text: validationError.message
+            }
+          })
+        }
+        AuthenticationManager.setUserPassword(
+          user._id,
+          req.body.newPassword1,
+          err => {
+            if (err) {
+              return res.status(500).json(internalError)
+            }
+            // log errors but do not wait for response
+            EmailHandler.sendEmail(
+              'passwordChanged',
+              { to: user.email },
+              err => {
+                if (err) {
+                  logger.warn(err)
+                }
+              }
+            )
+            UserSessionsManager.revokeAllUserSessions(
+              user,
+              [req.sessionID],
+              err => {
+                if (err != null) {
+                  return res.status(500).json(internalError)
+                }
+                res.json({
+                  message: {
+                    type: 'success',
+                    email: user.email,
+                    text: req.i18n.translate('password_change_successful')
+                  }
+                })
+              }
+            )
+          }
+        )
       }
     )
   }

services/web/app/views/user/setPassword.pug

@@ -25,12 +25,8 @@ block content
 										br
 										a(href="/user/password/reset"  style="text-decoration: underline;")
 											| Request a new password reset email
-								.alert.alert-danger(ng-show="passwordResetForm.response.data == 'SLInV2Error'")
+								.alert.alert-danger(ng-show="passwordResetForm.response.status == 400")
-										a(href=settings.accountMerge.sharelatexHost + "/user/password/reset")
+										| #{translate('invalid_password')}
-											| Please go to ShareLaTeX to reset your password
-								.alert.alert-danger(ng-show="passwordResetForm.response.data == 'NotInV2Error'")
-										a(href=settings.accountMerge.betaHost + "/user/password/reset" style="text-decoration: underline;")
-											| Please go to Overleaf to reset your password
 								.alert.alert-danger(ng-show="passwordResetForm.response.status == 500")
 										| #{translate('error_performing_request')}
 

services/web/app/views/user/settings.pug

@@ -98,10 +98,7 @@ block content
 											| #[a(href="/user/password/reset", target='_blank') #{translate("no_existing_password")}]
 									else
 										- var submitAction
-										if settings.overleaf
+										- submitAction = '/user/password/update'
-											- submitAction = '/user/change_password/v1'
-										else
-											- submitAction = '/user/password/update'
 										form(
 											async-form="changepassword"
 											name="changePasswordForm"

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

@@ -574,114 +574,40 @@ describe('AuthenticationManager', function() {
       })
     })
 
-    describe('password set attempt', function() {
+    describe('successful password set attempt', function() {
-      describe('with SL user in SL', function() {
+      beforeEach(function() {
-        beforeEach(function() {
+        this.UserGetter.getUser = sinon.stub().yields(null, { overleaf: null })
-          this.UserGetter.getUser = sinon
+        this.AuthenticationManager.setUserPassword(
-            .stub()
+          this.user_id,
-            .yields(null, { overleaf: null })
+          this.password,
-          return this.AuthenticationManager.setUserPassword(
+          this.callback
-            this.user_id,
+        )
-            this.password,
+      })
-            this.callback
-          )
-        })
 
-        it('should look up the user', function() {
+      it("should update the user's password in the database", function() {
-          return this.UserGetter.getUser
+        const { args } = this.db.users.update.lastCall
-            .calledWith(this.user_id)
+        expect(args[0]).to.deep.equal({
-            .should.equal(true)
+          _id: ObjectId(this.user_id.toString())
         })
-
+        return expect(args[1]).to.deep.equal({
-        it("should update the user's password in the database", function() {
+          $set: {
-          const { args } = this.db.users.update.lastCall
+            hashedPassword: this.hashedPassword
-          expect(args[0]).to.deep.equal({
+          },
-            _id: ObjectId(this.user_id.toString())
+          $unset: {
-          })
+            password: true
-          return expect(args[1]).to.deep.equal({
+          }
-            $set: {
-              hashedPassword: this.hashedPassword
-            },
-            $unset: {
-              password: true
-            }
-          })
-        })
-
-        it('should hash the password', function() {
-          this.bcrypt.genSalt.calledWith(12).should.equal(true)
-          return this.bcrypt.hash
-            .calledWith(this.password, this.salt)
-            .should.equal(true)
-        })
-
-        it('should call the callback', function() {
-          return this.callback.called.should.equal(true)
         })
       })
 
-      describe('with SL user in v2', function() {
+      it('should hash the password', function() {
-        beforeEach(function(done) {
+        this.bcrypt.genSalt.calledWith(12).should.equal(true)
-          this.settings.overleaf = true
+        return this.bcrypt.hash
-          this.UserGetter.getUser = sinon
+          .calledWith(this.password, this.salt)
-            .stub()
+          .should.equal(true)
-            .yields(null, { overleaf: null })
-          return this.AuthenticationManager.setUserPassword(
-            this.user_id,
-            this.password,
-            (err, changed) => {
-              this.callback(err, changed)
-              return done()
-            }
-          )
-        })
-        it('should error', function() {
-          return this.callback
-            .calledWith(new Errors.SLInV2Error('Password Reset Attempt'))
-            .should.equal(true)
-        })
       })
 
-      describe('with v2 user in SL', function() {
+      it('should call the callback', function() {
-        beforeEach(function(done) {
+        return this.callback.called.should.equal(true)
-          this.UserGetter.getUser = sinon
-            .stub()
-            .yields(null, { overleaf: { id: 1 } })
-          return this.AuthenticationManager.setUserPassword(
-            this.user_id,
-            this.password,
-            (err, changed) => {
-              this.callback(err, changed)
-              return done()
-            }
-          )
-        })
-        it('should error', function() {
-          return this.callback
-            .calledWith(new Errors.NotInV2Error('Password Reset Attempt'))
-            .should.equal(true)
-        })
-      })
-
-      describe('with v2 user in v2', function() {
-        beforeEach(function(done) {
-          this.settings.overleaf = true
-          this.UserGetter.getUser = sinon
-            .stub()
-            .yields(null, { overleaf: { id: 1 } })
-          this.V1Handler.doPasswordReset = sinon.stub().yields(null, true)
-          return this.AuthenticationManager.setUserPassword(
-            this.user_id,
-            this.password,
-            (err, changed) => {
-              this.callback(err, changed)
-              return done()
-            }
-          )
-        })
-        it('should set the password in v2', function() {
-          return this.callback.calledWith(null, true).should.equal(true)
-        })
       })
     })
   })

services/web/test/unit/src/PasswordReset/PasswordResetControllerTests.js

@@ -74,7 +74,7 @@ describe('PasswordResetController', function() {
       query: {}
     }
 
-    return (this.res = {})
+    this.res = {}
   })
 
   describe('requestReset', function() {
@@ -202,9 +202,9 @@ describe('PasswordResetController', function() {
         false,
         this.user_id
       )
-      this.res.sendStatus = code => {
+      this.res.status = code => {
         code.should.equal(404)
-        return done()
+        done()
       }
       return this.PasswordResetController.setNewUserPassword(this.req, this.res)
     })

services/web/test/unit/src/PasswordReset/PasswordResetHandlerTests.js

@@ -1,11 +1,29 @@
-const { expect } = require('chai')
+/* eslint-disable
+    handle-callback-err,
+    max-len,
+    no-return-assign,
+    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
+ * Full docs: https://github.com/decaffeinate/decaffeinate/blob/master/docs/suggestions.md
+ */
 const SandboxedModule = require('sandboxed-module')
+const assert = require('assert')
 const path = require('path')
 const sinon = require('sinon')
+const sinonChai = require('sinon-chai')
+const chai = require('chai')
 const modulePath = path.join(
   __dirname,
   '../../../../app/src/Features/PasswordReset/PasswordResetHandler'
 )
+const should = require('chai').should()
+chai.use(sinonChai)
+const { expect } = chai
 
 describe('PasswordResetHandler', function() {
   beforeEach(function() {
@@ -21,19 +39,17 @@ describe('PasswordResetHandler', function() {
     }
     this.EmailHandler = { sendEmail: sinon.stub() }
     this.AuthenticationManager = {
-      setUserPassword: sinon.stub()
+      setUserPassword: sinon.stub(),
+      setUserPasswordInV1: sinon.stub(),
+      setUserPasswordInV2: sinon.stub()
     }
-    this.V1Api = { request: sinon.stub() }
     this.PasswordResetHandler = SandboxedModule.require(modulePath, {
-      globals: {
+      globals: { console: console },
-        console: console
-      },
       requires: {
         '../User/UserGetter': this.UserGetter,
         '../Security/OneTimeTokenHandler': this.OneTimeTokenHandler,
         '../Email/EmailHandler': this.EmailHandler,
         '../Authentication/AuthenticationManager': this.AuthenticationManager,
-        '../V1/V1Api': this.V1Api,
         'settings-sharelatex': this.settings,
         'logger-sharelatex': {
           log() {},
@@ -42,128 +58,39 @@ describe('PasswordResetHandler', function() {
       }
     })
     this.token = '12312321i'
-    this.email = 'bob@bob.com'
+    this.user_id = 'user_id_here'
-    this.user = {
+    this.user = { email: (this.email = 'bob@bob.com'), _id: 'user-id' }
-      _id: 'user_id_here',
-      email: this.email
-    }
     this.password = 'my great secret password'
     this.callback = sinon.stub()
+    // this should not have any effect now
+    this.settings.overleaf = true
+  })
+
+  afterEach(function() {
+    this.settings.overleaf = false
   })
 
   describe('generateAndEmailResetToken', function() {
-    describe('when in ShareLaTeX', function() {
+    it('should check the user exists', function() {
-      it('should check the user exists', function(done) {
+      this.UserGetter.getUserByAnyEmail.yields()
-        this.UserGetter.getUserByMainEmail.callsArgWith(1)
+      this.PasswordResetHandler.generateAndEmailResetToken(
-        this.UserGetter.getUserByAnyEmail.callsArgWith(1)
+        this.user.email,
-        this.OneTimeTokenHandler.getNewToken.yields()
+        this.callback
-        this.PasswordResetHandler.generateAndEmailResetToken(
+      )
-          this.user.email,
+      this.UserGetter.getUserByAnyEmail.should.have.been.calledWith(
-          (err, status) => {
+        this.user.email
-            if (err) {
+      )
-              return done(err)
-            }
-            expect(status).to.be.null
-            done()
-          }
-        )
-      })
-
-      it('should send the email with the token', function(done) {
-        this.UserGetter.getUserByMainEmail.callsArgWith(1, null, this.user)
-        this.OneTimeTokenHandler.getNewToken.yields(null, this.token)
-        this.EmailHandler.sendEmail.callsArgWith(2)
-        this.PasswordResetHandler.generateAndEmailResetToken(
-          this.user.email,
-          (err, status) => {
-            if (err) {
-              return done(err)
-            }
-            this.EmailHandler.sendEmail.called.should.equal(true)
-            this.OneTimeTokenHandler.getNewToken.should.have.been.calledWith(
-              'password',
-              {
-                user_id: this.user._id,
-                email: this.email
-              }
-            )
-            status.should.equal('primary')
-            const args = this.EmailHandler.sendEmail.args[0]
-            args[0].should.equal('passwordResetRequested')
-            args[1].setNewPasswordUrl.should.equal(
-              `${this.settings.siteUrl}/user/password/set?passwordResetToken=${
-                this.token
-              }&email=${encodeURIComponent(this.user.email)}`
-            )
-            done()
-          }
-        )
-      })
-
-      it('should return exists == null for a holdingAccount', function(done) {
-        this.user.holdingAccount = true
-        this.UserGetter.getUserByMainEmail.callsArgWith(1, null, this.user)
-        this.UserGetter.getUserByAnyEmail.callsArgWith(1)
-        this.OneTimeTokenHandler.getNewToken.yields()
-        this.PasswordResetHandler.generateAndEmailResetToken(
-          this.user.email,
-          (err, status) => {
-            if (err) {
-              return done(err)
-            }
-            expect(status).to.be.null
-            done()
-          }
-        )
-      })
-
-      it('should set the password token data to the user id and email', function() {
-        this.UserGetter.getUserByMainEmail.callsArgWith(1, null, this.user)
-        this.OneTimeTokenHandler.getNewToken.yields(null, this.token)
-        this.EmailHandler.sendEmail.callsArgWith(2)
-      })
     })
 
-    describe('when in overleaf', function() {
+    it('should send the email with the token', function(done) {
-      beforeEach(function() {
+      this.UserGetter.getUserByAnyEmail.yields(null, this.user)
-        this.settings.overleaf = true
+      this.OneTimeTokenHandler.getNewToken.yields(null, this.token)
-      })
+      this.EmailHandler.sendEmail.yields()
-
+      this.PasswordResetHandler.generateAndEmailResetToken(
-      describe('when the email exists', function() {
+        this.user.email,
-        beforeEach(function() {
+        (err, status) => {
-          this.V1Api.request.yields(null, {}, { user_id: 42 })
-          this.OneTimeTokenHandler.getNewToken.yields(null, this.token)
-          this.EmailHandler.sendEmail.yields()
-          this.PasswordResetHandler.generateAndEmailResetToken(
-            this.email,
-            this.callback
-          )
-        })
-
-        it('should call the v1 api for the user', function() {
-          this.V1Api.request
-            .calledWith({
-              url: '/api/v1/sharelatex/user_emails',
-              qs: {
-                email: this.email
-              },
-              expectedStatusCodes: [404]
-            })
-            .should.equal(true)
-        })
-
-        it('should set the password token data to the user id and email', function() {
-          this.OneTimeTokenHandler.getNewToken.should.have.been.calledWith(
-            'password',
-            {
-              v1_user_id: 42,
-              email: this.email
-            }
-          )
-        })
-
-        it('should send an email with the token', function() {
           this.EmailHandler.sendEmail.called.should.equal(true)
+          status.should.equal('primary')
           const args = this.EmailHandler.sendEmail.args[0]
           args[0].should.equal('passwordResetRequested')
           args[1].setNewPasswordUrl.should.equal(
@@ -171,110 +98,107 @@ describe('PasswordResetHandler', function() {
               this.token
             }&email=${encodeURIComponent(this.user.email)}`
           )
-        })
+          done()
+        }
+      )
+    })
 
-        it('should return status == true', function() {
+    describe('when the email exists', function() {
-          this.callback.calledWith(null, 'primary').should.equal(true)
+      beforeEach(function() {
-        })
+        this.UserGetter.getUserByAnyEmail.yields(null, this.user)
+        this.OneTimeTokenHandler.getNewToken.yields(null, this.token)
+        this.EmailHandler.sendEmail.yields()
+        this.PasswordResetHandler.generateAndEmailResetToken(
+          this.email,
+          this.callback
+        )
       })
 
-      describe("when the email doesn't exist", function() {
+      it('should set the password token data to the user id and email', function() {
-        beforeEach(function() {
+        this.OneTimeTokenHandler.getNewToken.should.have.been.calledWith(
-          this.V1Api.request = sinon
+          'password',
-            .stub()
+          {
-            .yields(null, { statusCode: 404 }, {})
+            email: this.email,
-          this.UserGetter.getUserByAnyEmail.callsArgWith(1)
+            user_id: this.user._id
-          this.PasswordResetHandler.generateAndEmailResetToken(
+          }
-            this.email,
+        )
-            this.callback
-          )
-        })
-
-        it('should not set the password token data', function() {
-          this.OneTimeTokenHandler.getNewToken.called.should.equal(false)
-        })
-
-        it('should send an email with the token', function() {
-          this.EmailHandler.sendEmail.called.should.equal(false)
-        })
-
-        it('should return status == null', function() {
-          this.callback.calledWith(null, null).should.equal(true)
-        })
       })
 
-      describe("when the user isn't on v2", function() {
+      it('should send an email with the token', function() {
-        beforeEach(function() {
+        this.EmailHandler.sendEmail.called.should.equal(true)
-          this.V1Api.request = sinon
+        const args = this.EmailHandler.sendEmail.args[0]
-            .stub()
+        args[0].should.equal('passwordResetRequested')
-            .yields(null, { statusCode: 404 }, {})
+        args[1].setNewPasswordUrl.should.equal(
-          this.UserGetter.getUserByAnyEmail.callsArgWith(1, null, this.user)
+          `${this.settings.siteUrl}/user/password/set?passwordResetToken=${
-          this.PasswordResetHandler.generateAndEmailResetToken(
+            this.token
-            this.email,
+          }&email=${encodeURIComponent(this.user.email)}`
-            this.callback
+        )
-          )
-        })
-
-        it('should not set the password token data', function() {
-          this.OneTimeTokenHandler.getNewToken.called.should.equal(false)
-        })
-
-        it('should not send an email with the token', function() {
-          this.EmailHandler.sendEmail.called.should.equal(false)
-        })
-
-        it('should return status == sharelatex', function() {
-          this.callback.calledWith(null, 'sharelatex').should.equal(true)
-        })
       })
 
-      describe('when the email is a secondary email', function() {
+      it('should return status == true', function() {
-        beforeEach(function() {
+        this.callback.calledWith(null, 'primary').should.equal(true)
-          this.V1Api.request = sinon
+      })
-            .stub()
+    })
-            .yields(null, { statusCode: 404 }, {})
-          this.user.overleaf = { id: 101 }
-          this.UserGetter.getUserByAnyEmail.callsArgWith(1, null, this.user)
-          this.PasswordResetHandler.generateAndEmailResetToken(
-            this.email,
-            this.callback
-          )
-        })
 
-        it('should not set the password token data', function() {
+    describe("when the email doesn't exist", function() {
-          this.OneTimeTokenHandler.getNewToken.called.should.equal(false)
+      beforeEach(function() {
-        })
+        this.UserGetter.getUserByAnyEmail.yields(null, null)
+        this.PasswordResetHandler.generateAndEmailResetToken(
+          this.email,
+          this.callback
+        )
+      })
 
-        it('should not send an email with the token', function() {
+      it('should not set the password token data', function() {
-          this.EmailHandler.sendEmail.called.should.equal(false)
+        this.OneTimeTokenHandler.getNewToken.called.should.equal(false)
-        })
+      })
 
-        it('should return status == secondary', function() {
+      it('should send an email with the token', function() {
-          this.callback.calledWith(null, 'secondary').should.equal(true)
+        this.EmailHandler.sendEmail.called.should.equal(false)
-        })
+      })
+
+      it('should return status == null', function() {
+        this.callback.calledWith(null, null).should.equal(true)
+      })
+    })
+
+    describe('when the email is a secondary email', function() {
+      beforeEach(function() {
+        this.UserGetter.getUserByAnyEmail.callsArgWith(1, null, this.user)
+        this.PasswordResetHandler.generateAndEmailResetToken(
+          'secondary@email.com',
+          this.callback
+        )
+      })
+
+      it('should not set the password token data', function() {
+        this.OneTimeTokenHandler.getNewToken.called.should.equal(false)
+      })
+
+      it('should not send an email with the token', function() {
+        this.EmailHandler.sendEmail.called.should.equal(false)
+      })
+
+      it('should return status == secondary', function() {
+        this.callback.calledWith(null, 'secondary').should.equal(true)
       })
     })
   })
 
   describe('setNewUserPassword', function() {
-    describe('when no token is found', function() {
+    describe('when no data is found', function() {
       beforeEach(function() {
         this.OneTimeTokenHandler.getValueFromTokenAndExpire.yields(null, null)
-      })
-
-      it('should return found == false when', function(done) {
         this.PasswordResetHandler.setNewUserPassword(
           this.token,
           this.password,
-          (err, found) => {
+          this.callback
-            if (err != null) {
-              return done(err)
-            }
-            expect(found).to.be.false
-            done()
-          }
         )
       })
+
+      it('should return exists == false', function() {
+        this.callback.calledWith(null, false).should.equal(true)
+      })
     })
 
     describe('when the token has a user_id and email', function() {
@@ -336,8 +260,9 @@ describe('PasswordResetHandler', function() {
 
       describe('when the email and user match', function() {
         beforeEach(function() {
-          this.UserGetter.getUserByMainEmail
+          this.PasswordResetHandler.getUserForPasswordResetToken = sinon
-            .withArgs(this.email)
+            .stub()
+            .withArgs(this.token)
             .yields(null, this.user)
         })
 

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

@@ -42,7 +42,10 @@ describe('UserController', function() {
           email: 'old@something.com'
         }
       },
-      body: {}
+      body: {},
+      i18n: {
+        translate: text => text
+      }
     }
 
     this.UserDeleter = { deleteUser: sinon.stub().callsArgWith(1) }
@@ -109,10 +112,13 @@ describe('UserController', function() {
 
     this.res = {
       send: sinon.stub(),
+      status: sinon.stub(),
       sendStatus: sinon.stub(),
       json: sinon.stub()
     }
-    return (this.next = sinon.stub())
+    this.res.status.returns(this.res)
+    this.next = sinon.stub()
+    this.callback = sinon.stub()
   })
 
   describe('tryDeleteUser', function() {
@@ -511,62 +517,60 @@ describe('UserController', function() {
   })
 
   describe('changePassword', function() {
-    it('should check the old password is the current one at the moment', function(done) {
+    it('should check the old password is the current one at the moment', function() {
-      this.AuthenticationManager.authenticate.callsArgWith(2)
+      this.AuthenticationManager.authenticate.yields()
       this.req.body = { currentPassword: 'oldpasshere' }
-      this.res.send = () => {
+      this.UserController.changePassword(this.req, this.res, this.callback)
-        this.AuthenticationManager.authenticate
+      this.AuthenticationManager.authenticate.should.have.been.calledWith(
-          .calledWith({ _id: this.user._id }, 'oldpasshere')
+        { _id: this.user._id },
-          .should.equal(true)
+        'oldpasshere'
-        this.AuthenticationManager.setUserPassword.called.should.equal(false)
+      )
-        return done()
+      this.AuthenticationManager.setUserPassword.callCount.should.equal(0)
-      }
-      return this.UserController.changePassword(this.req, this.res)
     })
 
-    it('it should not set the new password if they do not match', function(done) {
+    it('it should not set the new password if they do not match', function() {
-      this.AuthenticationManager.authenticate.callsArgWith(2, null, {})
+      this.AuthenticationManager.authenticate.yields(null, {})
       this.req.body = {
         newPassword1: '1',
         newPassword2: '2'
       }
-      this.res.send = () => {
+      this.UserController.changePassword(this.req, this.res, this.callback)
-        this.AuthenticationManager.setUserPassword.called.should.equal(false)
+      this.res.status.should.have.been.calledWith(400)
-        return done()
+      this.AuthenticationManager.setUserPassword.callCount.should.equal(0)
-      }
-      return this.UserController.changePassword(this.req, this.res)
     })
 
-    it('should set the new password if they do match', function(done) {
+    it('should set the new password if they do match', function() {
-      this.AuthenticationManager.authenticate.callsArgWith(2, null, this.user)
+      this.AuthenticationManager.authenticate.yields(null, this.user)
-      this.AuthenticationManager.setUserPassword.callsArgWith(2)
+      this.AuthenticationManager.setUserPassword.yields()
       this.req.body = {
         newPassword1: 'newpass',
         newPassword2: 'newpass'
       }
-      this.res.send = () => {
+      this.UserController.changePassword(this.req, this.res, this.callback)
-        this.AuthenticationManager.setUserPassword
+      this.AuthenticationManager.setUserPassword.should.have.been.calledWith(
-          .calledWith(this.user._id, 'newpass')
+        this.user._id,
-          .should.equal(true)
+        'newpass'
-        return done()
+      )
-      }
-      return this.UserController.changePassword(this.req, this.res)
     })
 
-    it('it should not set the new password if it is invalid', function(done) {
+    it('it should not set the new password if it is invalid', function() {
       this.AuthenticationManager.validatePassword = sinon
         .stub()
-        .returns({ message: 'password contains invalid characters' })
+        .returns({ message: 'validation-error' })
-      this.AuthenticationManager.authenticate.callsArgWith(2, null, {})
+      this.AuthenticationManager.authenticate.yields(null, {})
       this.req.body = {
-        newPassword1: 'correct horse battery staple',
+        newPassword1: 'newpass',
-        newPassword2: 'correct horse battery staple'
+        newPassword2: 'newpass'
       }
-      this.res.send = () => {
+      this.UserController.changePassword(this.req, this.res, this.callback)
-        this.AuthenticationManager.setUserPassword.called.should.equal(false)
+      this.AuthenticationManager.setUserPassword.callCount.should.equal(0)
-        return done()
+      this.res.status.should.have.been.calledWith(400)
-      }
+      this.res.json.should.have.been.calledWith({
-      return this.UserController.changePassword(this.req, this.res)
+        message: {
+          type: 'error',
+          text: 'validation-error'
+        }
+      })
     })
   })
 })