@uppy/companion: remove oauthOrigin (#5311)

.env.example

@@ -8,10 +8,9 @@ COMPANION_DATADIR=./output
 COMPANION_DOMAIN=localhost:3020
 COMPANION_PROTOCOL=http
 COMPANION_PORT=3020
-COMPANION_CLIENT_ORIGINS=
+COMPANION_CLIENT_ORIGINS=true
 COMPANION_SECRET=development
 COMPANION_PREAUTH_SECRET=development2
-COMPANION_OAUTH_ORIGIN=*
 
 # NOTE: Only enable this in development. Enabling it in production is a security risk
 COMPANION_ALLOW_LOCAL_URLS=true

docs/companion.md

@@ -343,14 +343,6 @@ which has only the secret, nothing else.
 
 :::
 
-### `oauthOrigin` `COMPANION_OAUTH_ORIGIN` (required)
-
-An [origin](https://developer.mozilla.org/en-US/docs/Glossary/Origin) specifying
-allowed origins, or an array of origins (comma-separated origins in
-`COMPANION_OAUTH_ORIGIN`). Any browser request from an origin that is not listed
-will not receive OAuth2 tokens, and the OAuth request won’t complete. Set it to
-`'*'` to allow all origins (not recommended).
-
 #### `uploadUrls` `COMPANION_UPLOAD_URLS`
 
 An allowlist (array) of strings (exact URLs) or regular expressions. Companion
@@ -643,12 +635,27 @@ risk.**
 
 :::
 
-#### `corsOrigins` `COMPANION_CLIENT_ORIGINS`
+#### `corsOrigins` (required)
 
-Allowed CORS Origins (default `true`). Passed as the `origin` option in
-[cors](https://github.com/expressjs/cors#configuration-options))
+Allowed CORS Origins. Passed as the `origin` option in
+[cors](https://github.com/expressjs/cors#configuration-options).
 
-#### `COMPANION_CLIENT_ORIGINS_REGEX`
+Note this is used for both CORS’ `Access-Control-Allow-Origin` header, and for
+the
+[`targetOrigin`](https://developer.mozilla.org/en-US/docs/Web/API/Window/postMessage#targetorigin)
+for `postMessage` calls in the context of OAuth.
+
+Setting it to `true` treats any origin as a trusted one, making it easier to
+impersonate your brand. Setting it to `false` disables cross-origin supports,
+use this if you’re serving Companion and Uppy from the same domain name.
+
+##### `COMPANION_CLIENT_ORIGINS`
+
+A comma-separated string of origins, or `'true'` (which will be interpreted as
+the boolean value `true`), or `'false'` (which will be interpreted as the
+boolean value `false`).
+
+##### `COMPANION_CLIENT_ORIGINS_REGEX`
 
 Like COMPANION_CLIENT_ORIGINS, but allows a single regex instead.
 `COMPANION_CLIENT_ORIGINS` will be ignored if this is used. This is a

docs/guides/migration-guides.md

@@ -6,8 +6,10 @@ These cover all the major Uppy versions and how to migrate to them.
 
 - End-of-Life versions of Node.js are no longer supported (use latest 18.x LTS,
   20.x LTS, or 22.x current).
-- Setting the `oauthOrigin` option is now required. To get back to the unsafe
-  behavior of the previous version, set it to `'*'`.
+- Setting the `corsOrigin` option is now required. You should define the list of
+  origins you expect your app to be served from, otherwise it can be
+  impersonated from a different origin you don’t control. Set it to `true` if
+  you don’t care about impersonating.
 - `COMPANION_REDIS_EXPRESS_SESSION_PREFIX` now defaults to `companion-session:`
   (before `sess:`). To revert keep backwards compatibility, set the environment
   variable `COMPANION_REDIS_EXPRESS_SESSION_PREFIX=sess:`.
@@ -36,7 +38,8 @@ These cover all the major Uppy versions and how to migrate to them.
     (inverted boolean).
   - `downloadURL` 2nd (boolean) argument inverted.
   - `StreamHttpJsonError` renamed to `HttpError`.
-- Removed (undocumented) option `clients`.
+- Removed the `oauthOrigin` option, as well as the (undocumented) option
+  `clients`. Use `corsOrigin` instead.
 
 ### `@uppy/companion-client`
 

e2e/start-companion-with-load-balancer.mjs

@@ -67,7 +67,7 @@ const startCompanion = ({ name, port }) => {
       COMPANION_ALLOW_LOCAL_URLS: 'true',
       COMPANION_ENABLE_URL_ENDPOINT: 'true',
       COMPANION_LOGGER_PROCESS_NAME: name,
-      COMPANION_OAUTH_ORIGIN: '*',
+      COMPANION_CLIENT_ORIGINS: 'true',
     },
   })
   // Adding a `then` property so the return value is awaitable:

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

@@ -108,8 +108,8 @@ const validateConfig = (companionOptions) => {
     logger.error('Running without uploadUrls is a security risk and Companion will refuse to start up when running in production (NODE_ENV=production)', 'startup.uploadUrls')
   }
 
-  if (!companionOptions.oauthOrigin) {
-    throw new TypeError('Option oauthOrigin is required. To disable security, pass "*"')
+  if (companionOptions.corsOrigins == null) {
+    throw new TypeError('Option corsOrigins is required. To disable security, pass true')
   }
 
   if (periodicPingUrls != null && (

packages/@uppy/companion/src/server/controllers/connect.js

@@ -1,46 +1,30 @@
 const oAuthState = require('../helpers/oauth-state')
 
+/**
+ * Derived from `cors` npm package.
+ * @see https://github.com/expressjs/cors/blob/791983ebc0407115bc8ae8e64830d440da995938/lib/index.js#L19-L34
+ * @param {string} origin 
+ * @param {*} allowedOrigins 
+ * @returns {boolean}
+ */
+function isOriginAllowed(origin, allowedOrigins) {
+  if (Array.isArray(allowedOrigins)) {
+    return allowedOrigins.some(allowedOrigin => isOriginAllowed(origin, allowedOrigin))
+  }
+  if (typeof allowedOrigins === 'string'){
+    return origin === allowedOrigins;
+  }
+  return allowedOrigins.test?.(origin) ?? !!allowedOrigins;
+}
+
+
 const queryString = (params, prefix = '?') => {
   const str = new URLSearchParams(params).toString()
   return str ? `${prefix}${str}` : ''
 }
 
-/**
- * initializes the oAuth flow for a provider.
- *
- * @param {object} req
- * @param {object} res
- */
-module.exports = function connect(req, res) {
-  const { secret, oauthOrigin } = req.companion.options
-  const stateObj = oAuthState.generateState()
-
-  // not sure if we need to store origin in the session state (e.g. we could've just gotten it directly inside send-token)
-  // but we're afraid to change the logic there
-  if (!Array.isArray(oauthOrigin)) {
-    // If the server only allows a single origin, we ignore the client-supplied
-    // origin from query because we don't need it.
-    stateObj.origin = oauthOrigin
-  } else if (oauthOrigin.length < 2) {
-    // eslint-disable-next-line prefer-destructuring
-    stateObj.origin = oauthOrigin[0]
-  } else {
-    // If we have multiple allowed origins, we need to check the client-supplied origin from query.
-    // If the client provides an untrusted origin,
-    // we want to send `undefined`. `undefined` means `/`, which is the same origin when passed to `postMessage`.
-    // https://html.spec.whatwg.org/multipage/web-messaging.html#dom-window-postmessage-options-dev
-    const { origin } = JSON.parse(atob(req.query.state))
-    stateObj.origin = oauthOrigin.find(o => o === origin)
-  }
-
-  if (req.companion.options.server.oauthDomain) {
-    stateObj.companionInstance = req.companion.buildURL('', true)
-  }
-
-  if (req.query.uppyPreAuthToken) {
-    stateObj.preAuthToken = req.query.uppyPreAuthToken
-  }
-
+function encodeStateAndRedirect(req, res, stateObj) {
+  const { secret } = req.companion.options
   const state = oAuthState.encodeState(stateObj, secret)
   const { providerClass, providerGrantConfig } = req.companion
 
@@ -66,3 +50,66 @@ module.exports = function connect(req, res) {
   // Now we redirect to grant's /connect endpoint, see `app.use(Grant(grantConfig))`
   res.redirect(req.companion.buildURL(`/connect/${oauthProvider}${qs}`, true))
 }
+
+function getClientOrigin(base64EncodedState) {
+  try {
+    const { origin } = JSON.parse(atob(base64EncodedState))
+    return origin
+  } catch {
+    return undefined
+  }
+}
+
+
+/**
+ * Initializes the oAuth flow for a provider.
+ *
+ * The client has open a new tab and is about to be redirected to the auth
+ * provider. When the user will return to companion, we'll have to send the auth
+ * token back to Uppy with `window.postMessage()`. 
+ * To prevent other tabs and unauthorized origins from accessing that token, we
+ * reuse origin(s) from `corsOrigins` to limit the scope of `postMessage()`, which
+ * has `targetOrigin` parameter, required for cross-origin messages (i.e. if Uppy
+ * and Companion are served from different origins).
+ * We support multiple origins in `corsOrigins`, we have to figure out which
+ * origin the current connect request is coming from. Because the OAuth window
+ * was opened with `window.open()`, starting a new browsing context, the request
+ * is not cross origin and we don't have a `Origin` header to work with.
+ * That's why we use the client-provided base64-encoded parameter, check if it
+ * matches origin(s) allowed in `corsOrigins` Companion option, and use that as
+ * our `targetOrigin` for the `window.postMessage()` call (see `send-token.js`).
+ *
+ * @param {object} req
+ * @param {object} res
+ */
+module.exports = function connect(req, res, next) {
+  const stateObj = oAuthState.generateState()
+
+  if (req.companion.options.server.oauthDomain) {
+    stateObj.companionInstance = req.companion.buildURL('', true)
+  }
+
+  if (req.query.uppyPreAuthToken) {
+    stateObj.preAuthToken = req.query.uppyPreAuthToken
+  }
+
+  // Get the computed header generated by `cors` in a previous middleware.
+  stateObj.origin = res.getHeader('Access-Control-Allow-Origin')
+  let clientOrigin
+  if (!stateObj.origin && (clientOrigin = getClientOrigin(req.query.state))) {
+    const { corsOrigins } = req.companion.options
+
+    if (typeof corsOrigins === 'function') {
+      corsOrigins(clientOrigin, (err, finalOrigin) => {
+        if (err) next(err)
+        stateObj.origin = finalOrigin
+        encodeStateAndRedirect(req, res, stateObj)
+      })
+      return
+    }
+    if (isOriginAllowed(clientOrigin, req.companion.options.corsOrigins)) {
+      stateObj.origin = clientOrigin
+    }
+  }
+  encodeStateAndRedirect(req, res, stateObj)
+}

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

@@ -45,9 +45,17 @@ const companionProtocol = process.env.COMPANION_PROTOCOL || 'http'
 
 function getCorsOrigins () {
   if (process.env.COMPANION_CLIENT_ORIGINS) {
-    return process.env.COMPANION_CLIENT_ORIGINS
-      .split(',')
-      .map((url) => (hasProtocol(url) ? url : `${companionProtocol}://${url}`))
+    switch (process.env.COMPANION_CLIENT_ORIGINS) {
+
+      case 'true': return true
+      case 'false': return false
+      case '*': return '*'
+
+      default:
+      return process.env.COMPANION_CLIENT_ORIGINS
+        .split(',')
+        .map((url) => (hasProtocol(url) ? url : `${companionProtocol}://${url}`))
+  }
   }
   if (process.env.COMPANION_CLIENT_ORIGINS_REGEX) {
     return new RegExp(process.env.COMPANION_CLIENT_ORIGINS_REGEX)
@@ -183,9 +191,6 @@ const getConfigFromEnv = () => {
     corsOrigins: getCorsOrigins(),
     testDynamicOauthCredentials: process.env.COMPANION_TEST_DYNAMIC_OAUTH_CREDENTIALS === 'true',
     testDynamicOauthCredentialsSecret: process.env.COMPANION_TEST_DYNAMIC_OAUTH_CREDENTIALS_SECRET,
-    oauthOrigin: process.env.COMPANION_OAUTH_ORIGIN?.includes(',') ?
-      process.env.COMPANION_OAUTH_ORIGIN.split(',') :
-      process.env.COMPANION_OAUTH_ORIGIN,
   }
 }
 

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

@@ -20,7 +20,7 @@ afterAll(() => {
 
 process.env.COMPANION_DATADIR = './test/output'
 process.env.COMPANION_DOMAIN = 'localhost:3020'
-process.env.COMPANION_OAUTH_ORIGIN = '*'
+process.env.COMPANION_CLIENT_ORIGINS = 'true'
 const { companionOptions } = standalone()
 
 const mockReq = {}

packages/@uppy/companion/test/mockserver.js

@@ -46,7 +46,7 @@ const defaultEnv = {
 
   COMPANION_ENABLE_URL_ENDPOINT: 'true',
 
-  COMPANION_OAUTH_ORIGIN: '*',
+  COMPANION_CLIENT_ORIGINS: 'true',
 }
 
 function updateEnv (env) {