Просмотр исходного кода

Merge pull request #3325 from overleaf/jpa-session-cleanup

[UserEmailsController] clear sessions after changing the primary email

GitOrigin-RevId: 319b483a3c2851c37c0a340ba9c43a86225a9246
Jakob Ackermann 6 лет назад
Родитель
Сommit
15fd090e7a

+ 14 - 0
services/web/app/src/Features/User/UserEmailsController.js

@@ -1,6 +1,8 @@
+const logger = require('logger-sharelatex')
 const AuthenticationController = require('../Authentication/AuthenticationController')
 const AuthenticationController = require('../Authentication/AuthenticationController')
 const UserGetter = require('./UserGetter')
 const UserGetter = require('./UserGetter')
 const UserUpdater = require('./UserUpdater')
 const UserUpdater = require('./UserUpdater')
+const UserSessionsManager = require('./UserSessionsManager')
 const EmailHandler = require('../Email/EmailHandler')
 const EmailHandler = require('../Email/EmailHandler')
 const EmailHelper = require('../Helpers/EmailHelper')
 const EmailHelper = require('../Helpers/EmailHelper')
 const UserEmailsConfirmationHandler = require('./UserEmailsConfirmationHandler')
 const UserEmailsConfirmationHandler = require('./UserEmailsConfirmationHandler')
@@ -134,6 +136,18 @@ const UserEmailsController = {
           return UserEmailsController._handleEmailError(err, req, res, next)
           return UserEmailsController._handleEmailError(err, req, res, next)
         }
         }
         AuthenticationController.setInSessionUser(req, { email: email })
         AuthenticationController.setInSessionUser(req, { email: email })
+        const user = AuthenticationController.getSessionUser(req)
+        UserSessionsManager.revokeAllUserSessions(
+          user,
+          [req.sessionID],
+          err => {
+            if (err)
+              logger.warn(
+                { err },
+                'failed revoking secondary sessions after changing default email'
+              )
+          }
+        )
         res.sendStatus(200)
         res.sendStatus(200)
       }
       }
     )
     )

+ 75 - 0
services/web/test/acceptance/src/UserEmailsTests.js

@@ -820,6 +820,81 @@ describe('UserEmails', function() {
         })
         })
       })
       })
     })
     })
