Merge pull request #17732 from overleaf/bg-session-mitigation-initial-protoype

anonymous cookie-based sessions module

GitOrigin-RevId: 75fe2d48fa384ba8d07c0b478a9a5a907a2b3b67

package-lock.json

@@ -43377,6 +43377,7 @@
         "node-fetch": "^2.6.7",
         "nodemailer": "^6.7.0",
         "nodemailer-ses-transport": "^1.5.1",
+        "on-headers": "^1.0.2",
         "openai": "^4.36.0",
         "otplib": "^12.0.1",
         "p-limit": "^2.3.0",
@@ -43398,6 +43399,7 @@
         "sanitize-html": "^2.8.1",
         "tough-cookie": "^4.0.0",
         "tsscmp": "^1.0.6",
+        "uid-safe": "^2.1.5",
         "utf-8-validate": "^5.0.2",
         "uuid": "^9.0.1",
         "valid-data-url": "^2.0.0",
@@ -51977,6 +51979,7 @@
         "nodemailer": "^6.7.0",
         "nodemailer-ses-transport": "^1.5.1",
         "nvd3": "^1.8.6",
+        "on-headers": "^1.0.2",
         "openai": "^4.36.0",
         "otplib": "^12.0.1",
         "p-limit": "^2.3.0",
@@ -52035,6 +52038,7 @@
         "tough-cookie": "^4.0.0",
         "tsscmp": "^1.0.6",
         "typescript": "^5.0.4",
+        "uid-safe": "^2.1.5",
         "utf-8-validate": "^5.0.2",
         "uuid": "^9.0.1",
         "valid-data-url": "^2.0.0",

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

