From ad4050b51eb714f753931476df916ef791cb15ef Mon Sep 17 00:00:00 2001 From: Mikael Finstad Date: Sat, 9 May 2026 15:03:04 +0800 Subject: [PATCH] Send token using websocket (#6248) instead of window.opener - it is more robust inspired by #4110 closes #6246 closes #4107 Biggest caveat: In cases where window.opener is null (Dropbox), the auth window will not be auto closed (I don't know how to fix that). It will show the user a message "Authentication successful. You may now close this page." https://github.com/transloadit/uppy/pull/6248/changes#diff-ef5b69c4ab5b83168eaba6f57047bb07c53e3a426f375b42fcb32d79c746872cR22 --------- Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> Co-authored-by: Prakash --- .changeset/mighty-deers-sin.md | 8 + examples/react/test/index.test.tsx | 1 - examples/sveltekit/test/index.test.ts | 1 - examples/vue/test/index.test.ts | 1 - .../@uppy/companion-client/src/Provider.ts | 137 +++++------ .../src/getAllowedHosts.test.ts | 19 +- .../companion-client/src/getAllowedHosts.ts | 12 - .../src/server/controllers/callback.ts | 43 ++-- .../src/server/controllers/connect.ts | 5 + .../src/server/controllers/send-token.ts | 58 ++--- .../companion/src/server/helpers/html.ts | 92 ++++++++ .../src/server/helpers/oauth-state.ts | 1 + packages/@uppy/companion/src/server/socket.ts | 212 ++++++++++++------ .../@uppy/companion/src/standalone/index.ts | 4 +- .../companion/src/standalone/start-server.ts | 4 +- .../@uppy/companion/test/callback.test.ts | 60 ++++- packages/@uppy/companion/test/mockserver.ts | 15 +- 17 files changed, 440 insertions(+), 233 deletions(-) create mode 100644 .changeset/mighty-deers-sin.md create mode 100644 packages/@uppy/companion/src/server/helpers/html.ts diff --git a/.changeset/mighty-deers-sin.md b/.changeset/mighty-deers-sin.md new file mode 100644 index 000000000..71159166a --- /dev/null +++ b/.changeset/mighty-deers-sin.md @@ -0,0 +1,8 @@ +--- +"@uppy/companion-client": major +"@uppy/companion": major +--- + +Send token using websocket instead of window.opener. +Breaking in `@uppy/companion-client` because it needs newest version of Companion in order to work. +Breaking in `@uppy/companion` because `companion.socket()` now requires `companionOptions` to be passed as the second argument. diff --git a/examples/react/test/index.test.tsx b/examples/react/test/index.test.tsx index 1c69dc57e..886023456 100644 --- a/examples/react/test/index.test.tsx +++ b/examples/react/test/index.test.tsx @@ -129,6 +129,5 @@ describe('RemoteSource Component', () => { await expect.element(loginButton).toBeInTheDocument() await loginButton.click() - await expect.element(loginButton).toBeInTheDocument() }) }) diff --git a/examples/sveltekit/test/index.test.ts b/examples/sveltekit/test/index.test.ts index 2c11f4fb7..a14fb3537 100644 --- a/examples/sveltekit/test/index.test.ts +++ b/examples/sveltekit/test/index.test.ts @@ -132,7 +132,6 @@ describe('RemoteSource Component', () => { await expect.element(loginButton).toBeInTheDocument() await loginButton.click() - await expect.element(loginButton).toBeInTheDocument() }) }) diff --git a/examples/vue/test/index.test.ts b/examples/vue/test/index.test.ts index 8fd0be53b..1aa54275d 100644 --- a/examples/vue/test/index.test.ts +++ b/examples/vue/test/index.test.ts @@ -131,6 +131,5 @@ describe('RemoteSource Component', () => { await expect.element(loginButton).toBeInTheDocument() await loginButton.click() - await expect.element(loginButton).toBeInTheDocument() }) }) diff --git a/packages/@uppy/companion-client/src/Provider.ts b/packages/@uppy/companion-client/src/Provider.ts index 4e0ef1a3b..7a50897e4 100644 --- a/packages/@uppy/companion-client/src/Provider.ts +++ b/packages/@uppy/companion-client/src/Provider.ts @@ -5,8 +5,11 @@ import type { UnknownProviderPlugin, Uppy, } from '@uppy/core' -import type { CompanionClientProvider, RequestOptions } from '@uppy/utils' -import { isOriginAllowed } from './getAllowedHosts.js' +import { + type CompanionClientProvider, + getSocketHost, + type RequestOptions, +} from '@uppy/utils' import type { CompanionPluginOptions } from './index.js' import RequestClient, { authErrorStatusCode } from './RequestClient.js' @@ -135,22 +138,27 @@ export default class Provider authUrl({ authFormData, query, + authCallbackToken, }: { authFormData: unknown query: Record + authCallbackToken?: string | undefined }): string { - const params = new URLSearchParams({ + const searchParams = new URLSearchParams({ ...query, - // This is only used for Companion instances configured to accept multiple origins. + // `origin` is only used for Companion instances configured to accept multiple origins. state: btoa(JSON.stringify({ origin: getOrigin() })), ...this.authQuery({ authFormData }), }) if (this.preAuthToken) { - params.set('uppyPreAuthToken', this.preAuthToken) + searchParams.set('uppyPreAuthToken', this.preAuthToken) + } + if (authCallbackToken) { + searchParams.set('authCallbackToken', authCallbackToken) } - return `${this.hostname}/${this.id}/connect?${params}` + return `${this.hostname}/${this.id}/connect?${searchParams}` } protected async loginSimpleAuth({ @@ -181,67 +189,62 @@ export default class Provider signal: AbortSignal }): Promise { await this.ensurePreAuth() - signal.throwIfAborted() - const link = this.authUrl({ query: { uppyVersions }, authFormData }) + // Important: We need to do this synchronously, or else browsers might block the popup. + // (We cannot wait until the websocket connection is established). + // This opens up a race condtition if the websocket is not connected before the user + // completes(or cancels) authentication, but that’s a small compromose we gotta make. + const authCallbackToken = crypto.randomUUID() + + const link = this.authUrl({ + query: { uppyVersions }, + authFormData, + authCallbackToken, + }) const authWindow = window.open(link, '_blank') let interval: number | undefined - let handleMessage: ((e: MessageEvent) => void) | undefined + let webSocket: WebSocket | undefined try { - return await new Promise((resolve, reject) => { - handleMessage = (e: MessageEvent) => { - if (e.source !== authWindow) { - let jsonData = '' - try { - // TODO improve our uppy logger so that it can take an arbitrary number of arguments, - // each either objects, errors or strings, - // then we don’t have to manually do these things like json stringify when logging. - // the logger should never throw an error. - jsonData = JSON.stringify(e.data) - } catch (_err) { - // in case JSON.stringify fails (ignored) + const host = getSocketHost(this.opts.companionUrl) + + // Note that this promise is not guaranteed to settle in all cases + const token = await new Promise((resolve, reject) => { + webSocket = new WebSocket( + `${host}/api2/auth-callback/token/${authCallbackToken}`, + ) + + webSocket.addEventListener('close', () => { + reject(new Error('Socket closed')) + }) + + webSocket.addEventListener('error', (error) => { + this.uppy.log( + `Companion socket error ${JSON.stringify(error)}, closing socket`, + 'warning', + ) + webSocket?.close() // 'close' event will be emitted + }) + + webSocket.addEventListener('message', (e) => { + try { + const { token, error } = JSON.parse(e.data) + if (error) { + reject(new Error('Authentication reported error')) + } else if (!token) { + reject(new Error('Authentication did not return a token')) + } else { + resolve(token) } - this.uppy.log( - `ignoring event from unknown source ${jsonData}`, - 'warning', - ) - return + } catch (err) { + reject(err) } + }) - const { companionAllowedHosts } = this.#getPlugin().opts - if (!isOriginAllowed(e.origin, companionAllowedHosts)) { - this.uppy.log( - `ignoring event from ${e.origin} vs allowed pattern ${companionAllowedHosts}`, - 'warning', - ) - // We cannot reject here because the page might send events from other origins - // before sending the "real" auth completed event. - // for example Box has a "Pendo" tool that sends events to the opener - // https://github.com/transloadit/uppy/pull/5719 - return - } - - // Check if it's a string before doing the JSON.parse to maintain support - // for older Companion versions that used object references - const data = typeof e.data === 'string' ? JSON.parse(e.data) : e.data - - if (data.error) { - const { uppy } = this - const message = uppy.i18n('authAborted') - uppy.info({ message }, 'warning', 5000) - reject(new Error('auth aborted')) - return - } - - if (!data.token) { - reject(new Error('did not receive token from auth window')) - return - } - - resolve(this.setAuthToken(data.token)) - } + signal.addEventListener('abort', () => + reject(new Error('Authentication was aborted')), + ) // poll for user closure of the window, so we can reject when it happens if (authWindow) { @@ -251,15 +254,21 @@ export default class Provider } }, 500) } - - signal.addEventListener('abort', () => reject(new Error('Aborted'))) - window.addEventListener('message', handleMessage) }) + + this.setAuthToken(token) + } catch (err) { + const message = this.uppy.i18n('authAborted') + this.uppy.info({ message }, 'warning', 5000) + this.uppy.log(`Authentication failed: ${err.message}`, 'warning') + throw err } finally { // cleanup: - authWindow?.close() - window.clearInterval(interval) - if (handleMessage) window.removeEventListener('message', handleMessage) + // if we don't setTimeout, the window doesn't really close (I don't know why). + setTimeout(() => authWindow?.close(), 1) + this.uppy.log(`Closing auth callback socket ${authCallbackToken}`) + webSocket?.close() + clearInterval(interval) } } diff --git a/packages/@uppy/companion-client/src/getAllowedHosts.test.ts b/packages/@uppy/companion-client/src/getAllowedHosts.test.ts index d82c31583..45eef785e 100644 --- a/packages/@uppy/companion-client/src/getAllowedHosts.test.ts +++ b/packages/@uppy/companion-client/src/getAllowedHosts.test.ts @@ -1,5 +1,5 @@ import { describe, expect, it } from 'vitest' -import getAllowedHosts, { isOriginAllowed } from './getAllowedHosts.js' +import getAllowedHosts from './getAllowedHosts.js' describe('getAllowedHosts', () => { it('can convert companionAllowedHosts', () => { @@ -30,20 +30,3 @@ describe('getAllowedHosts', () => { ) }) }) - -describe('isOriginAllowed', () => { - it('should check origin', () => { - expect(isOriginAllowed('a', [/^.+$/])).toBeTruthy() - expect(isOriginAllowed('a', ['^.+$'])).toBeTruthy() - expect( - isOriginAllowed('www.transloadit.com', ['^www\\.transloadit\\.com$']), - ).toBeTruthy() - expect( - isOriginAllowed('www.transloadit.com', ['^transloadit\\.com$']), - ).toBeFalsy() - expect(isOriginAllowed('match', ['fail', 'match'])).toBeTruthy() - expect( - isOriginAllowed('www.transloadit.com', ['\\.transloadit\\.com$']), - ).toBeTruthy() - }) -}) diff --git a/packages/@uppy/companion-client/src/getAllowedHosts.ts b/packages/@uppy/companion-client/src/getAllowedHosts.ts index 5487dca03..ff58300d8 100644 --- a/packages/@uppy/companion-client/src/getAllowedHosts.ts +++ b/packages/@uppy/companion-client/src/getAllowedHosts.ts @@ -47,15 +47,3 @@ export default function getAllowedHosts( ret = escapeRegex(ret) return ret } - -export function isOriginAllowed( - origin: string, - allowedOrigin: string | RegExp | Array | undefined, -): boolean { - const patterns = Array.isArray(allowedOrigin) - ? allowedOrigin.map(wrapInRegex) - : [wrapInRegex(allowedOrigin)] - return patterns.some( - (pattern) => pattern?.test(origin) || pattern?.test(`${origin}/`), - ) // allowing for trailing '/' -} diff --git a/packages/@uppy/companion/src/server/controllers/callback.ts b/packages/@uppy/companion/src/server/controllers/callback.ts index 93f864d8f..7e1ab3a69 100644 --- a/packages/@uppy/companion/src/server/controllers/callback.ts +++ b/packages/@uppy/companion/src/server/controllers/callback.ts @@ -3,32 +3,23 @@ */ import type { NextFunction, Request, Response } from 'express' -import serialize from 'serialize-javascript' +import emitter from '../emitter/index.js' +import { + authCallbackErrorHtml, + legacyAuthCallbackHtml, +} from '../helpers/html.js' import * as tokenService from '../helpers/jwt.js' import * as oAuthState from '../helpers/oauth-state.js' import logger from '../logger.js' -const closePageHtml = (origin: string | undefined) => ` - - - - - - - Authentication failed. - ` - export default function callback( req: Request, res: Response, next: NextFunction, ): void { const providerName = req.params['providerName'] + const { companion } = req + if (providerName == null || providerName.length === 0) { res.sendStatus(400) return @@ -51,7 +42,25 @@ export default function callback( req.id, ) logger.debug(req.session?.grant?.response, 'callback.oauth.resp', req.id) - res.status(400).send(closePageHtml(originString)) + const authCallbackToken = + grantDynamic.state && + oAuthState.getFromState( + grantDynamic.state, + 'authCallbackToken', + companion.options.secret, + ) + // only new Uppy clients will set an authCallbackToken in the state + // in that case, we send the token through the emitter. + if (authCallbackToken) { + emitter().emit(authCallbackToken, { error: true }) + res.status(400).send(authCallbackErrorHtml()) + } else { + // This is backwards compatible with old Uppy clients: + res + .status(400) + .send(legacyAuthCallbackHtml({ error: true }, originString)) + } + return } diff --git a/packages/@uppy/companion/src/server/controllers/connect.ts b/packages/@uppy/companion/src/server/controllers/connect.ts index 2d62c4d95..a30e607da 100644 --- a/packages/@uppy/companion/src/server/controllers/connect.ts +++ b/packages/@uppy/companion/src/server/controllers/connect.ts @@ -123,6 +123,11 @@ export default function connect( stateObj.preAuthToken = preAuthTokenValue } + const authCallbackToken = req.query['authCallbackToken'] + if (typeof authCallbackToken === 'string') { + stateObj.authCallbackToken = authCallbackToken + } + // Get the computed header generated by `cors` in a previous middleware. stateObj.origin = res.getHeader('Access-Control-Allow-Origin') let clientOrigin: string | undefined diff --git a/packages/@uppy/companion/src/server/controllers/send-token.ts b/packages/@uppy/companion/src/server/controllers/send-token.ts index b0396fa28..44efb0679 100644 --- a/packages/@uppy/companion/src/server/controllers/send-token.ts +++ b/packages/@uppy/companion/src/server/controllers/send-token.ts @@ -1,45 +1,12 @@ import type { NextFunction, Request, Response } from 'express' -import serialize from 'serialize-javascript' +import emitter from '../emitter/index.js' +import { + authCallbackSuccessHtml, + legacyAuthCallbackHtml, +} from '../helpers/html.js' import * as oAuthState from '../helpers/oauth-state.js' import { isOriginAllowed } from './connect.js' -const htmlContent = (token: string, origin: string): string => { - return ` - - - - - - - - - - ` -} - export default function sendToken( req: Request, res: Response, @@ -74,5 +41,18 @@ export default function sendToken( return } - res.send(htmlContent(`${uppyAuthToken}`, clientOrigin)) + const authCallbackToken = oAuthState.getFromState( + state, + 'authCallbackToken', + companion.options.secret, + ) + // only new Uppy clients will set an authCallbackToken in the state + // in that case, we send the token through the emitter. + if (authCallbackToken) { + emitter().emit(authCallbackToken, { token: uppyAuthToken }) + res.send(authCallbackSuccessHtml()) + } else { + // This is backwards compatible with old Uppy clients: + res.send(legacyAuthCallbackHtml({ token: uppyAuthToken }, clientOrigin)) + } } diff --git a/packages/@uppy/companion/src/server/helpers/html.ts b/packages/@uppy/companion/src/server/helpers/html.ts new file mode 100644 index 000000000..a7e723db5 --- /dev/null +++ b/packages/@uppy/companion/src/server/helpers/html.ts @@ -0,0 +1,92 @@ +import serialize from 'serialize-javascript' + +export const authCallbackSuccessHtml = () => { + return ` + + + + + + + +
Authentication successful. You may now close this page.
+ + ` +} + +export const authCallbackErrorHtml = () => { + return ` + + + + + + + +
Authentication failed. You may now close this page.
+ + ` +} + +/** + * Generate an HTML page that will post a message to the opener window and then close itself. + * This is only used by old Uppy clients + * + * @param {unknown} data data payload + * @param {string} origin url string + */ +export const legacyAuthCallbackHtml = ( + data: unknown, + origin: string | undefined, +) => { + return ` + + + + + + + + + + ` +} diff --git a/packages/@uppy/companion/src/server/helpers/oauth-state.ts b/packages/@uppy/companion/src/server/helpers/oauth-state.ts index a1080ce79..2348e3ef8 100644 --- a/packages/@uppy/companion/src/server/helpers/oauth-state.ts +++ b/packages/@uppy/companion/src/server/helpers/oauth-state.ts @@ -9,6 +9,7 @@ export type OAuthState = { preAuthToken?: string companionInstance?: string customerDefinedAllowedOrigins?: string[] + authCallbackToken?: string } export const encodeState = ( diff --git a/packages/@uppy/companion/src/server/socket.ts b/packages/@uppy/companion/src/server/socket.ts index 71dc7581a..54d39ec53 100644 --- a/packages/@uppy/companion/src/server/socket.ts +++ b/packages/@uppy/companion/src/server/socket.ts @@ -1,10 +1,10 @@ -import assert from 'node:assert' import type { Server as HttpServer } from 'node:http' import type { Server as HttpsServer } from 'node:https' -import { WebSocketServer } from 'ws' +import { type WebSocket, WebSocketServer } from 'ws' +import type { CompanionInitOptions } from '../schemas/companion.js' import emitter from './emitter/index.js' import { isRecord } from './helpers/type-guards.js' -import { jsonStringify } from './helpers/utils.js' +import { getURLBuilder, jsonStringify } from './helpers/utils.js' import * as logger from './logger.js' import * as redis from './redis.js' import Uploader from './Uploader.js' @@ -19,57 +19,134 @@ function isSocketMessage(value: unknown): value is SocketMessage { ) } -/** - * The socket is used to send progress events during an upload. - */ -export default function setupSocket(server: HttpServer | HttpsServer): void { - const wss = new WebSocketServer({ server }) - const redisClient = redis.client() - +function handleUploadSocketConnection({ + token, + ws, +}: { + token: string + ws: WebSocket +}) { // A new connection is usually created when an upload begins, // or when connection fails while an upload is on-going and, // client attempts to reconnect. - wss.on('connection', (ws, req) => { - const fullPath = req.url - assert(fullPath != null, 'WebSocket connection URL is missing') - // the token identifies which ongoing upload's progress, the socket - // connection wishes to listen to. - const token = fullPath.replace(/^.*\/api\//, '') - logger.info(`connection received from ${token}`, 'socket.connect') - function send(data: SocketMessage): void { - ws.send(jsonStringify(data), (err) => { - if (err) { - logger.error(err, 'socket.redis.error', Uploader.shortenToken(token)) + // the token identifies which ongoing upload's progress, the socket + // connection wishes to listen to. + const redisClient = redis.client() + + function send(data: { action: string; payload: object }) { + ws.send(jsonStringify(data), (err) => { + if (err) + logger.error( + err, + 'socket.upload.redis.error', + Uploader.shortenToken(token), + ) + }) + } + + // if the redisClient is available, then we attempt to check the storage + // if we have any already stored state on the upload. + if (redisClient) { + redisClient + .get(`${Uploader.STORAGE_PREFIX}:${token}`) + .then((data) => { + if (data) { + const dataObj = JSON.parse(data.toString()) + if (isSocketMessage(dataObj)) send(dataObj) } }) - } + .catch((err) => + logger.error( + err, + 'socket.upload.redis.error', + Uploader.shortenToken(token), + ), + ) + } - // if the redisClient is available, then we attempt to check the storage - // if we have any already stored state on the upload, and send it to the client immediately after connection, - // so that the client can update the UI accordingly without the user having to wait for another event - if (redisClient) { - redisClient - .get(`${Uploader.STORAGE_PREFIX}:${token}`) - .then((data) => { - if (!data) return - const dataObj: unknown = JSON.parse(data) - if (isSocketMessage(dataObj)) { - send(dataObj) - } - }) - .catch((err) => - logger.error(err, 'socket.redis.error', Uploader.shortenToken(token)), + emitter().emit(`connection:${token}`) + const onRedisMessage = (...args: unknown[]) => { + const data = args[0] + if (!isSocketMessage(data)) return + send(data) + } + emitter().on(token, onRedisMessage) + + ws.on('message', (jsonData) => { + try { + const data: unknown = JSON.parse(jsonData.toString()) + // whitelist triggered actions + const action = isRecord(data) ? data['action'] : undefined + if (action === 'pause' || action === 'resume' || action === 'cancel') { + emitter().emit(`${action}:${token}`) + } + } catch (err) { + logger.error(err, 'websocket.error', Uploader.shortenToken(token)) + } + }) + + ws.on('close', () => { + emitter().removeListener(token, onRedisMessage) + }) +} + +function handleAuthCallbackSocketConnection({ + token, + ws, +}: { + token: string + ws: WebSocket +}) { + function send(data: unknown) { + ws.send(jsonStringify(data), (err) => { + if (err) + logger.error( + err, + 'socket.auth.redis.error', + Uploader.shortenToken(token), ) + }) + } + + // todo we should use a unique prefix for these and upload tokens, so that we can easily distinguish them in the emitter and avoid any potential conflicts. + // but it's a breaking change so let's not do it now + // it's unlikely there will be any collision + emitter().on(token, send) + + ws.on('close', () => { + emitter().removeListener(token, send) + }) +} + +export default function setupSockets( + server: HttpServer | HttpsServer, + companionOptions: CompanionInitOptions, +) { + const wss = new WebSocketServer({ server }) + + const urlBuilder = getURLBuilder(companionOptions) + + const externalBasePath = urlBuilder('', true, true) + + wss.on('connection', (ws, req) => { + // basic router: + let path = req.url + + // strip off base path if any + if (path != null && externalBasePath && path.startsWith(externalBasePath)) { + path = path.slice(externalBasePath.length) } - emitter().emit(`connection:${token}`) - const onTokenMessage = (...args: unknown[]) => { - const data = args[0] - if (!isSocketMessage(data)) return - send(data) - } - emitter().on(token, onTokenMessage) + // authentication callback token? + const authCallbackTokenMatch = path?.match( + /^\/api2\/auth-callback\/token\/([^/]+)/, + ) + const authCallbackToken = authCallbackTokenMatch?.[1] + + // or token that identifies which ongoing upload's progress, the socket connection wishes to listen to. + const uploadTokenMatch = path?.match(/^\/api\/([^/]+)/) + const uploadToken = uploadTokenMatch?.[1] ws.on('error', (err) => { // https://github.com/websockets/ws/issues/1543 @@ -81,33 +158,38 @@ export default function setupSocket(server: HttpServer | HttpsServer): void { ) { logger.error( 'WebSocket message too large', - 'websocket.error', - Uploader.shortenToken(token), + 'socket.upload.error', + uploadToken && Uploader.shortenToken(uploadToken), ) } else { - logger.error(err, 'websocket.error', Uploader.shortenToken(token)) - } - }) - - ws.on('message', (jsonData) => { - try { - const data: unknown = JSON.parse(jsonData.toString()) - // whitelist triggered actions - const action = isRecord(data) ? data['action'] : undefined - if (action === 'pause' || action === 'resume' || action === 'cancel') { - emitter().emit(`${action}:${token}`) - } - } catch (err) { logger.error( - err instanceof Error ? err : String(err), - 'websocket.error', - Uploader.shortenToken(token), + err, + 'socket.error', + uploadToken && Uploader.shortenToken(uploadToken), ) } }) - ws.on('close', () => { - emitter().removeListener(token, onTokenMessage) - }) + if (uploadToken) { + logger.info( + `Upload token connection received from ${uploadToken}`, + 'socket.upload.connect', + ) + + handleUploadSocketConnection({ token: uploadToken, ws }) + return + } + + if (authCallbackToken) { + logger.info( + `Auth callback token connection received from ${authCallbackToken}`, + 'socket.auth.callback.connect', + ) + + handleAuthCallbackSocketConnection({ token: authCallbackToken, ws }) + return + } + + ws.close() }) } diff --git a/packages/@uppy/companion/src/standalone/index.ts b/packages/@uppy/companion/src/standalone/index.ts index 221573841..488fb90a8 100644 --- a/packages/@uppy/companion/src/standalone/index.ts +++ b/packages/@uppy/companion/src/standalone/index.ts @@ -158,7 +158,7 @@ export default function server(inputCompanionOptions?: CompanionInitOptions) { } // initialize companion - const { app: companionApp } = companion.app(companionOptions) + const { app: companionApp, emitter } = companion.app(companionOptions) // add companion to server middleware router.use(companionApp) @@ -216,5 +216,5 @@ export default function server(inputCompanionOptions?: CompanionInitOptions) { } }) - return { app, companionOptions } + return { app, companionOptions, emitter } } diff --git a/packages/@uppy/companion/src/standalone/start-server.ts b/packages/@uppy/companion/src/standalone/start-server.ts index d585ba4c3..ec4dc713b 100644 --- a/packages/@uppy/companion/src/standalone/start-server.ts +++ b/packages/@uppy/companion/src/standalone/start-server.ts @@ -7,9 +7,9 @@ import standalone from './index.js' const port: string | number = process.env['COMPANION_PORT'] || process.env['PORT'] || 3020 -const { app } = standalone() +const { app, companionOptions } = standalone() -companion.socket(app.listen(port)) +companion.socket(app.listen(port), companionOptions) logger.info(`Welcome to Companion! v${packageJson.version}`) logger.info(`Listening on http://localhost:${port}`) diff --git a/packages/@uppy/companion/test/callback.test.ts b/packages/@uppy/companion/test/callback.test.ts index 72750ced2..3a19e0093 100644 --- a/packages/@uppy/companion/test/callback.test.ts +++ b/packages/@uppy/companion/test/callback.test.ts @@ -1,16 +1,19 @@ import request from 'supertest' -import { describe, expect, test, vi } from 'vitest' +import { beforeEach, describe, expect, test, vi } from 'vitest' import * as tokenService from '../src/server/helpers/jwt.js' -import mockOauthState from './mockoauthstate.js' -import { getServer, grantToken } from './mockserver.js' - -vi.mock('express-prom-bundle') -mockOauthState() const secret = 'secret' +beforeEach(() => { + vi.mock('express-prom-bundle') + + vi.resetModules() + vi.clearAllMocks() +}) + describe('test authentication callback', () => { test('authentication callback redirects to send-token url', async () => { + const { getServer } = await import('./mockserver.js') return request(await getServer()) .get('/drive/callback') .expect(302) @@ -22,6 +25,7 @@ describe('test authentication callback', () => { }) test('authentication callback sets cookie', async () => { + const { getServer, grantToken } = await import('./mockserver.js') return request(await getServer()) .get('/dropbox/callback') .expect(302) @@ -45,13 +49,55 @@ describe('test authentication callback', () => { }) }) - test('the token gets sent via html', async () => { + test('the token gets sent via websocket', async () => { + const callbackToken = 'auth-callback-token' + + const oauthState = await import('../src/server/helpers/oauth-state.js') + vi.spyOn(oauthState, 'getFromState').mockImplementation((state, key) => { + if (key === 'authCallbackToken') return callbackToken + + return 'http://localhost:3020' + }) + const authData = { dropbox: { accessToken: 'token value' }, drive: { accessToken: 'token value' }, } const token = tokenService.generateEncryptedAuthToken(authData, secret) + const onEmitted = vi.fn() + + const { getServerWithEmitter } = await import('./mockserver.js') + const { server, emitter } = await getServerWithEmitter() + emitter.on(callbackToken, onEmitted) + + await request(server) + .get(`/dropbox/send-token?uppyAuthToken=${encodeURIComponent(token)}`) + .expect(200) + .expect((res) => { + expect(res.text).toMatch('Authentication successful') + }) + + expect(onEmitted).toHaveBeenLastCalledWith({ + token, + }) + }) + + test('the token gets sent via legacy html mechanism', async () => { + const oauthState = await import('../src/server/helpers/oauth-state.js') + vi.spyOn(oauthState, 'getFromState').mockImplementation((state, key) => { + if (key === 'authCallbackToken') return undefined + + return 'http://localhost:3020' + }) + + const authData = { + dropbox: { accessToken: 'token value' }, + drive: { accessToken: 'token value' }, + } + const token = tokenService.generateEncryptedAuthToken(authData, secret) + + const { getServer } = await import('./mockserver.js') // see mock ../../src/server/helpers/oauth-state above for state values return request(await getServer()) .get(`/dropbox/send-token?uppyAuthToken=${encodeURIComponent(token)}`) diff --git a/packages/@uppy/companion/test/mockserver.ts b/packages/@uppy/companion/test/mockserver.ts index 0f2ed32ee..5fb178467 100644 --- a/packages/@uppy/companion/test/mockserver.ts +++ b/packages/@uppy/companion/test/mockserver.ts @@ -89,9 +89,9 @@ export const grantToken = 'fake token' // companion stores certain global state, so the user needs to reset modules for each test // todo rewrite companion to not use global state // https://github.com/transloadit/uppy/issues/3284 -export const getServer = async ( +export const getServerWithEmitter = async ( extraEnv: Record = {}, -): Promise> => { +) => { const { default: standalone } = await import('../src/standalone/index.js') const env = { @@ -125,7 +125,14 @@ export const getServer = async ( next() }) - const { app } = standalone() + const { app, emitter } = standalone() authServer.use(app) - return authServer + return { server: authServer, emitter } +} + +export const getServer = async ( + extraEnv?: Record | undefined, +) => { + const { server } = await getServerWithEmitter(extraEnv) + return server }