diff --git a/.env.example b/.env.example index 364b7e3e0..7df30462f 100644 --- a/.env.example +++ b/.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 diff --git a/docs/companion.md b/docs/companion.md index 6ebaea445..f85a154e5 100644 --- a/docs/companion.md +++ b/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 diff --git a/docs/guides/migration-guides.md b/docs/guides/migration-guides.md index c821abf0d..67d3df457 100644 --- a/docs/guides/migration-guides.md +++ b/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` diff --git a/e2e/start-companion-with-load-balancer.mjs b/e2e/start-companion-with-load-balancer.mjs index 697c2865c..c2d33d508 100755 --- a/e2e/start-companion-with-load-balancer.mjs +++ b/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: diff --git a/packages/@uppy/companion/src/config/companion.js b/packages/@uppy/companion/src/config/companion.js index 3acdbd06d..448c95cda 100644 --- a/packages/@uppy/companion/src/config/companion.js +++ b/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 && ( diff --git a/packages/@uppy/companion/src/server/controllers/connect.js b/packages/@uppy/companion/src/server/controllers/connect.js index d31a7adad..f2f835e95 100644 --- a/packages/@uppy/companion/src/server/controllers/connect.js +++ b/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) +} diff --git a/packages/@uppy/companion/src/standalone/helper.js b/packages/@uppy/companion/src/standalone/helper.js index 9f85d51be..6b575e331 100644 --- a/packages/@uppy/companion/src/standalone/helper.js +++ b/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, } } diff --git a/packages/@uppy/companion/test/__tests__/uploader.js b/packages/@uppy/companion/test/__tests__/uploader.js index 8cf3c2c93..8fb09ea38 100644 --- a/packages/@uppy/companion/test/__tests__/uploader.js +++ b/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 = {} diff --git a/packages/@uppy/companion/test/mockserver.js b/packages/@uppy/companion/test/mockserver.js index 6afe9cd11..9ee1779e9 100644 --- a/packages/@uppy/companion/test/mockserver.js +++ b/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) {