diff --git a/docs/companion.md b/docs/companion.md index 22b1120793..6ebaea4451 100644 --- a/docs/companion.md +++ b/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` diff --git a/docs/guides/migration-guides.md b/docs/guides/migration-guides.md index 28fb7db649..0b8345d3e4 100644 --- a/docs/guides/migration-guides.md +++ b/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` diff --git a/packages/@uppy/companion/src/config/companion.js b/packages/@uppy/companion/src/config/companion.js index 919d153a93..3acdbd06d8 100644 --- a/packages/@uppy/companion/src/config/companion.js +++ b/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 })) diff --git a/packages/@uppy/companion/src/server/controllers/connect.js b/packages/@uppy/companion/src/server/controllers/connect.js index 772f80fec2..d31a7adadd 100644 --- a/packages/@uppy/companion/src/server/controllers/connect.js +++ b/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) { diff --git a/packages/@uppy/companion/src/server/controllers/send-token.js b/packages/@uppy/companion/src/server/controllers/send-token.js index 60bac3474e..3f7ebe4be3 100644 --- a/packages/@uppy/companion/src/server/controllers/send-token.js +++ b/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() }