From 1dc325d1c7f9169019df98b4f2843c17b4099d1b Mon Sep 17 00:00:00 2001 From: Eric Mc Sween Date: Thu, 23 Apr 2020 07:50:40 -0400 Subject: [PATCH] Merge pull request #2750 from overleaf/ta-activate-finish-login Don't Bypass FinishLogin on Password Reset GitOrigin-RevId: 92567c893afb4aa64fa045151678d33c877d8f71 --- .../PasswordReset/PasswordResetController.js | 29 ++++++--------- .../src/Features/User/UserPagesController.js | 1 + services/web/app/views/user/activate.pug | 1 - .../PasswordResetControllerTests.js | 36 +++++++++---------- 4 files changed, 28 insertions(+), 39 deletions(-) diff --git a/services/web/app/src/Features/PasswordReset/PasswordResetController.js b/services/web/app/src/Features/PasswordReset/PasswordResetController.js index 592a96a1f1..b8f268edaf 100644 --- a/services/web/app/src/Features/PasswordReset/PasswordResetController.js +++ b/services/web/app/src/Features/PasswordReset/PasswordResetController.js @@ -94,31 +94,22 @@ module.exports = { if (err != null) { return next(err) } - if (!req.body.login_after) { + if (!req.session.doLoginAfterPasswordReset) { return res.sendStatus(200) } - UserGetter.getUser(userId, { email: 1 }, (err, user) => { + UserGetter.getUser(userId, (err, user) => { if (err != null) { return next(err) } - AuthenticationController.afterLoginSessionSetup( - req, - user, - err => { - if (err != null) { - logger.err( - { err, email: user.email }, - 'Error setting up session after setting password' - ) - return next(err) - } - res.json({ - redir: - AuthenticationController._getRedirectFromSession(req) || - '/project' - }) + AuthenticationController.finishLogin(user, req, res, err => { + if (err != null) { + logger.err( + { err, email: user.email }, + 'Error setting up session after setting password' + ) } - ) + next(err) + }) }) }) }) diff --git a/services/web/app/src/Features/User/UserPagesController.js b/services/web/app/src/Features/User/UserPagesController.js index 5fb2564d56..16d92817ac 100644 --- a/services/web/app/src/Features/User/UserPagesController.js +++ b/services/web/app/src/Features/User/UserPagesController.js @@ -54,6 +54,7 @@ const UserPagesController = { // as a way to log in which, if I know our users, they will. res.redirect(`/login?email=${encodeURIComponent(user.email)}`) } else { + req.session.doLoginAfterPasswordReset = true res.render('user/activate', { title: 'activate_account', email: user.email, diff --git a/services/web/app/views/user/activate.pug b/services/web/app/views/user/activate.pug index 190ee45cdd..4722b9c7e7 100644 --- a/services/web/app/views/user/activate.pug +++ b/services/web/app/views/user/activate.pug @@ -24,7 +24,6 @@ block content name="passwordResetToken", value=token ) - input(name='login_after', type='hidden', value="true") .alert.alert-danger(ng-show="activationForm.response.error") | #{translate("activation_token_expired")} diff --git a/services/web/test/unit/src/PasswordReset/PasswordResetControllerTests.js b/services/web/test/unit/src/PasswordReset/PasswordResetControllerTests.js index 72d480bb1e..3f26e54fbc 100644 --- a/services/web/test/unit/src/PasswordReset/PasswordResetControllerTests.js +++ b/services/web/test/unit/src/PasswordReset/PasswordResetControllerTests.js @@ -1,6 +1,7 @@ const SandboxedModule = require('sandboxed-module') const path = require('path') const sinon = require('sinon') +const { expect } = require('chai') const MODULE_PATH = path.join( __dirname, @@ -258,35 +259,32 @@ describe('PasswordResetController', function() { this.PasswordResetController.setNewUserPassword(this.req, this.res) }) - describe('when login_after is set', function() { + describe('when doLoginAfterPasswordReset is set', function() { beforeEach(function() { this.UserGetter.getUser = sinon .stub() - .callsArgWith(2, null, { email: 'joe@example.com' }) - this.req.body.login_after = 'true' + .callsArgWith(1, null, { email: 'joe@example.com' }) + this.req.session.doLoginAfterPasswordReset = 'true' this.res.json = sinon.stub() - this.AuthenticationController.afterLoginSessionSetup = sinon - .stub() - .callsArgWith(2, null) + this.AuthenticationController.finishLogin = sinon.stub().yields() this.AuthenticationController._getRedirectFromSession = sinon .stub() .returns('/some/path') }) - it('should login user if login_after is set', function(done) { - this.PasswordResetController.setNewUserPassword(this.req, this.res) - this.AuthenticationController.afterLoginSessionSetup.callCount.should.equal( - 1 + it('should login user', function(done) { + this.PasswordResetController.setNewUserPassword( + this.req, + this.res, + err => { + expect(err).to.not.exist + this.AuthenticationController.finishLogin.callCount.should.equal(1) + this.AuthenticationController.finishLogin + .calledWith({ email: 'joe@example.com' }, this.req) + .should.equal(true) + done() + } ) - this.AuthenticationController.afterLoginSessionSetup - .calledWith(this.req, { email: 'joe@example.com' }) - .should.equal(true) - this.AuthenticationController._getRedirectFromSession.callCount.should.equal( - 1 - ) - this.res.json.callCount.should.equal(1) - this.res.json.calledWith({ redir: '/some/path' }).should.equal(true) - done() }) }) })