diff --git a/.github/CONTRIBUTING.md b/.github/CONTRIBUTING.md index 362edbf88b..4f23743a16 100644 --- a/.github/CONTRIBUTING.md +++ b/.github/CONTRIBUTING.md @@ -115,7 +115,7 @@ Authenticates and Uploads from Dropbox through Companion: ### Unit tests -Unit tests are using Jest and can be run with: +Unit tests are using Vitest and can be run with: ```bash yarn test:unit diff --git a/packages/@uppy/companion-client/src/Provider.ts b/packages/@uppy/companion-client/src/Provider.ts index 2f6bdc066c..2aff9879d2 100644 --- a/packages/@uppy/companion-client/src/Provider.ts +++ b/packages/@uppy/companion-client/src/Provider.ts @@ -8,6 +8,7 @@ import type { import type { UnknownProviderPlugin } from '@uppy/core/lib/Uppy.js' import RequestClient, { authErrorStatusCode } from './RequestClient.ts' import type { CompanionPluginOptions } from './index.ts' +import { isOriginAllowed } from './getAllowedHosts.ts' export interface Opts extends PluginOpts, CompanionPluginOptions { pluginId: string @@ -28,29 +29,6 @@ function getOrigin() { return location.origin } -function getRegex(value?: string | RegExp) { - if (typeof value === 'string') { - return new RegExp(`^${value}$`) - } - if (value instanceof RegExp) { - return value - } - return undefined -} - -function isOriginAllowed( - origin: string, - allowedOrigin: string | RegExp | Array | undefined, -) { - const patterns = - Array.isArray(allowedOrigin) ? - allowedOrigin.map(getRegex) - : [getRegex(allowedOrigin)] - return patterns.some( - (pattern) => pattern?.test(origin) || pattern?.test(`${origin}/`), - ) // allowing for trailing '/' -} - export default class Provider extends RequestClient implements CompanionClientProvider diff --git a/packages/@uppy/companion-client/src/getAllowedHosts.test.ts b/packages/@uppy/companion-client/src/getAllowedHosts.test.ts new file mode 100644 index 0000000000..92dd634524 --- /dev/null +++ b/packages/@uppy/companion-client/src/getAllowedHosts.test.ts @@ -0,0 +1,48 @@ +import { describe, it, expect } from 'vitest' +import getAllowedHosts, { isOriginAllowed } from './getAllowedHosts.ts' + +describe('getAllowedHosts', () => { + it('can convert companionAllowedHosts', () => { + expect(getAllowedHosts('www.example.com', '')).toBe('www.example.com') + expect( + getAllowedHosts([/transloadit\.com/, 'www\\.example\\.com'], ''), + ).toEqual([/transloadit\.com/, 'www\\.example\\.com']) + expect(() => getAllowedHosts(['['], '')).toThrow( + /^Invalid regular expression/, + ) + }) + + it('can convert when companionAllowedHosts unset', () => { + expect(getAllowedHosts(undefined, 'http://server.com')).toBe( + 'http:\\/\\/server\\.com', + ) + expect(getAllowedHosts(undefined, 'https://server.com/')).toBe( + 'https:\\/\\/server\\.com', + ) + expect(getAllowedHosts(undefined, 'server.com')).toBe( + 'https:\\/\\/server\\.com', + ) + expect(getAllowedHosts(undefined, 'server.com/test')).toBe( + 'https:\\/\\/server\\.com', + ) + expect(getAllowedHosts(undefined, '//server.com:80/test')).toBe( + 'https:\\/\\/server\\.com:80', + ) + }) +}) + +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() + // todo maybe next major: + // 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 729a6adc9d..7129ea906a 100644 --- a/packages/@uppy/companion-client/src/getAllowedHosts.ts +++ b/packages/@uppy/companion-client/src/getAllowedHosts.ts @@ -1,22 +1,63 @@ +// https://stackoverflow.com/a/3561711/6519037 +function escapeRegex(string: string) { + return string.replace(/[/\-\\^$*+?.()|[\]{}]/g, '\\$&') +} + +function wrapInRegex(value?: string | RegExp): RegExp | undefined { + if (typeof value === 'string') { + // TODO in the next major we should change this to `new RegExp(value)` so that the user can control start/end characters + return new RegExp(`^${value}$`) // throws if invalid regex + } + if (value instanceof RegExp) { + return value + } + return undefined +} + export default function getAllowedHosts( - hosts: string | RegExp | Array | undefined, - url: string, + companionAllowedHosts: string | RegExp | Array | undefined, + companionUrl: string, ): string | RegExp | Array { - if (hosts) { - if ( - typeof hosts !== 'string' && - !Array.isArray(hosts) && - !(hosts instanceof RegExp) - ) { - throw new TypeError( - `The option "companionAllowedHosts" must be one of string, Array, RegExp`, - ) + if (companionAllowedHosts) { + const validate = (value: string | RegExp) => { + if ( + !(typeof value === 'string' && wrapInRegex(value)) && // wrapInRegex throws if invalid regex + !(value instanceof RegExp) + ) { + throw new TypeError( + `The option "companionAllowedHosts" must be one of string, Array, RegExp`, + ) + } + } + + if (Array.isArray(companionAllowedHosts)) { + companionAllowedHosts.every(validate) + } else { + validate(companionAllowedHosts) } - return hosts + return companionAllowedHosts } - // does not start with https:// - if (/^(?!https?:\/\/).*$/i.test(url)) { - return `https://${url.replace(/^\/\//, '')}` + + // if it does not start with https://, prefix it (and remove any leading slashes) + let ret = companionUrl + if (/^(?!https?:\/\/).*$/i.test(ret)) { + ret = `https://${companionUrl.replace(/^\/\//, '')}` } - return new URL(url).origin + ret = new URL(ret).origin + + 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 '/' }