+
+    describe('session cleanup', function() {
+      beforeEach(function setupSecondSession(done) {
+        this.userSession2 = new User()
+        this.userSession2.email = this.user.email
+        this.userSession2.emails = this.user.emails
+        this.userSession2.password = this.user.password
+        // login before adding the new email address
+        // User.login() performs a mongo update and resets the .emails field.
+        this.userSession2.login(done)
+      })
+
+      beforeEach(function checkSecondSessionLiveness(done) {
+        this.userSession2.request(
+          { method: 'GET', url: '/project', followRedirect: false },
+          (error, response) => {
+            expect(error).to.not.exist
+            expect(response.statusCode).to.equal(200)
+            done()
+          }
+        )
+      })
+
+      beforeEach(function addSecondaryEmail(done) {
+        this.user.request(
+          {
+            method: 'POST',
+            url: '/user/emails',
+            json: { email: 'new-confirmed-default@example.com' }
+          },
+          (error, response) => {
+            expect(error).to.not.exist
+            expect(response.statusCode).to.equal(204)
+            done()
+          }
+        )
+      })
+
+      beforeEach(function confirmSecondaryEmail(done) {
+        db.users.updateOne(
+          { 'emails.email': 'new-confirmed-default@example.com' },
+          { $set: { 'emails.$.confirmedAt': new Date() } },
+          done
+        )
+      })
+
+      beforeEach(function setDefault(done) {
+        this.user.request(
+          {
+            method: 'POST',
+            url: '/user/emails/default',
+            json: { email: 'new-confirmed-default@example.com' }
+          },
+          (error, response) => {
+            expect(error).to.not.exist
+            expect(response.statusCode).to.equal(200)
+            done()
+          }
+        )
+      })
+
+      it('should logout the other sessions', function(done) {
+        this.userSession2.request(
+          { method: 'GET', url: '/project', followRedirect: false },
+          (error, response) => {
+            expect(error).to.not.exist
+            expect(response.statusCode).to.equal(302)
+            expect(response.headers)
+              .to.have.property('location')
+              .to.match(new RegExp('^/login'))
+            done()
+          }
+        )
+      })
+    })
   })
   })
 
 
   describe('when not logged in', function() {
   describe('when not logged in', function() {

+ 45 - 2
services/web/test/unit/src/User/UserEmailsControllerTests.js

@@ -12,6 +12,7 @@ const Errors = require('../../../../app/src/Features/Errors/Errors')
 describe('UserEmailsController', function() {
 describe('UserEmailsController', function() {
   beforeEach(function() {
   beforeEach(function() {
     this.req = new MockRequest()
     this.req = new MockRequest()
+    this.req.sessionID = Math.random().toString()
     this.res = new MockResponse()
     this.res = new MockResponse()
     this.next = sinon.stub()
     this.next = sinon.stub()
     this.user = { _id: 'mock-user-id', email: 'example@overleaf.com' }
     this.user = { _id: 'mock-user-id', email: 'example@overleaf.com' }
@@ -23,12 +24,16 @@ describe('UserEmailsController', function() {
       }
       }
     }
     }
     this.AuthenticationController = {
     this.AuthenticationController = {
+      getSessionUser: sinon.stub().returns(this.user),
       getLoggedInUserId: sinon.stub().returns(this.user._id),
       getLoggedInUserId: sinon.stub().returns(this.user._id),
       setInSessionUser: sinon.stub()
       setInSessionUser: sinon.stub()
     }
     }
     this.Features = {
     this.Features = {
       hasFeature: sinon.stub()
       hasFeature: sinon.stub()
     }
     }
+    this.UserSessionsManager = {
+      revokeAllUserSessions: sinon.stub().yields()
+    }
     this.UserUpdater = {
     this.UserUpdater = {
       addEmailAddress: sinon.stub(),
       addEmailAddress: sinon.stub(),
       removeEmailAddress: sinon.stub(),
       removeEmailAddress: sinon.stub(),
@@ -58,6 +63,7 @@ describe('UserEmailsController', function() {
         '../Authentication/AuthenticationController': this
         '../Authentication/AuthenticationController': this
           .AuthenticationController,
           .AuthenticationController,
         '../../infrastructure/Features': this.Features,
         '../../infrastructure/Features': this.Features,
+        './UserSessionsManager': this.UserSessionsManager,
         './UserGetter': this.UserGetter,
         './UserGetter': this.UserGetter,
         './UserUpdater': this.UserUpdater,
         './UserUpdater': this.UserUpdater,
         '../Email/EmailHandler': (this.EmailHandler = {
         '../Email/EmailHandler': (this.EmailHandler = {
@@ -74,10 +80,11 @@ describe('UserEmailsController', function() {
         '../Institutions/InstitutionsAPI': this.InstitutionsAPI,
         '../Institutions/InstitutionsAPI': this.InstitutionsAPI,
         '../Errors/HttpErrorHandler': this.HttpErrorHandler,
         '../Errors/HttpErrorHandler': this.HttpErrorHandler,
         '../Errors/Errors': Errors,
         '../Errors/Errors': Errors,
-        'logger-sharelatex': {
+        'logger-sharelatex': (this.logger = {
           log() {},
           log() {},
+          warn: sinon.stub(),
           err() {}
           err() {}
-        }
+        })
       }
       }
     })
     })
   })
   })
@@ -317,6 +324,42 @@ describe('UserEmailsController', function() {
         }
         }
       })
       })
     })
     })
+
+    it('should reset the users other sessions', function(done) {
+      this.UserUpdater.setDefaultEmailAddress.yields()
+
+      this.res.callback = () => {
+        expect(
+          this.UserSessionsManager.revokeAllUserSessions
+        ).to.have.been.calledWith(this.user, [this.req.sessionID])
+        done()
+      }
+
+      this.UserEmailsController.setDefault(this.req, this.res, done)
+    })
+
+    it('handles error from revoking sessions and returns 200', function(done) {
+      this.UserUpdater.setDefaultEmailAddress.yields()
+      const redisError = new Error('redis error')
+      this.UserSessionsManager.revokeAllUserSessions = sinon
+        .stub()
+        .yields(redisError)
+
+      this.res.callback = () => {
+        expect(this.res.statusCode).to.equal(200)
+
+        // give revoke process time to run
+        setTimeout(() => {
+          expect(this.logger.warn).to.have.been.calledWith(
+            sinon.match({ err: redisError }),
+            'failed revoking secondary sessions after changing default email'
+          )
+          done()
+        })
+      }
+
+      this.UserEmailsController.setDefault(this.req, this.res, done)
+    })
   })
   })
 
 
   describe('endorse', function() {
   describe('endorse', function() {