Merge pull request #2750 from overleaf/ta-activate-finish-login

Don't Bypass FinishLogin on Password Reset

GitOrigin-RevId: 92567c893afb4aa64fa045151678d33c877d8f71

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)
+              })
             })
           })
         })

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,

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")}
 							

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
-        )
-        this.AuthenticationController.afterLoginSessionSetup
-          .calledWith(this.req, { email: 'joe@example.com' })
-          .should.equal(true)
-        this.AuthenticationController._getRedirectFromSession.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.res.json.callCount.should.equal(1)
-        this.res.json.calledWith({ redir: '/some/path' }).should.equal(true)
-        done()
       })
     })
   })