diff --git a/docs/companion.md b/docs/companion.md index 7d5720aac8..742c25458a 100644 --- a/docs/companion.md +++ b/docs/companion.md @@ -372,14 +372,10 @@ using many instances. See [How to scale Companion](#how-to-scale-companion). #### `COMPANION_REDIS_EXPRESS_SESSION_PREFIX` Set a custom prefix for redis keys created by -[connect-redis](https://github.com/tj/connect-redis). Defaults to `sess:`. -Sessions are used for storing authentication state and for allowing thumbnails -to be loaded by the browser via Companion. You might want to change this because -if you run a redis with many different apps in the same redis server, it’s hard -to know where `sess:` comes from and it might collide with other apps. **Note:** -in the future, we plan and changing the default to `companion:` and possibly -remove this option. This is a standalone-only option. See also -`COMPANION_REDIS_PUBSUB_SCOPE`. +[connect-redis](https://github.com/tj/connect-redis). Defaults to +`companion-session:`. Sessions are used for storing authentication state and for +allowing thumbnails to be loaded by the browser via Companion and for OAuth2. +See also `COMPANION_REDIS_PUBSUB_SCOPE`. #### `redisOptions` `COMPANION_REDIS_OPTIONS` @@ -655,8 +651,8 @@ as well as #### `enableUrlEndpoint` `COMPANION_ENABLE_URL_ENDPOINT` -Set this to `false` to disable the -[URL functionalily](https://uppy.io/docs/url/). Default: `true`. +Set this to `true` to enable the [URL functionalily](https://uppy.io/docs/url/). +Default: `false`. ### Events diff --git a/docs/guides/migration-guides.md b/docs/guides/migration-guides.md index 4413da8fea..34641255b0 100644 --- a/docs/guides/migration-guides.md +++ b/docs/guides/migration-guides.md @@ -2,6 +2,49 @@ These cover all the major Uppy versions and how to migrate to them. +## Migrate from Uppy 3.x to 4.x + +### Companion + +- `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:`. +- The URL endpoint (used by the `Url`/`Link` plugin) is now turned off by + default and must be explicitly enabled with + `COMPANION_ENABLE_URL_ENDPOINT=true` or `enableUrlEndpoint: true`. +- Custom provider breaking changes. If you have not implemented a custom + provider, you should not be affected. + - The static `getExtraConfig` property has been renamed to + `getExtraGrantConfig`. + - The static `authProvider` property has been renamed to `oauthProvider`. +- Endpoint `GET /s3/params` now returns `{ method: "POST" }` instead of + `{ method: "post" }`. This will not affect most people. +- The Companion [`error` event](https://uppy.io/docs/companion/#events) now no + longer includes `extraData` inside the `payload.error` property. `extraData` + is (and was also before) included in the `payload`. +- `access-control-allow-headers` is no longer included in + `Access-Control-Expose-Headers`, and `uppy-versions` is no longer an allowed + header. We are not aware of any issues this might cause. +- Internal refactoring (probably won’t affect you) + - `getProtectedGot` parameter `blockLocalIPs` changed to `allowLocalIPs` + (inverted boolean). + - `getURLMeta` 2nd (boolean) argument inverted. + - `getProtectedHttpAgent` parameter `blockLocalIPs` changed to `allowLocalIPs` + (inverted boolean). + - `downloadURL` 2nd (boolean) argument inverted. + +### `@uppy/companion-client` + +- `supportsRefreshToken` now defaults to `false` instead of `true`. If you have + implemented a custom provider, this might affect you. +- `Socket` class is no longer in use and has been removed. Unless you used this + class you don’t need to do anything. +- Remove deprecated options `serverUrl` and `serverPattern` (they were merely + defined in Typescript, not in use). +- `RequestClient` methods `get`, `post`, `delete` no longer accepts a boolean as + the third argument. Instead, pass `{ skipPostResponse: true | false }`. This + won’t affect you unless you’ve been using `RequestClient`. + ## Migrate from Robodog to Uppy plugins Uppy is flexible and extensible through plugins. But the integration code could diff --git a/docs/sources/companion-plugins/url.mdx b/docs/sources/companion-plugins/url.mdx index 000ecebf38..9a614d5e1f 100644 --- a/docs/sources/companion-plugins/url.mdx +++ b/docs/sources/companion-plugins/url.mdx @@ -90,8 +90,8 @@ new Uppy() ### Use in Companion -Companion supports this plugin out-of-the-box so integration is required on this -side. +Companion supports this plugin out-of-the-box, however it must be enabled in +Companion with the `enableUrlEndpoint` / `COMPANION_ENABLE_URL_ENDPOINT` option. ## API diff --git a/e2e/start-companion-with-load-balancer.mjs b/e2e/start-companion-with-load-balancer.mjs index a0f2be79b0..d228df8802 100755 --- a/e2e/start-companion-with-load-balancer.mjs +++ b/e2e/start-companion-with-load-balancer.mjs @@ -65,6 +65,7 @@ const startCompanion = ({ name, port }) => { COMPANION_SECRET: 'development', // multi instance will not work without secret set COMPANION_PREAUTH_SECRET: 'development', // multi instance will not work without secret set COMPANION_ALLOW_LOCAL_URLS: 'true', + COMPANION_ENABLE_URL_ENDPOINT: 'true', COMPANION_LOGGER_PROCESS_NAME: name, }, }) diff --git a/examples/custom-provider/server/CustomProvider.cjs b/examples/custom-provider/server/CustomProvider.cjs index 9b0e79be67..6e4292b14e 100644 --- a/examples/custom-provider/server/CustomProvider.cjs +++ b/examples/custom-provider/server/CustomProvider.cjs @@ -34,7 +34,7 @@ function adaptData (res) { class MyCustomProvider { static version = 2 - static get authProvider () { + static get oauthProvider () { return 'myunsplash' } diff --git a/packages/@uppy/aws-s3/src/index.ts b/packages/@uppy/aws-s3/src/index.ts index 88f346cf11..7d1b7f5fe9 100644 --- a/packages/@uppy/aws-s3/src/index.ts +++ b/packages/@uppy/aws-s3/src/index.ts @@ -338,7 +338,7 @@ export default class AwsS3Multipart< this.type = 'uploader' this.id = this.opts.id || 'AwsS3Multipart' // TODO: only initiate `RequestClient` is `companionUrl` is defined. - this.#client = new RequestClient(uppy, opts as any) + this.#client = new RequestClient(uppy, (opts as any) ?? {}) const dynamicDefaultOptions = { createMultipartUpload: this.createMultipartUpload, diff --git a/packages/@uppy/companion-client/src/Provider.ts b/packages/@uppy/companion-client/src/Provider.ts index aad0ef42ec..d252c97d12 100644 --- a/packages/@uppy/companion-client/src/Provider.ts +++ b/packages/@uppy/companion-client/src/Provider.ts @@ -9,12 +9,7 @@ import type { UnknownProviderPlugin } from '@uppy/core/lib/Uppy' import RequestClient, { authErrorStatusCode } from './RequestClient.ts' import type { CompanionPluginOptions } from './index.js' -// TODO: remove deprecated options in next major release export interface Opts extends PluginOpts, CompanionPluginOptions { - /** @deprecated */ - serverUrl?: string - /** @deprecated */ - serverPattern?: string pluginId: string name?: string supportsRefreshToken?: boolean @@ -87,7 +82,7 @@ export default class Provider this.tokenKey = `companion-${this.pluginId}-auth-token` this.companionKeysParams = this.opts.companionKeysParams this.preAuthToken = null - this.supportsRefreshToken = opts.supportsRefreshToken ?? true // todo false in next major + this.supportsRefreshToken = !!opts.supportsRefreshToken } async headers(): Promise> { diff --git a/packages/@uppy/companion-client/src/RequestClient.ts b/packages/@uppy/companion-client/src/RequestClient.ts index 7c33428853..93326f2e0c 100644 --- a/packages/@uppy/companion-client/src/RequestClient.ts +++ b/packages/@uppy/companion-client/src/RequestClient.ts @@ -27,10 +27,6 @@ export type Opts = { companionKeysParams?: Record } -type _RequestOptions = - | boolean // TODO: remove this on the next major - | RequestOptions - // Remove the trailing slash so we can always safely append /xyz. function stripSlash(url: string) { return url.replace(/\/$/, '') @@ -98,8 +94,7 @@ export default class RequestClient { this.uppy = uppy this.opts = opts this.onReceiveResponse = this.onReceiveResponse.bind(this) - // TODO: Remove optional chaining - this.#companionHeaders = opts?.companionHeaders + this.#companionHeaders = opts.companionHeaders } setCompanionHeaders(headers: Record): void { @@ -196,33 +191,24 @@ export default class RequestClient { async get( path: string, - options?: _RequestOptions, + options?: RequestOptions, ): Promise { - // TODO: remove boolean support for options that was added for backward compatibility. - // eslint-disable-next-line no-param-reassign - if (typeof options === 'boolean') options = { skipPostResponse: options } return this.request({ ...options, path }) } async post( path: string, data: Record, - options?: _RequestOptions, + options?: RequestOptions, ): Promise { - // TODO: remove boolean support for options that was added for backward compatibility. - // eslint-disable-next-line no-param-reassign - if (typeof options === 'boolean') options = { skipPostResponse: options } return this.request({ ...options, path, method: 'POST', data }) } async delete( path: string, data?: Record, - options?: _RequestOptions, + options?: RequestOptions, ): Promise { - // TODO: remove boolean support for options that was added for backward compatibility. - // eslint-disable-next-line no-param-reassign - if (typeof options === 'boolean') options = { skipPostResponse: options } return this.request({ ...options, path, method: 'DELETE', data }) } diff --git a/packages/@uppy/companion-client/src/Socket.test.ts b/packages/@uppy/companion-client/src/Socket.test.ts deleted file mode 100644 index aa79afeb96..0000000000 --- a/packages/@uppy/companion-client/src/Socket.test.ts +++ /dev/null @@ -1,202 +0,0 @@ -import { - afterEach, - beforeEach, - vi, - describe, - it, - expect, - type Mock, -} from 'vitest' -import UppySocket from './Socket.ts' - -describe('Socket', () => { - let webSocketConstructorSpy: Mock - let webSocketCloseSpy: Mock - let webSocketSendSpy: Mock - - beforeEach(() => { - webSocketConstructorSpy = vi.fn() - webSocketCloseSpy = vi.fn() - webSocketSendSpy = vi.fn() - - // @ts-expect-error WebSocket expects a lot more to be present but we don't care for this test - globalThis.WebSocket = class WebSocket { - constructor(target: string) { - webSocketConstructorSpy(target) - } - - // eslint-disable-next-line class-methods-use-this - close(args: any) { - webSocketCloseSpy(args) - } - - // eslint-disable-next-line class-methods-use-this - send(json: any) { - webSocketSendSpy(json) - } - - triggerOpen() { - // @ts-expect-error exist - this.onopen() - } - - triggerClose() { - // @ts-expect-error exist - this.onclose() - } - } - }) - afterEach(() => { - // @ts-expect-error not allowed but needed for test - globalThis.WebSocket = undefined - }) - - it('should expose a class', () => { - expect(UppySocket.name).toEqual('UppySocket') - expect( - new UppySocket({ - target: 'foo', - }) instanceof UppySocket, - ) - }) - - it('should setup a new WebSocket', () => { - new UppySocket({ target: 'foo' }) // eslint-disable-line no-new - expect(webSocketConstructorSpy.mock.calls[0][0]).toEqual('foo') - }) - - it('should send a message via the websocket if the connection is open', () => { - const uppySocket = new UppySocket({ target: 'foo' }) - // @ts-expect-error not allowed but needed for test - const webSocketInstance = uppySocket[Symbol.for('uppy test: getSocket')]() - webSocketInstance.triggerOpen() - - uppySocket.send('bar', 'boo') - expect(webSocketSendSpy.mock.calls.length).toEqual(1) - expect(webSocketSendSpy.mock.calls[0]).toEqual([ - JSON.stringify({ action: 'bar', payload: 'boo' }), - ]) - }) - - it('should queue the message for the websocket if the connection is not open', () => { - const uppySocket = new UppySocket({ target: 'foo' }) - - uppySocket.send('bar', 'boo') - // @ts-expect-error not allowed but needed for test - expect(uppySocket[Symbol.for('uppy test: getQueued')]()).toEqual([ - { action: 'bar', payload: 'boo' }, - ]) - expect(webSocketSendSpy.mock.calls.length).toEqual(0) - }) - - it('should queue any messages for the websocket if the connection is not open, then send them when the connection is open', () => { - const uppySocket = new UppySocket({ target: 'foo' }) - // @ts-expect-error not allowed but needed for test - const webSocketInstance = uppySocket[Symbol.for('uppy test: getSocket')]() - - uppySocket.send('bar', 'boo') - uppySocket.send('moo', 'baa') - // @ts-expect-error not allowed but needed for test - expect(uppySocket[Symbol.for('uppy test: getQueued')]()).toEqual([ - { action: 'bar', payload: 'boo' }, - { action: 'moo', payload: 'baa' }, - ]) - expect(webSocketSendSpy.mock.calls.length).toEqual(0) - - webSocketInstance.triggerOpen() - - // @ts-expect-error not allowed but needed for test - expect(uppySocket[Symbol.for('uppy test: getQueued')]()).toEqual([]) - expect(webSocketSendSpy.mock.calls.length).toEqual(2) - expect(webSocketSendSpy.mock.calls[0]).toEqual([ - JSON.stringify({ action: 'bar', payload: 'boo' }), - ]) - expect(webSocketSendSpy.mock.calls[1]).toEqual([ - JSON.stringify({ action: 'moo', payload: 'baa' }), - ]) - }) - - it('should start queuing any messages when the websocket connection is closed', () => { - const uppySocket = new UppySocket({ target: 'foo' }) - // @ts-expect-error not allowed but needed for test - const webSocketInstance = uppySocket[Symbol.for('uppy test: getSocket')]() - webSocketInstance.triggerOpen() - uppySocket.send('bar', 'boo') - // @ts-expect-error not allowed but needed for test - expect(uppySocket[Symbol.for('uppy test: getQueued')]()).toEqual([]) - - webSocketInstance.triggerClose() - uppySocket.send('bar', 'boo') - // @ts-expect-error not allowed but needed for test - expect(uppySocket[Symbol.for('uppy test: getQueued')]()).toEqual([ - { action: 'bar', payload: 'boo' }, - ]) - }) - - it('should close the websocket when it is force closed', () => { - const uppySocket = new UppySocket({ target: 'foo' }) - // @ts-expect-error not allowed but needed for test - const webSocketInstance = uppySocket[Symbol.for('uppy test: getSocket')]() - webSocketInstance.triggerOpen() - - uppySocket.close() - expect(webSocketCloseSpy.mock.calls.length).toEqual(1) - }) - - it('should be able to subscribe to messages received on the websocket', () => { - const uppySocket = new UppySocket({ target: 'foo' }) - // @ts-expect-error not allowed but needed for test - const webSocketInstance = uppySocket[Symbol.for('uppy test: getSocket')]() - - const emitterListenerMock = vi.fn() - uppySocket.on('hi', emitterListenerMock) - - webSocketInstance.triggerOpen() - webSocketInstance.onmessage({ - data: JSON.stringify({ action: 'hi', payload: 'ho' }), - }) - expect(emitterListenerMock.mock.calls).toEqual([ - ['ho', undefined, undefined, undefined, undefined, undefined], - ]) - }) - - it('should be able to emit messages and subscribe to them', () => { - const uppySocket = new UppySocket({ target: 'foo' }) - - const emitterListenerMock = vi.fn() - uppySocket.on('hi', emitterListenerMock) - - uppySocket.emit('hi', 'ho') - uppySocket.emit('hi', 'ho') - uppySocket.emit('hi', 'off to work we go') - - expect(emitterListenerMock.mock.calls).toEqual([ - ['ho', undefined, undefined, undefined, undefined, undefined], - ['ho', undefined, undefined, undefined, undefined, undefined], - [ - 'off to work we go', - undefined, - undefined, - undefined, - undefined, - undefined, - ], - ]) - }) - - it('should be able to subscribe to the first event for a particular action', () => { - const uppySocket = new UppySocket({ target: 'foo' }) - - const emitterListenerMock = vi.fn() - uppySocket.once('hi', emitterListenerMock) - - uppySocket.emit('hi', 'ho') - uppySocket.emit('hi', 'ho') - uppySocket.emit('hi', 'off to work we go') - - expect(emitterListenerMock.mock.calls.length).toEqual(1) - expect(emitterListenerMock.mock.calls).toEqual([ - ['ho', undefined, undefined, undefined, undefined, undefined], - ]) - }) -}) diff --git a/packages/@uppy/companion-client/src/Socket.ts b/packages/@uppy/companion-client/src/Socket.ts deleted file mode 100644 index 72c3284407..0000000000 --- a/packages/@uppy/companion-client/src/Socket.ts +++ /dev/null @@ -1,107 +0,0 @@ -// eslint-disable-next-line @typescript-eslint/ban-ts-comment -// @ts-ignore no types -import ee from 'namespace-emitter' - -type Opts = { - autoOpen?: boolean - target: string -} - -export default class UppySocket { - #queued: Array<{ action: string; payload: unknown }> = [] - - #emitter = ee() - - #isOpen = false - - #socket: WebSocket | null - - opts: Opts - - constructor(opts: Opts) { - this.opts = opts - - if (!opts || opts.autoOpen !== false) { - this.open() - } - } - - get isOpen(): boolean { - return this.#isOpen - } - - private [Symbol.for('uppy test: getSocket')](): WebSocket | null { - return this.#socket - } - - private [Symbol.for('uppy test: getQueued')](): Array<{ - action: string - payload: unknown - }> { - return this.#queued - } - - open(): void { - if (this.#socket != null) return - - this.#socket = new WebSocket(this.opts.target) - - this.#socket.onopen = () => { - this.#isOpen = true - - while (this.#queued.length > 0 && this.#isOpen) { - const first = this.#queued.shift()! - this.send(first.action, first.payload) - } - } - - this.#socket.onclose = () => { - this.#isOpen = false - this.#socket = null - } - - this.#socket.onmessage = this.#handleMessage - } - - close(): void { - this.#socket?.close() - } - - send(action: string, payload: unknown): void { - // attach uuid - - if (!this.#isOpen) { - this.#queued.push({ action, payload }) - return - } - - this.#socket!.send( - JSON.stringify({ - action, - payload, - }), - ) - } - - on(action: string, handler: () => void): void { - this.#emitter.on(action, handler) - } - - emit(action: string, payload: unknown): void { - this.#emitter.emit(action, payload) - } - - once(action: string, handler: () => void): void { - this.#emitter.once(action, handler) - } - - #handleMessage = (e: MessageEvent) => { - try { - const message = JSON.parse(e.data) - this.emit(message.action, message.payload) - } catch (err) { - // TODO: use a more robust error handler. - console.log(err) // eslint-disable-line no-console - } - } -} diff --git a/packages/@uppy/companion-client/src/index.ts b/packages/@uppy/companion-client/src/index.ts index e10ccf0a9a..9d5e7413e1 100644 --- a/packages/@uppy/companion-client/src/index.ts +++ b/packages/@uppy/companion-client/src/index.ts @@ -11,6 +11,3 @@ export { default as getAllowedHosts } from './getAllowedHosts.ts' export * as tokenStorage from './tokenStorage.ts' export type { CompanionPluginOptions } from './CompanionPluginOptions.ts' - -// TODO: remove in the next major -export { default as Socket } from './Socket.ts' diff --git a/packages/@uppy/companion/src/companion.js b/packages/@uppy/companion/src/companion.js index 683eeb3d1d..b42c1e9a6c 100644 --- a/packages/@uppy/companion/src/companion.js +++ b/packages/@uppy/companion/src/companion.js @@ -79,9 +79,9 @@ module.exports.app = (optionsArg = {}) => { providerManager.addCustomProviders(customProviders, providers, grantConfig) } - const getAuthProvider = (providerName) => providers[providerName]?.authProvider + const getOauthProvider = (providerName) => providers[providerName]?.oauthProvider - providerManager.addProviderOptions(options, grantConfig, getAuthProvider) + providerManager.addProviderOptions(options, grantConfig, getOauthProvider) // mask provider secrets from log messages logger.setMaskables(getMaskableSecrets(options)) @@ -103,7 +103,7 @@ module.exports.app = (optionsArg = {}) => { // override provider credentials at request time // Making `POST` request to the `/connect/:provider/:override?` route requires a form body parser middleware: // See https://github.com/simov/grant#dynamic-http - app.use('/connect/:authProvider/:override?', express.urlencoded({ extended: false }), getCredentialsOverrideMiddleware(providers, options)) + app.use('/connect/:oauthProvider/:override?', express.urlencoded({ extended: false }), getCredentialsOverrideMiddleware(providers, options)) app.use(Grant(grantConfig)) app.use((req, res, next) => { @@ -153,9 +153,9 @@ module.exports.app = (optionsArg = {}) => { const { key, secret } = options.providerOptions[providerName] function getRedirectUri() { - const authProvider = getAuthProvider(providerName) - if (!isOAuthProvider(authProvider)) return undefined - return grantConfig[authProvider]?.redirect_uri + const oauthProvider = getOauthProvider(providerName) + if (!isOAuthProvider(oauthProvider)) return undefined + return grantConfig[oauthProvider]?.redirect_uri } res.send({ diff --git a/packages/@uppy/companion/src/config/companion.js b/packages/@uppy/companion/src/config/companion.js index 5c6e8224ae..919d153a93 100644 --- a/packages/@uppy/companion/src/config/companion.js +++ b/packages/@uppy/companion/src/config/companion.js @@ -16,7 +16,7 @@ const defaultOptions = { getKey: defaultGetKey, expires: 800, // seconds }, - enableUrlEndpoint: true, // todo next major make this default false + enableUrlEndpoint: false, allowLocalUrls: false, periodicPingUrls: [], streamingUpload: false, diff --git a/packages/@uppy/companion/src/server/Uploader.js b/packages/@uppy/companion/src/server/Uploader.js index 2cdc4c4265..e3cfb3adb0 100644 --- a/packages/@uppy/companion/src/server/Uploader.js +++ b/packages/@uppy/companion/src/server/Uploader.js @@ -2,11 +2,11 @@ const tus = require('tus-js-client') const { randomUUID } = require('node:crypto') const validator = require('validator') -const { pipeline: pipelineCb } = require('node:stream') +const { pipeline } = require('node:stream/promises') const { join } = require('node:path') const fs = require('node:fs') -const { promisify } = require('node:util') const throttle = require('lodash/throttle') +const { once } = require('node:events') const { Upload } = require('@aws-sdk/lib-storage') @@ -14,9 +14,6 @@ const { rfc2047EncodeMetadata, getBucket } = require('./helpers/utils') const got = require('./got') -// TODO move to `require('streams/promises').pipeline` when dropping support for Node.js 14.x. -const pipeline = promisify(pipelineCb) - const { createReadStream, createWriteStream, ReadStream } = fs const { stat, unlink } = fs.promises @@ -40,23 +37,8 @@ function exceedsMaxFileSize(maxFileSize, size) { return maxFileSize && size && size > maxFileSize } -// TODO remove once we migrate away from form-data -function sanitizeMetadata(inputMetadata) { - if (inputMetadata == null) return {} - - const outputMetadata = {} - Object.keys(inputMetadata).forEach((key) => { - outputMetadata[key] = String(inputMetadata[key]) - }) - return outputMetadata -} - class ValidationError extends Error { - constructor(message) { - super(message) - - this.name = 'ValidationError' - } + name = 'ValidationError' } /** @@ -185,7 +167,7 @@ class Uploader { this.options = options this.token = randomUUID() this.fileName = `${Uploader.FILE_NAME_PREFIX}-${this.token}` - this.options.metadata = sanitizeMetadata(this.options.metadata) + this.options.metadata = this.options.metadata || {} this.options.fieldname = this.options.fieldname || DEFAULT_FIELD_NAME this.size = options.size this.uploadFileName = this.options.metadata.name @@ -343,7 +325,7 @@ class Uploader { return } logger.error(err, 'uploader.error', this.shortToken) - this.#emitError(err) + await this.#emitError(err) } finally { emitter().removeAllListeners(`pause:${this.token}`) emitter().removeAllListeners(`resume:${this.token}`) @@ -403,33 +385,9 @@ class Uploader { async awaitReady(timeout) { logger.debug('waiting for socket connection', 'uploader.socket.wait', this.shortToken) - // TODO: replace the Promise constructor call when dropping support for Node.js <16 with - // await once(emitter, eventName, timeout && { signal: AbortSignal.timeout(timeout) }) - await new Promise((resolve, reject) => { - const eventName = `connection:${this.token}` - let timer - let onEvent - - function cleanup() { - emitter().removeListener(eventName, onEvent) - clearTimeout(timer) - } - - if (timeout) { - // Need to timeout after a while, or we could leak emitters - timer = setTimeout(() => { - cleanup() - reject(new Error('Timed out waiting for socket connection')) - }, timeout) - } - - onEvent = () => { - cleanup() - resolve() - } - - emitter().once(eventName, onEvent) - }) + const eventName = `connection:${this.token}` + // eslint-disable-next-line compat/compat + await once(emitter(), eventName, timeout && { signal: AbortSignal.timeout(timeout) }) logger.debug('socket connection received', 'uploader.socket.wait', this.shortToken) } @@ -514,14 +472,13 @@ class Uploader { */ async #emitError(err) { // delete stack to avoid sending server info to client - // todo remove also extraData from serializedErr in next major, // see PR discussion https://github.com/transloadit/uppy/pull/3832 + // @ts-ignore const { serializeError } = await import('serialize-error') const { stack, ...serializedErr } = serializeError(err) const dataToEmit = { action: 'error', - // @ts-ignore - payload: { ...err.extraData, error: serializedErr }, + payload: { error: serializedErr }, } this.saveState(dataToEmit) emitter().emit(this.token, dataToEmit) diff --git a/packages/@uppy/companion/src/server/controllers/connect.js b/packages/@uppy/companion/src/server/controllers/connect.js index 01506dc197..b5d3b9c79c 100644 --- a/packages/@uppy/companion/src/server/controllers/connect.js +++ b/packages/@uppy/companion/src/server/controllers/connect.js @@ -44,12 +44,12 @@ module.exports = function connect(req, res) { ]] }) || []) - const { authProvider } = providerClass + const { oauthProvider } = providerClass const qs = queryString({ ...grantDynamicConfig, state, }) // Now we redirect to grant's /connect endpoint, see `app.use(Grant(grantConfig))` - res.redirect(req.companion.buildURL(`/connect/${authProvider}${qs}`, true)) + res.redirect(req.companion.buildURL(`/connect/${oauthProvider}${qs}`, true)) } diff --git a/packages/@uppy/companion/src/server/controllers/logout.js b/packages/@uppy/companion/src/server/controllers/logout.js index d1fc5597d5..30953e0658 100644 --- a/packages/@uppy/companion/src/server/controllers/logout.js +++ b/packages/@uppy/companion/src/server/controllers/logout.js @@ -26,7 +26,7 @@ async function logout (req, res, next) { const { accessToken } = providerUserSession const data = await companion.provider.logout({ token: accessToken, providerUserSession, companion }) delete companion.providerUserSession - tokenService.removeFromCookies(res, companion.options, companion.providerClass.authProvider) + tokenService.removeFromCookies(res, companion.options, companion.providerClass.oauthProvider) cleanSession() res.json({ ok: true, ...data }) } catch (err) { diff --git a/packages/@uppy/companion/src/server/controllers/oauth-redirect.js b/packages/@uppy/companion/src/server/controllers/oauth-redirect.js index 4805da245d..e52e7a51f9 100644 --- a/packages/@uppy/companion/src/server/controllers/oauth-redirect.js +++ b/packages/@uppy/companion/src/server/controllers/oauth-redirect.js @@ -10,9 +10,9 @@ const oAuthState = require('../helpers/oauth-state') */ module.exports = function oauthRedirect (req, res) { const params = qs.stringify(req.query) - const { authProvider } = req.companion.providerClass + const { oauthProvider } = req.companion.providerClass if (!req.companion.options.server.oauthDomain) { - res.redirect(req.companion.buildURL(`/connect/${authProvider}/callback?${params}`, true)) + res.redirect(req.companion.buildURL(`/connect/${oauthProvider}/callback?${params}`, true)) return } @@ -25,7 +25,7 @@ module.exports = function oauthRedirect (req, res) { const handlerHostName = (new URL(handler)).host if (hasMatch(handlerHostName, req.companion.options.server.validHosts)) { - const url = `${handler}/connect/${authProvider}/callback?${params}` + const url = `${handler}/connect/${oauthProvider}/callback?${params}` res.redirect(url) return } diff --git a/packages/@uppy/companion/src/server/controllers/s3.js b/packages/@uppy/companion/src/server/controllers/s3.js index 3a101711e2..fc189d3b27 100644 --- a/packages/@uppy/companion/src/server/controllers/s3.js +++ b/packages/@uppy/companion/src/server/controllers/s3.js @@ -80,7 +80,7 @@ module.exports = function s3 (config) { Key: key, }).then(data => { res.json({ - method: 'post', // TODO: switch to the uppercase 'POST' in the next major + method: 'POST', url: data.url, fields: data.fields, expires: config.expires, diff --git a/packages/@uppy/companion/src/server/controllers/url.js b/packages/@uppy/companion/src/server/controllers/url.js index 31303a3702..d54f2d10dd 100644 --- a/packages/@uppy/companion/src/server/controllers/url.js +++ b/packages/@uppy/companion/src/server/controllers/url.js @@ -17,15 +17,13 @@ const logger = require('../logger') * to the callback chunk by chunk. * * @param {string} url - * @param {boolean} blockLocalIPs + * @param {boolean} allowLocalIPs * @param {string} traceId * @returns {Promise} */ -const downloadURL = async (url, blockLocalIPs, traceId) => { - // TODO in next major, rename all blockLocalIPs to allowLocalUrls and invert the bool, to make it consistent - // see discussion https://github.com/transloadit/uppy/pull/4554/files#r1268677162 +const downloadURL = async (url, allowLocalIPs, traceId) => { try { - const protectedGot = await getProtectedGot({ blockLocalIPs }) + const protectedGot = await getProtectedGot({ allowLocalIPs }) const stream = protectedGot.stream.get(url, { responseType: 'json' }) await prepareStream(stream) return stream @@ -50,7 +48,7 @@ const meta = async (req, res) => { return res.status(400).json({ error: 'Invalid request body' }) } - const urlMeta = await getURLMeta(req.body.url, !allowLocalUrls) + const urlMeta = await getURLMeta(req.body.url, allowLocalUrls) return res.json(urlMeta) } catch (err) { logger.error(err, 'controller.url.meta.error', req.id) @@ -75,12 +73,12 @@ const get = async (req, res) => { } async function getSize () { - const { size } = await getURLMeta(req.body.url, !allowLocalUrls) + const { size } = await getURLMeta(req.body.url, allowLocalUrls) return size } async function download () { - return downloadURL(req.body.url, !allowLocalUrls, req.id) + return downloadURL(req.body.url, allowLocalUrls, req.id) } try { diff --git a/packages/@uppy/companion/src/server/helpers/jwt.js b/packages/@uppy/companion/src/server/helpers/jwt.js index 48d2f1c50a..73b8cfe384 100644 --- a/packages/@uppy/companion/src/server/helpers/jwt.js +++ b/packages/@uppy/companion/src/server/helpers/jwt.js @@ -106,16 +106,16 @@ function getCommonCookieOptions ({ companionOptions }) { return cookieOptions } -const getCookieName = (authProvider) => `uppyAuthToken--${authProvider}` +const getCookieName = (oauthProvider) => `uppyAuthToken--${oauthProvider}` -const addToCookies = ({ res, token, companionOptions, authProvider, maxAge = MAX_AGE_24H * 1000 }) => { +const addToCookies = ({ res, token, companionOptions, oauthProvider, maxAge = MAX_AGE_24H * 1000 }) => { const cookieOptions = { ...getCommonCookieOptions({ companionOptions }), maxAge, } // send signed token to client. - res.cookie(getCookieName(authProvider), token, cookieOptions) + res.cookie(getCookieName(oauthProvider), token, cookieOptions) } module.exports.addToCookiesIfNeeded = (req, res, uppyAuthToken, maxAge) => { @@ -125,7 +125,7 @@ module.exports.addToCookiesIfNeeded = (req, res, uppyAuthToken, maxAge) => { res, token: uppyAuthToken, companionOptions: req.companion.options, - authProvider: req.companion.providerClass.authProvider, + oauthProvider: req.companion.providerClass.oauthProvider, maxAge, }) } @@ -135,12 +135,12 @@ module.exports.addToCookiesIfNeeded = (req, res, uppyAuthToken, maxAge) => { * * @param {object} res * @param {object} companionOptions - * @param {string} authProvider + * @param {string} oauthProvider */ -module.exports.removeFromCookies = (res, companionOptions, authProvider) => { +module.exports.removeFromCookies = (res, companionOptions, oauthProvider) => { // options must be identical to those given to res.cookie(), excluding expires and maxAge. // https://expressjs.com/en/api.html#res.clearCookie const cookieOptions = getCommonCookieOptions({ companionOptions }) - res.clearCookie(getCookieName(authProvider), cookieOptions) + res.clearCookie(getCookieName(oauthProvider), cookieOptions) } diff --git a/packages/@uppy/companion/src/server/helpers/request.js b/packages/@uppy/companion/src/server/helpers/request.js index 672815bfff..c1afed5f02 100644 --- a/packages/@uppy/companion/src/server/helpers/request.js +++ b/packages/@uppy/companion/src/server/helpers/request.js @@ -46,7 +46,7 @@ module.exports.validateURL = validateURL /** * Returns http Agent that will prevent requests to private IPs (to prevent SSRF) */ -const getProtectedHttpAgent = ({ protocol, blockLocalIPs }) => { +const getProtectedHttpAgent = ({ protocol, allowLocalIPs }) => { function dnsLookup (hostname, options, callback) { dns.lookup(hostname, options, (err, addresses, maybeFamily) => { if (err) { @@ -58,7 +58,7 @@ const getProtectedHttpAgent = ({ protocol, blockLocalIPs }) => { // because dns.lookup seems to be called with option `all: true`, if we are on an ipv6 system, // `addresses` could contain a list of ipv4 addresses as well as ipv6 mapped addresses (rfc6052) which we cannot allow // however we should still allow any valid ipv4 addresses, so we filter out the invalid addresses - const validAddresses = !blockLocalIPs ? toValidate : toValidate.filter(({ address }) => !isDisallowedIP(address)) + const validAddresses = allowLocalIPs ? toValidate : toValidate.filter(({ address }) => !isDisallowedIP(address)) // and check if there's anything left after we filtered: if (validAddresses.length === 0) { @@ -73,7 +73,7 @@ const getProtectedHttpAgent = ({ protocol, blockLocalIPs }) => { return class HttpAgent extends (protocol.startsWith('https') ? https : http).Agent { createConnection (options, callback) { - if (ipaddr.isValid(options.host) && blockLocalIPs && isDisallowedIP(options.host)) { + if (ipaddr.isValid(options.host) && !allowLocalIPs && isDisallowedIP(options.host)) { callback(new Error(FORBIDDEN_IP_ADDRESS)) return undefined } @@ -85,9 +85,9 @@ const getProtectedHttpAgent = ({ protocol, blockLocalIPs }) => { module.exports.getProtectedHttpAgent = getProtectedHttpAgent -async function getProtectedGot ({ blockLocalIPs }) { - const HttpAgent = getProtectedHttpAgent({ protocol: 'http', blockLocalIPs }) - const HttpsAgent = getProtectedHttpAgent({ protocol: 'https', blockLocalIPs }) +async function getProtectedGot ({ allowLocalIPs }) { + const HttpAgent = getProtectedHttpAgent({ protocol: 'http', allowLocalIPs }) + const HttpsAgent = getProtectedHttpAgent({ protocol: 'https', allowLocalIPs }) const httpAgent = new HttpAgent() const httpsAgent = new HttpsAgent() @@ -102,12 +102,12 @@ module.exports.getProtectedGot = getProtectedGot * Gets the size and content type of a url's content * * @param {string} url - * @param {boolean} blockLocalIPs + * @param {boolean} allowLocalIPs * @returns {Promise<{name: string, type: string, size: number}>} */ -exports.getURLMeta = async (url, blockLocalIPs = false) => { +exports.getURLMeta = async (url, allowLocalIPs = false) => { async function requestWithMethod (method) { - const protectedGot = await getProtectedGot({ blockLocalIPs }) + const protectedGot = await getProtectedGot({ allowLocalIPs }) const stream = protectedGot.stream(url, { method, throwHttpErrors: false }) return new Promise((resolve, reject) => ( diff --git a/packages/@uppy/companion/src/server/jobs.js b/packages/@uppy/companion/src/server/jobs.js index 68fde58210..d067b62111 100644 --- a/packages/@uppy/companion/src/server/jobs.js +++ b/packages/@uppy/companion/src/server/jobs.js @@ -1,16 +1,13 @@ const schedule = require('node-schedule') const fs = require('node:fs') const path = require('node:path') -const { promisify } = require('node:util') +const { setTimeout: sleep } = require('node:timers/promises') const got = require('./got') const { FILE_NAME_PREFIX } = require('./Uploader') const logger = require('./logger') -// TODO rewrite to use require('timers/promises').setTimeout when we support newer node versions -const sleep = promisify(setTimeout) - const cleanUpFinishedUploads = (dirPath) => { logger.info(`running clean up job for path: ${dirPath}`, 'jobs.cleanup.progress.read') fs.readdir(dirPath, (err, files) => { diff --git a/packages/@uppy/companion/src/server/middlewares.js b/packages/@uppy/companion/src/server/middlewares.js index 2b4dad6562..2ecb4620a9 100644 --- a/packages/@uppy/companion/src/server/middlewares.js +++ b/packages/@uppy/companion/src/server/middlewares.js @@ -23,7 +23,7 @@ exports.hasSessionAndProvider = (req, res, next) => { return next() } -const isOAuthProviderReq = (req) => isOAuthProvider(req.companion.providerClass.authProvider) +const isOAuthProviderReq = (req) => isOAuthProvider(req.companion.providerClass.oauthProvider) const isSimpleAuthProviderReq = (req) => !!req.companion.providerClass.hasSimpleAuth /** @@ -122,7 +122,7 @@ exports.gentleVerifyToken = (req, res, next) => { } exports.cookieAuthToken = (req, res, next) => { - req.companion.authToken = req.cookies[`uppyAuthToken--${req.companion.providerClass.authProvider}`] + req.companion.authToken = req.cookies[`uppyAuthToken--${req.companion.providerClass.oauthProvider}`] return next() } @@ -133,14 +133,11 @@ exports.cors = (options = {}) => (req, res, next) => { const existingExposeHeaders = res.get('Access-Control-Expose-Headers') const exposeHeadersSet = new Set(existingExposeHeaders?.split(',')?.map((method) => method.trim().toLowerCase())) - // exposed so it can be accessed for our custom uppy client preflight - exposeHeadersSet.add('access-control-allow-headers') // todo remove in next major, see https://github.com/transloadit/uppy/pull/4462 if (options.sendSelfEndpoint) exposeHeadersSet.add('i-am') // Needed for basic operation: https://github.com/transloadit/uppy/issues/3021 const allowedHeaders = [ 'uppy-auth-token', - 'uppy-versions', // todo remove in the future? see https://github.com/transloadit/uppy/pull/4462 'uppy-credentials-params', 'authorization', 'origin', diff --git a/packages/@uppy/companion/src/server/provider/Provider.js b/packages/@uppy/companion/src/server/provider/Provider.js index b8b8c87f8a..43d3a978e9 100644 --- a/packages/@uppy/companion/src/server/provider/Provider.js +++ b/packages/@uppy/companion/src/server/provider/Provider.js @@ -18,9 +18,8 @@ class Provider { /** * config to extend the grant config - * todo major: rename to getExtraGrantConfig */ - static getExtraConfig () { + static getExtraGrantConfig () { return {} } @@ -102,8 +101,7 @@ class Provider { * * @returns {string} */ - // todo next major: rename authProvider to oauthProvider (we have other non-oauth auth types too now) - static get authProvider () { + static get oauthProvider () { return undefined } @@ -122,5 +120,5 @@ class Provider { } module.exports = Provider -// OAuth providers are those that have an `authProvider` set. It means they require OAuth authentication to work -module.exports.isOAuthProvider = (authProvider) => typeof authProvider === 'string' && authProvider.length > 0 +// OAuth providers are those that have an `oauthProvider` set. It means they require OAuth authentication to work +module.exports.isOAuthProvider = (oauthProvider) => typeof oauthProvider === 'string' && oauthProvider.length > 0 diff --git a/packages/@uppy/companion/src/server/provider/box/index.js b/packages/@uppy/companion/src/server/provider/box/index.js index 923884e35c..3488363082 100644 --- a/packages/@uppy/companion/src/server/provider/box/index.js +++ b/packages/@uppy/companion/src/server/provider/box/index.js @@ -34,7 +34,7 @@ class Box extends Provider { this.needsCookieAuth = true } - static get authProvider () { + static get oauthProvider () { return 'box' } @@ -119,7 +119,7 @@ class Box extends Provider { return withProviderErrorHandling({ fn, tag, - providerName: Box.authProvider, + providerName: Box.oauthProvider, isAuthError: (response) => response.statusCode === 401, getJsonErrorMessage: (body) => body?.message, }) diff --git a/packages/@uppy/companion/src/server/provider/credentials.js b/packages/@uppy/companion/src/server/provider/credentials.js index 166327fdd3..e716e92251 100644 --- a/packages/@uppy/companion/src/server/provider/credentials.js +++ b/packages/@uppy/companion/src/server/provider/credentials.js @@ -71,8 +71,8 @@ async function fetchProviderKeys (providerName, companionOptions, credentialRequ exports.getCredentialsOverrideMiddleware = (providers, companionOptions) => { return async (req, res, next) => { try { - const { authProvider, override } = req.params - const [providerName] = Object.keys(providers).filter((name) => providers[name].authProvider === authProvider) + const { oauthProvider, override } = req.params + const [providerName] = Object.keys(providers).filter((name) => providers[name].oauthProvider === oauthProvider) if (!providerName) { next() return diff --git a/packages/@uppy/companion/src/server/provider/drive/index.js b/packages/@uppy/companion/src/server/provider/drive/index.js index d750c06bb7..5c2b4ff327 100644 --- a/packages/@uppy/companion/src/server/provider/drive/index.js +++ b/packages/@uppy/companion/src/server/provider/drive/index.js @@ -50,7 +50,7 @@ async function getStats ({ id, token }) { * Adapter for API https://developers.google.com/drive/api/v3/ */ class Drive extends Provider { - static get authProvider () { + static get oauthProvider () { return 'google' } @@ -214,7 +214,7 @@ class Drive extends Provider { return withProviderErrorHandling({ fn, tag, - providerName: Drive.authProvider, + providerName: Drive.oauthProvider, isAuthError: (response) => ( response.statusCode === 401 || (response.statusCode === 400 && response.body?.error === 'invalid_grant') // Refresh token has expired or been revoked diff --git a/packages/@uppy/companion/src/server/provider/dropbox/index.js b/packages/@uppy/companion/src/server/provider/dropbox/index.js index 236bb0419c..d33e274ecf 100644 --- a/packages/@uppy/companion/src/server/provider/dropbox/index.js +++ b/packages/@uppy/companion/src/server/provider/dropbox/index.js @@ -60,7 +60,7 @@ class DropBox extends Provider { this.needsCookieAuth = true } - static get authProvider () { + static get oauthProvider () { return 'dropbox' } @@ -141,7 +141,7 @@ class DropBox extends Provider { return withProviderErrorHandling({ fn, tag, - providerName: DropBox.authProvider, + providerName: DropBox.oauthProvider, isAuthError: (response) => response.statusCode === 401, getJsonErrorMessage: (body) => body?.error_summary, }) diff --git a/packages/@uppy/companion/src/server/provider/facebook/index.js b/packages/@uppy/companion/src/server/provider/facebook/index.js index 22e6367137..b48a24349b 100644 --- a/packages/@uppy/companion/src/server/provider/facebook/index.js +++ b/packages/@uppy/companion/src/server/provider/facebook/index.js @@ -24,7 +24,7 @@ async function getMediaUrl ({ token, id }) { * Adapter for API https://developers.facebook.com/docs/graph-api/using-graph-api/ */ class Facebook extends Provider { - static get authProvider () { + static get oauthProvider () { return 'facebook' } @@ -69,7 +69,7 @@ class Facebook extends Provider { async size ({ id, token }) { return this.#withErrorHandling('provider.facebook.size.error', async () => { const url = await getMediaUrl({ token, id }) - const { size } = await getURLMeta(url, true) + const { size } = await getURLMeta(url) return size }) } @@ -86,7 +86,7 @@ class Facebook extends Provider { return withProviderErrorHandling({ fn, tag, - providerName: Facebook.authProvider, + providerName: Facebook.oauthProvider, isAuthError: (response) => typeof response.body === 'object' && response.body?.error?.code === 190, // Invalid OAuth 2.0 Access Token getJsonErrorMessage: (body) => body?.error?.message, }) diff --git a/packages/@uppy/companion/src/server/provider/index.js b/packages/@uppy/companion/src/server/provider/index.js index fb7d4b1b59..c556395b30 100644 --- a/packages/@uppy/companion/src/server/provider/index.js +++ b/packages/@uppy/companion/src/server/provider/index.js @@ -42,12 +42,12 @@ module.exports.getProviderMiddleware = (providers, grantConfig) => { const ProviderClass = providers[providerName] if (ProviderClass && validOptions(req.companion.options)) { const { allowLocalUrls } = req.companion.options - const { authProvider } = ProviderClass + const { oauthProvider } = ProviderClass let providerGrantConfig - if (isOAuthProvider(authProvider)) { + if (isOAuthProvider(oauthProvider)) { req.companion.getProviderCredentials = getCredentialsResolver(providerName, req.companion.options, req) - providerGrantConfig = grantConfig[authProvider] + providerGrantConfig = grantConfig[oauthProvider] req.companion.providerGrantConfig = providerGrantConfig } @@ -85,11 +85,11 @@ module.exports.addCustomProviders = (customProviders, providers, grantConfig) => // eslint-disable-next-line no-param-reassign providers[providerName] = customProvider.module - const { authProvider } = customProvider.module + const { oauthProvider } = customProvider.module - if (isOAuthProvider(authProvider)) { + if (isOAuthProvider(oauthProvider)) { // eslint-disable-next-line no-param-reassign - grantConfig[authProvider] = { + grantConfig[oauthProvider] = { ...customProvider.config, // todo: consider setting these options from a universal point also used // by official providers. It'll prevent these from getting left out if the @@ -105,9 +105,9 @@ module.exports.addCustomProviders = (customProviders, providers, grantConfig) => * * @param {{server: object, providerOptions: object}} companionOptions * @param {object} grantConfig - * @param {(a: string) => string} getAuthProvider + * @param {(a: string) => string} getOauthProvider */ -module.exports.addProviderOptions = (companionOptions, grantConfig, getAuthProvider) => { +module.exports.addProviderOptions = (companionOptions, grantConfig, getOauthProvider) => { const { server, providerOptions } = companionOptions if (!validOptions({ server })) { logger.warn('invalid provider options detected. Providers will not be loaded', 'provider.options.invalid') @@ -124,40 +124,40 @@ module.exports.addProviderOptions = (companionOptions, grantConfig, getAuthProvi const { oauthDomain } = server const keys = Object.keys(providerOptions).filter((key) => key !== 'server') keys.forEach((providerName) => { - const authProvider = getAuthProvider?.(providerName) + const oauthProvider = getOauthProvider?.(providerName) - if (isOAuthProvider(authProvider) && grantConfig[authProvider]) { + if (isOAuthProvider(oauthProvider) && grantConfig[oauthProvider]) { // explicitly add providerOptions so users don't override other providerOptions. // eslint-disable-next-line no-param-reassign - grantConfig[authProvider].key = providerOptions[providerName].key + grantConfig[oauthProvider].key = providerOptions[providerName].key // eslint-disable-next-line no-param-reassign - grantConfig[authProvider].secret = providerOptions[providerName].secret + grantConfig[oauthProvider].secret = providerOptions[providerName].secret if (providerOptions[providerName].credentialsURL) { // eslint-disable-next-line no-param-reassign - grantConfig[authProvider].dynamic = ['key', 'secret', 'redirect_uri'] + grantConfig[oauthProvider].dynamic = ['key', 'secret', 'redirect_uri'] } const provider = exports.getDefaultProviders()[providerName] - Object.assign(grantConfig[authProvider], provider.getExtraConfig()) + Object.assign(grantConfig[oauthProvider], provider.getExtraGrantConfig()) // override grant.js redirect uri with companion's custom redirect url const isExternal = !!server.implicitPath const redirectPath = `/${providerName}/redirect` // eslint-disable-next-line no-param-reassign - grantConfig[authProvider].redirect_uri = getURLBuilder(companionOptions)(redirectPath, isExternal) + grantConfig[oauthProvider].redirect_uri = getURLBuilder(companionOptions)(redirectPath, isExternal) if (oauthDomain) { const fullRedirectPath = getURLBuilder(companionOptions)(redirectPath, isExternal, true) // eslint-disable-next-line no-param-reassign - grantConfig[authProvider].redirect_uri = `${server.protocol}://${oauthDomain}${fullRedirectPath}` + grantConfig[oauthProvider].redirect_uri = `${server.protocol}://${oauthDomain}${fullRedirectPath}` } if (server.implicitPath) { // no url builder is used for this because grant internally adds the path // eslint-disable-next-line no-param-reassign - grantConfig[authProvider].callback = `${server.implicitPath}${grantConfig[authProvider].callback}` + grantConfig[oauthProvider].callback = `${server.implicitPath}${grantConfig[oauthProvider].callback}` } else if (server.path) { // eslint-disable-next-line no-param-reassign - grantConfig[authProvider].callback = `${server.path}${grantConfig[authProvider].callback}` + grantConfig[oauthProvider].callback = `${server.path}${grantConfig[oauthProvider].callback}` } } }) diff --git a/packages/@uppy/companion/src/server/provider/instagram/graph/index.js b/packages/@uppy/companion/src/server/provider/instagram/graph/index.js index 13c52aa055..db124452df 100644 --- a/packages/@uppy/companion/src/server/provider/instagram/graph/index.js +++ b/packages/@uppy/companion/src/server/provider/instagram/graph/index.js @@ -24,14 +24,14 @@ async function getMediaUrl ({ token, id }) { */ class Instagram extends Provider { // for "grant" - static getExtraConfig () { + static getExtraGrantConfig () { return { protocol: 'https', scope: ['user_profile', 'user_media'], } } - static get authProvider () { + static get oauthProvider () { return 'instagram' } @@ -70,7 +70,7 @@ class Instagram extends Provider { async size ({ id, token }) { return this.#withErrorHandling('provider.instagram.size.error', async () => { const url = await getMediaUrl({ token, id }) - const { size } = await getURLMeta(url, true) + const { size } = await getURLMeta(url) return size }) } @@ -86,7 +86,7 @@ class Instagram extends Provider { return withProviderErrorHandling({ fn, tag, - providerName: Instagram.authProvider, + providerName: Instagram.oauthProvider, isAuthError: (response) => typeof response.body === 'object' && response.body?.error?.code === 190, // Invalid OAuth 2.0 Access Token getJsonErrorMessage: (body) => body?.error?.message, }) diff --git a/packages/@uppy/companion/src/server/provider/onedrive/index.js b/packages/@uppy/companion/src/server/provider/onedrive/index.js index 2d89efaf23..191222dd5f 100644 --- a/packages/@uppy/companion/src/server/provider/onedrive/index.js +++ b/packages/@uppy/companion/src/server/provider/onedrive/index.js @@ -23,7 +23,7 @@ const getRootPath = (query) => (query.driveId ? `drives/${query.driveId}` : 'me/ * Adapter for API https://docs.microsoft.com/en-us/onedrive/developer/rest-api/ */ class OneDrive extends Provider { - static get authProvider () { + static get oauthProvider () { return 'microsoft' } @@ -98,7 +98,7 @@ class OneDrive extends Provider { return withProviderErrorHandling({ fn, tag, - providerName: OneDrive.authProvider, + providerName: OneDrive.oauthProvider, isAuthError: (response) => response.statusCode === 401, isUserFacingError: (response) => [400, 403].includes(response.statusCode), // onedrive gives some errors here that the user might want to know about diff --git a/packages/@uppy/companion/src/server/provider/unsplash/index.js b/packages/@uppy/companion/src/server/provider/unsplash/index.js index 77c0d5c001..b097e92287 100644 --- a/packages/@uppy/companion/src/server/provider/unsplash/index.js +++ b/packages/@uppy/companion/src/server/provider/unsplash/index.js @@ -58,7 +58,7 @@ class Unsplash extends Provider { async size ({ id, token }) { return this.#withErrorHandling('provider.unsplash.size.error', async () => { const { links: { download: url } } = await getPhotoMeta(await getClient({ token }), id) - const { size } = await getURLMeta(url, true) + const { size } = await getURLMeta(url) return size }) } diff --git a/packages/@uppy/companion/src/server/provider/zoom/index.js b/packages/@uppy/companion/src/server/provider/zoom/index.js index 7cf2284185..b9f5ec84ce 100644 --- a/packages/@uppy/companion/src/server/provider/zoom/index.js +++ b/packages/@uppy/companion/src/server/provider/zoom/index.js @@ -30,7 +30,7 @@ async function findFile ({ client, meetingId, fileId, recordingStart }) { * Adapter for API https://marketplace.zoom.us/docs/api-reference/zoom-api */ class Zoom extends Provider { - static get authProvider () { + static get oauthProvider () { return 'zoom' } @@ -163,7 +163,7 @@ class Zoom extends Provider { return withProviderErrorHandling({ fn, tag, - providerName: Zoom.authProvider, + providerName: Zoom.oauthProvider, isAuthError: (response) => authErrorCodes.includes(response.statusCode), getJsonErrorMessage: (body) => body?.message, }) diff --git a/packages/@uppy/companion/src/standalone/helper.js b/packages/@uppy/companion/src/standalone/helper.js index 426a317dbd..0b0d35902b 100644 --- a/packages/@uppy/companion/src/standalone/helper.js +++ b/packages/@uppy/companion/src/standalone/helper.js @@ -137,8 +137,7 @@ const getConfigFromEnv = () => { oauthDomain: process.env.COMPANION_OAUTH_DOMAIN, validHosts, }, - // todo next major make this default false - enableUrlEndpoint: process.env.COMPANION_ENABLE_URL_ENDPOINT == null || process.env.COMPANION_ENABLE_URL_ENDPOINT === 'true', + enableUrlEndpoint: process.env.COMPANION_ENABLE_URL_ENDPOINT === 'true', periodicPingUrls: process.env.COMPANION_PERIODIC_PING_URLS ? process.env.COMPANION_PERIODIC_PING_URLS.split(',') : [], periodicPingInterval: process.env.COMPANION_PERIODIC_PING_INTERVAL ? parseInt(process.env.COMPANION_PERIODIC_PING_INTERVAL, 10) : undefined, diff --git a/packages/@uppy/companion/src/standalone/index.js b/packages/@uppy/companion/src/standalone/index.js index e7b1d5cc1d..155b081052 100644 --- a/packages/@uppy/companion/src/standalone/index.js +++ b/packages/@uppy/companion/src/standalone/index.js @@ -117,8 +117,7 @@ module.exports = function server(inputCompanionOptions) { const redisClient = redis.client(companionOptions) if (redisClient) { - // todo next major: change default prefix to something like "companion-session:" and possibly remove this option - sessionOptions.store = new RedisStore({ client: redisClient, prefix: process.env.COMPANION_REDIS_EXPRESS_SESSION_PREFIX || 'sess:' }) + sessionOptions.store = new RedisStore({ client: redisClient, prefix: process.env.COMPANION_REDIS_EXPRESS_SESSION_PREFIX || 'companion-session:' }) } if (process.env.COMPANION_COOKIE_DOMAIN) { diff --git a/packages/@uppy/companion/test/__tests__/cors.js b/packages/@uppy/companion/test/__tests__/cors.js index 8cf5b063d6..908e5e5932 100644 --- a/packages/@uppy/companion/test/__tests__/cors.js +++ b/packages/@uppy/companion/test/__tests__/cors.js @@ -39,8 +39,8 @@ describe('cors', () => { ['Vary', 'Origin'], ['Access-Control-Allow-Credentials', 'true'], ['Access-Control-Allow-Methods', 'PATCH,OPTIONS,POST,GET,DELETE'], - ['Access-Control-Allow-Headers', 'test-allow-header,uppy-auth-token,uppy-versions,uppy-credentials-params,authorization,origin,content-type,accept'], - ['Access-Control-Expose-Headers', 'test,access-control-allow-headers,i-am'], + ['Access-Control-Allow-Headers', 'test-allow-header,uppy-auth-token,uppy-credentials-params,authorization,origin,content-type,accept'], + ['Access-Control-Expose-Headers', 'test,i-am'], ['Content-Length', '0'], ]) // expect(next).toHaveBeenCalled() @@ -53,8 +53,7 @@ describe('cors', () => { ['Vary', 'Origin'], ['Access-Control-Allow-Credentials', 'true'], ['Access-Control-Allow-Methods', 'GET,POST,OPTIONS,DELETE'], - ['Access-Control-Allow-Headers', 'uppy-auth-token,uppy-versions,uppy-credentials-params,authorization,origin,content-type,accept'], - ['Access-Control-Expose-Headers', 'access-control-allow-headers'], + ['Access-Control-Allow-Headers', 'uppy-auth-token,uppy-credentials-params,authorization,origin,content-type,accept'], ['Content-Length', '0'], ]) }) @@ -70,8 +69,7 @@ describe('cors', () => { ['Vary', 'Origin'], ['Access-Control-Allow-Credentials', 'true'], ['Access-Control-Allow-Methods', 'GET,POST,OPTIONS,DELETE'], - ['Access-Control-Allow-Headers', 'uppy-auth-token,uppy-versions,uppy-credentials-params,authorization,origin,content-type,accept'], - ['Access-Control-Expose-Headers', 'access-control-allow-headers'], + ['Access-Control-Allow-Headers', 'uppy-auth-token,uppy-credentials-params,authorization,origin,content-type,accept'], ['Content-Length', '0'], ]) }) @@ -83,8 +81,7 @@ describe('cors', () => { ['Vary', 'Origin'], ['Access-Control-Allow-Credentials', 'true'], ['Access-Control-Allow-Methods', 'GET,POST,OPTIONS,DELETE'], - ['Access-Control-Allow-Headers', 'uppy-auth-token,uppy-versions,uppy-credentials-params,authorization,origin,content-type,accept'], - ['Access-Control-Expose-Headers', 'access-control-allow-headers'], + ['Access-Control-Allow-Headers', 'uppy-auth-token,uppy-credentials-params,authorization,origin,content-type,accept'], ['Content-Length', '0'], ]) }) diff --git a/packages/@uppy/companion/test/__tests__/http-agent.js b/packages/@uppy/companion/test/__tests__/http-agent.js index 5e4452ebfa..c2c48809e6 100644 --- a/packages/@uppy/companion/test/__tests__/http-agent.js +++ b/packages/@uppy/companion/test/__tests__/http-agent.js @@ -11,24 +11,24 @@ describe('test protected request Agent', () => { test('allows URLs without IP addresses', async () => { nock('https://transloadit.com').get('/').reply(200) const url = 'https://transloadit.com' - return (await getProtectedGot({ blockLocalIPs: true })).get(url) + return (await getProtectedGot({ allowLocalIPs: false })).get(url) }) test('blocks url that resolves to forbidden IP', async () => { const url = 'https://localhost' - const promise = getProtectedGot({ blockLocalIPs: true }).then(got => got.get(url)) + const promise = getProtectedGot({ allowLocalIPs: false }).then(got => got.get(url)) await expect(promise).rejects.toThrow(/^Forbidden resolved IP address/) }) test('blocks private http IP address', async () => { const url = 'http://172.20.10.4:8090' - const promise = getProtectedGot({ blockLocalIPs: true }).then(got => got.get(url)) + const promise = getProtectedGot({ allowLocalIPs: false }).then(got => got.get(url)) await expect(promise).rejects.toThrow(new Error(FORBIDDEN_IP_ADDRESS)) }) test('blocks private https IP address', async () => { const url = 'https://172.20.10.4:8090' - const promise = getProtectedGot({ blockLocalIPs: true }).then(got => got.get(url)) + const promise = getProtectedGot({ allowLocalIPs: false }).then(got => got.get(url)) await expect(promise).rejects.toThrow(new Error(FORBIDDEN_IP_ADDRESS)) }) @@ -57,12 +57,12 @@ describe('test protected request Agent', () => { for (const ip of ipv4s) { const url = `http://${ip}:8090` - const promise = getProtectedGot({ blockLocalIPs: true }).then(got => got.get(url)) + const promise = getProtectedGot({ allowLocalIPs: false }).then(got => got.get(url)) await expect(promise).rejects.toThrow(new Error(FORBIDDEN_IP_ADDRESS)) } for (const ip of ipv6s) { const url = `http://[${ip}]:8090` - const promise = getProtectedGot({ blockLocalIPs: true }).then(got => got.get(url)) + const promise = getProtectedGot({ allowLocalIPs: false }).then(got => got.get(url)) await expect(promise).rejects.toThrow(new Error(FORBIDDEN_IP_ADDRESS)) } }) diff --git a/packages/@uppy/companion/test/__tests__/provider-manager.js b/packages/@uppy/companion/test/__tests__/provider-manager.js index 084e2ec9f1..6d8d5322c4 100644 --- a/packages/@uppy/companion/test/__tests__/provider-manager.js +++ b/packages/@uppy/companion/test/__tests__/provider-manager.js @@ -5,7 +5,7 @@ const { setDefaultEnv } = require('../mockserver') let grantConfig let companionOptions -const getAuthProvider = (providerName) => providerManager.getDefaultProviders()[providerName]?.authProvider +const getOauthProvider = (providerName) => providerManager.getDefaultProviders()[providerName]?.oauthProvider describe('Test Provider options', () => { beforeEach(() => { @@ -16,7 +16,7 @@ describe('Test Provider options', () => { }) test('adds provider options', () => { - providerManager.addProviderOptions(companionOptions, grantConfig, getAuthProvider) + providerManager.addProviderOptions(companionOptions, grantConfig, getOauthProvider) expect(grantConfig.dropbox.key).toBe('dropbox_key') expect(grantConfig.dropbox.secret).toBe('dropbox_secret') @@ -35,7 +35,7 @@ describe('Test Provider options', () => { test('adds extra provider config', () => { process.env.COMPANION_INSTAGRAM_KEY = '123456' - providerManager.addProviderOptions(getCompanionOptions(), grantConfig, getAuthProvider) + providerManager.addProviderOptions(getCompanionOptions(), grantConfig, getOauthProvider) expect(grantConfig.instagram).toEqual({ transport: 'session', callback: '/instagram/callback', @@ -104,7 +104,7 @@ describe('Test Provider options', () => { companionOptions = getCompanionOptions() - providerManager.addProviderOptions(companionOptions, grantConfig, getAuthProvider) + providerManager.addProviderOptions(companionOptions, grantConfig, getOauthProvider) expect(grantConfig.dropbox.secret).toBe('xobpord') expect(grantConfig.box.secret).toBe('xwbepqd') @@ -118,7 +118,7 @@ describe('Test Provider options', () => { delete companionOptions.server.host delete companionOptions.server.protocol - providerManager.addProviderOptions(companionOptions, grantConfig, getAuthProvider) + providerManager.addProviderOptions(companionOptions, grantConfig, getOauthProvider) expect(grantConfig.dropbox.key).toBeUndefined() expect(grantConfig.dropbox.secret).toBeUndefined() @@ -137,7 +137,7 @@ describe('Test Provider options', () => { test('sets a main redirect uri, if oauthDomain is set', () => { companionOptions.server.oauthDomain = 'domain.com' - providerManager.addProviderOptions(companionOptions, grantConfig, getAuthProvider) + providerManager.addProviderOptions(companionOptions, grantConfig, getOauthProvider) expect(grantConfig.dropbox.redirect_uri).toBe('http://domain.com/dropbox/redirect') expect(grantConfig.box.redirect_uri).toBe('http://domain.com/box/redirect') @@ -156,7 +156,7 @@ describe('Test Custom Provider options', () => { key: 'foo_key', secret: 'foo_secret', }, - module: { authProvider: 'some_provider' }, + module: { oauthProvider: 'some_provider' }, }, }, providers, grantConfig) diff --git a/packages/@uppy/companion/test/__tests__/providers.js b/packages/@uppy/companion/test/__tests__/providers.js index bd1c31ae8c..6012490549 100644 --- a/packages/@uppy/companion/test/__tests__/providers.js +++ b/packages/@uppy/companion/test/__tests__/providers.js @@ -25,10 +25,11 @@ const OAUTH_STATE = 'some-cool-nice-encrytpion' const providers = require('../../src/server/provider').getDefaultProviders() const providerNames = Object.keys(providers) -const AUTH_PROVIDERS = { - drive: 'google', - onedrive: 'microsoft', -} +const oauthProviders = Object.fromEntries( + Object.entries(providers).flatMap(([name, provider]) => ( + provider.oauthProvider != null ? [[name, provider.oauthProvider]] : [] + )) +) const authData = {} providerNames.forEach((provider) => { authData[provider] = { accessToken: 'token value' } @@ -372,16 +373,16 @@ describe('provider file gets downloaded from', () => { }) describe('connect to provider', () => { - test.each(providerNames)('connect to %s via grant.js endpoint', (providerName) => { - const authProvider = AUTH_PROVIDERS[providerName] || providerName + test.each(providerNames)('connect to %s via grant.js endpoint', async (providerName) => { + const oauthProvider = oauthProviders[providerName] - if (authProvider.authProvider == null) return + if (oauthProvider == null) return - request(authServer) + await request(authServer) .get(`/${providerName}/connect?foo=bar`) .set('uppy-auth-token', token) .expect(302) - .expect('Location', `http://localhost:3020/connect/${authProvider}?state=${OAUTH_STATE}`) + .expect('Location', `http://localhost:3020/connect/${oauthProvider}?state=${OAUTH_STATE}`) }) }) diff --git a/packages/@uppy/companion/test/__tests__/uploader.js b/packages/@uppy/companion/test/__tests__/uploader.js index 9b857394a4..ac258c8678 100644 --- a/packages/@uppy/companion/test/__tests__/uploader.js +++ b/packages/@uppy/companion/test/__tests__/uploader.js @@ -227,12 +227,12 @@ describe('uploader with tus protocol', () => { }) // https://github.com/transloadit/uppy/issues/3477 - test('upload functions with xhr formdata and metadata', async () => { - nock('http://localhost').post('/', /^--form-data-boundary-[a-z0-9]+\r\nContent-Disposition: form-data; name="key1"\r\n\r\nnull\r\n--form-data-boundary-[a-z0-9]+\r\nContent-Disposition: form-data; name="key2"\r\n\r\ntrue\r\n--form-data-boundary-[a-z0-9]+\r\nContent-Disposition: form-data; name="key3"\r\n\r\n\d+\r\n--form-data-boundary-[a-z0-9]+\r\nContent-Disposition: form-data; name="key4"\r\n\r\n\[object Object\]\r\n--form-data-boundary-[a-z0-9]+\r\nContent-Disposition: form-data; name="key5"\r\n\r\n\(\) => \{\}\r\n--form-data-boundary-[a-z0-9]+\r\nContent-Disposition: form-data; name="key6"\r\n\r\nSymbol\(\)\r\n--form-data-boundary-[a-z0-9]+\r\nContent-Disposition: form-data; name="files\[\]"; filename="uppy-file-[^"]+"\r\nContent-Type: application\/octet-stream\r\n\r\nSome file content\r\n--form-data-boundary-[a-z0-9]+--\r\n\r\n$/) + test('upload functions with xhr formdata and metadata without crashing the node.js process', async () => { + nock('http://localhost').post('/', /^--form-data-boundary-[a-z0-9]+\r\nContent-Disposition: form-data; name="key1"\r\n\r\nnull\r\n--form-data-boundary-[a-z0-9]+\r\nContent-Disposition: form-data; name="key2"\r\n\r\ntrue\r\n--form-data-boundary-[a-z0-9]+\r\nContent-Disposition: form-data; name="key3"\r\n\r\n\d+\r\n--form-data-boundary-[a-z0-9]+\r\nContent-Disposition: form-data; name="key4"\r\n\r\n\[object Object\]\r\n--form-data-boundary-[a-z0-9]+\r\nContent-Disposition: form-data; name="key5"\r\n\r\n\(\) => \{\}\r\n--form-data-boundary-[a-z0-9]+\r\nContent-Disposition: form-data; name="files\[\]"; filename="uppy-file-[^"]+"\r\nContent-Type: application\/octet-stream\r\n\r\nSome file content\r\n--form-data-boundary-[a-z0-9]+--\r\n\r\n$/) .reply(200) const metadata = { - key1: null, key2: true, key3: 1234, key4: {}, key5: () => {}, key6: Symbol(''), + key1: null, key2: true, key3: 1234, key4: {}, key5: () => {}, } const ret = await runMultipartTest({ useFormData: true, metadata }) expect(ret).toMatchObject({ url: null, extraData: { response: expect.anything(), bytesUploaded: 17 } }) diff --git a/packages/@uppy/companion/test/mockoauthstate.js b/packages/@uppy/companion/test/mockoauthstate.js index 8a7db59057..2739e328b9 100644 --- a/packages/@uppy/companion/test/mockoauthstate.js +++ b/packages/@uppy/companion/test/mockoauthstate.js @@ -9,5 +9,6 @@ module.exports = () => { return 'http://localhost:3020' }, + encodeState: () => 'some-cool-nice-encrytpion', } } diff --git a/packages/@uppy/companion/test/mockserver.js b/packages/@uppy/companion/test/mockserver.js index 7bdf519f8d..765306844f 100644 --- a/packages/@uppy/companion/test/mockserver.js +++ b/packages/@uppy/companion/test/mockserver.js @@ -40,6 +40,8 @@ const defaultEnv = { COMPANION_PERIODIC_PING_URLS: '', COMPANION_CLIENT_SOCKET_CONNECT_TIMEOUT: '', + + COMPANION_ENABLE_URL_ENDPOINT: 'true', } function updateEnv (env) {