From ec4bc5850871702a99de472bb76be8645bc8fa1f Mon Sep 17 00:00:00 2001 From: Mikael Finstad Date: Wed, 6 Dec 2023 05:55:41 +0800 Subject: [PATCH] Provider user sessions (#4619) New concept "simple auth" - authentication that happens immediately (in one http request) without redirecting to any third party. uppyAuthToken initially used to simply contain an encrypted & json encoded OAuth2 access_token for a specific provider. Then we added refresh tokens as well inside uppyAuthToken #4448. Now we also allow storing other state or parameters needed for that specific provider, like username, password, host name, webdav URL etc... This is needed for providers like webdav, ftp etc, where the user needs to give some more input data while authenticating Companion: - `providerTokens` has been renamed to `providerUserSession` because it now includes not only tokens, but a user's session with a provider. Companion `Provider` class: - New `hasSimpleAuth` static boolean property - whether this provider uses simple auth - uppyAuthToken expiry default 24hr again for providers that don't support refresh tokens - make uppyAuthToken expiry configurable per provider - new `authStateExpiry` static property (defaults to 24hr) - new static property `grantDynamicToUserSession`, allows providers to specify which state from Grant `dynamic` to include into the provider's `providerUserSession`. --- e2e/cypress/integration/dashboard-tus.spec.ts | 4 - e2e/cypress/integration/dashboard-xhr.spec.ts | 14 +-- e2e/cypress/integration/reusable-tests.ts | 25 ++++-- package.json | 2 +- packages/@uppy/box/src/Box.jsx | 1 + .../@uppy/companion-client/src/AuthError.js | 5 +- .../@uppy/companion-client/src/Provider.js | 86 +++++++++++++------ .../companion-client/src/RequestClient.js | 58 +++++++------ packages/@uppy/companion/src/companion.js | 8 +- .../@uppy/companion/src/server/Uploader.js | 72 +++++++++------- .../src/server/controllers/callback.js | 15 ++-- .../src/server/controllers/connect.js | 43 ++++++++-- .../companion/src/server/controllers/get.js | 5 +- .../companion/src/server/controllers/index.js | 1 + .../companion/src/server/controllers/list.js | 8 +- .../src/server/controllers/logout.js | 11 ++- .../src/server/controllers/oauth-redirect.js | 2 +- .../src/server/controllers/refresh-token.js | 23 ++--- .../src/server/controllers/send-token.js | 2 +- .../src/server/controllers/simple-auth.js | 31 +++++++ .../src/server/controllers/thumbnail.js | 15 ++-- .../@uppy/companion/src/server/helpers/jwt.js | 73 ++++++++++------ .../src/server/helpers/oauth-state.js | 25 ++---- .../companion/src/server/helpers/upload.js | 24 +++--- .../companion/src/server/helpers/utils.js | 11 +-- packages/@uppy/companion/src/server/logger.js | 8 +- .../@uppy/companion/src/server/middlewares.js | 57 +++++++----- .../companion/src/server/provider/Provider.js | 33 ++++++- .../src/server/provider/box/index.js | 2 +- .../src/server/provider/credentials.js | 6 +- .../src/server/provider/drive/index.js | 5 ++ .../src/server/provider/dropbox/index.js | 9 +- .../companion/src/server/provider/error.d.ts | 19 ---- .../companion/src/server/provider/error.js | 43 +++++++--- .../companion/src/server/provider/index.js | 14 +-- .../src/server/provider/providerErrors.js | 53 +++++++----- .../@uppy/companion/src/standalone/index.js | 6 +- packages/@uppy/core/src/BasePlugin.js | 3 +- packages/@uppy/core/src/Uppy.js | 3 +- packages/@uppy/dropbox/src/Dropbox.jsx | 1 + packages/@uppy/facebook/src/Facebook.jsx | 1 + .../@uppy/google-drive/src/GoogleDrive.jsx | 1 + packages/@uppy/instagram/src/Instagram.jsx | 1 + packages/@uppy/onedrive/src/OneDrive.jsx | 1 + .../src/ProviderView/AuthView.jsx | 62 ++++++++----- .../src/ProviderView/ProviderView.jsx | 49 ++++++----- packages/@uppy/utils/package.json | 3 +- packages/@uppy/utils/src/Translator.ts | 27 ++++-- .../@uppy/utils/src/UserFacingApiError.js | 5 ++ packages/@uppy/zoom/src/Zoom.jsx | 1 + 50 files changed, 620 insertions(+), 357 deletions(-) create mode 100644 packages/@uppy/companion/src/server/controllers/simple-auth.js delete mode 100644 packages/@uppy/companion/src/server/provider/error.d.ts create mode 100644 packages/@uppy/utils/src/UserFacingApiError.js diff --git a/e2e/cypress/integration/dashboard-tus.spec.ts b/e2e/cypress/integration/dashboard-tus.spec.ts index 053089c7df..8432b969c7 100644 --- a/e2e/cypress/integration/dashboard-tus.spec.ts +++ b/e2e/cypress/integration/dashboard-tus.spec.ts @@ -1,6 +1,4 @@ import { - interceptCompanionUrlRequest, - interceptCompanionUnsplashRequest, runRemoteUrlImageUploadTest, runRemoteUnsplashUploadTest, } from './reusable-tests' @@ -15,8 +13,6 @@ describe('Dashboard with Tus', () => { cy.intercept('/files/*').as('tus') cy.intercept({ method: 'POST', pathname: '/files' }).as('post') cy.intercept({ method: 'PATCH', pathname: '/files/*' }).as('patch') - interceptCompanionUrlRequest() - interceptCompanionUnsplashRequest() }) it('should upload cat image successfully', () => { diff --git a/e2e/cypress/integration/dashboard-xhr.spec.ts b/e2e/cypress/integration/dashboard-xhr.spec.ts index e21ee08caa..f1e6fffd98 100644 --- a/e2e/cypress/integration/dashboard-xhr.spec.ts +++ b/e2e/cypress/integration/dashboard-xhr.spec.ts @@ -1,6 +1,5 @@ import { - interceptCompanionUrlRequest, - interceptCompanionUnsplashRequest, + interceptCompanionUrlMetaRequest, runRemoteUrlImageUploadTest, runRemoteUnsplashUploadTest, } from './reusable-tests' @@ -8,8 +7,6 @@ import { describe('Dashboard with XHR', () => { beforeEach(() => { cy.visit('/dashboard-xhr') - interceptCompanionUrlRequest() - interceptCompanionUnsplashRequest() }) it('should upload remote image with URL plugin', () => { @@ -22,8 +19,9 @@ describe('Dashboard with XHR', () => { cy.get('.uppy-Url-input').type( 'http://localhost:4678/file-with-content-disposition', ) + interceptCompanionUrlMetaRequest() cy.get('.uppy-Url-importButton').click() - cy.wait('@url').then(() => { + cy.wait('@url-meta').then(() => { cy.get('.uppy-Dashboard-Item-name').should('contain', fileName) cy.get('.uppy-Dashboard-Item-status').should('contain', '84 KB') }) @@ -32,8 +30,9 @@ describe('Dashboard with XHR', () => { it('should return correct file name with URL plugin from remote image without Content-Disposition', () => { cy.get('[data-cy="Url"]').click() cy.get('.uppy-Url-input').type('http://localhost:4678/file-no-headers') + interceptCompanionUrlMetaRequest() cy.get('.uppy-Url-importButton').click() - cy.wait('@url').then(() => { + cy.wait('@url-meta').then(() => { cy.get('.uppy-Dashboard-Item-name').should('contain', 'file-no') cy.get('.uppy-Dashboard-Item-status').should('contain', '0') }) @@ -50,8 +49,9 @@ describe('Dashboard with XHR', () => { cy.get('.uppy-Url-input').type( 'http://localhost:4678/file-with-content-disposition', ) + interceptCompanionUrlMetaRequest() cy.get('.uppy-Url-importButton').click() - cy.wait('@url').then(() => { + cy.wait('@url-meta').then(() => { cy.get('.uppy-Dashboard-Item-name').should('contain', 'file-with') cy.get('.uppy-Dashboard-Item-status').should('contain', '123 B') }) diff --git a/e2e/cypress/integration/reusable-tests.ts b/e2e/cypress/integration/reusable-tests.ts index f60d29de53..d311b60e39 100644 --- a/e2e/cypress/integration/reusable-tests.ts +++ b/e2e/cypress/integration/reusable-tests.ts @@ -1,9 +1,13 @@ /* global cy */ -export const interceptCompanionUrlRequest = () => - cy.intercept('http://localhost:3020/url/*').as('url') -export const interceptCompanionUnsplashRequest = () => - cy.intercept('http://localhost:3020/search/unsplash/*').as('unsplash') +const interceptCompanionUrlRequest = () => + cy + .intercept({ method: 'POST', url: 'http://localhost:3020/url/get' }) + .as('url') +export const interceptCompanionUrlMetaRequest = () => + cy + .intercept({ method: 'POST', url: 'http://localhost:3020/url/meta' }) + .as('url-meta') export function runRemoteUrlImageUploadTest() { cy.get('[data-cy="Url"]').click() @@ -11,6 +15,7 @@ export function runRemoteUrlImageUploadTest() { 'https://raw.githubusercontent.com/transloadit/uppy/main/e2e/cypress/fixtures/images/cat.jpg', ) cy.get('.uppy-Url-importButton').click() + interceptCompanionUrlRequest() cy.get('.uppy-StatusBar-actionBtn--upload').click() cy.wait('@url').then(() => { cy.get('.uppy-StatusBar-statusPrimary').should('contain', 'Complete') @@ -20,8 +25,12 @@ export function runRemoteUrlImageUploadTest() { export function runRemoteUnsplashUploadTest() { cy.get('[data-cy="Unsplash"]').click() cy.get('.uppy-SearchProvider-input').type('book') + cy.intercept({ + method: 'GET', + url: 'http://localhost:3020/search/unsplash/list?q=book', + }).as('unsplash-list') cy.get('.uppy-SearchProvider-searchButton').click() - cy.wait('@unsplash') + cy.wait('@unsplash-list') // Test that the author link is visible cy.get('.uppy-ProviderBrowserItem') .first() @@ -34,8 +43,12 @@ export function runRemoteUnsplashUploadTest() { cy.get('a').should('have.css', 'display', 'block') }) cy.get('.uppy-c-btn-primary').click() + cy.intercept({ + method: 'POST', + url: 'http://localhost:3020/search/unsplash/get/*', + }).as('unsplash-get') cy.get('.uppy-StatusBar-actionBtn--upload').click() - cy.wait('@unsplash').then(() => { + cy.wait('@unsplash-get').then(() => { cy.get('.uppy-StatusBar-statusPrimary').should('contain', 'Complete') }) } diff --git a/package.json b/package.json index d4b99be78e..a72531a80a 100644 --- a/package.json +++ b/package.json @@ -129,7 +129,7 @@ "contributors:save": "yarn node ./bin/update-contributors.mjs", "dev:with-companion": "npm-run-all --parallel start:companion dev", "dev": "yarn workspace @uppy-dev/dev dev", - "lint:fix": "yarn run lint -- --fix", + "lint:fix": "yarn lint --fix", "lint:markdown": "remark -f -q -i .remarkignore . .github/CONTRIBUTING.md", "lint:staged": "lint-staged", "lint:css": "stylelint ./packages/**/*.scss", diff --git a/packages/@uppy/box/src/Box.jsx b/packages/@uppy/box/src/Box.jsx index 93a202a1db..2fd823b383 100644 --- a/packages/@uppy/box/src/Box.jsx +++ b/packages/@uppy/box/src/Box.jsx @@ -30,6 +30,7 @@ export default class Box extends UIPlugin { companionCookiesRule: this.opts.companionCookiesRule, provider: 'box', pluginId: this.id, + supportsRefreshToken: false, }) this.defaultLocale = locale diff --git a/packages/@uppy/companion-client/src/AuthError.js b/packages/@uppy/companion-client/src/AuthError.js index 14517d3400..6c77e794f0 100644 --- a/packages/@uppy/companion-client/src/AuthError.js +++ b/packages/@uppy/companion-client/src/AuthError.js @@ -1,9 +1,12 @@ 'use strict' class AuthError extends Error { - constructor () { + constructor() { super('Authorization required') this.name = 'AuthError' + + // we use a property because of instanceof is unsafe: + // https://github.com/transloadit/uppy/pull/4619#discussion_r1406225982 this.isAuthError = true } } diff --git a/packages/@uppy/companion-client/src/Provider.js b/packages/@uppy/companion-client/src/Provider.js index a3ebb5c7ec..fbd3a434aa 100644 --- a/packages/@uppy/companion-client/src/Provider.js +++ b/packages/@uppy/companion-client/src/Provider.js @@ -1,18 +1,19 @@ 'use strict' -import RequestClient from './RequestClient.js' +import RequestClient, { authErrorStatusCode } from './RequestClient.js' import * as tokenStorage from './tokenStorage.js' + const getName = (id) => { return id.split('-').map((s) => s.charAt(0).toUpperCase() + s.slice(1)).join(' ') } -function getOrigin () { +function getOrigin() { // eslint-disable-next-line no-restricted-globals return location.origin } -function getRegex (value) { +function getRegex(value) { if (typeof value === 'string') { return new RegExp(`^${value}$`) } if (value instanceof RegExp) { @@ -21,7 +22,7 @@ function getRegex (value) { return undefined } -function isOriginAllowed (origin, allowedOrigin) { +function isOriginAllowed(origin, allowedOrigin) { const patterns = Array.isArray(allowedOrigin) ? allowedOrigin.map(getRegex) : [getRegex(allowedOrigin)] return patterns .some((pattern) => pattern?.test(origin) || pattern?.test(`${origin}/`)) // allowing for trailing '/' @@ -30,7 +31,7 @@ function isOriginAllowed (origin, allowedOrigin) { export default class Provider extends RequestClient { #refreshingTokenPromise - constructor (uppy, opts) { + constructor(uppy, opts) { super(uppy, opts) this.provider = opts.provider this.id = this.provider @@ -39,9 +40,10 @@ export default class Provider extends RequestClient { 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 } - async headers () { + async headers() { const [headers, token] = await Promise.all([super.headers(), this.#getAuthToken()]) const authHeaders = {} if (token) { @@ -56,24 +58,25 @@ export default class Provider extends RequestClient { return { ...headers, ...authHeaders } } - onReceiveResponse (response) { + onReceiveResponse(response) { super.onReceiveResponse(response) const plugin = this.uppy.getPlugin(this.pluginId) const oldAuthenticated = plugin.getPluginState().authenticated - const authenticated = oldAuthenticated ? response.status !== 401 : response.status < 400 + const authenticated = oldAuthenticated ? response.status !== authErrorStatusCode : response.status < 400 plugin.setPluginState({ authenticated }) return response } - async setAuthToken (token) { + async setAuthToken(token) { return this.uppy.getPlugin(this.pluginId).storage.setItem(this.tokenKey, token) } - async #getAuthToken () { + async #getAuthToken() { return this.uppy.getPlugin(this.pluginId).storage.getItem(this.tokenKey) } - async #removeAuthToken () { + /** @protected */ + async removeAuthToken() { return this.uppy.getPlugin(this.pluginId).storage.removeItem(this.tokenKey) } @@ -81,7 +84,7 @@ export default class Provider extends RequestClient { * Ensure we have a preauth token if necessary. Attempts to fetch one if we don't, * or rejects if loading one fails. */ - async ensurePreAuth () { + async ensurePreAuth() { if (this.companionKeysParams && !this.preAuthToken) { await this.fetchPreAuthToken() @@ -91,11 +94,18 @@ export default class Provider extends RequestClient { } } - authUrl (queries = {}) { + // eslint-disable-next-line class-methods-use-this + authQuery() { + return {} + } + + authUrl({ authFormData, query } = {}) { const params = new URLSearchParams({ + ...query, state: btoa(JSON.stringify({ origin: getOrigin() })), - ...queries, + ...this.authQuery({ authFormData }), }) + if (this.preAuthToken) { params.set('uppyPreAuthToken', this.preAuthToken) } @@ -103,12 +113,24 @@ export default class Provider extends RequestClient { return `${this.hostname}/${this.id}/connect?${params}` } - async login (queries) { + /** @protected */ + async loginSimpleAuth({ uppyVersions, authFormData, signal }) { + const response = await this.post(`${this.id}/simple-auth`, { form: authFormData }, { qs: { uppyVersions }, signal }) + this.setAuthToken(response.uppyAuthToken) + } + + /** @protected */ + async loginOAuth({ uppyVersions, authFormData, signal }) { await this.ensurePreAuth() + signal.throwIfAborted() + return new Promise((resolve, reject) => { - const link = this.authUrl(queries) + const link = this.authUrl({ query: { uppyVersions }, authFormData }) const authWindow = window.open(link, '_blank') + + let cleanup + const handleToken = (e) => { if (e.source !== authWindow) { let jsonData = '' @@ -148,24 +170,35 @@ export default class Provider extends RequestClient { return } + cleanup() + resolve(this.setAuthToken(data.token)) + } + + cleanup = () => { authWindow.close() window.removeEventListener('message', handleToken) - this.setAuthToken(data.token).then(() => resolve()).catch(reject) + signal.removeEventListener('abort', cleanup) } + + signal.addEventListener('abort', cleanup) window.addEventListener('message', handleToken) }) } - refreshTokenUrl () { + async login({ uppyVersions, authFormData, signal }) { + return this.loginOAuth({ uppyVersions, authFormData, signal }) + } + + refreshTokenUrl() { return `${this.hostname}/${this.id}/refresh-token` } - fileUrl (id) { + fileUrl(id) { return `${this.hostname}/${this.id}/get/${id}` } /** @protected */ - async request (...args) { + async request(...args) { await this.#refreshingTokenPromise try { @@ -177,6 +210,7 @@ export default class Provider extends RequestClient { return await super.request(...args) } catch (err) { + if (!this.supportsRefreshToken) throw err // only handle auth errors (401 from provider), and only handle them if we have a (refresh) token const authTokenAfter = await this.#getAuthToken() if (!err.isAuthError || !authTokenAfter) throw err @@ -192,7 +226,7 @@ export default class Provider extends RequestClient { } catch (refreshTokenErr) { if (refreshTokenErr.isAuthError) { // if refresh-token has failed with auth error, delete token, so we don't keep trying to refresh in future - await this.#removeAuthToken() + await this.removeAuthToken() } throw err } finally { @@ -208,7 +242,7 @@ export default class Provider extends RequestClient { } } - async fetchPreAuthToken () { + async fetchPreAuthToken() { if (!this.companionKeysParams) { return } @@ -221,17 +255,17 @@ export default class Provider extends RequestClient { } } - list (directory, options) { + list(directory, options) { return this.get(`${this.id}/list/${directory || ''}`, options) } - async logout (options) { + async logout(options) { const response = await this.get(`${this.id}/logout`, options) - await this.#removeAuthToken() + await this.removeAuthToken() return response } - static initPlugin (plugin, opts, defaultOpts) { + static initPlugin(plugin, opts, defaultOpts) { /* eslint-disable no-param-reassign */ plugin.type = 'acquirer' plugin.files = [] diff --git a/packages/@uppy/companion-client/src/RequestClient.js b/packages/@uppy/companion-client/src/RequestClient.js index 972315b8ef..c0fae74069 100644 --- a/packages/@uppy/companion-client/src/RequestClient.js +++ b/packages/@uppy/companion-client/src/RequestClient.js @@ -1,5 +1,6 @@ 'use strict' +import UserFacingApiError from '@uppy/utils/lib/UserFacingApiError' // eslint-disable-next-line import/no-extraneous-dependencies import pRetry, { AbortError } from 'p-retry' @@ -13,25 +14,26 @@ import AuthError from './AuthError.js' import packageJson from '../package.json' // Remove the trailing slash so we can always safely append /xyz. -function stripSlash (url) { +function stripSlash(url) { return url.replace(/\/$/, '') } const retryCount = 10 // set to a low number, like 2 to test manual user retries const socketActivityTimeoutMs = 5 * 60 * 1000 // set to a low number like 10000 to test this -const authErrorStatusCode = 401 +export const authErrorStatusCode = 401 class HttpError extends Error { statusCode constructor({ statusCode, message }) { super(message) + this.name = 'HttpError' this.statusCode = statusCode } } -async function handleJSONResponse (res) { +async function handleJSONResponse(res) { if (res.status === authErrorStatusCode) { throw new AuthError() } @@ -41,15 +43,19 @@ async function handleJSONResponse (res) { } let errMsg = `Failed request with status: ${res.status}. ${res.statusText}` + let errData try { - const errData = await res.json() - - errMsg = errData.message ? `${errMsg} message: ${errData.message}` : errMsg - errMsg = errData.requestId - ? `${errMsg} request-Id: ${errData.requestId}` - : errMsg - } catch { - /* if the response contains invalid JSON, let's ignore the error */ + errData = await res.json() + + if (errData.message) errMsg = `${errMsg} message: ${errData.message}` + if (errData.requestId) errMsg = `${errMsg} request-Id: ${errData.requestId}` + } catch (cause) { + // if the response contains invalid JSON, let's ignore the error data + throw new Error(errMsg, { cause }) + } + + if (res.status >= 400 && res.status <= 499 && errData.message) { + throw new UserFacingApiError(errData.message) } throw new HttpError({ statusCode: res.status, message: errMsg }) @@ -60,22 +66,22 @@ export default class RequestClient { #companionHeaders - constructor (uppy, opts) { + constructor(uppy, opts) { this.uppy = uppy this.opts = opts this.onReceiveResponse = this.onReceiveResponse.bind(this) this.#companionHeaders = opts?.companionHeaders } - setCompanionHeaders (headers) { + setCompanionHeaders(headers) { this.#companionHeaders = headers } - [Symbol.for('uppy test: getCompanionHeaders')] () { + [Symbol.for('uppy test: getCompanionHeaders')]() { return this.#companionHeaders } - get hostname () { + get hostname() { const { companion } = this.uppy.getState() const host = this.opts.companionUrl return stripSlash(companion && companion[host] ? companion[host] : host) @@ -96,7 +102,7 @@ export default class RequestClient { } } - onReceiveResponse ({ headers }) { + onReceiveResponse({ headers }) { const state = this.uppy.getState() const companion = state.companion || {} const host = this.opts.companionUrl @@ -109,7 +115,7 @@ export default class RequestClient { } } - #getUrl (url) { + #getUrl(url) { if (/^(https?:|)\/\//.test(url)) { return url } @@ -117,7 +123,7 @@ export default class RequestClient { } /** @protected */ - async request ({ path, method = 'GET', data, skipPostResponse, signal }) { + async request({ path, method = 'GET', data, skipPostResponse, signal }) { try { const headers = await this.headers(!data) const response = await fetchWithNetworkError(this.#getUrl(path), { @@ -132,7 +138,7 @@ export default class RequestClient { return await handleJSONResponse(response) } catch (err) { // pass these through - if (err instanceof AuthError || err.name === 'AbortError') throw err + if (err.isAuthError || err.name === 'UserFacingApiError' || err.name === 'AbortError') throw err throw new ErrorWithCause(`Could not ${method} ${this.#getUrl(path)}`, { cause: err, @@ -140,21 +146,21 @@ export default class RequestClient { } } - async get (path, options = undefined) { + async get(path, options = undefined) { // 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, data, options = undefined) { + async post(path, data, options = undefined) { // 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, data = undefined, options) { + async delete(path, data = undefined, options) { // 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 } @@ -174,7 +180,7 @@ export default class RequestClient { * @param {*} options * @returns */ - async uploadRemoteFile (file, reqBody, options = {}) { + async uploadRemoteFile(file, reqBody, options = {}) { try { const { signal, getQueue } = options @@ -191,7 +197,7 @@ export default class RequestClient { return await this.#requestSocketToken(...args) } catch (outerErr) { // throwing AbortError will cause p-retry to stop retrying - if (outerErr instanceof AuthError) throw new AbortError(outerErr) + if (outerErr.isAuthError) throw new AbortError(outerErr) if (outerErr.cause == null) throw outerErr const err = outerErr.cause @@ -200,7 +206,7 @@ export default class RequestClient { [408, 409, 429, 418, 423].includes(err.statusCode) || (err.statusCode >= 500 && err.statusCode <= 599 && ![501, 505].includes(err.statusCode)) ) - if (err instanceof HttpError && !isRetryableHttpError()) throw new AbortError(err); + if (err.name === 'HttpError' && !isRetryableHttpError()) throw new AbortError(err); // p-retry will retry most other errors, // but it will not retry TypeError (except network error TypeErrors) @@ -253,7 +259,7 @@ export default class RequestClient { * * @param {{ file: UppyFile, queue: RateLimitedQueue, signal: AbortSignal }} file */ - async #awaitRemoteFileUpload ({ file, queue, signal }) { + async #awaitRemoteFileUpload({ file, queue, signal }) { let removeEventHandlers const { capabilities } = this.uppy.getState() diff --git a/packages/@uppy/companion/src/companion.js b/packages/@uppy/companion/src/companion.js index c3deec6f92..564a108fb7 100644 --- a/packages/@uppy/companion/src/companion.js +++ b/packages/@uppy/companion/src/companion.js @@ -16,7 +16,7 @@ const jobs = require('./server/jobs') const logger = require('./server/logger') const middlewares = require('./server/middlewares') const { getMaskableSecrets, defaultOptions, validateConfig } = require('./config/companion') -const { ProviderApiError, ProviderAuthError } = require('./server/provider/error') +const { ProviderApiError, ProviderUserError, ProviderAuthError } = require('./server/provider/error') const { getCredentialsOverrideMiddleware } = require('./server/provider/credentials') // @ts-ignore const { version } = require('../package.json') @@ -52,7 +52,7 @@ const interceptGrantErrorResponse = interceptor((req, res) => { }) // make the errors available publicly for custom providers -module.exports.errors = { ProviderApiError, ProviderAuthError } +module.exports.errors = { ProviderApiError, ProviderUserError, ProviderAuthError } module.exports.socket = require('./server/socket') module.exports.setLoggerProcessName = setLoggerProcessName @@ -126,6 +126,8 @@ module.exports.app = (optionsArg = {}) => { app.get('/:providerName/logout', middlewares.hasSessionAndProvider, middlewares.hasOAuthProvider, middlewares.gentleVerifyToken, controllers.logout) app.get('/:providerName/send-token', middlewares.hasSessionAndProvider, middlewares.hasOAuthProvider, middlewares.verifyToken, controllers.sendToken) + app.post('/:providerName/simple-auth', express.json(), middlewares.hasSessionAndProvider, middlewares.hasBody, middlewares.hasSimpleAuthProvider, controllers.simpleAuth) + app.get('/:providerName/list/:id?', middlewares.hasSessionAndProvider, middlewares.verifyToken, controllers.list) // backwards compat: app.get('/search/:providerName/list', middlewares.hasSessionAndProvider, middlewares.verifyToken, controllers.list) @@ -140,8 +142,8 @@ module.exports.app = (optionsArg = {}) => { if (options.testDynamicOauthCredentials) { app.post('/:providerName/test-dynamic-oauth-credentials', (req, res) => { if (req.query.secret !== options.testDynamicOauthCredentialsSecret) throw new Error('Invalid secret') - logger.info('Returning dynamic OAuth2 credentials') const { providerName } = req.params + logger.info(`Returning dynamic OAuth2 credentials for ${providerName}`) // for simplicity, we just return the normal credentials for the provider, but in a real-world scenario, // we would query based on parameters const { key, secret } = options.providerOptions[providerName] diff --git a/packages/@uppy/companion/src/server/Uploader.js b/packages/@uppy/companion/src/server/Uploader.js index 994580513b..dd10fb9718 100644 --- a/packages/@uppy/companion/src/server/Uploader.js +++ b/packages/@uppy/companion/src/server/Uploader.js @@ -41,12 +41,12 @@ const PROTOCOLS = Object.freeze({ tus: 'tus', }) -function exceedsMaxFileSize (maxFileSize, size) { +function exceedsMaxFileSize(maxFileSize, size) { return maxFileSize && size && size > maxFileSize } // TODO remove once we migrate away from form-data -function sanitizeMetadata (inputMetadata) { +function sanitizeMetadata(inputMetadata) { if (inputMetadata == null) return {} const outputMetadata = {} @@ -56,16 +56,24 @@ function sanitizeMetadata (inputMetadata) { return outputMetadata } -class AbortError extends Error {} +class AbortError extends Error { + isAbortError = true +} + +class ValidationError extends Error { + constructor(message) { + super(message) -class ValidationError extends Error {} + this.name = 'ValidationError' + } +} /** * Validate the options passed down to the uplaoder * * @param {UploaderOptions} options */ -function validateOptions (options) { +function validateOptions(options) { // validate HTTP Method if (options.httpMethod) { if (typeof options.httpMethod !== 'string') { @@ -155,7 +163,7 @@ class Uploader { * * @param {UploaderOptions} options */ - constructor (options) { + constructor(options) { validateOptions(options) this.options = options @@ -200,18 +208,18 @@ class Uploader { this._paused = true if (this.tus) { const shouldTerminate = !!this.tus.url - this.tus.abort(shouldTerminate).catch(() => {}) + this.tus.abort(shouldTerminate).catch(() => { }) } this.abortReadStream(new AbortError()) }) } - abortReadStream (err) { + abortReadStream(err) { this.uploadStopped = true if (this.readStream) this.readStream.destroy(err) } - async _uploadByProtocol () { + async _uploadByProtocol() { // todo a default protocol should not be set. We should ensure that the user specifies their protocol. // after we drop old versions of uppy client we can remove this const protocol = this.options.protocol || PROTOCOLS.multipart @@ -228,7 +236,7 @@ class Uploader { } } - async _downloadStreamAsFile (stream) { + async _downloadStreamAsFile(stream) { this.tmpPath = join(this.options.pathPrefix, this.fileName) logger.debug('fully downloading file', 'uploader.download', this.shortToken) @@ -253,7 +261,7 @@ class Uploader { this.readStream = fileStream } - _needDownloadFirst () { + _needDownloadFirst() { return !this.options.size || !this.options.companionOptions.streamingUpload } @@ -261,7 +269,7 @@ class Uploader { * * @param {import('stream').Readable} stream */ - async uploadStream (stream) { + async uploadStream(stream) { try { if (this.uploadStopped) throw new Error('Cannot upload stream after upload stopped') if (this.readStream) throw new Error('Already uploading') @@ -289,15 +297,15 @@ class Uploader { } } - tryDeleteTmpPath () { - if (this.tmpPath) unlink(this.tmpPath).catch(() => {}) + tryDeleteTmpPath() { + if (this.tmpPath) unlink(this.tmpPath).catch(() => { }) } /** * * @param {import('stream').Readable} stream */ - async tryUploadStream (stream) { + async tryUploadStream(stream) { try { emitter().emit('upload-start', { token: this.token }) @@ -306,7 +314,7 @@ class Uploader { const { url, extraData } = ret this.#emitSuccess(url, extraData) } catch (err) { - if (err instanceof AbortError) { + if (err?.isAbortError) { logger.error('Aborted upload', 'uploader.aborted', this.shortToken) return } @@ -328,11 +336,11 @@ class Uploader { * @param {string} token the token to Shorten * @returns {string} */ - static shortenToken (token) { + static shortenToken(token) { return token.substring(0, 8) } - static reqToOptions (req, size) { + static reqToOptions(req, size) { const useFormDataIsSet = Object.prototype.hasOwnProperty.call(req.body, 'useFormData') const useFormData = useFormDataIsSet ? req.body.useFormData : true @@ -365,11 +373,11 @@ class Uploader { * we avoid using the entire token because this is meant to be a short term * access token between uppy client and companion websocket */ - get shortToken () { + get shortToken() { return Uploader.shortenToken(this.token) } - async awaitReady (timeout) { + 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 @@ -379,7 +387,7 @@ class Uploader { let timer let onEvent - function cleanup () { + function cleanup() { emitter().removeListener(eventName, onEvent) clearTimeout(timer) } @@ -407,7 +415,7 @@ class Uploader { * @typedef {{action: string, payload: object}} State * @param {State} state */ - saveState (state) { + saveState(state) { if (!this.storage) return // make sure the keys get cleaned up. // https://github.com/transloadit/uppy/issues/3748 @@ -434,7 +442,7 @@ class Uploader { * @param {number} [bytesUploaded] * @param {number | null} [bytesTotalIn] */ - onProgress (bytesUploaded = 0, bytesTotalIn = 0) { + onProgress(bytesUploaded = 0, bytesTotalIn = 0) { const bytesTotal = bytesTotalIn || this.size || 0 // If fully downloading before uploading, combine downloaded and uploaded bytes @@ -470,7 +478,7 @@ class Uploader { * @param {string} url * @param {object} extraData */ - #emitSuccess (url, extraData) { + #emitSuccess(url, extraData) { const emitData = { action: 'success', payload: { ...extraData, complete: true, url }, @@ -483,7 +491,7 @@ class Uploader { * * @param {Error} err */ - #emitError (err) { + #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 @@ -502,7 +510,7 @@ class Uploader { * * @param {any} stream */ - async #uploadTus (stream) { + async #uploadTus(stream) { const uploader = this const isFileStream = stream instanceof ReadStream @@ -531,7 +539,7 @@ class Uploader { * * @param {Error} error */ - onError (error) { + onError(error) { logger.error(error, 'uploader.tus.error') // deleting tus originalRequest field because it uses the same http-agent // as companion, and this agent may contain sensitive request details (e.g headers) @@ -550,10 +558,10 @@ class Uploader { * @param {number} [bytesUploaded] * @param {number} [bytesTotal] */ - onProgress (bytesUploaded, bytesTotal) { + onProgress(bytesUploaded, bytesTotal) { uploader.onProgress(bytesUploaded, bytesTotal) }, - onSuccess () { + onSuccess() { resolve({ url: uploader.tus.url }) }, }) @@ -564,12 +572,12 @@ class Uploader { }) } - async #uploadMultipart (stream) { + async #uploadMultipart(stream) { if (!this.options.endpoint) { throw new Error('No multipart endpoint set') } - function getRespObj (response) { + function getRespObj(response) { // remove browser forbidden headers const { 'set-cookie': deleted, 'set-cookie2': deleted2, ...responseHeaders } = response.headers @@ -642,7 +650,7 @@ class Uploader { /** * Upload the file to S3 using a Multipart upload. */ - async #uploadS3Multipart (stream) { + async #uploadS3Multipart(stream) { if (!this.options.s3) { throw new Error('The S3 client is not configured on this companion instance.') } diff --git a/packages/@uppy/companion/src/server/controllers/callback.js b/packages/@uppy/companion/src/server/controllers/callback.js index cee35ced16..effcd397fc 100644 --- a/packages/@uppy/companion/src/server/controllers/callback.js +++ b/packages/@uppy/companion/src/server/controllers/callback.js @@ -33,27 +33,30 @@ module.exports = function callback (req, res, next) { // eslint-disable-line no- const grant = req.session.grant || {} + const grantDynamic = oAuthState.getGrantDynamicFromRequest(req) + const origin = grantDynamic.state && oAuthState.getFromState(grantDynamic.state, 'origin', req.companion.options.secret) + if (!grant.response?.access_token) { logger.debug(`Did not receive access token for provider ${providerName}`, null, req.id) logger.debug(grant.response, 'callback.oauth.resp', req.id) - const state = oAuthState.getDynamicStateFromRequest(req) - const origin = state && oAuthState.getFromState(state, 'origin', req.companion.options.secret) return res.status(400).send(closePageHtml(origin)) } const { access_token: accessToken, refresh_token: refreshToken } = grant.response - if (!req.companion.allProvidersTokens) req.companion.allProvidersTokens = {} - req.companion.allProvidersTokens[providerName] = { + req.companion.providerUserSession = { accessToken, refreshToken, // might be undefined for some providers + ...req.companion.providerClass.grantDynamicToUserSession({ grantDynamic }), } + logger.debug(`Generating auth token for provider ${providerName}. refreshToken: ${refreshToken ? 'yes' : 'no'}`, null, req.id) const uppyAuthToken = tokenService.generateEncryptedAuthToken( - req.companion.allProvidersTokens, req.companion.options.secret, + { [providerName]: req.companion.providerUserSession }, + req.companion.options.secret, req.companion.providerClass.authStateExpiry, ) - tokenService.addToCookiesIfNeeded(req, res, uppyAuthToken) + tokenService.addToCookiesIfNeeded(req, res, uppyAuthToken, req.companion.providerClass.authStateExpiry) return res.redirect(req.companion.buildURL(`/${providerName}/send-token?uppyAuthToken=${uppyAuthToken}`, true)) } diff --git a/packages/@uppy/companion/src/server/controllers/connect.js b/packages/@uppy/companion/src/server/controllers/connect.js index 7e8f9b8e5d..1bfbc06593 100644 --- a/packages/@uppy/companion/src/server/controllers/connect.js +++ b/packages/@uppy/companion/src/server/controllers/connect.js @@ -1,27 +1,56 @@ const atob = require('atob') const oAuthState = require('../helpers/oauth-state') +const queryString = (params, prefix = '?') => { + const str = new URLSearchParams(params).toString() + return str ? `${prefix}${str}` : '' +} + /** * initializes the oAuth flow for a provider. * * @param {object} req * @param {object} res */ -module.exports = function connect (req, res) { +module.exports = function connect(req, res) { const { secret } = req.companion.options - let state = oAuthState.generateState(secret) + const stateObj = oAuthState.generateState() + if (req.query.state) { - const origin = JSON.parse(atob(req.query.state)) - state = oAuthState.addToState(state, origin, secret) + const { origin } = JSON.parse(atob(req.query.state)) + stateObj.origin = origin } if (req.companion.options.server.oauthDomain) { - state = oAuthState.addToState(state, { companionInstance: req.companion.buildURL('', true) }, secret) + stateObj.companionInstance = req.companion.buildURL('', true) } if (req.query.uppyPreAuthToken) { - state = oAuthState.addToState(state, { preAuthToken: req.query.uppyPreAuthToken }, secret) + stateObj.preAuthToken = req.query.uppyPreAuthToken } - res.redirect(req.companion.buildURL(`/connect/${req.companion.provider.authProvider}?state=${state}`, true)) + const state = oAuthState.encodeState(stateObj, secret) + const { provider, providerGrantConfig } = req.companion + + // pass along grant's dynamic config (if specified for the provider in its grant config `dynamic` section) + // this is needed for things like custom oauth domain (e.g. webdav) + const grantDynamicConfig = Object.fromEntries(providerGrantConfig.dynamic?.flatMap((dynamicKey) => { + const queryValue = req.query[dynamicKey]; + + // note: when using credentialsURL (dynamic oauth credentials), dynamic has ['key', 'secret', 'redirect_uri'] + // but in that case, query string is empty, so we need to only fetch these parameters from QS if they exist. + if (!queryValue) return [] + return [[ + dynamicKey, queryValue + ]] + }) || []) + + const providerName = provider.authProvider + const qs = queryString({ + ...grantDynamicConfig, + state, + }) + + // Now we redirect to grant's /connect endpoint, see `app.use(Grant(grantConfig))` + res.redirect(req.companion.buildURL(`/connect/${providerName}${qs}`, true)) } diff --git a/packages/@uppy/companion/src/server/controllers/get.js b/packages/@uppy/companion/src/server/controllers/get.js index f160e5c66f..e3bd4da759 100644 --- a/packages/@uppy/companion/src/server/controllers/get.js +++ b/packages/@uppy/companion/src/server/controllers/get.js @@ -3,7 +3,8 @@ const { startDownUpload } = require('../helpers/upload') async function get (req, res) { const { id } = req.params - const { accessToken } = req.companion.providerTokens + const { providerUserSession } = req.companion + const { accessToken } = providerUserSession const { provider } = req.companion async function getSize () { @@ -11,7 +12,7 @@ async function get (req, res) { } async function download () { - const { stream } = await provider.download({ id, token: accessToken, query: req.query }) + const { stream } = await provider.download({ id, token: accessToken, providerUserSession, query: req.query }) return stream } diff --git a/packages/@uppy/companion/src/server/controllers/index.js b/packages/@uppy/companion/src/server/controllers/index.js index 4410eec3b9..90dd93fb12 100644 --- a/packages/@uppy/companion/src/server/controllers/index.js +++ b/packages/@uppy/companion/src/server/controllers/index.js @@ -6,6 +6,7 @@ module.exports = { get: require('./get'), thumbnail: require('./thumbnail'), list: require('./list'), + simpleAuth: require('./simple-auth'), logout: require('./logout'), connect: require('./connect'), preauth: require('./preauth'), diff --git a/packages/@uppy/companion/src/server/controllers/list.js b/packages/@uppy/companion/src/server/controllers/list.js index 284660c1be..43afb0b7c9 100644 --- a/packages/@uppy/companion/src/server/controllers/list.js +++ b/packages/@uppy/companion/src/server/controllers/list.js @@ -1,10 +1,14 @@ const { respondWithError } = require('../provider/error') async function list ({ query, params, companion }, res, next) { - const { accessToken } = companion.providerTokens + const { providerUserSession } = companion + const { accessToken } = providerUserSession try { - const data = await companion.provider.list({ companion, token: accessToken, directory: params.id, query }) + // todo remove backward compat `token` param from all provider methods (because it can be found in providerUserSession) + const data = await companion.provider.list({ + companion, token: accessToken, providerUserSession, directory: params.id, query, + }) res.json(data) } catch (err) { if (respondWithError(err, res)) return diff --git a/packages/@uppy/companion/src/server/controllers/logout.js b/packages/@uppy/companion/src/server/controllers/logout.js index 1ec87e303f..741dc61dda 100644 --- a/packages/@uppy/companion/src/server/controllers/logout.js +++ b/packages/@uppy/companion/src/server/controllers/logout.js @@ -13,20 +13,19 @@ async function logout (req, res, next) { req.session.grant.dynamic = null } } - const { providerName } = req.params const { companion } = req - const tokens = companion.allProvidersTokens ? companion.allProvidersTokens[providerName] : null + const { providerUserSession } = companion - if (!tokens) { + if (!providerUserSession) { cleanSession() res.json({ ok: true, revoked: false }) return } try { - const { accessToken } = tokens - const data = await companion.provider.logout({ token: accessToken, companion }) - delete companion.allProvidersTokens[providerName] + const { accessToken } = providerUserSession + const data = await companion.provider.logout({ token: accessToken, providerUserSession, companion }) + delete companion.providerUserSession tokenService.removeFromCookies(res, companion.options, companion.provider.authProvider) cleanSession() res.json({ ok: true, ...data }) diff --git a/packages/@uppy/companion/src/server/controllers/oauth-redirect.js b/packages/@uppy/companion/src/server/controllers/oauth-redirect.js index a0ef7ba477..ff4f095448 100644 --- a/packages/@uppy/companion/src/server/controllers/oauth-redirect.js +++ b/packages/@uppy/companion/src/server/controllers/oauth-redirect.js @@ -16,7 +16,7 @@ module.exports = function oauthRedirect (req, res) { return } - const state = oAuthState.getDynamicStateFromRequest(req) + const { state } = oAuthState.getGrantDynamicFromRequest(req) if (!state) { res.status(400).send('Cannot find state in session') return diff --git a/packages/@uppy/companion/src/server/controllers/refresh-token.js b/packages/@uppy/companion/src/server/controllers/refresh-token.js index ab37cd3cd7..ed6d7a4748 100644 --- a/packages/@uppy/companion/src/server/controllers/refresh-token.js +++ b/packages/@uppy/companion/src/server/controllers/refresh-token.js @@ -11,10 +11,10 @@ async function refreshToken (req, res, next) { const { key: clientId, secret: clientSecret } = req.companion.options.providerOptions[providerName] const { redirect_uri: redirectUri } = req.companion.providerGrantConfig - const providerTokens = req.companion.allProvidersTokens[providerName] + const { providerUserSession } = req.companion // not all providers have refresh tokens - if (providerTokens.refreshToken == null || providerTokens.refreshToken === '') { + if (providerUserSession.refreshToken == null || providerUserSession.refreshToken === '') { logger.warn('Tried to refresh token without having a token') res.sendStatus(401) return @@ -22,26 +22,21 @@ async function refreshToken (req, res, next) { try { const data = await req.companion.provider.refreshToken({ - redirectUri, clientId, clientSecret, refreshToken: providerTokens.refreshToken, + redirectUri, clientId, clientSecret, refreshToken: providerUserSession.refreshToken, }) - const newAllProvidersTokens = { - ...req.companion.allProvidersTokens, - [providerName]: { - ...providerTokens, - accessToken: data.accessToken, - }, + req.companion.providerUserSession = { + ...providerUserSession, + accessToken: data.accessToken, } - req.companion.allProvidersTokens = newAllProvidersTokens - req.companion.providerTokens = newAllProvidersTokens[providerName] - logger.debug(`Generating refreshed auth token for provider ${providerName}`, null, req.id) const uppyAuthToken = tokenService.generateEncryptedAuthToken( - req.companion.allProvidersTokens, req.companion.options.secret, + { [providerName]: req.companion.providerUserSession }, + req.companion.options.secret, req.companion.providerClass.authStateExpiry, ) - tokenService.addToCookiesIfNeeded(req, res, uppyAuthToken) + tokenService.addToCookiesIfNeeded(req, res, uppyAuthToken, req.companion.providerClass.authStateExpiry) res.send({ uppyAuthToken }) } catch (err) { diff --git a/packages/@uppy/companion/src/server/controllers/send-token.js b/packages/@uppy/companion/src/server/controllers/send-token.js index 17cc4c1601..60bac3474e 100644 --- a/packages/@uppy/companion/src/server/controllers/send-token.js +++ b/packages/@uppy/companion/src/server/controllers/send-token.js @@ -33,7 +33,7 @@ const htmlContent = (token, origin) => { module.exports = function sendToken (req, res, next) { const uppyAuthToken = req.companion.authToken - const state = oAuthState.getDynamicStateFromRequest(req) + const { state } = oAuthState.getGrantDynamicFromRequest(req) if (state) { const origin = oAuthState.getFromState(state, 'origin', req.companion.options.secret) const allowedClients = req.companion.options.clients diff --git a/packages/@uppy/companion/src/server/controllers/simple-auth.js b/packages/@uppy/companion/src/server/controllers/simple-auth.js new file mode 100644 index 0000000000..91d900a152 --- /dev/null +++ b/packages/@uppy/companion/src/server/controllers/simple-auth.js @@ -0,0 +1,31 @@ +const tokenService = require('../helpers/jwt') +const { respondWithError } = require('../provider/error') +const logger = require('../logger') + +async function simpleAuth (req, res, next) { + const { providerName } = req.params + + try { + const simpleAuthResponse = await req.companion.provider.simpleAuth({ requestBody: req.body }) + + req.companion.providerUserSession = { + ...req.companion.providerUserSession, + ...simpleAuthResponse, + } + + logger.debug(`Generating simple auth token for provider ${providerName}`, null, req.id) + const uppyAuthToken = tokenService.generateEncryptedAuthToken( + { [providerName]: req.companion.providerUserSession }, + req.companion.options.secret, req.companion.providerClass.authStateExpiry, + ) + + tokenService.addToCookiesIfNeeded(req, res, uppyAuthToken, req.companion.providerClass.authStateExpiry) + + res.send({ uppyAuthToken }) + } catch (err) { + if (respondWithError(err, res)) return + next(err) + } +} + +module.exports = simpleAuth diff --git a/packages/@uppy/companion/src/server/controllers/thumbnail.js b/packages/@uppy/companion/src/server/controllers/thumbnail.js index 0994964ed2..b578bbb5c2 100644 --- a/packages/@uppy/companion/src/server/controllers/thumbnail.js +++ b/packages/@uppy/companion/src/server/controllers/thumbnail.js @@ -1,19 +1,22 @@ +const { respondWithError } = require('../provider/error') + /** * * @param {object} req * @param {object} res */ async function thumbnail (req, res, next) { - const { providerName, id } = req.params - const { accessToken } = req.companion.allProvidersTokens[providerName] - const { provider } = req.companion + const { id } = req.params + const { provider, providerUserSession } = req.companion + const { accessToken } = providerUserSession try { - const { stream } = await provider.thumbnail({ id, token: accessToken }) + const { stream, contentType } = await provider.thumbnail({ id, token: accessToken, providerUserSession }) + if (contentType != null) res.set('Content-Type', contentType) stream.pipe(res) } catch (err) { - if (err.isAuthError) res.sendStatus(401) - else next(err) + if (respondWithError(err, res)) return + next(err) } } diff --git a/packages/@uppy/companion/src/server/helpers/jwt.js b/packages/@uppy/companion/src/server/helpers/jwt.js index b1a9b75dfd..0db6249a47 100644 --- a/packages/@uppy/companion/src/server/helpers/jwt.js +++ b/packages/@uppy/companion/src/server/helpers/jwt.js @@ -1,10 +1,13 @@ const jwt = require('jsonwebtoken') const { encrypt, decrypt } = require('./utils') -// The Uppy auth token is a (JWT) container around provider OAuth access & refresh tokens. -// Providers themselves will verify these inner tokens. +// The Uppy auth token is an encrypted JWT & JSON encoded container. +// It used to simply contain an OAuth access_token and refresh_token for a specific provider. +// However now we allow more data to be stored in it. This allows for storing other state or parameters needed for that +// specific provider, like username, password, host names etc. +// The different providers APIs themselves will verify these inner tokens through Provider classes. // The expiry of the Uppy auth token should be higher than the expiry of the refresh token. -// Because some refresh tokens never expire, we set the Uppy auth token expiry very high. +// Because some refresh tokens normally never expire, we set the Uppy auth token expiry very high. // Chrome has a maximum cookie expiry of 400 days, so we'll use that (we also store the auth token in a cookie) // // If the Uppy auth token expiry were set too low (e.g. 24hr), we could risk this situation: @@ -14,16 +17,21 @@ const { encrypt, decrypt } = require('./utils') // even though the provider refresh token would still have been accepted and // there's no way for them to retry their failed files. // With 400 days, there's still a theoretical possibility but very low. -const EXPIRY = 60 * 60 * 24 * 400 -const EXPIRY_MS = EXPIRY * 1000 +const MAX_AGE_REFRESH_TOKEN = 60 * 60 * 24 * 400 + +const MAX_AGE_24H = 60 * 60 * 24 + +module.exports.MAX_AGE_24H = MAX_AGE_24H +module.exports.MAX_AGE_REFRESH_TOKEN = MAX_AGE_REFRESH_TOKEN /** * * @param {*} data * @param {string} secret + * @param {number} maxAge */ -const generateToken = (data, secret) => { - return jwt.sign({ data }, secret, { expiresIn: EXPIRY }) +const generateToken = (data, secret, maxAge) => { + return jwt.sign({ data }, secret, { expiresIn: maxAge }) } /** @@ -41,18 +49,17 @@ const verifyToken = (token, secret) => { * @param {*} payload * @param {string} secret */ -module.exports.generateEncryptedToken = (payload, secret) => { +module.exports.generateEncryptedToken = (payload, secret, maxAge = MAX_AGE_24H) => { // return payload // for easier debugging - return encrypt(generateToken(payload, secret), secret) + return encrypt(generateToken(payload, secret, maxAge), secret) } /** - * * @param {*} payload * @param {string} secret */ -module.exports.generateEncryptedAuthToken = (payload, secret) => { - return module.exports.generateEncryptedToken(JSON.stringify(payload), secret) +module.exports.generateEncryptedAuthToken = (payload, secret, maxAge) => { + return module.exports.generateEncryptedToken(JSON.stringify(payload), secret, maxAge) } /** @@ -78,14 +85,15 @@ module.exports.verifyEncryptedAuthToken = (token, secret, providerName) => { return tokens } -const addToCookies = (res, token, companionOptions, authProvider, prefix) => { +function getCommonCookieOptions ({ companionOptions }) { const cookieOptions = { - maxAge: EXPIRY_MS, httpOnly: true, } // Fix to show thumbnails on Chrome // https://community.transloadit.com/t/dropbox-and-box-thumbnails-returning-401-unauthorized/15781/2 + // Note that sameSite cookies also require secure (which needs https), so thumbnails don't work from localhost + // to test locally, you can manually find the URL of the image and open it in a separate browser tab if (companionOptions.server && companionOptions.server.protocol === 'https') { cookieOptions.sameSite = 'none' cookieOptions.secure = true @@ -94,14 +102,32 @@ const addToCookies = (res, token, companionOptions, authProvider, prefix) => { if (companionOptions.cookieDomain) { cookieOptions.domain = companionOptions.cookieDomain } + + return cookieOptions +} + +const getCookieName = (authProvider) => `uppyAuthToken--${authProvider}` + +const addToCookies = ({ res, token, companionOptions, authProvider, maxAge = MAX_AGE_24H * 1000 }) => { + const cookieOptions = { + ...getCommonCookieOptions({ companionOptions }), + maxAge, + } + // send signed token to client. - res.cookie(`${prefix}--${authProvider}`, token, cookieOptions) + res.cookie(getCookieName(authProvider), token, cookieOptions) } -module.exports.addToCookiesIfNeeded = (req, res, uppyAuthToken) => { +module.exports.addToCookiesIfNeeded = (req, res, uppyAuthToken, maxAge) => { // some providers need the token in cookies for thumbnail/image requests if (req.companion.provider.needsCookieAuth) { - addToCookies(res, uppyAuthToken, req.companion.options, req.companion.provider.authProvider, 'uppyAuthToken') + addToCookies({ + res, + token: uppyAuthToken, + companionOptions: req.companion.options, + authProvider: req.companion.provider.authProvider, + maxAge, + }) } } @@ -112,14 +138,9 @@ module.exports.addToCookiesIfNeeded = (req, res, uppyAuthToken) => { * @param {string} authProvider */ module.exports.removeFromCookies = (res, companionOptions, authProvider) => { - const cookieOptions = { - maxAge: EXPIRY_MS, - httpOnly: true, - } - - if (companionOptions.cookieDomain) { - cookieOptions.domain = companionOptions.cookieDomain - } + // 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(`uppyAuthToken--${authProvider}`, cookieOptions) + res.clearCookie(getCookieName(authProvider), cookieOptions) } diff --git a/packages/@uppy/companion/src/server/helpers/oauth-state.js b/packages/@uppy/companion/src/server/helpers/oauth-state.js index d833376079..4a7d1a9b9c 100644 --- a/packages/@uppy/companion/src/server/helpers/oauth-state.js +++ b/packages/@uppy/companion/src/server/helpers/oauth-state.js @@ -2,33 +2,26 @@ const crypto = require('node:crypto') const atob = require('atob') const { encrypt, decrypt } = require('./utils') -const setState = (state, secret) => { +module.exports.encodeState = (state, secret) => { const encodedState = Buffer.from(JSON.stringify(state)).toString('base64') return encrypt(encodedState, secret) } -const getState = (state, secret) => { +const decodeState = (state, secret) => { const encodedState = decrypt(state, secret) return JSON.parse(atob(encodedState)) } -module.exports.generateState = (secret) => { - const state = {} - state.id = crypto.randomBytes(10).toString('hex') - return setState(state, secret) -} - -module.exports.addToState = (state, data, secret) => { - const stateObj = getState(state, secret) - return setState(Object.assign(stateObj, data), secret) +module.exports.generateState = () => { + return { + id: crypto.randomBytes(10).toString('hex'), + } } module.exports.getFromState = (state, name, secret) => { - return getState(state, secret)[name] + return decodeState(state, secret)[name] } -module.exports.getDynamicStateFromRequest = (req) => { - const dynamic = (req.session.grant || {}).dynamic || {} - const { state } = dynamic - return state +module.exports.getGrantDynamicFromRequest = (req) => { + return req.session.grant?.dynamic ?? {} } diff --git a/packages/@uppy/companion/src/server/helpers/upload.js b/packages/@uppy/companion/src/server/helpers/upload.js index 31554e4169..5f637c43bf 100644 --- a/packages/@uppy/companion/src/server/helpers/upload.js +++ b/packages/@uppy/companion/src/server/helpers/upload.js @@ -2,9 +2,7 @@ const Uploader = require('../Uploader') const logger = require('../logger') const { respondWithError } = require('../provider/error') -const { ValidationError } = Uploader - -async function startDownUpload ({ req, res, getSize, download }) { +async function startDownUpload({ req, res, getSize, download }) { try { const size = await getSize() const { clientSocketConnectTimeout } = req.companion.options @@ -15,22 +13,22 @@ async function startDownUpload ({ req, res, getSize, download }) { logger.debug('Starting download stream.', null, req.id) const stream = await download() - // "Forking" off the upload operation to background, so we can return the http request: - ;(async () => { - // wait till the client has connected to the socket, before starting - // the download, so that the client can receive all download/upload progress. - logger.debug('Waiting for socket connection before beginning remote download/upload.', null, req.id) - await uploader.awaitReady(clientSocketConnectTimeout) - logger.debug('Socket connection received. Starting remote download/upload.', null, req.id) + // "Forking" off the upload operation to background, so we can return the http request: + ; (async () => { + // wait till the client has connected to the socket, before starting + // the download, so that the client can receive all download/upload progress. + logger.debug('Waiting for socket connection before beginning remote download/upload.', null, req.id) + await uploader.awaitReady(clientSocketConnectTimeout) + logger.debug('Socket connection received. Starting remote download/upload.', null, req.id) - await uploader.tryUploadStream(stream) - })().catch((err) => logger.error(err)) + await uploader.tryUploadStream(stream) + })().catch((err) => logger.error(err)) // Respond the request // NOTE: the Uploader will continue running after the http request is responded res.status(200).json({ token: uploader.token }) } catch (err) { - if (err instanceof ValidationError) { + if (err.name === 'ValidationError') { logger.debug(err.message, 'uploader.validator.fail') res.status(400).json({ message: err.message }) return diff --git a/packages/@uppy/companion/src/server/helpers/utils.js b/packages/@uppy/companion/src/server/helpers/utils.js index fadae31b54..627d87fcb1 100644 --- a/packages/@uppy/companion/src/server/helpers/utils.js +++ b/packages/@uppy/companion/src/server/helpers/utils.js @@ -74,7 +74,7 @@ module.exports.getURLBuilder = (options) => { * * @param {string|Buffer} secret */ -function createSecret (secret) { +function createSecret(secret) { const hash = crypto.createHash('sha256') hash.update(secret) return hash.digest() @@ -85,15 +85,15 @@ function createSecret (secret) { * * @returns {Buffer} */ -function createIv () { +function createIv() { return crypto.randomBytes(16) } -function urlEncode (unencoded) { +function urlEncode(unencoded) { return unencoded.replace(/\+/g, '-').replace(/\//g, '_').replace(/=/g, '~') } -function urlDecode (encoded) { +function urlDecode(encoded) { return encoded.replace(/-/g, '+').replace(/_/g, '/').replace(/~/g, '=') } @@ -157,6 +157,7 @@ class StreamHttpJsonError extends Error { super(`Request failed with status ${statusCode}`) this.statusCode = statusCode this.responseJson = responseJson + this.name = 'StreamHttpJsonError' } } @@ -188,7 +189,7 @@ module.exports.prepareStream = async (stream) => new Promise((resolve, reject) = reject(err) }) - }) +}) module.exports.getBasicAuthHeader = (key, secret) => { const base64 = Buffer.from(`${key}:${secret}`, 'binary').toString('base64') diff --git a/packages/@uppy/companion/src/server/logger.js b/packages/@uppy/companion/src/server/logger.js index 112a35555b..f2788ff18f 100644 --- a/packages/@uppy/companion/src/server/logger.js +++ b/packages/@uppy/companion/src/server/logger.js @@ -1,7 +1,6 @@ const chalk = require('chalk') const escapeStringRegexp = require('escape-string-regexp') const util = require('node:util') -const { ProviderApiError, ProviderAuthError } = require('./provider/error') const valuesToMask = [] /** @@ -24,7 +23,7 @@ exports.setMaskables = (maskables) => { * @param {string} msg the message whose content should be masked * @returns {string} */ -function maskMessage (msg) { +function maskMessage(msg) { let out = msg for (const toBeMasked of valuesToMask) { const toBeReplaced = new RegExp(toBeMasked, 'gi') @@ -53,10 +52,11 @@ const log = ({ arg, tag = '', level, traceId = '', color = (message) => message const time = new Date().toISOString() const whitespace = tag && traceId ? ' ' : '' - function msgToString () { + function msgToString() { // We don't need to log stack trace on special errors that we ourselves have produced // (to reduce log noise) - if ((arg instanceof ProviderApiError || arg instanceof ProviderAuthError) && typeof arg.message === 'string') { + // @ts-ignore + if ((arg instanceof Error && arg.name === 'ProviderApiError') && typeof arg.message === 'string') { return arg.message } if (typeof arg === 'string') return arg diff --git a/packages/@uppy/companion/src/server/middlewares.js b/packages/@uppy/companion/src/server/middlewares.js index 7c9cf93cc1..73e8d798cb 100644 --- a/packages/@uppy/companion/src/server/middlewares.js +++ b/packages/@uppy/companion/src/server/middlewares.js @@ -24,6 +24,7 @@ exports.hasSessionAndProvider = (req, res, next) => { } const isOAuthProviderReq = (req) => isOAuthProvider(req.companion.providerClass.authProvider) +const isSimpleAuthProviderReq = (req) => !!req.companion.providerClass.hasSimpleAuth /** * Middleware can be used to verify that the current request is to an OAuth provider @@ -38,6 +39,15 @@ exports.hasOAuthProvider = (req, res, next) => { return next() } +exports.hasSimpleAuthProvider = (req, res, next) => { + if (!isSimpleAuthProviderReq(req)) { + logger.debug('Provider does not support simple auth.', null, req.id) + return res.sendStatus(400) + } + + return next() +} + exports.hasBody = (req, res, next) => { if (!req.body) { logger.debug('No body attached to req object. Exiting dispatcher.', null, req.id) @@ -57,7 +67,28 @@ exports.hasSearchQuery = (req, res, next) => { } exports.verifyToken = (req, res, next) => { - // for non oauth providers, we just load the static key from options + if (isOAuthProviderReq(req) || isSimpleAuthProviderReq(req)) { + // For OAuth / simple auth provider, we find the encrypted auth token from the header: + const token = req.companion.authToken + if (token == null) { + logger.info('cannot auth token', 'token.verify.unset', req.id) + res.sendStatus(401) + return + } + const { providerName } = req.params + try { + const payload = tokenService.verifyEncryptedAuthToken(token, req.companion.options.secret, providerName) + req.companion.providerUserSession = payload[providerName] + } catch (err) { + logger.error(err.message, 'token.verify.error', req.id) + res.sendStatus(401) + return + } + next() + return + } + + // for non auth providers, we just load the static key from options if (!isOAuthProviderReq(req)) { const { providerOptions } = req.companion.options const { providerName } = req.params @@ -67,31 +98,11 @@ exports.verifyToken = (req, res, next) => { return } - req.companion.providerTokens = { + req.companion.providerUserSession = { accessToken: providerOptions[providerName].key, } next() - return - } - - // Ok, OAuth provider, we fetch the token: - const token = req.companion.authToken - if (token == null) { - logger.info('cannot auth token', 'token.verify.unset', req.id) - res.sendStatus(401) - return - } - const { providerName } = req.params - try { - const payload = tokenService.verifyEncryptedAuthToken(token, req.companion.options.secret, providerName) - req.companion.allProvidersTokens = payload - req.companion.providerTokens = payload[providerName] - } catch (err) { - logger.error(err.message, 'token.verify.error', req.id) - res.sendStatus(401) - return } - next() } // does not fail if token is invalid @@ -102,7 +113,7 @@ exports.gentleVerifyToken = (req, res, next) => { const payload = tokenService.verifyEncryptedAuthToken( req.companion.authToken, req.companion.options.secret, providerName, ) - req.companion.allProvidersTokens = payload + req.companion.providerUserSession = payload[providerName] } catch (err) { logger.error(err.message, 'token.gentle.verify.error', req.id) } diff --git a/packages/@uppy/companion/src/server/provider/Provider.js b/packages/@uppy/companion/src/server/provider/Provider.js index 074d18d156..e649cab1a6 100644 --- a/packages/@uppy/companion/src/server/provider/Provider.js +++ b/packages/@uppy/companion/src/server/provider/Provider.js @@ -1,20 +1,24 @@ +const { MAX_AGE_24H } = require('../helpers/jwt') + /** * Provider interface defines the specifications of any provider implementation */ class Provider { /** * - * @param {{providerName: string, allowLocalUrls: boolean}} options + * @param {{providerName: string, allowLocalUrls: boolean, providerGrantConfig?: object}} options */ - constructor ({ allowLocalUrls }) { + constructor ({ allowLocalUrls, providerGrantConfig }) { // Some providers might need cookie auth for the thumbnails fetched via companion this.needsCookieAuth = false this.allowLocalUrls = allowLocalUrls + this.providerGrantConfig = providerGrantConfig return this } /** * config to extend the grant config + * todo major: rename to getExtraGrantConfig */ static getExtraConfig () { return {} @@ -85,13 +89,36 @@ class Provider { } /** - * Name of the OAuth provider. Return empty string if no OAuth provider is needed. + * @param {any} param0 + * @returns {Promise} + */ + // eslint-disable-next-line no-unused-vars, class-methods-use-this + async simpleAuth ({ requestBody }) { + throw new Error('method not implemented') + } + + /** + * Name of the OAuth provider (passed to Grant). Return empty string if no OAuth provider is needed. * * @returns {string} */ + // todo next major: rename authProvider to oauthProvider (we have other non-oauth auth types too now) static get authProvider () { return undefined } + + // eslint-disable-next-line no-unused-vars + static grantDynamicToUserSession ({ grantDynamic }) { + return {} + } + + static get hasSimpleAuth () { + return false + } + + static get authStateExpiry () { + return MAX_AGE_24H + } } module.exports = Provider diff --git a/packages/@uppy/companion/src/server/provider/box/index.js b/packages/@uppy/companion/src/server/provider/box/index.js index a05913e435..0de108136e 100644 --- a/packages/@uppy/companion/src/server/provider/box/index.js +++ b/packages/@uppy/companion/src/server/provider/box/index.js @@ -88,7 +88,7 @@ class Box extends Provider { }) await prepareStream(stream) - return { stream } + return { stream, contentType: 'image/jpeg' } }) } diff --git a/packages/@uppy/companion/src/server/provider/credentials.js b/packages/@uppy/companion/src/server/provider/credentials.js index 20fbe39dff..154378f9a6 100644 --- a/packages/@uppy/companion/src/server/provider/credentials.js +++ b/packages/@uppy/companion/src/server/provider/credentials.js @@ -83,17 +83,15 @@ exports.getCredentialsOverrideMiddleware = (providers, companionOptions) => { return } - const dynamicState = oAuthState.getDynamicStateFromRequest(req) + const grantDynamic = oAuthState.getGrantDynamicFromRequest(req) // only use state via session object if user isn't making intial "connect" request. // override param indicates subsequent requests from the oauth flow - const state = override ? dynamicState : req.query.state + const state = override ? grantDynamic.state : req.query.state if (!state) { next() return } - // pre auth token is companionKeysParams encoded and encrypted by companion before the oauth flow, - // I believe this has been done so that it cannot be modified by the client later. const preAuthToken = oAuthState.getFromState(state, 'preAuthToken', companionOptions.secret) if (!preAuthToken) { next() diff --git a/packages/@uppy/companion/src/server/provider/drive/index.js b/packages/@uppy/companion/src/server/provider/drive/index.js index f0b4e9a850..98cab16569 100644 --- a/packages/@uppy/companion/src/server/provider/drive/index.js +++ b/packages/@uppy/companion/src/server/provider/drive/index.js @@ -5,6 +5,7 @@ const logger = require('../../logger') const { VIRTUAL_SHARED_DIR, adaptData, isShortcut, isGsuiteFile, getGsuiteExportType } = require('./adapter') const { withProviderErrorHandling } = require('../providerErrors') const { prepareStream } = require('../../helpers/utils') +const { MAX_AGE_REFRESH_TOKEN } = require('../../helpers/jwt') const { ProviderAuthError } = require('../error') @@ -59,6 +60,10 @@ class Drive extends Provider { return 'google' } + static get authStateExpiry () { + return MAX_AGE_REFRESH_TOKEN + } + async list (options) { return this.#withErrorHandling('provider.drive.list.error', async () => { const directory = options.directory || 'root' diff --git a/packages/@uppy/companion/src/server/provider/dropbox/index.js b/packages/@uppy/companion/src/server/provider/dropbox/index.js index 50b4989ea6..9eda38bef3 100644 --- a/packages/@uppy/companion/src/server/provider/dropbox/index.js +++ b/packages/@uppy/companion/src/server/provider/dropbox/index.js @@ -4,6 +4,7 @@ const Provider = require('../Provider') const adaptData = require('./adapter') const { withProviderErrorHandling } = require('../providerErrors') const { prepareStream } = require('../../helpers/utils') +const { MAX_AGE_REFRESH_TOKEN } = require('../../helpers/jwt') // From https://www.dropbox.com/developers/reference/json-encoding: // @@ -63,6 +64,10 @@ class DropBox extends Provider { return 'dropbox' } + static get authStateExpiry () { + return MAX_AGE_REFRESH_TOKEN + } + /** * * @param {object} options @@ -100,13 +105,13 @@ class DropBox extends Provider { return this.#withErrorHandling('provider.dropbox.thumbnail.error', async () => { const stream = getClient({ token }).stream.post('files/get_thumbnail_v2', { prefixUrl: 'https://content.dropboxapi.com/2', - headers: { 'Dropbox-API-Arg': httpHeaderSafeJson({ resource: { '.tag': 'path', path: `${id}` }, size: 'w256h256' }) }, + headers: { 'Dropbox-API-Arg': httpHeaderSafeJson({ resource: { '.tag': 'path', path: `${id}` }, size: 'w256h256', format: 'jpeg' }) }, body: Buffer.alloc(0), responseType: 'json', }) await prepareStream(stream) - return { stream } + return { stream, contentType: 'image/jpeg' } }) } diff --git a/packages/@uppy/companion/src/server/provider/error.d.ts b/packages/@uppy/companion/src/server/provider/error.d.ts deleted file mode 100644 index e6aaed6288..0000000000 --- a/packages/@uppy/companion/src/server/provider/error.d.ts +++ /dev/null @@ -1,19 +0,0 @@ -// We need explicit type declarations for `errors.js` because of a typescript bug when generating declaration files. -// I think it's this one: -// https://github.com/microsoft/TypeScript/issues/37832 -// -// We could try removing this file when we upgrade to 4.1 :) - -export class ProviderApiError extends Error { - constructor(message: string, statusCode: number) -} -export class ProviderAuthError extends ProviderApiError { - constructor() -} - -export function errorToResponse(anyError: Error): { - code: number - message: string -} - -export function respondWithError(anyError: Error, res: any): boolean diff --git a/packages/@uppy/companion/src/server/provider/error.js b/packages/@uppy/companion/src/server/provider/error.js index 278d2df29f..efb4a53381 100644 --- a/packages/@uppy/companion/src/server/provider/error.js +++ b/packages/@uppy/companion/src/server/provider/error.js @@ -1,3 +1,4 @@ +/* eslint-disable max-classes-per-file */ /** * ProviderApiError is error returned when an adapter encounters * an http error while communication with its corresponding provider @@ -7,7 +8,7 @@ class ProviderApiError extends Error { * @param {string} message error message * @param {number} statusCode the http status code from the provider api */ - constructor (message, statusCode) { + constructor(message, statusCode) { super(`HTTP ${statusCode}: ${message}`) // Include statusCode to make it easier to debug this.name = 'ProviderApiError' this.statusCode = statusCode @@ -15,12 +16,23 @@ class ProviderApiError extends Error { } } +class ProviderUserError extends ProviderApiError { + /** + * @param {object} json arbitrary JSON.stringify-able object that will be passed to the client + */ + constructor(json) { + super('User error', undefined) + this.name = 'ProviderUserError' + this.json = json + } +} + /** * AuthError is error returned when an adapter encounters * an authorization error while communication with its corresponding provider */ class ProviderAuthError extends ProviderApiError { - constructor () { + constructor() { super('invalid access token detected by Provider', 401) this.name = 'AuthError' this.isAuthError = true @@ -32,37 +44,46 @@ class ProviderAuthError extends ProviderApiError { * * @param {Error | ProviderApiError} err the error instance to convert to an http json response */ -function errorToResponse (err) { - if (err instanceof ProviderAuthError && err.isAuthError) { - return { code: 401, message: err.message } +function errorToResponse(err) { + // @ts-ignore + if (err?.isAuthError) { + return { code: 401, json: { message: err.message } } + } + + if (err?.name === 'ProviderUserError') { + // @ts-ignore + return { code: 400, json: err.json } } - if (err instanceof ProviderApiError) { + if (err?.name === 'ProviderApiError') { + // @ts-ignore if (err.statusCode >= 500) { // bad gateway i.e the provider APIs gateway - return { code: 502, message: err.message } + return { code: 502, json: { message: err.message } } } + // @ts-ignore if (err.statusCode === 429) { return { code: 429, message: err.message } } + // @ts-ignore if (err.statusCode >= 400) { // 424 Failed Dependency - return { code: 424, message: err.message } + return { code: 424, json: { message: err.message } } } } return undefined } -function respondWithError (err, res) { +function respondWithError(err, res) { const errResp = errorToResponse(err) if (errResp) { - res.status(errResp.code).json({ message: errResp.message }) + res.status(errResp.code).json(errResp.json) return true } return false } -module.exports = { ProviderAuthError, ProviderApiError, errorToResponse, respondWithError } +module.exports = { ProviderAuthError, ProviderApiError, ProviderUserError, respondWithError } diff --git a/packages/@uppy/companion/src/server/provider/index.js b/packages/@uppy/companion/src/server/provider/index.js index 78ddee1ea2..e0b927dbc8 100644 --- a/packages/@uppy/companion/src/server/provider/index.js +++ b/packages/@uppy/companion/src/server/provider/index.js @@ -36,7 +36,7 @@ const providerNameToAuthName = (name, options) => { // eslint-disable-line no-un return (providers[name] || {}).authProvider } -function getGrantConfigForProvider ({ providerName, companionOptions, grantConfig }) { +function getGrantConfigForProvider({ providerName, companionOptions, grantConfig }) { const authProvider = providerNameToAuthName(providerName, companionOptions) if (!isOAuthProvider(authProvider)) return undefined @@ -63,13 +63,17 @@ module.exports.getProviderMiddleware = (providers, grantConfig) => { const ProviderClass = providers[providerName] if (ProviderClass && validOptions(req.companion.options)) { const { allowLocalUrls } = req.companion.options - req.companion.provider = new ProviderClass({ providerName, allowLocalUrls }) - req.companion.providerClass = ProviderClass - req.companion.providerGrantConfig = grantConfig[ProviderClass.authProvider] + const { authProvider } = ProviderClass - if (isOAuthProvider(ProviderClass.authProvider)) { + let providerGrantConfig + if (isOAuthProvider(authProvider)) { req.companion.getProviderCredentials = getCredentialsResolver(providerName, req.companion.options, req) + providerGrantConfig = grantConfig[authProvider] + req.companion.providerGrantConfig = providerGrantConfig } + + req.companion.provider = new ProviderClass({ providerName, providerGrantConfig, allowLocalUrls }) + req.companion.providerClass = ProviderClass } else { logger.warn('invalid provider options detected. Provider will not be loaded', 'provider.middleware.invalid', req.id) } diff --git a/packages/@uppy/companion/src/server/provider/providerErrors.js b/packages/@uppy/companion/src/server/provider/providerErrors.js index 62eba3eba5..715de19592 100644 --- a/packages/@uppy/companion/src/server/provider/providerErrors.js +++ b/packages/@uppy/companion/src/server/provider/providerErrors.js @@ -1,29 +1,37 @@ -const { HTTPError } = require('got').default - const logger = require('../logger') -const { ProviderApiError, ProviderAuthError } = require('./error') -const { StreamHttpJsonError } = require('../helpers/utils') +const { ProviderApiError, ProviderUserError, ProviderAuthError } = require('./error') /** * * @param {{ - * fn: () => any, tag: string, providerName: string, isAuthError: (a: { statusCode: number, body?: object }) => boolean, + * fn: () => any, + * tag: string, + * providerName: string, + * isAuthError?: (a: { statusCode: number, body?: object }) => boolean, + * isUserFacingError?: (a: { statusCode: number, body?: object }) => boolean, * getJsonErrorMessage: (a: object) => string * }} param0 * @returns */ -async function withProviderErrorHandling ({ fn, tag, providerName, isAuthError = () => false, getJsonErrorMessage }) { - function getErrorMessage (response) { - if (typeof response.body === 'object') { - const message = getJsonErrorMessage(response.body) +async function withProviderErrorHandling({ + fn, + tag, + providerName, + isAuthError = () => false, + isUserFacingError = () => false, + getJsonErrorMessage, +}) { + function getErrorMessage({ statusCode, body }) { + if (typeof body === 'object') { + const message = getJsonErrorMessage(body) if (message != null) return message } - if (typeof response.body === 'string') { - return response.body + if (typeof body === 'string') { + return body } - return `request to ${providerName} returned ${response.statusCode}` + return `request to ${providerName} returned ${statusCode}` } try { @@ -32,21 +40,26 @@ async function withProviderErrorHandling ({ fn, tag, providerName, isAuthError = let statusCode let body - if (err instanceof HTTPError) { + if (err?.name === 'HTTPError') { statusCode = err.response?.statusCode body = err.response?.body - } else if (err instanceof StreamHttpJsonError) { - statusCode = err.statusCode + } else if (err?.name === 'StreamHttpJsonError') { + statusCode = err.statusCode body = err.responseJson } if (statusCode != null) { - const err2 = isAuthError({ statusCode, body }) - ? new ProviderAuthError() - : new ProviderApiError(getErrorMessage(body), statusCode) + let knownErr + if (isAuthError({ statusCode, body })) { + knownErr = new ProviderAuthError() + } else if (isUserFacingError({ statusCode, body })) { + knownErr = new ProviderUserError({ message: getErrorMessage({ statusCode, body }) }) + } else { + knownErr = new ProviderApiError(getErrorMessage({ statusCode, body }), statusCode) + } - logger.error(err2, tag) - throw err2 + logger.error(knownErr, tag) + throw knownErr } logger.error(err, tag) diff --git a/packages/@uppy/companion/src/standalone/index.js b/packages/@uppy/companion/src/standalone/index.js index d4c926929a..04708a5493 100644 --- a/packages/@uppy/companion/src/standalone/index.js +++ b/packages/@uppy/companion/src/standalone/index.js @@ -17,7 +17,7 @@ const { getCompanionOptions, generateSecret, buildHelpfulStartupMessage } = requ * * @returns {object} */ -module.exports = function server (inputCompanionOptions) { +module.exports = function server(inputCompanionOptions) { const companionOptions = getCompanionOptions(inputCompanionOptions) companion.setLoggerProcessName(companionOptions) @@ -52,7 +52,7 @@ module.exports = function server (inputCompanionOptions) { * censored: boolean * }} */ - function censorQuery (rawQuery) { + function censorQuery(rawQuery) { /** @type {Record} */ const query = {} let censored = false @@ -172,7 +172,7 @@ module.exports = function server (inputCompanionOptions) { if (app.get('env') === 'production') { // if the error is a URIError from the requested URL we only log the error message // to avoid uneccessary error alerts - if (err.status === 400 && err instanceof URIError) { + if (err.status === 400 && err.name === 'URIError') { logger.error(err.message, 'root.error', req.id) } else { logger.error(err, 'root.error', req.id) diff --git a/packages/@uppy/core/src/BasePlugin.js b/packages/@uppy/core/src/BasePlugin.js index c802f97b74..9ad2e433c1 100644 --- a/packages/@uppy/core/src/BasePlugin.js +++ b/packages/@uppy/core/src/BasePlugin.js @@ -41,7 +41,8 @@ export default class BasePlugin { } i18nInit () { - const translator = new Translator([this.defaultLocale, this.uppy.locale, this.opts.locale]) + const onMissingKey = (key) => this.uppy.log(`Missing i18n string: ${key}`, 'error') + const translator = new Translator([this.defaultLocale, this.uppy.locale, this.opts.locale], { onMissingKey }) this.i18n = translator.translate.bind(translator) this.i18nArray = translator.translateArray.bind(translator) this.setPluginState() // so that UI re-renders and we see the updated locale diff --git a/packages/@uppy/core/src/Uppy.js b/packages/@uppy/core/src/Uppy.js index 4baac95bca..82470ef0ec 100644 --- a/packages/@uppy/core/src/Uppy.js +++ b/packages/@uppy/core/src/Uppy.js @@ -204,7 +204,8 @@ class Uppy { } i18nInit () { - const translator = new Translator([this.defaultLocale, this.opts.locale]) + const onMissingKey = (key) => this.log(`Missing i18n string: ${key}`, 'error') + const translator = new Translator([this.defaultLocale, this.opts.locale], { onMissingKey }) this.i18n = translator.translate.bind(translator) this.i18nArray = translator.translateArray.bind(translator) this.locale = translator.locale diff --git a/packages/@uppy/dropbox/src/Dropbox.jsx b/packages/@uppy/dropbox/src/Dropbox.jsx index c591933087..b836ba0d90 100644 --- a/packages/@uppy/dropbox/src/Dropbox.jsx +++ b/packages/@uppy/dropbox/src/Dropbox.jsx @@ -27,6 +27,7 @@ export default class Dropbox extends UIPlugin { companionCookiesRule: this.opts.companionCookiesRule, provider: 'dropbox', pluginId: this.id, + supportsRefreshToken: true, }) this.defaultLocale = locale diff --git a/packages/@uppy/facebook/src/Facebook.jsx b/packages/@uppy/facebook/src/Facebook.jsx index f8ceac492e..30d8c5faba 100644 --- a/packages/@uppy/facebook/src/Facebook.jsx +++ b/packages/@uppy/facebook/src/Facebook.jsx @@ -30,6 +30,7 @@ export default class Facebook extends UIPlugin { companionCookiesRule: this.opts.companionCookiesRule, provider: 'facebook', pluginId: this.id, + supportsRefreshToken: false, }) this.defaultLocale = locale diff --git a/packages/@uppy/google-drive/src/GoogleDrive.jsx b/packages/@uppy/google-drive/src/GoogleDrive.jsx index 96433c33ef..fa857f429f 100644 --- a/packages/@uppy/google-drive/src/GoogleDrive.jsx +++ b/packages/@uppy/google-drive/src/GoogleDrive.jsx @@ -41,6 +41,7 @@ export default class GoogleDrive extends UIPlugin { companionCookiesRule: this.opts.companionCookiesRule, provider: 'drive', pluginId: this.id, + supportsRefreshToken: true, }) this.defaultLocale = locale diff --git a/packages/@uppy/instagram/src/Instagram.jsx b/packages/@uppy/instagram/src/Instagram.jsx index 7ba14fd33e..c0094a86b8 100644 --- a/packages/@uppy/instagram/src/Instagram.jsx +++ b/packages/@uppy/instagram/src/Instagram.jsx @@ -40,6 +40,7 @@ export default class Instagram extends UIPlugin { companionCookiesRule: this.opts.companionCookiesRule, provider: 'instagram', pluginId: this.id, + supportsRefreshToken: false, }) this.onFirstRender = this.onFirstRender.bind(this) diff --git a/packages/@uppy/onedrive/src/OneDrive.jsx b/packages/@uppy/onedrive/src/OneDrive.jsx index 6f4b939f56..b1696a0178 100644 --- a/packages/@uppy/onedrive/src/OneDrive.jsx +++ b/packages/@uppy/onedrive/src/OneDrive.jsx @@ -32,6 +32,7 @@ export default class OneDrive extends UIPlugin { companionCookiesRule: this.opts.companionCookiesRule, provider: 'onedrive', pluginId: this.id, + supportsRefreshToken: false, }) this.defaultLocale = locale diff --git a/packages/@uppy/provider-views/src/ProviderView/AuthView.jsx b/packages/@uppy/provider-views/src/ProviderView/AuthView.jsx index 5cf51f82f6..c0ec896f4a 100644 --- a/packages/@uppy/provider-views/src/ProviderView/AuthView.jsx +++ b/packages/@uppy/provider-views/src/ProviderView/AuthView.jsx @@ -1,4 +1,5 @@ import { h } from 'preact' +import { useCallback } from 'preact/hooks' function GoogleIcon () { return ( @@ -36,46 +37,65 @@ function GoogleIcon () { ) } -function AuthView (props) { - const { pluginName, pluginIcon, i18nArray, handleAuth } = props +const DefaultForm = ({ pluginName, i18n, onAuth }) => { // In order to comply with Google's brand we need to create a different button // for the Google Drive plugin const isGoogleDrive = pluginName === 'Google Drive' - const pluginNameComponent = ( - - {pluginName} -
-
- ) + const onSubmit = useCallback((e) => { + e.preventDefault() + onAuth() + }, [onAuth]) + return ( -
-
{pluginIcon()}
-
- {i18nArray('authenticateWithTitle', { - pluginName: pluginNameComponent, - })} -
+
{isGoogleDrive ? ( ) : ( )} +
+ ) +} + +const defaultRenderForm = ({ pluginName, i18n, onAuth }) => ( + +) + +function AuthView (props) { + const { loading, pluginName, pluginIcon, i18n, handleAuth, renderForm = defaultRenderForm } = props + + const pluginNameComponent = ( + + {pluginName} +
+
+ ) + return ( +
+
{pluginIcon()}
+
+ {i18n('authenticateWithTitle', { + pluginName: pluginNameComponent, + })} +
+ +
+ {renderForm({ pluginName, i18n, loading, onAuth: handleAuth })} +
) } diff --git a/packages/@uppy/provider-views/src/ProviderView/ProviderView.jsx b/packages/@uppy/provider-views/src/ProviderView/ProviderView.jsx index 314ee6bd10..ef79b256b6 100644 --- a/packages/@uppy/provider-views/src/ProviderView/ProviderView.jsx +++ b/packages/@uppy/provider-views/src/ProviderView/ProviderView.jsx @@ -6,7 +6,6 @@ import { getSafeFileId } from '@uppy/utils/lib/generateFileID' import AuthView from './AuthView.jsx' import Header from './Header.jsx' import Browser from '../Browser.jsx' -import LoaderView from '../Loader.jsx' import CloseWrapper from '../CloseWrapper.js' import View from '../View.js' @@ -68,7 +67,7 @@ export default class ProviderView extends View { // Set default state for the plugin this.plugin.setPluginState({ - authenticated: false, + authenticated: undefined, // we don't know yet files: [], folders: [], breadcrumbs: [], @@ -186,6 +185,13 @@ export default class ProviderView extends View { this.plugin.setPluginState({ folders, files, breadcrumbs, filterInput: '' }) }) } catch (err) { + // This is the first call that happens when the provider view loads, after auth, so it's probably nice to show any + // error occurring here to the user. + if (err?.name === 'UserFacingApiError') { + this.plugin.uppy.info({ message: this.plugin.uppy.i18n(err.message) }, 'warning', 5000) + return + } + this.handleError(err) } finally { this.setLoading(false) @@ -241,13 +247,23 @@ export default class ProviderView extends View { this.plugin.setPluginState({ filterInput: '' }) } - async handleAuth () { + async handleAuth (authFormData) { try { - await this.provider.login() - this.plugin.setPluginState({ authenticated: true }) - this.preFirstRender() - } catch (e) { - this.plugin.uppy.log(`login failed: ${e.message}`) + await this.#withAbort(async (signal) => { + this.setLoading(true) + await this.provider.login({ authFormData, signal }) + this.plugin.setPluginState({ authenticated: true }) + this.preFirstRender() + }) + } catch (err) { + if (err.name === 'UserFacingApiError') { + this.plugin.uppy.info({ message: this.plugin.uppy.i18n(err.message) }, 'warning', 5000) + return + } + + this.plugin.uppy.log(`login failed: ${err.message}`) + } finally { + this.setLoading(false) } } @@ -429,7 +445,6 @@ export default class ProviderView extends View { currentSelection, files: hasInput ? filterItems(files) : files, folders: hasInput ? filterItems(folders) : folders, - username: this.username, getNextFolder: this.getNextFolder, getFolder: this.getFolder, loadAllFiles: this.opts.loadAllFiles, @@ -457,25 +472,19 @@ export default class ProviderView extends View { i18n: this.plugin.uppy.i18n, uppyFiles: this.plugin.uppy.getFiles(), validateRestrictions: (...args) => this.plugin.uppy.validateRestrictions(...args), + isLoading: loading, } - if (loading) { - return ( - - - - ) - } - - if (!authenticated) { + if (authenticated === false) { return ( ) diff --git a/packages/@uppy/utils/package.json b/packages/@uppy/utils/package.json index 496fad47a8..fa5d44a7d5 100644 --- a/packages/@uppy/utils/package.json +++ b/packages/@uppy/utils/package.json @@ -65,7 +65,8 @@ "./lib/FOCUSABLE_ELEMENTS.js": "./lib/FOCUSABLE_ELEMENTS.js", "./lib/fileFilters": "./lib/fileFilters.js", "./lib/VirtualList": "./lib/VirtualList.js", - "./src/microtip.scss": "./src/microtip.scss" + "./src/microtip.scss": "./src/microtip.scss", + "./lib/UserFacingApiError": "./lib/UserFacingApiError.js" }, "dependencies": { "lodash": "^4.17.21", diff --git a/packages/@uppy/utils/src/Translator.ts b/packages/@uppy/utils/src/Translator.ts index 34a4f202a3..3fcef5991d 100644 --- a/packages/@uppy/utils/src/Translator.ts +++ b/packages/@uppy/utils/src/Translator.ts @@ -1,5 +1,3 @@ -import has from './hasProperty.ts' - // We're using a generic because languages have different plural rules. export interface Locale { strings: Record> @@ -84,6 +82,10 @@ function interpolate( return interpolated } +const defaultOnMissingKey = (key: string): void => { + throw new Error(`missing string: ${key}`) +} + /** * Translates strings with interpolation & pluralization support. * Extensible with custom dictionaries and pluralization functions. @@ -98,7 +100,10 @@ function interpolate( export default class Translator { protected locale: Locale - constructor(locales: Locale | Locale[]) { + constructor( + locales: Locale | Locale[], + { onMissingKey = defaultOnMissingKey } = {}, + ) { this.locale = { strings: {}, pluralize(n: number): 0 | 1 { @@ -114,8 +119,12 @@ export default class Translator { } else { this.#apply(locales) } + + this.#onMissingKey = onMissingKey } + #onMissingKey + #apply(locale?: Locale): void { if (!locale?.strings) { return @@ -146,11 +155,11 @@ export default class Translator { * @returns The translated and interpolated parts, in order. */ translateArray(key: string, options?: Options): Array { - if (!has(this.locale.strings, key)) { - throw new Error(`missing string: ${key}`) + let string = this.locale.strings[key] + if (string == null) { + this.#onMissingKey(key) + string = key } - - const string = this.locale.strings[key] const hasPluralForms = typeof string === 'object' if (hasPluralForms) { @@ -163,6 +172,10 @@ export default class Translator { ) } + if (typeof string !== 'string') { + throw new Error(`string was not a string`) + } + return interpolate(string, options) } } diff --git a/packages/@uppy/utils/src/UserFacingApiError.js b/packages/@uppy/utils/src/UserFacingApiError.js new file mode 100644 index 0000000000..1bfe9da950 --- /dev/null +++ b/packages/@uppy/utils/src/UserFacingApiError.js @@ -0,0 +1,5 @@ +class UserFacingApiError extends Error { + name = 'UserFacingApiError' +} + +export default UserFacingApiError diff --git a/packages/@uppy/zoom/src/Zoom.jsx b/packages/@uppy/zoom/src/Zoom.jsx index c7c3d392ba..dc465e05a3 100644 --- a/packages/@uppy/zoom/src/Zoom.jsx +++ b/packages/@uppy/zoom/src/Zoom.jsx @@ -28,6 +28,7 @@ export default class Zoom extends UIPlugin { companionCookiesRule: this.opts.companionCookiesRule, provider: 'zoom', pluginId: this.id, + supportsRefreshToken: false, }) this.defaultLocale = locale