From 334484f9a57f9309c9c0d21096dd0e36e32c17d7 Mon Sep 17 00:00:00 2001 From: Murderlon Date: Thu, 28 Nov 2024 15:06:35 +0100 Subject: [PATCH 1/3] @uppy/companion: pass fetched origins to window.postMessage() --- packages/@uppy/companion/src/companion.js | 1 + .../src/server/controllers/connect.js | 8 ++--- .../src/server/controllers/send-token.js | 33 +++++++++++++------ .../src/server/provider/credentials.js | 22 +++++++++++++ .../companion/src/server/provider/index.js | 2 +- 5 files changed, 51 insertions(+), 15 deletions(-) diff --git a/packages/@uppy/companion/src/companion.js b/packages/@uppy/companion/src/companion.js index 01f55c2fa5..67c41e5c46 100644 --- a/packages/@uppy/companion/src/companion.js +++ b/packages/@uppy/companion/src/companion.js @@ -163,6 +163,7 @@ module.exports.app = (optionsArg = {}) => { key, secret, redirect_uri: getRedirectUri(), + origins: ['http://localhost:5173'], }, }) }) diff --git a/packages/@uppy/companion/src/server/controllers/connect.js b/packages/@uppy/companion/src/server/controllers/connect.js index f2f835e954..154ba67046 100644 --- a/packages/@uppy/companion/src/server/controllers/connect.js +++ b/packages/@uppy/companion/src/server/controllers/connect.js @@ -3,8 +3,8 @@ 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 + * @param {string} origin + * @param {*} allowedOrigins * @returns {boolean} */ function isOriginAllowed(origin, allowedOrigins) { @@ -17,7 +17,6 @@ function isOriginAllowed(origin, allowedOrigins) { return allowedOrigins.test?.(origin) ?? !!allowedOrigins; } - const queryString = (params, prefix = '?') => { const str = new URLSearchParams(params).toString() return str ? `${prefix}${str}` : '' @@ -66,7 +65,7 @@ function getClientOrigin(base64EncodedState) { * * 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()`. + * 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 @@ -113,3 +112,4 @@ module.exports = function connect(req, res, next) { } encodeStateAndRedirect(req, res, stateObj) } +module.exports.isOriginAllowed = isOriginAllowed diff --git a/packages/@uppy/companion/src/server/controllers/send-token.js b/packages/@uppy/companion/src/server/controllers/send-token.js index 2ac5489691..475185c1b2 100644 --- a/packages/@uppy/companion/src/server/controllers/send-token.js +++ b/packages/@uppy/companion/src/server/controllers/send-token.js @@ -1,4 +1,5 @@ const serialize = require('serialize-javascript') +const { isOriginAllowed } = require('./connect') const oAuthState = require('../helpers/oauth-state') @@ -46,18 +47,30 @@ const htmlContent = (token, origin) => { /** * - * @param {object} req - * @param {object} res - * @param {Function} next + * @param {import('express').Request} req + * @param {import('express').Response} res + * @param {import('express').NextFunction} next */ -module.exports = function sendToken (req, res, next) { - const uppyAuthToken = req.companion.authToken +module.exports = function sendToken(req, res, next) { + // @ts-expect-error untyped + const {companion} = req + const uppyAuthToken = companion.authToken const { state } = oAuthState.getGrantDynamicFromRequest(req) - if (state) { - const origin = oAuthState.getFromState(state, 'origin', req.companion.options.secret) - res.send(htmlContent(uppyAuthToken, origin)) - return + + if (!state) { + return next() } - next() + + const decoded = oAuthState.decodeState(state, companion.options.secret) + const { origin: clientOrigin, customerDefinedAllowedOrigins } = decoded + + if ( + customerDefinedAllowedOrigins && + !isOriginAllowed(clientOrigin, customerDefinedAllowedOrigins) + ) { + return next() + } + + return res.send(htmlContent(uppyAuthToken, clientOrigin)) } diff --git a/packages/@uppy/companion/src/server/provider/credentials.js b/packages/@uppy/companion/src/server/provider/credentials.js index e716e92251..7d0aaf847f 100644 --- a/packages/@uppy/companion/src/server/provider/credentials.js +++ b/packages/@uppy/companion/src/server/provider/credentials.js @@ -108,10 +108,32 @@ exports.getCredentialsOverrideMiddleware = (providers, companionOptions) => { const credentials = await fetchProviderKeys(providerName, companionOptions, payload) + // Besides the key and secret the fetched credentials can also contain `origins`, + // which is an array of strings of allowed origins to prevent any origin from getting the OAuth + // token through window.postMessage (see comment in connect.js). + // postMessage happens in send-token.js, which is a different request, so we need to put the allowed origins + // on the encrypted session state to access it later there. + if (credentials.origins) { + const decodedState = oAuthState.decodeState(state, companionOptions.secret) + decodedState.customerDefinedAllowedOrigins = credentials.origins + const newState = oAuthState.encodeState(decodedState, companionOptions.secret) + // @ts-expect-error untyped + req.session.grant = { + // @ts-expect-error untyped + ...(req.session.grant || {}), + dynamic: { + // @ts-expect-error untyped + ...(req.session.grant?.dynamic || {}), + state: newState, + }, + } + } + res.locals.grant = { dynamic: { key: credentials.key, secret: credentials.secret, + origins: credentials.origins, }, } diff --git a/packages/@uppy/companion/src/server/provider/index.js b/packages/@uppy/companion/src/server/provider/index.js index 52c707dac0..32b63bace4 100644 --- a/packages/@uppy/companion/src/server/provider/index.js +++ b/packages/@uppy/companion/src/server/provider/index.js @@ -136,7 +136,7 @@ module.exports.addProviderOptions = (companionOptions, grantConfig, getOauthProv grantConfig[oauthProvider].secret = providerOptions[providerName].secret if (providerOptions[providerName].credentialsURL) { // eslint-disable-next-line no-param-reassign - grantConfig[oauthProvider].dynamic = ['key', 'secret', 'redirect_uri'] + grantConfig[oauthProvider].dynamic = ['key', 'secret', 'redirect_uri', 'origins'] } const provider = exports.getDefaultProviders()[providerName] From 82a35b1f6743073bab5bc2982194a216e97a6172 Mon Sep 17 00:00:00 2001 From: Murderlon Date: Mon, 2 Dec 2024 12:06:50 +0100 Subject: [PATCH 2/3] Fix test --- .../@uppy/companion/src/server/controllers/send-token.js | 6 +++--- packages/@uppy/companion/src/server/provider/credentials.js | 2 +- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/packages/@uppy/companion/src/server/controllers/send-token.js b/packages/@uppy/companion/src/server/controllers/send-token.js index 475185c1b2..290aaba4f0 100644 --- a/packages/@uppy/companion/src/server/controllers/send-token.js +++ b/packages/@uppy/companion/src/server/controllers/send-token.js @@ -53,7 +53,7 @@ const htmlContent = (token, origin) => { */ module.exports = function sendToken(req, res, next) { // @ts-expect-error untyped - const {companion} = req + const { companion } = req const uppyAuthToken = companion.authToken const { state } = oAuthState.getGrantDynamicFromRequest(req) @@ -62,8 +62,8 @@ module.exports = function sendToken(req, res, next) { return next() } - const decoded = oAuthState.decodeState(state, companion.options.secret) - const { origin: clientOrigin, customerDefinedAllowedOrigins } = decoded + const clientOrigin = oAuthState.getFromState(state, 'origin', companion.options.secret) + const customerDefinedAllowedOrigins = oAuthState.getFromState(state, 'customerDefinedAllowedOrigins', companion.options.secret) if ( customerDefinedAllowedOrigins && diff --git a/packages/@uppy/companion/src/server/provider/credentials.js b/packages/@uppy/companion/src/server/provider/credentials.js index 7d0aaf847f..277ea5e379 100644 --- a/packages/@uppy/companion/src/server/provider/credentials.js +++ b/packages/@uppy/companion/src/server/provider/credentials.js @@ -113,7 +113,7 @@ exports.getCredentialsOverrideMiddleware = (providers, companionOptions) => { // token through window.postMessage (see comment in connect.js). // postMessage happens in send-token.js, which is a different request, so we need to put the allowed origins // on the encrypted session state to access it later there. - if (credentials.origins) { + if (Array.isArray(credentials.origins) && credentials.origins.length > 0) { const decodedState = oAuthState.decodeState(state, companionOptions.secret) decodedState.customerDefinedAllowedOrigins = credentials.origins const newState = oAuthState.encodeState(decodedState, companionOptions.secret) From 86908936f7e86503acbe3b36e9dd28e1a70490e9 Mon Sep 17 00:00:00 2001 From: Merlijn Vos Date: Mon, 16 Dec 2024 14:18:16 +0100 Subject: [PATCH 3/3] Simplify Co-authored-by: Mikael Finstad --- packages/@uppy/companion/src/server/provider/credentials.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/@uppy/companion/src/server/provider/credentials.js b/packages/@uppy/companion/src/server/provider/credentials.js index 277ea5e379..d962849dd8 100644 --- a/packages/@uppy/companion/src/server/provider/credentials.js +++ b/packages/@uppy/companion/src/server/provider/credentials.js @@ -120,10 +120,10 @@ exports.getCredentialsOverrideMiddleware = (providers, companionOptions) => { // @ts-expect-error untyped req.session.grant = { // @ts-expect-error untyped - ...(req.session.grant || {}), + ...req.session.grant, dynamic: { // @ts-expect-error untyped - ...(req.session.grant?.dynamic || {}), + ...req.session.grant?.dynamic, state: newState, }, }