diff --git a/services/web/app/src/Features/PasswordReset/PasswordResetController.js b/services/web/app/src/Features/PasswordReset/PasswordResetController.js index f9ea0cc7a7..44dc70ee65 100644 --- a/services/web/app/src/Features/PasswordReset/PasswordResetController.js +++ b/services/web/app/src/Features/PasswordReset/PasswordResetController.js @@ -30,7 +30,6 @@ async function setNewUserPassword(req, res, next) { } passwordResetToken = passwordResetToken.trim() - delete req.session.resetToken const initiatorId = SessionManager.getLoggedInUserId(req.session) // password reset via tokens can be done while logged in, or not @@ -145,6 +144,51 @@ async function requestReset(req, res, next) { } } +async function renderSetPasswordForm(req, res, next) { + if (req.query.passwordResetToken != null) { + try { + const result = + await PasswordResetHandler.promises.getUserForPasswordResetToken( + req.query.passwordResetToken + ) + + const { user, remainingPeeks } = result || {} + if (!user || remainingPeeks <= 0) { + return res.redirect('/user/password/reset?error=token_expired') + } + req.session.resetToken = req.query.passwordResetToken + let emailQuery = '' + + if (typeof req.query.email === 'string') { + const email = EmailsHelper.parseEmail(req.query.email) + if (email) { + emailQuery = `?email=${encodeURIComponent(email)}` + } + } + + return res.redirect('/user/password/set' + emailQuery) + } catch (err) { + return res.redirect('/user/password/reset?error=token_expired') + } + } + + if (req.session.resetToken == null) { + return res.redirect('/user/password/reset') + } + + const email = EmailsHelper.parseEmail(req.query.email) + + // clean up to avoid leaking the token in the session object + const passwordResetToken = req.session.resetToken + delete req.session.resetToken + + res.render('user/setPassword', { + title: 'set_password', + email, + passwordResetToken, + }) +} + module.exports = { renderRequestResetForm(req, res) { const errorQuery = req.query.error @@ -159,38 +203,6 @@ module.exports = { }, requestReset: expressify(requestReset), - - renderSetPasswordForm(req, res) { - if (req.query.passwordResetToken != null) { - return PasswordResetHandler.getUserForPasswordResetToken( - req.query.passwordResetToken, - (err, result) => { - const { user, remainingPeeks } = result || {} - if (err || !user || remainingPeeks <= 0) { - return res.redirect('/user/password/reset?error=token_expired') - } - req.session.resetToken = req.query.passwordResetToken - let emailQuery = '' - if (typeof req.query.email === 'string') { - const email = EmailsHelper.parseEmail(req.query.email) - if (email) { - emailQuery = `?email=${encodeURIComponent(email)}` - } - } - return res.redirect('/user/password/set' + emailQuery) - } - ) - } - 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, - }) - }, - + renderSetPasswordForm: expressify(renderSetPasswordForm), setNewUserPassword: expressify(setNewUserPassword), } diff --git a/services/web/test/unit/src/PasswordReset/PasswordResetControllerTests.js b/services/web/test/unit/src/PasswordReset/PasswordResetControllerTests.js index 24a57ff5dd..b794e58d87 100644 --- a/services/web/test/unit/src/PasswordReset/PasswordResetControllerTests.js +++ b/services/web/test/unit/src/PasswordReset/PasswordResetControllerTests.js @@ -39,11 +39,14 @@ describe('PasswordResetController', function () { setNewUserPassword: sinon .stub() .resolves({ found: true, reset: true, userID: this.user_id }), + getUserForPasswordResetToken: sinon + .stub() + .withArgs(this.token) + .resolves({ + user: { _id: this.user_id }, + remainingPeeks: 1, + }), }, - getUserForPasswordResetToken: sinon - .stub() - .withArgs(this.token) - .yields(null, { user: { _id: this.user_id }, remainingPeeks: 1 }), } this.UserSessionsManager = { promises: { @@ -259,15 +262,6 @@ describe('PasswordResetController', function () { this.PasswordResetController.setNewUserPassword(this.req, this.res) }) - it('should clear the session.resetToken', function (done) { - this.res.sendStatus = code => { - code.should.equal(200) - this.req.session.should.not.have.property('resetToken') - done() - } - this.PasswordResetController.setNewUserPassword(this.req, this.res) - }) - it('should clear sessions', function (done) { this.res.sendStatus = code => { this.UserSessionsManager.promises.revokeAllUserSessions.callCount.should.equal( @@ -374,10 +368,10 @@ describe('PasswordResetController', function () { describe('with expired token in query', function () { beforeEach(function () { this.req.query.passwordResetToken = this.token - this.PasswordResetHandler.getUserForPasswordResetToken = sinon + this.PasswordResetHandler.promises.getUserForPasswordResetToken = sinon .stub() .withArgs(this.token) - .yields(null, { user: { _id: this.user_id }, remainingPeeks: 0 }) + .resolves({ user: { _id: this.user_id }, remainingPeeks: 0 }) }) it('should redirect to the reset request page with an error message', function (done) { @@ -452,7 +446,15 @@ describe('PasswordResetController', function () { it('should render the page, passing the reset token', function (done) { this.res.render = (templatePath, options) => { - options.passwordResetToken.should.equal(this.req.session.resetToken) + options.passwordResetToken.should.equal(this.token) + done() + } + this.PasswordResetController.renderSetPasswordForm(this.req, this.res) + }) + + it('should clear the req.session.resetToken', function (done) { + this.res.render = (templatePath, options) => { + this.req.session.should.not.have.property('resetToken') done() } this.PasswordResetController.renderSetPasswordForm(this.req, this.res)