Companion should respect previously set value for Accesss-Control-Allow-Methods (#2726)

Co-authored-by: Mikael Finstad <finstaden@gmail.com>

packages/@uppy/companion/package.json

@@ -49,7 +49,7 @@
     "ip-address": "6.2.0",
     "isobject": "3.0.1",
     "jsonwebtoken": "8.5.1",
-    "lodash.merge": "4.6.2",
+    "lodash": "^4.17.20",
     "mime-types": "2.1.25",
     "moment-timezone": "^0.5.31",
     "morgan": "1.10.0",

packages/@uppy/companion/src/companion.js

@@ -10,7 +10,7 @@ const s3 = require('./server/controllers/s3')
 const getS3Client = require('./server/s3-client')
 const url = require('./server/controllers/url')
 const emitter = require('./server/emitter')
-const merge = require('lodash.merge')
+const merge = require('lodash/merge')
 const redis = require('./server/redis')
 const cookieParser = require('cookie-parser')
 const { getURLBuilder } = require('./server/helpers/utils')
@@ -77,8 +77,10 @@ module.exports.app = (options = {}) => {
   // override provider credentials at request time
   app.use('/connect/:authProvider/:override?', getCredentialsOverrideMiddleware(providers, options))
   app.use(Grant(grantConfig))
+
+  app.use(middlewares.mergeAccessControlAllowMethods)
+
   app.use((req, res, next) => {
-    res.header('Access-Control-Allow-Methods', 'GET, POST, OPTIONS, DELETE')
     res.header(
       'Access-Control-Allow-Headers',
       [

packages/@uppy/companion/src/server/middlewares.js

@@ -1,5 +1,6 @@
 const tokenService = require('./helpers/jwt')
 const logger = require('./logger')
+const uniq = require('lodash/uniq')
 
 exports.hasSessionAndProvider = (req, res, next) => {
   if (!req.session || !req.body) {
@@ -71,3 +72,16 @@ exports.loadSearchProviderToken = (req, res, next) => {
   req.companion.providerToken = searchProviders[providerName].key
   next()
 }
+
+exports.mergeAccessControlAllowMethods = (req, res, next) => {
+  const existingHeader = res.get('Access-Control-Allow-Methods')
+  let existingMethods = []
+  if (existingHeader) {
+    existingMethods = existingHeader.replace(/\s/g, '').split(',').map((method) => method.toUpperCase())
+  }
+
+  const mergedMethods = uniq([...existingMethods, 'GET', 'POST', 'OPTIONS', 'DELETE'])
+
+  res.header('Access-Control-Allow-Methods', mergedMethods.join(','))
+  next()
+}

packages/@uppy/companion/src/standalone/helper.js

@@ -1,5 +1,5 @@
 const fs = require('fs')
-const merge = require('lodash.merge')
+const merge = require('lodash/merge')
 const stripIndent = require('common-tags/lib/stripIndent')
 const utils = require('../server/helpers/utils')
 const logger = require('../server/logger')

packages/@uppy/companion/src/standalone/index.js

@@ -7,7 +7,7 @@ const bodyParser = require('body-parser')
 const redis = require('../server/redis')
 const logger = require('../server/logger')
 const { URL } = require('url')
-const merge = require('lodash.merge')
+const merge = require('lodash/merge')
 // @ts-ignore
 const promBundle = require('express-prom-bundle')
 const session = require('express-session')

packages/@uppy/companion/test/__tests__/middlewares.js

@@ -0,0 +1,26 @@
+/* global jest:false, test:false, describe:false, expect:false */
+
+const { mergeAccessControlAllowMethods } = require('../../src/server/middlewares')
+
+describe('mergeAccessControlAllowMethods', () => {
+  test('should properly merge', () => {
+    const res = {
+      get: () => 'PATCH,OPTIONS, post',
+      header: jest.fn()
+    }
+    const next = jest.fn()
+    mergeAccessControlAllowMethods(undefined, res, next)
+    expect(res.header).toHaveBeenCalledWith('Access-Control-Allow-Methods', 'PATCH,OPTIONS,POST,GET,DELETE')
+    expect(next).toHaveBeenCalled()
+  })
+  test('should also work when nothing added', () => {
+    const res = {
+      get: () => undefined,
+      header: jest.fn()
+    }
+    const next = jest.fn()
+    mergeAccessControlAllowMethods(undefined, res, next)
+    expect(res.header).toHaveBeenCalledWith('Access-Control-Allow-Methods', 'GET,POST,OPTIONS,DELETE')
+    expect(next).toHaveBeenCalled()
+  })
+})