diff --git a/packages/@uppy/companion/src/companion.js b/packages/@uppy/companion/src/companion.js index 564a108fb7..683eeb3d1d 100644 --- a/packages/@uppy/companion/src/companion.js +++ b/packages/@uppy/companion/src/companion.js @@ -20,8 +20,10 @@ const { ProviderApiError, ProviderUserError, ProviderAuthError } = require('./se const { getCredentialsOverrideMiddleware } = require('./server/provider/credentials') // @ts-ignore const { version } = require('../package.json') +const { isOAuthProvider } = require('./server/provider/Provider') -function setLoggerProcessName ({ loggerProcessName }) { + +function setLoggerProcessName({ loggerProcessName }) { if (loggerProcessName != null) logger.setProcessName(loggerProcessName) } @@ -72,13 +74,15 @@ module.exports.app = (optionsArg = {}) => { const providers = providerManager.getDefaultProviders() - providerManager.addProviderOptions(options, grantConfig) - const { customProviders } = options if (customProviders) { providerManager.addCustomProviders(customProviders, providers, grantConfig) } + const getAuthProvider = (providerName) => providers[providerName]?.authProvider + + providerManager.addProviderOptions(options, grantConfig, getAuthProvider) + // mask provider secrets from log messages logger.setMaskables(getMaskableSecrets(options)) @@ -147,13 +151,18 @@ module.exports.app = (optionsArg = {}) => { // 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] + + function getRedirectUri() { + const authProvider = getAuthProvider(providerName) + if (!isOAuthProvider(authProvider)) return undefined + return grantConfig[authProvider]?.redirect_uri + } + res.send({ credentials: { key, secret, - redirect_uri: providerManager.getGrantConfigForProvider({ - providerName, companionOptions: options, grantConfig, - })?.redirect_uri, + redirect_uri: getRedirectUri(), }, }) }) diff --git a/packages/@uppy/companion/src/server/controllers/connect.js b/packages/@uppy/companion/src/server/controllers/connect.js index 1bfbc06593..7018e18e38 100644 --- a/packages/@uppy/companion/src/server/controllers/connect.js +++ b/packages/@uppy/companion/src/server/controllers/connect.js @@ -30,7 +30,7 @@ module.exports = function connect(req, res) { } const state = oAuthState.encodeState(stateObj, secret) - const { provider, providerGrantConfig } = req.companion + const { providerClass, 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) @@ -45,12 +45,12 @@ module.exports = function connect(req, res) { ]] }) || []) - const providerName = provider.authProvider + const { authProvider } = providerClass const qs = queryString({ ...grantDynamicConfig, state, }) // Now we redirect to grant's /connect endpoint, see `app.use(Grant(grantConfig))` - res.redirect(req.companion.buildURL(`/connect/${providerName}${qs}`, true)) + res.redirect(req.companion.buildURL(`/connect/${authProvider}${qs}`, true)) } diff --git a/packages/@uppy/companion/src/server/controllers/logout.js b/packages/@uppy/companion/src/server/controllers/logout.js index 741dc61dda..d1fc5597d5 100644 --- a/packages/@uppy/companion/src/server/controllers/logout.js +++ b/packages/@uppy/companion/src/server/controllers/logout.js @@ -26,7 +26,7 @@ async function logout (req, res, next) { const { accessToken } = providerUserSession const data = await companion.provider.logout({ token: accessToken, providerUserSession, companion }) delete companion.providerUserSession - tokenService.removeFromCookies(res, companion.options, companion.provider.authProvider) + tokenService.removeFromCookies(res, companion.options, companion.providerClass.authProvider) cleanSession() res.json({ ok: true, ...data }) } catch (err) { diff --git a/packages/@uppy/companion/src/server/controllers/oauth-redirect.js b/packages/@uppy/companion/src/server/controllers/oauth-redirect.js index ff4f095448..4805da245d 100644 --- a/packages/@uppy/companion/src/server/controllers/oauth-redirect.js +++ b/packages/@uppy/companion/src/server/controllers/oauth-redirect.js @@ -10,7 +10,7 @@ const oAuthState = require('../helpers/oauth-state') */ module.exports = function oauthRedirect (req, res) { const params = qs.stringify(req.query) - const { authProvider } = req.companion.provider + const { authProvider } = req.companion.providerClass if (!req.companion.options.server.oauthDomain) { res.redirect(req.companion.buildURL(`/connect/${authProvider}/callback?${params}`, true)) return diff --git a/packages/@uppy/companion/src/server/helpers/jwt.js b/packages/@uppy/companion/src/server/helpers/jwt.js index 0db6249a47..48d2f1c50a 100644 --- a/packages/@uppy/companion/src/server/helpers/jwt.js +++ b/packages/@uppy/companion/src/server/helpers/jwt.js @@ -125,7 +125,7 @@ module.exports.addToCookiesIfNeeded = (req, res, uppyAuthToken, maxAge) => { res, token: uppyAuthToken, companionOptions: req.companion.options, - authProvider: req.companion.provider.authProvider, + authProvider: req.companion.providerClass.authProvider, maxAge, }) } diff --git a/packages/@uppy/companion/src/server/middlewares.js b/packages/@uppy/companion/src/server/middlewares.js index 73e8d798cb..a699b22c43 100644 --- a/packages/@uppy/companion/src/server/middlewares.js +++ b/packages/@uppy/companion/src/server/middlewares.js @@ -122,7 +122,7 @@ exports.gentleVerifyToken = (req, res, next) => { } exports.cookieAuthToken = (req, res, next) => { - req.companion.authToken = req.cookies[`uppyAuthToken--${req.companion.provider.authProvider}`] + req.companion.authToken = req.cookies[`uppyAuthToken--${req.companion.providerClass.authProvider}`] return next() } diff --git a/packages/@uppy/companion/src/server/provider/Provider.js b/packages/@uppy/companion/src/server/provider/Provider.js index e649cab1a6..b8b8c87f8a 100644 --- a/packages/@uppy/companion/src/server/provider/Provider.js +++ b/packages/@uppy/companion/src/server/provider/Provider.js @@ -122,5 +122,5 @@ class Provider { } module.exports = Provider -// OAuth providers are those that have a `static authProvider` set. It means they require OAuth authentication to work +// OAuth providers are those that have an `authProvider` set. It means they require OAuth authentication to work module.exports.isOAuthProvider = (authProvider) => typeof authProvider === 'string' && authProvider.length > 0 diff --git a/packages/@uppy/companion/src/server/provider/box/index.js b/packages/@uppy/companion/src/server/provider/box/index.js index 0de108136e..5ce6278136 100644 --- a/packages/@uppy/companion/src/server/provider/box/index.js +++ b/packages/@uppy/companion/src/server/provider/box/index.js @@ -31,7 +31,6 @@ async function list ({ directory, query, token }) { class Box extends Provider { constructor (options) { super(options) - this.authProvider = Box.authProvider // needed for the thumbnails fetched via companion this.needsCookieAuth = true } @@ -116,11 +115,12 @@ class Box extends Provider { }) } + // eslint-disable-next-line class-methods-use-this async #withErrorHandling (tag, fn) { return withProviderErrorHandling({ fn, tag, - providerName: this.authProvider, + providerName: Box.authProvider, isAuthError: (response) => response.statusCode === 401, getJsonErrorMessage: (body) => body?.message, }) diff --git a/packages/@uppy/companion/src/server/provider/drive/index.js b/packages/@uppy/companion/src/server/provider/drive/index.js index 98cab16569..384a86b4ed 100644 --- a/packages/@uppy/companion/src/server/provider/drive/index.js +++ b/packages/@uppy/companion/src/server/provider/drive/index.js @@ -51,11 +51,6 @@ async function getStats ({ id, token }) { * Adapter for API https://developers.google.com/drive/api/v3/ */ class Drive extends Provider { - constructor (options) { - super(options) - this.authProvider = Drive.authProvider - } - static get authProvider () { return 'google' } @@ -200,11 +195,12 @@ class Drive extends Provider { }) } + // eslint-disable-next-line class-methods-use-this async #withErrorHandling (tag, fn) { return withProviderErrorHandling({ fn, tag, - providerName: this.authProvider, + providerName: Drive.authProvider, isAuthError: (response) => ( response.statusCode === 401 || (response.statusCode === 400 && response.body?.error === 'invalid_grant') // Refresh token has expired or been revoked diff --git a/packages/@uppy/companion/src/server/provider/dropbox/index.js b/packages/@uppy/companion/src/server/provider/dropbox/index.js index 9eda38bef3..d696f4cc74 100644 --- a/packages/@uppy/companion/src/server/provider/dropbox/index.js +++ b/packages/@uppy/companion/src/server/provider/dropbox/index.js @@ -56,7 +56,6 @@ async function userInfo ({ token }) { class DropBox extends Provider { constructor (options) { super(options) - this.authProvider = DropBox.authProvider this.needsCookieAuth = true } @@ -136,11 +135,12 @@ class DropBox extends Provider { }) } + // eslint-disable-next-line class-methods-use-this async #withErrorHandling (tag, fn) { return withProviderErrorHandling({ fn, tag, - providerName: this.authProvider, + providerName: DropBox.authProvider, isAuthError: (response) => response.statusCode === 401, getJsonErrorMessage: (body) => body?.error_summary, }) diff --git a/packages/@uppy/companion/src/server/provider/facebook/index.js b/packages/@uppy/companion/src/server/provider/facebook/index.js index 4d5de8ce1a..e29f98f183 100644 --- a/packages/@uppy/companion/src/server/provider/facebook/index.js +++ b/packages/@uppy/companion/src/server/provider/facebook/index.js @@ -24,11 +24,6 @@ async function getMediaUrl ({ token, id }) { * Adapter for API https://developers.facebook.com/docs/graph-api/using-graph-api/ */ class Facebook extends Provider { - constructor (options) { - super(options) - this.authProvider = Facebook.authProvider - } - static get authProvider () { return 'facebook' } @@ -86,11 +81,12 @@ class Facebook extends Provider { }) } + // eslint-disable-next-line class-methods-use-this async #withErrorHandling (tag, fn) { return withProviderErrorHandling({ fn, tag, - providerName: this.authProvider, + providerName: Facebook.authProvider, isAuthError: (response) => typeof response.body === 'object' && response.body?.error?.code === 190, // Invalid OAuth 2.0 Access Token getJsonErrorMessage: (body) => body?.error?.message, }) diff --git a/packages/@uppy/companion/src/server/provider/index.js b/packages/@uppy/companion/src/server/provider/index.js index e0b927dbc8..fb7d4b1b59 100644 --- a/packages/@uppy/companion/src/server/provider/index.js +++ b/packages/@uppy/companion/src/server/provider/index.js @@ -12,7 +12,6 @@ const zoom = require('./zoom') const { getURLBuilder } = require('../helpers/utils') const logger = require('../logger') const { getCredentialsResolver } = require('./credentials') -// eslint-disable-next-line const Provider = require('./Provider') const { isOAuthProvider } = Provider @@ -25,26 +24,6 @@ const validOptions = (options) => { return options.server.host && options.server.protocol } -/** - * - * @param {string} name of the provider - * @param {{server: object, providerOptions: object}} options - * @returns {string} the authProvider for this provider - */ -const providerNameToAuthName = (name, options) => { // eslint-disable-line no-unused-vars - const providers = exports.getDefaultProviders() - return (providers[name] || {}).authProvider -} - -function getGrantConfigForProvider({ providerName, companionOptions, grantConfig }) { - const authProvider = providerNameToAuthName(providerName, companionOptions) - - if (!isOAuthProvider(authProvider)) return undefined - return grantConfig[authProvider] -} - -module.exports.getGrantConfigForProvider = getGrantConfigForProvider - /** * adds the desired provider module to the request object, * based on the providerName parameter specified @@ -106,10 +85,11 @@ module.exports.addCustomProviders = (customProviders, providers, grantConfig) => // eslint-disable-next-line no-param-reassign providers[providerName] = customProvider.module + const { authProvider } = customProvider.module - if (isOAuthProvider(customProvider.module.authProvider)) { + if (isOAuthProvider(authProvider)) { // eslint-disable-next-line no-param-reassign - grantConfig[providerName] = { + grantConfig[authProvider] = { ...customProvider.config, // todo: consider setting these options from a universal point also used // by official providers. It'll prevent these from getting left out if the @@ -125,8 +105,9 @@ module.exports.addCustomProviders = (customProviders, providers, grantConfig) => * * @param {{server: object, providerOptions: object}} companionOptions * @param {object} grantConfig + * @param {(a: string) => string} getAuthProvider */ -module.exports.addProviderOptions = (companionOptions, grantConfig) => { +module.exports.addProviderOptions = (companionOptions, grantConfig, getAuthProvider) => { const { server, providerOptions } = companionOptions if (!validOptions({ server })) { logger.warn('invalid provider options detected. Providers will not be loaded', 'provider.options.invalid') @@ -143,7 +124,8 @@ module.exports.addProviderOptions = (companionOptions, grantConfig) => { const { oauthDomain } = server const keys = Object.keys(providerOptions).filter((key) => key !== 'server') keys.forEach((providerName) => { - const authProvider = providerNameToAuthName(providerName, companionOptions) + const authProvider = getAuthProvider?.(providerName) + if (isOAuthProvider(authProvider) && grantConfig[authProvider]) { // explicitly add providerOptions so users don't override other providerOptions. // eslint-disable-next-line no-param-reassign diff --git a/packages/@uppy/companion/src/server/provider/instagram/graph/index.js b/packages/@uppy/companion/src/server/provider/instagram/graph/index.js index 92c1c6457d..4bf05c3e0f 100644 --- a/packages/@uppy/companion/src/server/provider/instagram/graph/index.js +++ b/packages/@uppy/companion/src/server/provider/instagram/graph/index.js @@ -23,11 +23,6 @@ async function getMediaUrl ({ token, id }) { * Adapter for API https://developers.facebook.com/docs/instagram-api/overview */ class Instagram extends Provider { - constructor (options) { - super(options) - this.authProvider = Instagram.authProvider - } - // for "grant" static getExtraConfig () { return { @@ -86,11 +81,12 @@ class Instagram extends Provider { return { revoked: false, manual_revoke_url: 'https://www.instagram.com/accounts/manage_access/' } } + // eslint-disable-next-line class-methods-use-this async #withErrorHandling (tag, fn) { return withProviderErrorHandling({ fn, tag, - providerName: this.authProvider, + providerName: Instagram.authProvider, isAuthError: (response) => typeof response.body === 'object' && response.body?.error?.code === 190, // Invalid OAuth 2.0 Access Token getJsonErrorMessage: (body) => body?.error?.message, }) diff --git a/packages/@uppy/companion/src/server/provider/onedrive/index.js b/packages/@uppy/companion/src/server/provider/onedrive/index.js index a318b5d43b..46a599ea5f 100644 --- a/packages/@uppy/companion/src/server/provider/onedrive/index.js +++ b/packages/@uppy/companion/src/server/provider/onedrive/index.js @@ -23,11 +23,6 @@ const getRootPath = (query) => (query.driveId ? `drives/${query.driveId}` : 'me/ * Adapter for API https://docs.microsoft.com/en-us/onedrive/developer/rest-api/ */ class OneDrive extends Provider { - constructor (options) { - super(options) - this.authProvider = OneDrive.authProvider - } - static get authProvider () { return 'microsoft' } @@ -98,11 +93,12 @@ class OneDrive extends Provider { }) } + // eslint-disable-next-line class-methods-use-this async #withErrorHandling (tag, fn) { return withProviderErrorHandling({ fn, tag, - providerName: this.authProvider, + providerName: OneDrive.authProvider, isAuthError: (response) => response.statusCode === 401, isUserFacingError: (response) => [400, 403].includes(response.statusCode), // onedrive gives some errors here that the user might want to know about diff --git a/packages/@uppy/companion/src/server/provider/zoom/index.js b/packages/@uppy/companion/src/server/provider/zoom/index.js index 9be59a399d..ea0852dda5 100644 --- a/packages/@uppy/companion/src/server/provider/zoom/index.js +++ b/packages/@uppy/companion/src/server/provider/zoom/index.js @@ -29,11 +29,6 @@ async function findFile ({ client, meetingId, fileId, recordingStart }) { * Adapter for API https://marketplace.zoom.us/docs/api-reference/zoom-api */ class Zoom extends Provider { - constructor (options) { - super(options) - this.authProvider = Zoom.authProvider - } - static get authProvider () { return 'zoom' } @@ -157,6 +152,7 @@ class Zoom extends Provider { }) } + // eslint-disable-next-line class-methods-use-this async #withErrorHandling (tag, fn) { const authErrorCodes = [ 124, // expired token @@ -166,7 +162,7 @@ class Zoom extends Provider { return withProviderErrorHandling({ fn, tag, - providerName: this.authProvider, + providerName: Zoom.authProvider, isAuthError: (response) => authErrorCodes.includes(response.statusCode), getJsonErrorMessage: (body) => body?.message, }) diff --git a/packages/@uppy/companion/test/__tests__/provider-manager.js b/packages/@uppy/companion/test/__tests__/provider-manager.js index 5c1a8aa7f0..084e2ec9f1 100644 --- a/packages/@uppy/companion/test/__tests__/provider-manager.js +++ b/packages/@uppy/companion/test/__tests__/provider-manager.js @@ -5,6 +5,8 @@ const { setDefaultEnv } = require('../mockserver') let grantConfig let companionOptions +const getAuthProvider = (providerName) => providerManager.getDefaultProviders()[providerName]?.authProvider + describe('Test Provider options', () => { beforeEach(() => { setDefaultEnv() @@ -14,7 +16,7 @@ describe('Test Provider options', () => { }) test('adds provider options', () => { - providerManager.addProviderOptions(companionOptions, grantConfig) + providerManager.addProviderOptions(companionOptions, grantConfig, getAuthProvider) expect(grantConfig.dropbox.key).toBe('dropbox_key') expect(grantConfig.dropbox.secret).toBe('dropbox_secret') @@ -33,7 +35,7 @@ describe('Test Provider options', () => { test('adds extra provider config', () => { process.env.COMPANION_INSTAGRAM_KEY = '123456' - providerManager.addProviderOptions(getCompanionOptions(), grantConfig) + providerManager.addProviderOptions(getCompanionOptions(), grantConfig, getAuthProvider) expect(grantConfig.instagram).toEqual({ transport: 'session', callback: '/instagram/callback', @@ -102,7 +104,7 @@ describe('Test Provider options', () => { companionOptions = getCompanionOptions() - providerManager.addProviderOptions(companionOptions, grantConfig) + providerManager.addProviderOptions(companionOptions, grantConfig, getAuthProvider) expect(grantConfig.dropbox.secret).toBe('xobpord') expect(grantConfig.box.secret).toBe('xwbepqd') @@ -116,7 +118,7 @@ describe('Test Provider options', () => { delete companionOptions.server.host delete companionOptions.server.protocol - providerManager.addProviderOptions(companionOptions, grantConfig) + providerManager.addProviderOptions(companionOptions, grantConfig, getAuthProvider) expect(grantConfig.dropbox.key).toBeUndefined() expect(grantConfig.dropbox.secret).toBeUndefined() @@ -135,7 +137,7 @@ describe('Test Provider options', () => { test('sets a main redirect uri, if oauthDomain is set', () => { companionOptions.server.oauthDomain = 'domain.com' - providerManager.addProviderOptions(companionOptions, grantConfig) + providerManager.addProviderOptions(companionOptions, grantConfig, getAuthProvider) expect(grantConfig.dropbox.redirect_uri).toBe('http://domain.com/dropbox/redirect') expect(grantConfig.box.redirect_uri).toBe('http://domain.com/box/redirect') @@ -158,8 +160,8 @@ describe('Test Custom Provider options', () => { }, }, providers, grantConfig) - expect(grantConfig.foo.key).toBe('foo_key') - expect(grantConfig.foo.secret).toBe('foo_secret') + expect(grantConfig.some_provider.key).toBe('foo_key') + expect(grantConfig.some_provider.secret).toBe('foo_secret') expect(providers.foo).toBeTruthy() }) })