Browse Source

Merge pull request #5688 from overleaf/jpa-invalid-password-message

[web] password reset: validate user password ahead of invalidating token

GitOrigin-RevId: ba3e6549f53675a2216e2fc24293276c1968d416
Hugh O'Brien 4 years ago
parent
commit
3b95ac6d88

+ 12 - 1
services/web/app/src/Features/PasswordReset/PasswordResetController.js

@@ -1,5 +1,6 @@
 const PasswordResetHandler = require('./PasswordResetHandler')
 const AuthenticationController = require('../Authentication/AuthenticationController')
+const AuthenticationManager = require('../Authentication/AuthenticationManager')
 const SessionManager = require('../Authentication/SessionManager')
 const UserGetter = require('../User/UserGetter')
 const UserUpdater = require('../User/UserUpdater')
@@ -10,7 +11,7 @@ const { expressify } = require('../../util/promises')
 
 async function setNewUserPassword(req, res, next) {
   let user
-  let { passwordResetToken, password } = req.body
+  let { passwordResetToken, password, email } = req.body
   if (!passwordResetToken || !password) {
     return res.status(400).json({
       message: {
@@ -18,6 +19,14 @@ async function setNewUserPassword(req, res, next) {
       },
     })
   }
+
+  const err = AuthenticationManager.validatePassword(password, email)
+  if (err) {
+    return res.status(400).json({
+      message: { text: err.message },
+    })
+  }
+
   passwordResetToken = passwordResetToken.trim()
   delete req.session.resetToken
 
@@ -128,8 +137,10 @@ module.exports = {
     if (req.session.resetToken == null) {
       return res.redirect('/user/password/reset')
     }
+    const email = EmailsHelper.parseEmail(req.query.email)
     res.render('user/setPassword', {
       title: 'set_password',
+      email,
       passwordResetToken: req.session.resetToken,
     })
   },

+ 1 - 0
services/web/app/views/user/setPassword.pug

@@ -37,6 +37,7 @@ block content
 								a(href='/login') #{translate("login_here")}
 
 							input(type="hidden", name="_csrf", value=csrfToken)
+							input(type="hidden", name="email", value=email)
 
 							.form-group
 								input.form-control#passwordField(

+ 45 - 1
services/web/test/acceptance/src/PasswordResetTests.js

@@ -175,7 +175,7 @@ describe('PasswordReset', function () {
         expect(auditLog).to.deep.equal([])
       })
 
-      it('without a valid password should return 400 and log the change', async function () {
+      it('without a valid password should return 400 and not log the change', async function () {
         // send reset request
         response = await userHelper.request.post('/user/password/set', {
           form: {
@@ -187,6 +187,50 @@ describe('PasswordReset', function () {
         expect(response.statusCode).to.equal(400)
         userHelper = await UserHelper.getUser({ email })
 
+        const auditLog = userHelper.getAuditLogWithoutNoise()
+        expect(auditLog).to.deep.equal([])
+      })
+
+      it('should flag email in password', async function () {
+        const localPart = email.split('@').shift()
+        // send bad password
+        response = await userHelper.request.post('/user/password/set', {
+          form: {
+            passwordResetToken: token,
+            password: localPart,
+            email,
+          },
+          json: true,
+          simple: false,
+        })
+        expect(response.statusCode).to.equal(400)
+        expect(response.body).to.deep.equal({
+          message: { text: 'password contains part of email address' },
+        })
+      })
+
+      it('should be able to retry after providing an invalid password', async function () {
+        // send bad password
+        response = await userHelper.request.post('/user/password/set', {
+          form: {
+            passwordResetToken: token,
+            password: 'short',
+          },
+          simple: false,
+        })
+        expect(response.statusCode).to.equal(400)
+
+        // send good password
+        response = await userHelper.request.post('/user/password/set', {
+          form: {
+            passwordResetToken: token,
+            password: 'SomeThingVeryStrong!11',
+          },
+          simple: false,
+        })
+        expect(response.statusCode).to.equal(200)
+        userHelper = await UserHelper.getUser({ email })
+
         const auditLog = userHelper.getAuditLogWithoutNoise()
         expect(auditLog.length).to.equal(1)
       })

+ 3 - 0
services/web/test/unit/src/PasswordReset/PasswordResetControllerTests.js

@@ -54,6 +54,9 @@ describe('PasswordResetController', function () {
       requires: {
         '@overleaf/settings': this.settings,
         './PasswordResetHandler': this.PasswordResetHandler,
+        '../Authentication/AuthenticationManager': {
+          validatePassword: sinon.stub().returns(null),
+        },
         '../Authentication/AuthenticationController': (this.AuthenticationController = {
           getLoggedInUserId: sinon.stub(),
           finishLogin: sinon.stub(),