Merge pull request #2516 from overleaf/spd-oio-samesite-cookies

Handle 'samesite=lax' session cookies on POST to open-in-overleaf

GitOrigin-RevId: d856f372e91134be47cc301a515ec08511618631

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

@@ -12,6 +12,7 @@ const Csrf = require('./Csrf')
 
 const sessionsRedisClient = UserSessionsRedis.client()
 
+const SessionAutostartMiddleware = require('./SessionAutostartMiddleware')
 const SessionStoreManager = require('./SessionStoreManager')
 const session = require('express-session')
 const RedisStore = require('connect-redis')(session)
@@ -95,6 +96,7 @@ RedirectManager.apply(webRouter)
 ProxyManager.apply(publicApiRouter)
 
 webRouter.use(cookieParser(Settings.security.sessionSecret))
+SessionAutostartMiddleware.applyInitialMiddleware(webRouter)
 webRouter.use(
   session({
     resave: false,
@@ -149,16 +151,18 @@ webRouter.use(translations.setLangBasedOnDomainMiddlewear)
 
 // Measure expiry from last request, not last login
 webRouter.use(function(req, res, next) {
-  req.session.touch()
-  if (AuthenticationController.isUserLoggedIn(req)) {
-    UserSessionsManager.touch(
-      AuthenticationController.getSessionUser(req),
-      err => {
-        if (err) {
-          logger.err({ err }, 'error extending user session')
+  if (!req.session.noSessionCallback) {
+    req.session.touch()
+    if (AuthenticationController.isUserLoggedIn(req)) {
+      UserSessionsManager.touch(
+        AuthenticationController.getSessionUser(req),
+        err => {
+          if (err) {
+            logger.err({ err }, 'error extending user session')
+          }
         }
-      }
-    )
+      )
+    }
   }
   next()
 })
@@ -166,6 +170,8 @@ webRouter.use(function(req, res, next) {
 webRouter.use(ReferalConnect.use)
 expressLocals(webRouter, privateApiRouter, publicApiRouter)
 
+webRouter.use(SessionAutostartMiddleware.invokeCallbackMiddleware)
+
 if (app.get('env') === 'production') {
   logger.info('Production Enviroment')
   app.enable('view cache')
@@ -240,6 +246,7 @@ if (enableWebRouter || notDefined(enableWebRouter)) {
 }
 
 metrics.injectMetricsRoute(webRouter)
+
 Router.initialize(webRouter, privateApiRouter, publicApiRouter)
 
 module.exports = {

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

@@ -0,0 +1,67 @@
+const Settings = require('settings-sharelatex')
+
+// SessionAutostartMiddleware provides a mechanism to force certain routes not
+// to get an automatic session where they don't have one already. This allows us
+// to work around issues where we might overwrite a user's login cookie with one
+// that is hidden by a `SameSite` setting.
+//
+// When registering a route with disableSessionAutostartForRoute, a callback
+// should be provided that handles the case that a session is not available.
+// This will be called as a standard middleware with (req, res, next) - calling
+// next will continue and sett up a session as normal, otherwise the app can
+// perform a different operation as usual
+
+class SessionAutostartMiddleware {
+  constructor() {
+    this.middleware = this.middleware.bind(this)
+    this._cookieName = Settings.cookieName
+    this._noAutostartCallbacks = new Map()
+  }
+
+  static applyInitialMiddleware(router) {
+    const middleware = new SessionAutostartMiddleware()
+    router.sessionAutostartMiddleware = middleware
+    router.use(middleware.middleware)
+  }
+
+  disableSessionAutostartForRoute(route, method, callback) {
+    if (typeof callback !== 'function') {
+      throw new Error('callback not provided when disabling session autostart')
+    }
+
+    if (!this._noAutostartCallbacks[route]) {
+      this._noAutostartCallbacks[route] = new Map()
+    }
+
+    this._noAutostartCallbacks[route][method] = callback
+  }
+
+  autostartCallbackForRequest(req) {
+    return (
+      this._noAutostartCallbacks[req.path] &&
+      this._noAutostartCallbacks[req.path][req.method]
+    )
+  }
+
+  middleware(req, res, next) {
+    if (!req.signedCookies[this._cookieName]) {
+      const callback = this.autostartCallbackForRequest(req)
+      if (callback) {
+        req.session = {
+          noSessionCallback: callback
+        }
+      }
+    }
+
+    next()
+  }
+
+  static invokeCallbackMiddleware(req, res, next) {
+    if (req.session.noSessionCallback) {
+      return req.session.noSessionCallback(req, res, next)
+    }
+    next()
+  }
+}
+
+module.exports = SessionAutostartMiddleware

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

@@ -53,14 +53,15 @@ module.exports = {
   },
 
   validationMiddleware(req, res, next) {
-    if (!checkValidationToken(req)) {
-      // the session must exist for it to fail validation
-      req.session.destroy(() => {
-        return next(new Error('invalid session'))
-      })
-    } else {
-      return next()
+    if (!req.session.noSessionCallback) {
+      if (!checkValidationToken(req)) {
+        // the session must exist for it to fail validation
+        return req.session.destroy(() => {
+          return next(new Error('invalid session'))
+        })
+      }
     }
+    next()
   },
 
   hasValidationToken(req) {

services/web/config/settings.defaults.coffee

@@ -390,6 +390,11 @@ module.exports = settings =
 	# cookie with a secure flag (recommended).
 	secureCookie: false
 
+	# 'SameSite' cookie setting. Can be set to 'lax', 'none' or 'strict'
+	# 'lax' is recommended, as 'strict' will prevent people linking to projects
+	# https://tools.ietf.org/html/draft-ietf-httpbis-rfc6265bis-03#section-4.1.2.7
+	sameSiteCookie: 'lax'
+
 	# If you are running ShareLaTeX behind a proxy (like Apache, Nginx, etc)
 	# then set this to true to allow it to correctly detect the forwarded IP
 	# address and http/https protocol information.

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

@@ -0,0 +1,68 @@
+const sinon = require('sinon')
+const chai = require('chai')
+const { expect } = chai
+const modulePath =
+  '../../../../app/src/infrastructure/SessionAutostartMiddleware.js'
+const SandboxedModule = require('sandboxed-module')
+
+describe('SessionAutostartMiddleware', function() {
+  let SessionAutostartMiddleware, middleware, Settings
+  const cookieName = 'coookieee'
+  const excludedRoute = '/wombat/potato'
+  const excludedMethod = 'POST'
+  const excludedCallback = () => 'call me'
+
+  beforeEach(function() {
+    Settings = {
+      cookieName: cookieName
+    }
+
+    SessionAutostartMiddleware = SandboxedModule.require(modulePath, {
+      globals: {
+        console: console
+      },
+      requires: {
+        'settings-sharelatex': Settings
+      }
+    })
+
+    middleware = new SessionAutostartMiddleware()
+    middleware.disableSessionAutostartForRoute(
+      excludedRoute,
+      excludedMethod,
+      excludedCallback
+    )
+  })
+
+  describe('middleware', function() {
+    let req, next
+
+    beforeEach(function() {
+      req = { path: excludedRoute, method: excludedMethod, signedCookies: {} }
+      next = sinon.stub()
+    })
+
+    it('executes the callback for the excluded route', function() {
+      middleware.middleware(req, {}, next)
+      expect(req.session.noSessionCallback).to.equal(excludedCallback)
+    })
+
+    it('does not execute the callback if the method is not excluded', function() {
+      req.method = 'GET'
+      middleware.middleware(req, {}, next)
+      expect(req.session).not.to.exist
+    })
+
+    it('does not execute the callback if the path is not excluded', function() {
+      req.path = '/giraffe'
+      middleware.middleware(req, {}, next)
+      expect(req.session).not.to.exist
+    })
+
+    it('does not execute the callback if there is a cookie', function() {
+      req.signedCookies[cookieName] = 'a very useful session cookie'
+      middleware.middleware(req, {}, next)
+      expect(req.session).not.to.exist
+    })
+  })
+})