Default allow headers (transloadit#3167)

* refactor cors middleware to avoid duplicates * Make more ACAH headers default Add the following headers to access-control-allow-headers by default: Authorization, Origin, Content-Type, Accept They are needed for basic operation. See transloadit#3021 therefore also remove custom middlewares in examples and standalone * Update outdated readme for S3 AWS now requires JSON instead of XML format * Update packages/@uppy/companion/src/server/middlewares.js Co-authored-by: Antoine du Hamel <duhamelantoine1995@gmail.com> * Update packages/@uppy/companion/src/server/middlewares.js Co-authored-by: Antoine du Hamel <duhamelantoine1995@gmail.com> * fix review comments Co-authored-by: Antoine du Hamel <duhamelantoine1995@gmail.com>
docsend · Sep 30, 2021 · 86f5015 · 86f5015
1 parent d08d377
commit 86f5015
Show file tree

Hide file tree

Showing 3 changed files with 38 additions and 32 deletions.
diff --git a/src/server/middlewares.js b/src/server/middlewares.js
@@ -79,25 +79,39 @@ exports.loadSearchProviderToken = (req, res, next) => {
 }
 
 exports.cors = (options = {}) => (req, res, next) => {
-  const exposedHeaders = [
-    // exposed so it can be accessed for our custom uppy client preflight
-    'Access-Control-Allow-Headers',
-  ]
-  if (options.sendSelfEndpoint) exposedHeaders.push('i-am')
-  if (res.get('Access-Control-Expose-Headers')) exposedHeaders.push(res.get('Access-Control-Expose-Headers'))
+  // HTTP headers are not case sensitive, and express always handles them in lower case, so that's why we lower case them.
+  // I believe that HTTP verbs are case sensitive, and should be uppercase.
+
+  // TODO: Move to optional chaining when we drop Node.js v12.x support
+  const existingExposeHeaders = res.get('Access-Control-Expose-Headers')
+  const exposeHeadersSet = new Set(existingExposeHeaders && existingExposeHeaders.split(',').map(method => method.trim().toLowerCase()))
 
+  // exposed so it can be accessed for our custom uppy client preflight
+  exposeHeadersSet.add('access-control-allow-headers')
+  if (options.sendSelfEndpoint) exposeHeadersSet.add('i-am')
+
+  // Needed for basic operation: https://github.com/transloadit/uppy/issues/3021
   const allowedHeaders = [
     'uppy-auth-token',
     'uppy-versions',
     'uppy-credentials-params',
+    'authorization',
+    'origin',
+    'content-type',
+    'accept',
   ]
-  if (res.get('Access-Control-Allow-Headers')) allowedHeaders.push(res.get('Access-Control-Allow-Headers'))
-
-  // TODO: Move to optional chaining when we drop Node.js v12.x support
-  const ACAMHeader = res.get('Access-Control-Allow-Methods')
-  const existingAllowMethodsHeader = new Set(ACAMHeader && ACAMHeader.split(',').map(method => method.trim().toUpperCase()))
-  existingAllowMethodsHeader.add('GET').add('POST').add('OPTIONS').add('DELETE')
-  const methods = Array.from(existingAllowMethodsHeader)
+  const existingAllowHeaders = res.get('Access-Control-Allow-Headers')
+  const allowHeadersSet = new Set(existingAllowHeaders
+    ? existingAllowHeaders
+      .split(',')
+      .map((method) => method.trim().toLowerCase())
+      .concat(allowedHeaders)
+    : allowedHeaders)
+
+  const existingAllowMethods = res.get('Access-Control-Allow-Methods')
+  const allowMethodsSet = new Set(existingAllowMethods && existingAllowMethods.split(',').map(method => method.trim().toUpperCase()))
+  // Needed for basic operation:
+  allowMethodsSet.add('GET').add('POST').add('OPTIONS').add('DELETE')
 
   // If endpoint urls are specified, then we only allow those endpoints.
   // Otherwise, we allow any client url to access companion.
@@ -110,9 +124,9 @@ exports.cors = (options = {}) => (req, res, next) => {
   return cors({
     credentials: true,
     origin,
-    methods,
-    allowedHeaders: allowedHeaders.join(','),
-    exposedHeaders: exposedHeaders.join(','),
+    methods: Array.from(allowMethodsSet),
+    allowedHeaders: Array.from(allowHeadersSet).join(','),
+    exposedHeaders: Array.from(exposeHeadersSet).join(','),
   })(req, res, next)
 }
 

diff --git a/src/standalone/index.js b/src/standalone/index.js
@@ -133,14 +133,6 @@ module.exports = function server (inputCompanionOptions = {}) {
 
   app.use(session(sessionOptions))
 
-  app.use((req, res, next) => {
-    res.setHeader(
-      'Access-Control-Allow-Headers',
-      'Authorization, Origin, Content-Type, Accept'
-    )
-    next()
-  })
-
   // Routes
   if (process.env.COMPANION_HIDE_WELCOME !== 'true') {
     app.get('/', (req, res) => {

diff --git a/test/__tests__/cors.js b/test/__tests__/cors.js
@@ -41,8 +41,8 @@ describe('cors', () => {
       ['Vary', 'Origin'],
       ['Access-Control-Allow-Credentials', 'true'],
       ['Access-Control-Allow-Methods', 'PATCH,OPTIONS,POST,GET,DELETE'],
-      ['Access-Control-Allow-Headers', 'uppy-auth-token,uppy-versions,uppy-credentials-params,test-allow-header'],
-      ['Access-Control-Expose-Headers', 'Access-Control-Allow-Headers,i-am,test'],
+      ['Access-Control-Allow-Headers', 'test-allow-header,uppy-auth-token,uppy-versions,uppy-credentials-params,authorization,origin,content-type,accept'],
+      ['Access-Control-Expose-Headers', 'test,access-control-allow-headers,i-am'],
       ['Content-Length', '0'],
     ])
     // expect(next).toHaveBeenCalled()
@@ -55,8 +55,8 @@ describe('cors', () => {
       ['Vary', 'Origin'],
       ['Access-Control-Allow-Credentials', 'true'],
       ['Access-Control-Allow-Methods', 'GET,POST,OPTIONS,DELETE'],
-      ['Access-Control-Allow-Headers', 'uppy-auth-token,uppy-versions,uppy-credentials-params'],
-      ['Access-Control-Expose-Headers', 'Access-Control-Allow-Headers'],
+      ['Access-Control-Allow-Headers', 'uppy-auth-token,uppy-versions,uppy-credentials-params,authorization,origin,content-type,accept'],
+      ['Access-Control-Expose-Headers', 'access-control-allow-headers'],
       ['Content-Length', '0'],
     ])
   })
@@ -72,8 +72,8 @@ describe('cors', () => {
       ['Vary', 'Origin'],
       ['Access-Control-Allow-Credentials', 'true'],
       ['Access-Control-Allow-Methods', 'GET,POST,OPTIONS,DELETE'],
-      ['Access-Control-Allow-Headers', 'uppy-auth-token,uppy-versions,uppy-credentials-params'],
-      ['Access-Control-Expose-Headers', 'Access-Control-Allow-Headers'],
+      ['Access-Control-Allow-Headers', 'uppy-auth-token,uppy-versions,uppy-credentials-params,authorization,origin,content-type,accept'],
+      ['Access-Control-Expose-Headers', 'access-control-allow-headers'],
       ['Content-Length', '0'],
     ])
   })
@@ -85,8 +85,8 @@ describe('cors', () => {
       ['Vary', 'Origin'],
       ['Access-Control-Allow-Credentials', 'true'],
       ['Access-Control-Allow-Methods', 'GET,POST,OPTIONS,DELETE'],
-      ['Access-Control-Allow-Headers', 'uppy-auth-token,uppy-versions,uppy-credentials-params'],
-      ['Access-Control-Expose-Headers', 'Access-Control-Allow-Headers'],
+      ['Access-Control-Allow-Headers', 'uppy-auth-token,uppy-versions,uppy-credentials-params,authorization,origin,content-type,accept'],
+      ['Access-Control-Expose-Headers', 'access-control-allow-headers'],
       ['Content-Length', '0'],
     ])
   })