@@ -419,8 +419,9 @@ const AuthenticationController = {
     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(() => {
+        // Force user to update session by destroying the current one.
+        // A new session will be created on the next request.
+        req.session.destroy(() => {
           // 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

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

@@ -88,10 +88,15 @@ function loadViewIncludes(app) {
   }
 }
 
-function registerAppMiddleware(app) {
+function registerMiddleware(appOrRouter, middlewareName, options) {
+  if (!middlewareName) {
+    throw new Error(
+      'middleware name must be provided to register module middleware'
+    )
+  }
   for (const module of modules()) {
-    if (module.appMiddleware) {
-      module.appMiddleware(app)
+    if (module[middlewareName]) {
+      module[middlewareName](appOrRouter, options)
     }
   }
 }
@@ -164,7 +169,7 @@ module.exports = {
   loadViewIncludes,
   moduleIncludes,
   moduleIncludesAvailable,
-  registerAppMiddleware,
+  registerMiddleware,
   hooks: {
     attach: attachHook,
     fire: fireHook,

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

@@ -133,7 +133,7 @@ Modules.loadViewIncludes(app)
 
 app.use(metrics.http.monitor(logger))
 
-Modules.registerAppMiddleware(app)
+Modules.registerMiddleware(app, 'appMiddleware')
 app.use(bodyParser.urlencoded({ extended: true, limit: '2mb' }))
 app.use(bodyParser.json({ limit: Settings.max_json_request_size }))
 app.use(methodOverride())
@@ -157,6 +157,9 @@ RedirectManager.apply(webRouter)
 
 webRouter.use(cookieParser(Settings.security.sessionSecret))
 SessionAutostartMiddleware.applyInitialMiddleware(webRouter)
+Modules.registerMiddleware(webRouter, 'sessionMiddleware', {
+  store: sessionStore,
+})
 webRouter.use(
   session({
     resave: false,

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

@@ -71,4 +71,7 @@ module.exports = {
       return false
     }
   },
+
+  computeValidationToken,
+  checkValidationToken,
 }

services/web/package.json

@@ -136,6 +136,7 @@
     "node-fetch": "^2.6.7",
     "nodemailer": "^6.7.0",
     "nodemailer-ses-transport": "^1.5.1",
+    "on-headers": "^1.0.2",
     "openai": "^4.36.0",
     "otplib": "^12.0.1",
     "p-limit": "^2.3.0",
@@ -157,6 +158,7 @@
     "sanitize-html": "^2.8.1",
     "tough-cookie": "^4.0.0",
     "tsscmp": "^1.0.6",
+    "uid-safe": "^2.1.5",
     "utf-8-validate": "^5.0.2",
     "uuid": "^9.0.1",
     "valid-data-url": "^2.0.0",

services/web/test/acceptance/src/helpers/InitApp.js

@@ -28,6 +28,11 @@ before('start main app', function (done) {
     route => route.path && route.path === '/dev/csrf',
     router => {
       router.get('/dev/session', (req, res) => {
+        // allow changing the session directly for testing, assign any
+        // properties in the query string to req.session
+        if (req.query && Object.keys(req.query).length > 0) {
+          Object.assign(req.session, req.query)
+        }
         return res.json(req.session)
       })
     }
@@ -75,6 +80,27 @@ before('start main app', function (done) {
       })
     }
   )
+  injectRouteAfter(
+    app,
+    route => route.path && route.path === '/dev/csrf',
+    router => {
+      router.csrf.disableDefaultCsrfProtection(
+        '/dev/no_autostart_post_gateway',
+        'POST'
+      )
+      router.sessionAutostartMiddleware.disableSessionAutostartForRoute(
+        '/dev/no_autostart_post_gateway',
+        'POST',
+        (req, res, next) => {
+          next()
+        }
+      )
+      router.post('/dev/no_autostart_post_gateway', (req, res) => {
+        res.status(200).json({ message: 'no autostart' })
+      })
+    }
+  )
+
   server = App.listen(23000, '127.0.0.1', done)
 })
 

services/web/test/acceptance/src/helpers/User.js

@@ -8,6 +8,8 @@ const AuthenticationManager = require('../../../../app/src/Features/Authenticati
 const { promisifyClass } = require('@overleaf/promise-utils')
 const fs = require('fs')
 const Path = require('path')
+const { Cookie } = require('tough-cookie')
+const COOKIE_DOMAIN = settings.cookieDomain
 
 let count = settings.test.counterInit
 
@@ -32,10 +34,15 @@ class User {
     })
   }
 
-  getSession(callback) {
+  getSession(options, callback) {
+    if (typeof options === 'function') {
+      callback = options
+      options = {}
+    }
     this.request.get(
       {
         url: '/dev/session',
+        qs: options.set,
       },
       (err, response, body) => {
         if (err != null) {
@@ -186,6 +193,22 @@ class User {
     )
   }
 
+  /* Return the session cookie, url decoded. Use the option {raw:true} to get the original undecoded value */
+
+  sessionCookie(options) {
+    const cookie = Cookie.parse(
+      this.jar.getCookieString(
+        // The cookie domain has a leading '.' but
+        // the cookie jar stores it without.
+        'https://' + COOKIE_DOMAIN.replace(/^\./, '') + '/'
+      )
+    )
+    if (cookie?.value && !options?.raw) {
+      cookie.value = decodeURIComponent(cookie.value)
+    }
+    return cookie
+  }
+
   getEmailConfirmationCode(callback) {
     this.getSession((err, session) => {
       if (err != null) {
@@ -1224,7 +1247,7 @@ class User {
 }
 
 User.promises = promisifyClass(User, {
-  without: ['setExtraAttributes'],
+  without: ['setExtraAttributes', 'sessionCookie'],
 })
 
 User.promises.prototype.doRequest = async function (method, params) {

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

@@ -624,7 +624,7 @@ describe('AuthenticationController', function () {
       beforeEach(function () {
         this.req.session = {
           user: this.user,
-          regenerate: sinon.stub().yields(),
+          destroy: sinon.stub().yields(),
         }
         this.req.user = this.user
         this.AuthenticationController._redirectToLoginOrRegisterPage =
@@ -637,7 +637,7 @@ describe('AuthenticationController', function () {
       })
 
       it('should destroy the current session', function () {
-        this.req.session.regenerate.called.should.equal(true)
+        this.req.session.destroy.called.should.equal(true)
       })
 
       it('should redirect to the register or login page', function () {