@uppy/companion: make `oauthOrigin` option required (#5276)

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

docs/companion.md

@@ -343,20 +343,13 @@ which has only the secret, nothing else.
 
 :::
 
-### `oauthOrigin` `COMPANION_OAUTH_ORIGIN`
-
-:::caution
-
-Setting this option is strongly recommended. If left unset (or set to `'*'`),
-your app could be impersonated.
-
-:::
+### `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). Default: `'*'`.
+`'*'` to allow all origins (not recommended).
 
 #### `uploadUrls` `COMPANION_UPLOAD_URLS`
 

docs/guides/migration-guides.md

@@ -5,6 +5,8 @@ These cover all the major Uppy versions and how to migrate to them.
 ## Migrate from Companion 4.x to 5.x
 
 - Node.js `>=18.20.0` is now required.
+- Setting the `oauthOrigin` option is now required. To get back to the unsafe
+  behavior of the previous version, set it to `'*'`.
 - `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:`.
@@ -33,6 +35,7 @@ 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`.
 
 ### `@uppy/companion-client`
 

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

@@ -108,6 +108,10 @@ 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 (periodicPingUrls != null && (
     !Array.isArray(periodicPingUrls)
     || periodicPingUrls.some((url2) => !isURL(url2, { protocols: ['http', 'https'], require_protocol: true, require_tld: false }))

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

@@ -17,11 +17,11 @@ module.exports = function connect(req, res) {
 
   // 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 (oauthOrigin && !Array.isArray(oauthOrigin)) {
+  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 && oauthOrigin.length < 2) {
+  } else if (oauthOrigin.length < 2) {
     // eslint-disable-next-line prefer-destructuring
     stateObj.origin = oauthOrigin[0]
   } else {
@@ -30,7 +30,7 @@ module.exports = function connect(req, res) {
     // 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 ? oauthOrigin.find(o => o === origin) : origin
+    stateObj.origin = oauthOrigin.find(o => o === origin)
   }
 
   if (req.companion.options.server.oauthDomain) {

packages/@uppy/companion/src/server/controllers/send-token.js

@@ -1,7 +1,5 @@
-const { URL } = require('node:url')
 const serialize = require('serialize-javascript')
 
-const { hasMatch } = require('../helpers/utils')
 const oAuthState = require('../helpers/oauth-state')
 
 /**
@@ -36,12 +34,8 @@ module.exports = function sendToken (req, res, next) {
   const { state } = oAuthState.getGrantDynamicFromRequest(req)
   if (state) {
     const origin = oAuthState.getFromState(state, 'origin', req.companion.options.secret)
-    const allowedClients = req.companion.options.clients
-    // if no preset clients then allow any client
-    if (!allowedClients || hasMatch(origin, allowedClients) || hasMatch((new URL(origin)).host, allowedClients)) {
-      res.send(htmlContent(uppyAuthToken, origin))
-      return
-    }
+    res.send(htmlContent(uppyAuthToken, origin))
+    return
   }
   next()
 }