@uppy/companion: fix `authProvider` property inconsistency (#4672)

* remove useless line

* fix broken cookie removal logic

related #4426

* fix mime type of thumbnails

not critical but some browsers might have problems

* simplify/speedup token generation

so we don't have to decode/decrypt/encode/encrypt so many times

* use instanceof instead of prop check

* Implement alternative provider auth

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`.

* refactor

* use respondWithError

also for thumbnails
for consistency

* fix prepareStream

it wasn't returning the status code (like `got` does on error)
it's needed to respond properly with a http error

* don't throw when missing i18n key

instead log error and show the key
this in on par with other i18n frameworks

* fix bugged try/catch

* allow aborting login too

and don't replace the whole view with a loader when plugin state loading
it will cause auth views to lose state
an inter-view loading text looks much more graceful and is how SearchProviderView works too

* add json http error support

add support for passing objects and messages from companion to uppy
this allows companion to for example give a more detailed error when authenticating

* don't tightly couple auth form with html form

don't force the user to use html form
and use preact for it, for flexibility

* fix i18n

* make contentType parameterized

* allow sending certain errors to the user

this is useful because:

      // onedrive gives some errors here that the user might want to know about
      // e.g. these happen if you try to login to a users in an organization,
      // without an Office365 licence or OneDrive account setup completed
      // 400: Tenant does not have a SPO license
      // 403: You do not have access to create this personal site or you do not have a valid license

* make `authProvider` consistent

always use the static property
ignoring the instance propety

fixes #4460

* fix bug

* fix test also

* don't have default content-type

* make a loginSimpleAuth api too

* make removeAuthToken protected

(cherry picked from commit 4be2b6fd9136d178966a6cca68204bf7ea38d09e)

* fix lint

* run yarn format

* Apply suggestions from code review

Co-authored-by: Antoine du Hamel <antoine@transloadit.com>

* fix broken merge conflict

* improve inheritance

* fix bug

* fix bug with dynamic grant config

* use duck typing for error checks

see discussion here: https://github.com/transloadit/uppy/pull/4619#discussion_r1406225982

* Apply suggestions from code review

Co-authored-by: Antoine du Hamel <antoine@transloadit.com>

* fix broken lint fix script

* fix broken merge code

* try to fix flakey tets

* fix lint

* fix merge issue

---------

Co-authored-by: Antoine du Hamel <antoine@transloadit.com>

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(),
         },
       })
     })

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))
 }

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) {

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

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,
     })
   }

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()
 }
 

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

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,
     })

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

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,
     })

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,
     })

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

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,
     })

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

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,
     })

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()
   })
 })