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 | 56 ++++++------ 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 | 75 ++++++++++------ .../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 | 66 +++++++++----- .../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, 622 insertions(+), 359 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 053089c7d..8432b969c 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 e21ee08ca..f1e6fffd9 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 f60d29de5..d311b60e3 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 d4b99be78..a72531a80 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 93a202a1d..2fd823b38 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 14517d340..6c77e794f 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 a3ebb5c7e..fbd3a434a 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 972315b8e..c0fae7406 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() + 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 */ + 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 c3deec6f9..564a108fb 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 994580513..dd10fb971 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 {} +class ValidationError extends Error { + constructor(message) { + super(message) + + 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 cee35ced1..effcd397f 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 7e8f9b8e5..1bfbc0659 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 f160e5c66..e3bd4da75 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 4410eec3b..90dd93fb1 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 284660c1b..43afb0b7c 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 1ec87e303..741dc61dd 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 a0ef7ba47..ff4f09544 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 ab37cd3cd..ed6d7a474 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 17cc4c160..60bac3474 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 000000000..91d900a15 --- /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 0994964ed..b578bbb5c 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 b1a9b75df..0db6249a4 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 } - // send signed token to client. - res.cookie(`${prefix}--${authProvider}`, token, cookieOptions) + + return cookieOptions } -module.exports.addToCookiesIfNeeded = (req, res, uppyAuthToken) => { +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(getCookieName(authProvider), token, cookieOptions) +} + +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, - } + // 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 }) - if (companionOptions.cookieDomain) { - cookieOptions.domain = companionOptions.cookieDomain - } - - 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 d83337607..4a7d1a9b9 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 31554e416..5f637c43b 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 fadae31b5..627d87fcb 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 112a35555..f2788ff18 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 7c9cf93cc..73e8d798c 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 074d18d15..e649cab1a 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 a05913e43..0de108136 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 20fbe39df..154378f9a 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 f0b4e9a85..98cab1656 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 50b4989ea..9eda38bef 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 e6aaed628..000000000 --- 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 278d2df29..efb4a5338 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 instanceof ProviderApiError) { + if (err?.name === 'ProviderUserError') { + // @ts-ignore + return { code: 400, json: err.json } + } + + 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 78ddee1ea..e0b927dbc 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 62eba3eba..715de1959 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 d4c926929..04708a549 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 c802f97b7..9ad2e433c 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 4baac95bc..82470ef0e 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 c59193308..b836ba0d9 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 f8ceac492..30d8c5fab 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 96433c33e..fa857f429 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 7ba14fd33..c0094a86b 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 6f4b939f5..b1696a017 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 5cf51f82f..c0ec896f4 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,12 +37,47 @@ 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 onSubmit = useCallback((e) => { + e.preventDefault() + onAuth() + }, [onAuth]) + + return ( +
+ {isGoogleDrive ? ( + + ) : ( + + )} +
+ ) +} + +const defaultRenderForm = ({ pluginName, i18n, onAuth }) => ( + +) + +function AuthView (props) { + const { loading, pluginName, pluginIcon, i18n, handleAuth, renderForm = defaultRenderForm } = props + const pluginNameComponent = ( {pluginName} @@ -52,30 +88,14 @@ function AuthView (props) {
{pluginIcon()}
- {i18nArray('authenticateWithTitle', { + {i18n('authenticateWithTitle', { pluginName: pluginNameComponent, })}
- {isGoogleDrive ? ( - - ) : ( - - )} + +
+ {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 314ee6bd1..ef79b256b 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 496fad47a..fa5d44a7d 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 34a4f202a..3fcef5991 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 000000000..1bfe9da95 --- /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 c7c3d392b..dc465e05a 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