Merge pull request #2276 from overleaf/bg-reject-invalid-sessions

reject invalid sessions

GitOrigin-RevId: 5dc59609d01d7ad9bc29f9bf18faee1165d10689

services/web/app/src/Features/Authentication/AuthenticationController.js

@@ -8,6 +8,7 @@ const Settings = require('settings-sharelatex')
 const basicAuth = require('basic-auth-connect')
 const UserHandler = require('../User/UserHandler')
 const UserSessionsManager = require('../User/UserSessionsManager')
+const SessionStoreManager = require('../../infrastructure/SessionStoreManager')
 const Analytics = require('../Analytics/AnalyticsManager')
 const passport = require('passport')
 const NotificationsBuilder = require('../Notifications/NotificationsBuilder')
@@ -309,6 +310,29 @@ const AuthenticationController = (module.exports = {
     }
   },
 
+  validateUserSession: function() {
+    // Middleware to check that the user's session is still good on key actions,
+    // such as opening a a project. Could be used to check that session has not
+    // exceeded a maximum lifetime (req.session.session_created), or for session
+    // hijacking checks (e.g. change of ip address, req.session.ip_address). For
+    // now, just check that the session has been loaded from the session store
+    // correctly.
+    return function(req, res, next) {
+      // check that the session store is returning valid results
+      if (req.session && !SessionStoreManager.hasValidationToken(req)) {
+        // force user to update session
+        req.session.regenerate(() => {
+          // need to destroy the existing session and generate a new one
+          // otherwise they will already be logged in when they are redirected
+          // to the login page
+          AuthenticationController._redirectToLoginOrRegisterPage(req, res)
+        })
+      } else {
+        next()
+      }
+    }
+  },
+
   _globalLoginWhitelist: [],
   addEndpointToLoginWhitelist(endpoint) {
     return AuthenticationController._globalLoginWhitelist.push(endpoint)

services/web/app/src/infrastructure/SessionStoreManager.js

@@ -61,5 +61,13 @@ module.exports = {
     } else {
       return next()
     }
+  },
+
+  hasValidationToken(req) {
+    if (req && req.session && req.session.validationToken) {
+      return true
+    } else {
+      return false
+    }
   }
 }

services/web/app/src/router.js

@@ -288,6 +288,7 @@ function initialize(webRouter, privateApiRouter, publicApiRouter) {
       maxRequests: 15,
       timeInterval: 60
     }),
+    AuthenticationController.validateUserSession(),
     AuthorizationMiddleware.ensureUserCanReadProject,
     ProjectController.loadEditor
   )

services/web/test/unit/src/Authentication/AuthenticationControllerTests.js

@@ -33,6 +33,7 @@ describe('AuthenticationController', function() {
         '../Analytics/AnalyticsManager': (this.AnalyticsManager = {
           recordEvent: sinon.stub()
         }),
+        '../../infrastructure/SessionStoreManager': (this.SessionStoreManager = {}),
         'logger-sharelatex': (this.logger = {
           log: sinon.stub(),
           warn: sinon.stub(),
@@ -582,6 +583,54 @@ describe('AuthenticationController', function() {
     })
   })
 
+  describe('validateUserSession', function() {
+    beforeEach(function() {
+      this.user = {
+        _id: 'user-id-123',
+        email: 'user@sharelatex.com'
+      }
+      this.middleware = this.AuthenticationController.validateUserSession()
+    })
+
+    describe('when the user has a session token', function() {
+      beforeEach(function() {
+        this.req.user = this.user
+        this.SessionStoreManager.hasValidationToken = sinon.stub().returns(true)
+        this.middleware(this.req, this.res, this.next)
+      })
+
+      it('should call the next method in the chain', function() {
+        this.next.called.should.equal(true)
+      })
+    })
+
+    describe('when the user does not have a session token', function() {
+      beforeEach(function() {
+        this.req.session = {
+          user: this.user,
+          regenerate: sinon.stub().yields()
+        }
+        this.req.user = this.user
+        this.AuthenticationController._redirectToLoginOrRegisterPage = sinon.stub()
+        this.req.query = {}
+        this.SessionStoreManager.hasValidationToken = sinon
+          .stub()
+          .returns(false)
+        this.middleware(this.req, this.res, this.next)
+      })
+
+      it('should destroy the current session', function() {
+        this.req.session.regenerate.called.should.equal(true)
+      })
+
+      it('should redirect to the register or login page', function() {
+        this.AuthenticationController._redirectToLoginOrRegisterPage
+          .calledWith(this.req, this.res)
+          .should.equal(true)
+      })
+    })
+  })
+
   describe('requireOauth', function() {
     beforeEach(function() {
       this.res.send = sinon.stub()

services/web/test/unit/src/Security/SessionStoreManagerTests.js

@@ -84,4 +84,24 @@ describe('SessionStoreManager', function() {
       expect(this.next).to.be.calledWithExactly()
     })
   })
+  describe('hasValidationToken', function() {
+    this.beforeEach(function() {
+      this.req = { sessionID: '123456789' }
+    })
+    it('should return true when the session is valid', function() {
+      this.req.session = { validationToken: 'v1:6789' }
+      const result = this.SessionStoreManager.hasValidationToken(this.req)
+      expect(result).to.equal(true)
+    })
+    it('should return false when the session is valid', function() {
+      this.req.session = { validationToken: 'v1:abcd' }
+      const result = this.SessionStoreManager.hasValidationToken(this.req)
+      expect(result).to.equal(true)
+    })
+    it('should return false when the validation token is missing', function() {
+      this.req.session = {}
+      const result = this.SessionStoreManager.hasValidationToken(this.req)
+      expect(result).to.equal(false)
+    })
+  })
 })