diff --git a/services/web/app/src/Features/PasswordReset/PasswordResetController.js b/services/web/app/src/Features/PasswordReset/PasswordResetController.js index c64e34aab1..1ae043a3db 100644 --- a/services/web/app/src/Features/PasswordReset/PasswordResetController.js +++ b/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, }) }, diff --git a/services/web/app/views/user/setPassword.pug b/services/web/app/views/user/setPassword.pug index f7c25f3841..5df2554d24 100644 --- a/services/web/app/views/user/setPassword.pug +++ b/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( diff --git a/services/web/test/acceptance/src/PasswordResetTests.js b/services/web/test/acceptance/src/PasswordResetTests.js index 6ed609677e..e7e41eb4a3 100644 --- a/services/web/test/acceptance/src/PasswordResetTests.js +++ b/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) }) diff --git a/services/web/test/unit/src/PasswordReset/PasswordResetControllerTests.js b/services/web/test/unit/src/PasswordReset/PasswordResetControllerTests.js index 075ffcfd07..6d53110abf 100644 --- a/services/web/test/unit/src/PasswordReset/PasswordResetControllerTests.js +++ b/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(),