Browse Source

Refactor/dedupe cookie/session logic (#4420)

* refactor/dedupe cookie logic

so that we also send sameSite for session cookies
this might fix the issue where sessions are recreated for every single request

* "fix" test

* Apply suggestions from code review

Co-authored-by: Antoine du Hamel <antoine@transloadit.com>

---------

Co-authored-by: Antoine du Hamel <antoine@transloadit.com>
Mikael Finstad 2 năm trước cách đây
mục cha
commit
354cc303ef

+ 17 - 14
packages/@uppy/companion/src/server/helpers/jwt.js

@@ -48,9 +48,11 @@ module.exports.verifyEncryptedToken = (token, secret) => {
   }
 }
 
-const addToCookies = (res, token, companionOptions, authProvider, prefix) => {
+const getCookieName = (authProvider) => `uppyAuthToken--${authProvider}`
+
+function getCookieOptions (companionOptions) {
   const cookieOptions = {
-    maxAge: 1000 * EXPIRY, // would expire after one day (24 hrs)
+    maxAge: 1000 * EXPIRY,
     httpOnly: true,
   }
 
@@ -64,10 +66,12 @@ const addToCookies = (res, token, companionOptions, authProvider, prefix) => {
   if (companionOptions.cookieDomain) {
     cookieOptions.domain = companionOptions.cookieDomain
   }
-  // send signed token to client.
-  res.cookie(`${prefix}--${authProvider}`, token, cookieOptions)
+
+  return cookieOptions
 }
 
+module.exports.getCookieOptions = getCookieOptions
+
 /**
  *
  * @param {object} res
@@ -76,7 +80,10 @@ const addToCookies = (res, token, companionOptions, authProvider, prefix) => {
  * @param {string} authProvider
  */
 module.exports.addToCookies = (res, token, companionOptions, authProvider) => {
-  addToCookies(res, token, companionOptions, authProvider, 'uppyAuthToken')
+  const cookieOptions = getCookieOptions(companionOptions)
+
+  // send signed token to client.
+  res.cookie(getCookieName(authProvider), token, cookieOptions)
 }
 
 /**
@@ -86,14 +93,10 @@ module.exports.addToCookies = (res, token, companionOptions, authProvider) => {
  * @param {string} authProvider
  */
 module.exports.removeFromCookies = (res, companionOptions, authProvider) => {
-  const cookieOptions = {
-    maxAge: 1000 * EXPIRY, // would expire after one day (24 hrs)
-    httpOnly: true,
-  }
-
-  if (companionOptions.cookieDomain) {
-    cookieOptions.domain = companionOptions.cookieDomain
-  }
+  // https://expressjs.com/en/api.html
+  // Web browsers and other compliant clients will only clear the cookie if the given options is
+  // identical to those given to res.cookie(), excluding expires and maxAge.
+  const cookieOptions = getCookieOptions(companionOptions)
 
-  res.clearCookie(`uppyAuthToken--${authProvider}`, cookieOptions)
+  res.clearCookie(getCookieName(authProvider), cookieOptions)
 }

+ 2 - 6
packages/@uppy/companion/src/standalone/index.js

@@ -11,6 +11,7 @@ const logger = require('../server/logger')
 const redis = require('../server/redis')
 const companion = require('../companion')
 const { getCompanionOptions, generateSecret, buildHelpfulStartupMessage } = require('./helper')
+const { getCookieOptions } = require('../server/helpers/jwt')
 
 /**
  * Configures an Express app for running Companion standalone
@@ -116,12 +117,7 @@ module.exports = function server (inputCompanionOptions) {
     sessionOptions.store = new RedisStore({ client: redisClient, prefix: process.env.COMPANION_REDIS_EXPRESS_SESSION_PREFIX || 'sess:' })
   }
 
-  if (process.env.COMPANION_COOKIE_DOMAIN) {
-    sessionOptions.cookie = {
-      domain: process.env.COMPANION_COOKIE_DOMAIN,
-      maxAge: 24 * 60 * 60 * 1000, // 1 day
-    }
-  }
+  sessionOptions.cookie = getCookieOptions(companionOptions)
 
   // Session is used for grant redirects, so that we don't need to expose secret tokens in URLs
   // See https://github.com/transloadit/uppy/pull/1668

+ 1 - 0
packages/@uppy/companion/test/__tests__/preauth.js

@@ -10,6 +10,7 @@ jest.mock('../../src/server/helpers/jwt', () => {
     },
     addToCookies: () => {},
     removeFromCookies: () => {},
+    getCookieOptions: () => {},
   }
 })