Bladeren bron

@uppy/companion: implement refresh for authentication tokens (#4448)

* allow storing multiple tokens

inside uppy auth token

* de-duplicate uploadRemote

by creating a new superclass UploaderPlugin

* pull out requestSocketToken

from MiniXHRUpload

* add class UploaderPlugin

* refactor

* refactor

* refactor/reuse

* refactor/make getAuthToken private

* fix bug

* implement refresh token

for dropbox and google drive

closes #2721

* fix test

* also update auth token cookie
when refreshing token

* fix build error on node 14

* increase auth token expiry

to workaround expiry

* Update packages/@uppy/companion-client/src/RequestClient.js

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

* make queueRequestSocketToken private

* rename arg

* fix lint

* log error message

* fix s3

* Update packages/@uppy/companion-client/src/Provider.js

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

---------

Co-authored-by: Antoine du Hamel <antoine@transloadit.com>
Mikael Finstad 1 jaar geleden
bovenliggende
commit
2c75ddacfe
37 gewijzigde bestanden met toevoegingen van 439 en 347 verwijderingen
  1. 5 33
      packages/@uppy/aws-s3-multipart/src/index.js
  2. 5 87
      packages/@uppy/aws-s3/src/MiniXHRUpload.js
  3. 64 4
      packages/@uppy/aws-s3/src/index.js
  4. 47 9
      packages/@uppy/companion-client/src/Provider.js
  5. 5 4
      packages/@uppy/companion-client/src/RequestClient.js
  6. 1 0
      packages/@uppy/companion/src/companion.js
  7. 2 0
      packages/@uppy/companion/src/config/grant.js
  8. 19 13
      packages/@uppy/companion/src/server/controllers/callback.js
  9. 2 6
      packages/@uppy/companion/src/server/controllers/deauth-callback.js
  10. 3 3
      packages/@uppy/companion/src/server/controllers/get.js
  11. 1 0
      packages/@uppy/companion/src/server/controllers/index.js
  12. 4 8
      packages/@uppy/companion/src/server/controllers/list.js
  13. 7 10
      packages/@uppy/companion/src/server/controllers/logout.js
  14. 51 0
      packages/@uppy/companion/src/server/controllers/refresh-token.js
  15. 0 5
      packages/@uppy/companion/src/server/controllers/send-token.js
  16. 2 2
      packages/@uppy/companion/src/server/controllers/thumbnail.js
  17. 54 28
      packages/@uppy/companion/src/server/helpers/jwt.js
  18. 2 6
      packages/@uppy/companion/src/server/helpers/upload.js
  19. 16 11
      packages/@uppy/companion/src/server/middlewares.js
  20. 9 0
      packages/@uppy/companion/src/server/provider/Provider.js
  21. 10 6
      packages/@uppy/companion/src/server/provider/credentials.js
  22. 11 0
      packages/@uppy/companion/src/server/provider/drive/index.js
  23. 11 1
      packages/@uppy/companion/src/server/provider/dropbox/index.js
  24. 2 0
      packages/@uppy/companion/src/server/provider/error.d.ts
  25. 10 1
      packages/@uppy/companion/src/server/provider/error.js
  26. 13 16
      packages/@uppy/companion/src/server/provider/providerErrors.js
  27. 18 7
      packages/@uppy/companion/test/__tests__/callback.js
  28. 4 4
      packages/@uppy/companion/test/__tests__/companion.js
  29. 2 2
      packages/@uppy/companion/test/__tests__/credentials.js
  30. 3 9
      packages/@uppy/companion/test/__tests__/preauth.js
  31. 6 0
      packages/@uppy/companion/test/__tests__/provider-manager.js
  32. 2 2
      packages/@uppy/companion/test/__tests__/providers.js
  33. 3 1
      packages/@uppy/companion/test/mockserver.js
  34. 34 0
      packages/@uppy/core/src/UploaderPlugin.js
  35. 2 2
      packages/@uppy/core/src/Uppy.js
  36. 5 36
      packages/@uppy/tus/src/index.js
  37. 4 31
      packages/@uppy/xhr-upload/src/index.js

+ 5 - 33
packages/@uppy/aws-s3-multipart/src/index.js

@@ -1,4 +1,4 @@
-import BasePlugin from '@uppy/core/lib/BasePlugin.js'
+import UploaderPlugin from '@uppy/core/lib/UploaderPlugin.js'
 import { Socket, Provider, RequestClient } from '@uppy/companion-client'
 import EventManager from '@uppy/utils/lib/EventManager'
 import emitSocketProgress from '@uppy/utils/lib/emitSocketProgress'
@@ -303,11 +303,9 @@ class HTTPCommunicationQueue {
   }
 }
 
-export default class AwsS3Multipart extends BasePlugin {
+export default class AwsS3Multipart extends UploaderPlugin {
   static VERSION = packageJson.version
 
-  #queueRequestSocketToken
-
   #companionCommunicationQueue
 
   #client
@@ -359,7 +357,7 @@ export default class AwsS3Multipart extends BasePlugin {
     this.uploaderEvents = Object.create(null)
     this.uploaderSockets = Object.create(null)
 
-    this.#queueRequestSocketToken = this.requests.wrapPromiseFunction(this.#requestSocketToken, { priority: -1 })
+    this.setQueueRequestSocketToken(this.requests.wrapPromiseFunction(this.#requestSocketToken, { priority: -1 }))
   }
 
   [Symbol.for('uppy test: getClient')] () { return this.#client }
@@ -683,32 +681,6 @@ export default class AwsS3Multipart extends BasePlugin {
     return res.token
   }
 
-  // NOTE! Keep this duplicated code in sync with other plugins
-  // TODO we should probably abstract this into a common function
-  async #uploadRemote (file, options) {
-    this.resetUploaderReferences(file.id)
-
-    try {
-      if (file.serverToken) {
-        return await this.connectToServerSocket(file)
-      }
-      const serverToken = await this.#queueRequestSocketToken(file, options).abortOn(options?.signal)
-
-      if (!this.uppy.getState().files[file.id]) return undefined
-
-      this.uppy.setFileState(file.id, { serverToken })
-      return await this.connectToServerSocket(this.uppy.getFile(file.id))
-    } catch (err) {
-      if (err?.cause?.name !== 'AbortError') {
-        this.uppy.setFileState(file.id, { serverToken: undefined })
-        this.uppy.emit('upload-error', file, err)
-        throw err
-      }
-      // The file upload was aborted, it’s not an error
-      return undefined
-    }
-  }
-
   async connectToServerSocket (file) {
     return new Promise((resolve, reject) => {
       let queuedRequest
@@ -841,10 +813,10 @@ export default class AwsS3Multipart extends BasePlugin {
         const removedHandler = (removedFile) => {
           if (removedFile.id === file.id) controller.abort()
         }
-
         this.uppy.on('file-removed', removedHandler)
 
-        const uploadPromise = this.#uploadRemote(file, { signal: controller.signal })
+        this.resetUploaderReferences(file.id)
+        const uploadPromise = this.uploadRemoteFile(file, { signal: controller.signal })
 
         this.requests.wrapSyncFunction(() => {
           this.uppy.off('file-removed', removedHandler)

+ 5 - 87
packages/@uppy/aws-s3/src/MiniXHRUpload.js

@@ -1,5 +1,5 @@
 import { nanoid } from 'nanoid/non-secure'
-import { Provider, RequestClient, Socket } from '@uppy/companion-client'
+import { Socket } from '@uppy/companion-client'
 import emitSocketProgress from '@uppy/utils/lib/emitSocketProgress'
 import getSocketHost from '@uppy/utils/lib/getSocketHost'
 import EventManager from '@uppy/utils/lib/EventManager'
@@ -53,8 +53,6 @@ function createFormDataUpload (file, opts) {
 const createBareUpload = file => file.data
 
 export default class MiniXHRUpload {
-  #queueRequestSocketToken
-
   constructor (uppy, opts) {
     this.uppy = uppy
     this.opts = {
@@ -67,11 +65,9 @@ export default class MiniXHRUpload {
     this.requests = opts[internalRateLimitedQueue]
     this.uploaderEvents = Object.create(null)
     this.i18n = opts.i18n
-
-    this.#queueRequestSocketToken = this.requests.wrapPromiseFunction(this.#requestSocketToken, { priority: -1 })
   }
 
-  #getOptions (file) {
+  getOptions (file) {
     const { uppy } = this
 
     const overrides = uppy.getState().xhrUpload
@@ -89,31 +85,6 @@ export default class MiniXHRUpload {
     return opts
   }
 
-  uploadFile (id, current, total) {
-    const file = this.uppy.getFile(id)
-    this.uppy.log(`uploading ${current} of ${total}`)
-
-    if (file.error) {
-      throw new Error(file.error)
-    } else if (file.isRemote) {
-      const controller = new AbortController()
-
-      const removedHandler = (removedFile) => {
-        if (removedFile.id === file.id) controller.abort()
-      }
-      this.uppy.on('file-removed', removedHandler)
-
-      const uploadPromise = this.#uploadRemoteFile(file, { signal: controller.signal })
-
-      this.requests.wrapSyncFunction(() => {
-        this.uppy.off('file-removed', removedHandler)
-      }, { priority: -1 })()
-
-      return uploadPromise
-    }
-    return this.#uploadLocalFile(file, current, total)
-  }
-
   #addEventHandlerForFile (eventName, fileID, eventHandler) {
     this.uploaderEvents[fileID].on(eventName, (fileOrID) => {
       // TODO (major): refactor Uppy events to consistently send file objects (or consistently IDs)
@@ -130,8 +101,8 @@ export default class MiniXHRUpload {
     })
   }
 
-  #uploadLocalFile (file) {
-    const opts = this.#getOptions(file)
+  uploadLocalFile (file) {
+    const opts = this.getOptions(file)
 
     return new Promise((resolve, reject) => {
       // This is done in index.js in the S3 plugin.
@@ -265,62 +236,9 @@ export default class MiniXHRUpload {
     })
   }
 
-  #requestSocketToken = async (file) => {
-    const opts = this.#getOptions(file)
-    const Client = file.remote.providerOptions.provider ? Provider : RequestClient
-    const client = new Client(this.uppy, file.remote.providerOptions)
-    const allowedMetaFields = Array.isArray(opts.allowedMetaFields)
-      ? opts.allowedMetaFields
-      // Send along all fields by default.
-      : Object.keys(file.meta)
-
-    if (file.tus) {
-      // Install file-specific upload overrides.
-      Object.assign(opts, file.tus)
-    }
-
-    const res = await client.post(file.remote.url, {
-      ...file.remote.body,
-      protocol: 'multipart',
-      endpoint: opts.endpoint,
-      size: file.data.size,
-      fieldname: opts.fieldName,
-      metadata: Object.fromEntries(allowedMetaFields.map(name => [name, file.meta[name]])),
-      httpMethod: opts.method,
-      useFormData: opts.formData,
-      headers: opts.headers,
-    })
-    return res.token
-  }
-
-  // NOTE! Keep this duplicated code in sync with other plugins
-  // TODO we should probably abstract this into a common function
-  async #uploadRemoteFile (file, options) {
-    // TODO: we could rewrite this to use server-sent events instead of creating WebSockets.
-    try {
-      if (file.serverToken) {
-        return await this.connectToServerSocket(file)
-      }
-      const serverToken = await this.#queueRequestSocketToken(file, options).abortOn(options?.signal)
-
-      if (!this.uppy.getState().files[file.id]) return undefined
-
-      this.uppy.setFileState(file.id, { serverToken })
-      return await this.connectToServerSocket(this.uppy.getFile(file.id))
-    } catch (err) {
-      if (err?.cause?.name !== 'AbortError') {
-        this.uppy.setFileState(file.id, { serverToken: undefined })
-        this.uppy.emit('upload-error', file, err)
-        throw err
-      }
-      // The file upload was aborted, it’s not an error
-      return undefined
-    }
-  }
-
   async connectToServerSocket (file) {
     return new Promise((resolve, reject) => {
-      const opts = this.#getOptions(file)
+      const opts = this.getOptions(file)
       const token = file.serverToken
       const host = getSocketHost(file.remote.companionUrl)
       let socket

+ 64 - 4
packages/@uppy/aws-s3/src/index.js

@@ -25,10 +25,10 @@
  * the XHRUpload code, but at least it's not horrifically broken :)
  */
 
-import BasePlugin from '@uppy/core/lib/BasePlugin.js'
+import UploaderPlugin from '@uppy/core/lib/UploaderPlugin.js'
 import AwsS3Multipart from '@uppy/aws-s3-multipart'
 import { RateLimitedQueue, internalRateLimitedQueue } from '@uppy/utils/lib/RateLimitedQueue'
-import { RequestClient } from '@uppy/companion-client'
+import { RequestClient, Provider } from '@uppy/companion-client'
 import { filterNonFailedFiles, filterFilesToEmitUploadStarted } from '@uppy/utils/lib/fileFilters'
 
 import packageJson from '../package.json'
@@ -103,7 +103,7 @@ function defaultGetResponseError (content, xhr) {
 let warnedSuccessActionStatus = false
 
 // TODO deprecate this, will use s3-multipart instead
-export default class AwsS3 extends BasePlugin {
+export default class AwsS3 extends UploaderPlugin {
   static VERSION = packageJson.version
 
   #client
@@ -144,6 +144,8 @@ export default class AwsS3 extends BasePlugin {
 
     this.#client = new RequestClient(uppy, opts)
     this.#requests = new RateLimitedQueue(this.opts.limit)
+
+    this.setQueueRequestSocketToken(this.#requests.wrapPromiseFunction(this.#requestSocketToken, { priority: -1 }))
   }
 
   [Symbol.for('uppy test: getClient')] () { return this.#client }
@@ -228,7 +230,7 @@ export default class AwsS3 extends BasePlugin {
           xhrUpload: xhrOpts,
         })
 
-        return this.#uploader.uploadFile(file.id, index, numberOfFiles)
+        return this.uploadFile(file.id, index, numberOfFiles)
       }).catch((error) => {
         delete paramsPromises[id]
 
@@ -247,6 +249,64 @@ export default class AwsS3 extends BasePlugin {
     return Promise.resolve()
   }
 
+  connectToServerSocket (file) {
+    return this.#uploader.connectToServerSocket(file)
+  }
+
+  #requestSocketToken = async (file) => {
+    const opts = this.#uploader.getOptions(file)
+    const Client = file.remote.providerOptions.provider ? Provider : RequestClient
+    const client = new Client(this.uppy, file.remote.providerOptions)
+    const allowedMetaFields = Array.isArray(opts.allowedMetaFields)
+      ? opts.allowedMetaFields
+      // Send along all fields by default.
+      : Object.keys(file.meta)
+
+    if (file.tus) {
+      // Install file-specific upload overrides.
+      Object.assign(opts, file.tus)
+    }
+
+    const res = await client.post(file.remote.url, {
+      ...file.remote.body,
+      protocol: 'multipart',
+      endpoint: opts.endpoint,
+      size: file.data.size,
+      fieldname: opts.fieldName,
+      metadata: Object.fromEntries(allowedMetaFields.map(name => [name, file.meta[name]])),
+      httpMethod: opts.method,
+      useFormData: opts.formData,
+      headers: opts.headers,
+    })
+    return res.token
+  }
+
+  uploadFile (id, current, total) {
+    const file = this.uppy.getFile(id)
+    this.uppy.log(`uploading ${current} of ${total}`)
+
+    if (file.error) throw new Error(file.error)
+
+    if (file.isRemote) {
+      const controller = new AbortController()
+
+      const removedHandler = (removedFile) => {
+        if (removedFile.id === file.id) controller.abort()
+      }
+      this.uppy.on('file-removed', removedHandler)
+
+      const uploadPromise = this.uploadRemoteFile(file, { signal: controller.signal })
+
+      this.requests.wrapSyncFunction(() => {
+        this.uppy.off('file-removed', removedHandler)
+      }, { priority: -1 })()
+
+      return uploadPromise
+    }
+
+    return this.#uploader.uploadLocalFile(file, current, total)
+  }
+
   install () {
     const { uppy } = this
     uppy.addPreProcessor(this.#setCompanionHeaders)

+ 47 - 9
packages/@uppy/companion-client/src/Provider.js

@@ -8,6 +8,8 @@ const getName = (id) => {
 }
 
 export default class Provider extends RequestClient {
+  #refreshingTokenPromise
+
   constructor (uppy, opts) {
     super(uppy, opts)
     this.provider = opts.provider
@@ -20,7 +22,7 @@ export default class Provider extends RequestClient {
   }
 
   async headers () {
-    const [headers, token] = await Promise.all([super.headers(), this.getAuthToken()])
+    const [headers, token] = await Promise.all([super.headers(), this.#getAuthToken()])
     const authHeaders = {}
     if (token) {
       authHeaders['uppy-auth-token'] = token
@@ -43,14 +45,18 @@ export default class Provider extends RequestClient {
     return response
   }
 
-  setAuthToken (token) {
+  async setAuthToken (token) {
     return this.uppy.getPlugin(this.pluginId).storage.setItem(this.tokenKey, token)
   }
 
-  getAuthToken () {
+  async #getAuthToken () {
     return this.uppy.getPlugin(this.pluginId).storage.getItem(this.tokenKey)
   }
 
+  async #removeAuthToken () {
+    return this.uppy.getPlugin(this.pluginId).storage.removeItem(this.tokenKey)
+  }
+
   /**
    * Ensure we have a preauth token if necessary. Attempts to fetch one if we don't,
    * or rejects if loading one fails.
@@ -74,10 +80,44 @@ export default class Provider extends RequestClient {
     return `${this.hostname}/${this.id}/connect?${params}`
   }
 
+  refreshTokenUrl () {
+    return `${this.hostname}/${this.id}/refresh-token`
+  }
+
   fileUrl (id) {
     return `${this.hostname}/${this.id}/get/${id}`
   }
 
+  /** @protected */
+  async request (...args) {
+    await this.#refreshingTokenPromise
+
+    try {
+      // throw Object.assign(new Error(), { isAuthError: true }) // testing simulate access token expired (to refresh token)
+      return await super.request(...args)
+    } catch (err) {
+      if (!err.isAuthError) throw err // only handle auth errors (401 from provider)
+
+      await this.#refreshingTokenPromise
+
+      // Many provider requests may be starting at once, however refresh token should only be called once.
+      // Once a refresh token operation has started, we need all other request to wait for this operation (atomically)
+      this.#refreshingTokenPromise = (async () => {
+        try {
+          const response = await super.request({ path: this.refreshTokenUrl(), method: 'POST' })
+          await this.setAuthToken(response.uppyAuthToken)
+        } finally {
+          this.#refreshingTokenPromise = undefined
+        }
+      })()
+
+      await this.#refreshingTokenPromise
+
+      // now retry the request with our new refresh token
+      return super.request(...args)
+    }
+  }
+
   async fetchPreAuthToken () {
     if (!this.companionKeysParams) {
       return
@@ -95,12 +135,10 @@ export default class Provider extends RequestClient {
     return this.get(`${this.id}/list/${directory || ''}`)
   }
 
-  logout () {
-    return this.get(`${this.id}/logout`)
-      .then((response) => Promise.all([
-        response,
-        this.uppy.getPlugin(this.pluginId).storage.removeItem(this.tokenKey),
-      ])).then(([response]) => response)
+  async logout () {
+    const response = await this.get(`${this.id}/logout`)
+    await this.#removeAuthToken()
+    return response
   }
 
   static initPlugin (plugin, opts, defaultOpts) {

+ 5 - 4
packages/@uppy/companion-client/src/RequestClient.js

@@ -150,7 +150,8 @@ export default class RequestClient {
     }))
   }
 
-  async #request ({ path, method = 'GET', data, skipPostResponse, signal }) {
+  /** @protected */
+  async request ({ path, method = 'GET', data, skipPostResponse, signal }) {
     try {
       const headers = await this.preflightAndHeaders(path)
       const response = await fetchWithNetworkError(this.#getUrl(path), {
@@ -172,20 +173,20 @@ export default class RequestClient {
     // 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 })
+    return this.request({ ...options, path })
   }
 
   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 })
+    return this.request({ ...options, path, method: 'POST', data })
   }
 
   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 }
-    return this.#request({ ...options, path, method: 'DELETE', data })
+    return this.request({ ...options, path, method: 'DELETE', data })
   }
 }

+ 1 - 0
packages/@uppy/companion/src/companion.js

@@ -123,6 +123,7 @@ module.exports.app = (optionsArg = {}) => {
   app.get('/:providerName/connect', middlewares.hasSessionAndProvider, middlewares.hasOAuthProvider, controllers.connect)
   app.get('/:providerName/redirect', middlewares.hasSessionAndProvider, middlewares.hasOAuthProvider, controllers.redirect)
   app.get('/:providerName/callback', middlewares.hasSessionAndProvider, middlewares.hasOAuthProvider, controllers.callback)
+  app.post('/:providerName/refresh-token', middlewares.hasSessionAndProvider, middlewares.hasOAuthProvider, middlewares.verifyToken, controllers.refreshToken)
   app.post('/:providerName/deauthorization/callback', express.json(), middlewares.hasSessionAndProvider, middlewares.hasBody, middlewares.hasOAuthProvider, controllers.deauthorizationCallback)
   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)

+ 2 - 0
packages/@uppy/companion/src/config/grant.js

@@ -8,12 +8,14 @@ module.exports = () => {
         'https://www.googleapis.com/auth/drive.readonly',
       ],
       callback: '/drive/callback',
+      custom_params: { access_type : 'offline' },
     },
     dropbox: {
       transport: 'session',
       authorize_url: 'https://www.dropbox.com/oauth2/authorize',
       access_url: 'https://api.dropbox.com/oauth2/token',
       callback: '/dropbox/callback',
+      custom_params: { token_access_type : 'offline' },
     },
     box: {
       transport: 'session',

+ 19 - 13
packages/@uppy/companion/src/server/controllers/callback.js

@@ -31,21 +31,27 @@ const closePageHtml = (origin) => `
 module.exports = function callback (req, res, next) { // eslint-disable-line no-unused-vars
   const { providerName } = req.params
 
-  if (!req.companion.providerTokens) {
-    req.companion.providerTokens = {}
+  const grant = req.session.grant || {}
+
+  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 grant = req.session.grant || {}
-  if (grant.response && grant.response.access_token) {
-    req.companion.providerTokens[providerName] = grant.response.access_token
-    logger.debug(`Generating auth token for provider ${providerName}`, null, req.id)
-    const uppyAuthToken = tokenService.generateEncryptedToken(req.companion.providerTokens, req.companion.options.secret)
-    return res.redirect(req.companion.buildURL(`/${providerName}/send-token?uppyAuthToken=${uppyAuthToken}`, true))
+  if (!req.companion.allProvidersTokens) req.companion.allProvidersTokens = {}
+  req.companion.allProvidersTokens[providerName] = {
+    accessToken: grant.response.access_token,
+    refreshToken: grant.response.refresh_token, // might be undefined for some providers
   }
+  logger.debug(`Generating auth token for provider ${providerName}`, null, req.id)
+  const uppyAuthToken = tokenService.generateEncryptedAuthToken(
+    req.companion.allProvidersTokens, req.companion.options.secret,
+  )
+
+  tokenService.addToCookiesIfNeeded(req, res, uppyAuthToken)
 
-  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))
+  return res.redirect(req.companion.buildURL(`/${providerName}/send-token?uppyAuthToken=${uppyAuthToken}`, true))
 }

+ 2 - 6
packages/@uppy/companion/src/server/controllers/deauth-callback.js

@@ -1,4 +1,4 @@
-const { errorToResponse } = require('../provider/error')
+const { respondWithError } = require('../provider/error')
 
 async function deauthCallback ({ body, companion, headers }, res, next) {
   // we need the provider instance to decide status codes because
@@ -10,11 +10,7 @@ async function deauthCallback ({ body, companion, headers }, res, next) {
     res.status(status || 200).json(data)
     return
   } catch (err) {
-    const errResp = errorToResponse(err)
-    if (errResp) {
-      res.status(errResp.code).json({ message: errResp.message })
-      return
-    }
+    if (respondWithError(err, res)) return
     next(err)
   }
 }

+ 3 - 3
packages/@uppy/companion/src/server/controllers/get.js

@@ -3,15 +3,15 @@ const { startDownUpload } = require('../helpers/upload')
 
 async function get (req, res) {
   const { id } = req.params
-  const token = req.companion.providerToken
+  const { accessToken } = req.companion.providerTokens
   const { provider } = req.companion
 
   async function getSize () {
-    return provider.size({ id, token, query: req.query })
+    return provider.size({ id, token: accessToken, query: req.query })
   }
 
   async function download () {
-    const { stream } = await provider.download({ id, token, query: req.query })
+    const { stream } = await provider.download({ id, token: accessToken, query: req.query })
     return stream
   }
 

+ 1 - 0
packages/@uppy/companion/src/server/controllers/index.js

@@ -10,4 +10,5 @@ module.exports = {
   connect: require('./connect'),
   preauth: require('./preauth'),
   redirect: require('./oauth-redirect'),
+  refreshToken: require('./refresh-token'),
 }

+ 4 - 8
packages/@uppy/companion/src/server/controllers/list.js

@@ -1,17 +1,13 @@
-const { errorToResponse } = require('../provider/error')
+const { respondWithError } = require('../provider/error')
 
 async function list ({ query, params, companion }, res, next) {
-  const token = companion.providerToken
+  const { accessToken } = companion.providerTokens
 
   try {
-    const data = await companion.provider.list({ companion, token, directory: params.id, query })
+    const data = await companion.provider.list({ companion, token: accessToken, directory: params.id, query })
     res.json(data)
   } catch (err) {
-    const errResp = errorToResponse(err)
-    if (errResp) {
-      res.status(errResp.code).json({ message: errResp.message })
-      return
-    }
+    if (respondWithError(err, res)) return
     next(err)
   }
 }

+ 7 - 10
packages/@uppy/companion/src/server/controllers/logout.js

@@ -1,5 +1,5 @@
 const tokenService = require('../helpers/jwt')
-const { errorToResponse } = require('../provider/error')
+const { respondWithError } = require('../provider/error')
 
 /**
  *
@@ -15,26 +15,23 @@ async function logout (req, res, next) {
   }
   const { providerName } = req.params
   const { companion } = req
-  const token = companion.providerTokens ? companion.providerTokens[providerName] : null
+  const tokens = companion.allProvidersTokens ? companion.allProvidersTokens[providerName] : null
 
-  if (!token) {
+  if (!tokens) {
     cleanSession()
     res.json({ ok: true, revoked: false })
     return
   }
 
   try {
-    const data = await companion.provider.logout({ token, companion })
-    delete companion.providerTokens[providerName]
+    const { accessToken } = tokens
+    const data = await companion.provider.logout({ token: accessToken, companion })
+    delete companion.allProvidersTokens[providerName]
     tokenService.removeFromCookies(res, companion.options, companion.provider.authProvider)
     cleanSession()
     res.json({ ok: true, ...data })
   } catch (err) {
-    const errResp = errorToResponse(err)
-    if (errResp) {
-      res.status(errResp.code).json({ message: errResp.message })
-      return
-    }
+    if (respondWithError(err, res)) return
     next(err)
   }
 }

+ 51 - 0
packages/@uppy/companion/src/server/controllers/refresh-token.js

@@ -0,0 +1,51 @@
+const tokenService = require('../helpers/jwt')
+const { respondWithError } = require('../provider/error')
+const logger = require('../logger')
+
+// https://www.dropboxforum.com/t5/Dropbox-API-Support-Feedback/Get-refresh-token-from-access-token/td-p/596739
+// https://developers.dropbox.com/oauth-guide
+// https://github.com/simov/grant/issues/149
+async function refreshToken (req, res, next) {
+  const { providerName } = req.params
+
+  const { key: clientId, secret: clientSecret } = req.companion.options.providerOptions[providerName]
+
+  const providerTokens = req.companion.allProvidersTokens[providerName]
+
+  // not all providers have refresh tokens
+  if (providerTokens.refreshToken == null) {
+    res.sendStatus(401)
+    return
+  }
+
+  try {
+    const data = await req.companion.provider.refreshToken({
+      clientId, clientSecret, refreshToken: providerTokens.refreshToken,
+    })
+
+    const newAllProvidersTokens = {
+      ...req.companion.allProvidersTokens,
+      [providerName]: {
+        ...providerTokens,
+        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,
+    )
+
+    tokenService.addToCookiesIfNeeded(req, res, uppyAuthToken)
+
+    res.send({ uppyAuthToken })
+  } catch (err) {
+    if (respondWithError(err, res)) return
+    next(err)
+  }
+}
+
+module.exports = refreshToken

+ 0 - 5
packages/@uppy/companion/src/server/controllers/send-token.js

@@ -1,7 +1,6 @@
 const { URL } = require('node:url')
 const serialize = require('serialize-javascript')
 
-const tokenService = require('../helpers/jwt')
 const { hasMatch } = require('../helpers/utils')
 const oAuthState = require('../helpers/oauth-state')
 
@@ -33,10 +32,6 @@ const htmlContent = (token, origin) => {
  */
 module.exports = function sendToken (req, res, next) {
   const uppyAuthToken = req.companion.authToken
-  // some providers need the token in cookies for thumbnail/image requests
-  if (req.companion.provider.needsCookieAuth) {
-    tokenService.addToCookies(res, uppyAuthToken, req.companion.options, req.companion.provider.authProvider)
-  }
 
   const state = oAuthState.getDynamicStateFromRequest(req)
   if (state) {

+ 2 - 2
packages/@uppy/companion/src/server/controllers/thumbnail.js

@@ -5,11 +5,11 @@
  */
 async function thumbnail (req, res, next) {
   const { providerName, id } = req.params
-  const token = req.companion.providerTokens[providerName]
+  const { accessToken } = req.companion.allProvidersTokens[providerName]
   const { provider } = req.companion
 
   try {
-    const { stream } = await provider.thumbnail({ id, token })
+    const { stream } = await provider.thumbnail({ id, token: accessToken })
     stream.pipe(res)
   } catch (err) {
     if (err.isAuthError) res.sendStatus(401)

+ 54 - 28
packages/@uppy/companion/src/server/helpers/jwt.js

@@ -1,15 +1,29 @@
 const jwt = require('jsonwebtoken')
 const { encrypt, decrypt } = require('./utils')
 
-const EXPIRY = 60 * 60 * 24 // one day (24 hrs)
+// The Uppy auth token is a (JWT) container around provider OAuth access & refresh tokens.
+// Providers themselves will verify these inner tokens.
+// 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.
+// 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:
+// A user starts an upload, thus creating an auth token. They leave the upload running over night.
+// The upload finishes after a few hours, but with some failed files. Then the next day (say after 25hr)
+// they would retry the failed tiles, however now the Uppy auth token has expired and
+// 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
 
 /**
  *
- * @param {*} payload
+ * @param {*} data
  * @param {string} secret
  */
-module.exports.generateToken = (payload, secret) => {
-  return jwt.sign({ data: payload }, secret, { expiresIn: EXPIRY })
+const generateToken = (data, secret) => {
+  return jwt.sign({ data }, secret, { expiresIn: EXPIRY })
 }
 
 /**
@@ -17,13 +31,9 @@ module.exports.generateToken = (payload, secret) => {
  * @param {string} token
  * @param {string} secret
  */
-module.exports.verifyToken = (token, secret) => {
-  try {
-    // @ts-ignore
-    return { payload: jwt.verify(token, secret, {}).data }
-  } catch (err) {
-    return { err }
-  }
+const verifyToken = (token, secret) => {
+  // @ts-ignore
+  return jwt.verify(token, secret, {}).data
 }
 
 /**
@@ -32,7 +42,17 @@ module.exports.verifyToken = (token, secret) => {
  * @param {string} secret
  */
 module.exports.generateEncryptedToken = (payload, secret) => {
-  return encrypt(module.exports.generateToken(payload, secret), secret)
+  // return payload // for easier debugging
+  return encrypt(generateToken(payload, secret), secret)
+}
+
+/**
+ *
+ * @param {*} payload
+ * @param {string} secret
+ */
+module.exports.generateEncryptedAuthToken = (payload, secret) => {
+  return module.exports.generateEncryptedToken(JSON.stringify(payload), secret)
 }
 
 /**
@@ -41,16 +61,26 @@ module.exports.generateEncryptedToken = (payload, secret) => {
  * @param {string} secret
  */
 module.exports.verifyEncryptedToken = (token, secret) => {
-  try {
-    return module.exports.verifyToken(decrypt(token, secret), secret)
-  } catch (err) {
-    return { err }
-  }
+  const ret = verifyToken(decrypt(token, secret), secret)
+  if (!ret) throw new Error('No payload')
+  return ret
+}
+
+/**
+ *
+ * @param {string} token
+ * @param {string} secret
+ */
+module.exports.verifyEncryptedAuthToken = (token, secret, providerName) => {
+  const json = module.exports.verifyEncryptedToken(token, secret)
+  const tokens = JSON.parse(json)
+  if (!tokens[providerName]) throw new Error(`Missing token payload for provider ${providerName}`)
+  return tokens
 }
 
 const addToCookies = (res, token, companionOptions, authProvider, prefix) => {
   const cookieOptions = {
-    maxAge: 1000 * EXPIRY, // would expire after one day (24 hrs)
+    maxAge: EXPIRY_MS,
     httpOnly: true,
   }
 
@@ -68,15 +98,11 @@ const addToCookies = (res, token, companionOptions, authProvider, prefix) => {
   res.cookie(`${prefix}--${authProvider}`, token, cookieOptions)
 }
 
-/**
- *
- * @param {object} res
- * @param {string} token
- * @param {object} companionOptions
- * @param {string} authProvider
- */
-module.exports.addToCookies = (res, token, companionOptions, authProvider) => {
-  addToCookies(res, token, companionOptions, authProvider, 'uppyAuthToken')
+module.exports.addToCookiesIfNeeded = (req, res, uppyAuthToken) => {
+  // 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')
+  }
 }
 
 /**
@@ -87,7 +113,7 @@ module.exports.addToCookies = (res, token, companionOptions, authProvider) => {
  */
 module.exports.removeFromCookies = (res, companionOptions, authProvider) => {
   const cookieOptions = {
-    maxAge: 1000 * EXPIRY, // would expire after one day (24 hrs)
+    maxAge: EXPIRY_MS,
     httpOnly: true,
   }
 

+ 2 - 6
packages/@uppy/companion/src/server/helpers/upload.js

@@ -1,6 +1,6 @@
 const Uploader = require('../Uploader')
 const logger = require('../logger')
-const { errorToResponse } = require('../provider/error')
+const { respondWithError } = require('../provider/error')
 
 const { ValidationError } = Uploader
 
@@ -36,11 +36,7 @@ async function startDownUpload ({ req, res, getSize, download, onUnhandledError
       return
     }
 
-    const errResp = errorToResponse(err)
-    if (errResp) {
-      res.status(errResp.code).json({ message: errResp.message })
-      return
-    }
+    if (respondWithError(err, res)) return
 
     onUnhandledError(err)
   }

+ 16 - 11
packages/@uppy/companion/src/server/middlewares.js

@@ -67,7 +67,9 @@ exports.verifyToken = (req, res, next) => {
       return
     }
 
-    req.companion.providerToken = providerOptions[providerName].key
+    req.companion.providerTokens = {
+      accessToken: providerOptions[providerName].key,
+    }
     next()
     return
   }
@@ -80,16 +82,15 @@ exports.verifyToken = (req, res, next) => {
     return
   }
   const { providerName } = req.params
-  const { err, payload } = tokenService.verifyEncryptedToken(token, req.companion.options.secret)
-  if (err || !payload[providerName]) {
-    if (err) {
-      logger.error(err.message, 'token.verify.error', req.id)
-    }
+  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
   }
-  req.companion.providerTokens = payload
-  req.companion.providerToken = payload[providerName]
   next()
 }
 
@@ -97,9 +98,13 @@ exports.verifyToken = (req, res, next) => {
 exports.gentleVerifyToken = (req, res, next) => {
   const { providerName } = req.params
   if (req.companion.authToken) {
-    const { err, payload } = tokenService.verifyEncryptedToken(req.companion.authToken, req.companion.options.secret)
-    if (!err && payload[providerName]) {
-      req.companion.providerTokens = payload
+    try {
+      const payload = tokenService.verifyEncryptedAuthToken(
+        req.companion.authToken, req.companion.options.secret, providerName,
+      )
+      req.companion.allProvidersTokens = payload
+    } catch (err) {
+      logger.error(err.message, 'token.gentle.verify.error', req.id)
     }
   }
   next()

+ 9 - 0
packages/@uppy/companion/src/server/provider/Provider.js

@@ -7,6 +7,7 @@ class Provider {
    * @param {object} options
    */
   constructor (options) { // eslint-disable-line no-unused-vars
+    // Some providers might need cookie auth for the thumbnails fetched via companion
     this.needsCookieAuth = false
     return this
   }
@@ -74,6 +75,14 @@ class Provider {
     throw new Error('method not implemented')
   }
 
+  /**
+   * Generate a new access token based on the refresh token
+   */
+  // eslint-disable-next-line class-methods-use-this,no-unused-vars
+  async refreshToken (options) {
+    throw new Error('method not implemented')
+  }
+
   /**
    * Name of the OAuth provider. Return empty string if no OAuth provider is needed.
    *

+ 10 - 6
packages/@uppy/companion/src/server/provider/credentials.js

@@ -69,7 +69,7 @@ async function fetchProviderKeys (providerName, companionOptions, credentialRequ
  * @returns {import('express').RequestHandler}
  */
 exports.getCredentialsOverrideMiddleware = (providers, companionOptions) => {
-  return (req, res, next) => {
+  return async (req, res, next) => {
     const { authProvider, override } = req.params
     const [providerName] = Object.keys(providers).filter((name) => providers[name].authProvider === authProvider)
     if (!providerName) {
@@ -97,13 +97,17 @@ exports.getCredentialsOverrideMiddleware = (providers, companionOptions) => {
       return
     }
 
-    const { err, payload } = tokenService.verifyEncryptedToken(preAuthToken, companionOptions.preAuthSecret)
-    if (err || !payload) {
+    let payload
+    try {
+      payload = tokenService.verifyEncryptedToken(preAuthToken, companionOptions.preAuthSecret)
+    } catch (err) {
       next()
       return
     }
 
-    fetchProviderKeys(providerName, companionOptions, payload).then((credentials) => {
+    try {
+      const credentials = await fetchProviderKeys(providerName, companionOptions, payload)
+
       res.locals.grant = {
         dynamic: {
           key: credentials.key,
@@ -116,7 +120,7 @@ exports.getCredentialsOverrideMiddleware = (providers, companionOptions) => {
       }
 
       next()
-    }).catch((keyErr) => {
+    } catch (keyErr) {
       // TODO we should return an html page here that can communicate the error
       // back to the Uppy client, just like /send-token does
       res.send(`
@@ -134,7 +138,7 @@ exports.getCredentialsOverrideMiddleware = (providers, companionOptions) => {
         </body>
         </html>
       `)
-    })
+    }
   }
 }
 

+ 11 - 0
packages/@uppy/companion/src/server/provider/drive/index.js

@@ -18,6 +18,10 @@ const getClient = ({ token }) => got.extend({
   },
 })
 
+const getOauthClient = () => got.extend({
+  prefixUrl: 'https://oauth2.googleapis.com',
+})
+
 async function getStats ({ id, token }) {
   const client = getClient({ token })
 
@@ -168,6 +172,13 @@ class Drive extends Provider {
     })
   }
 
+  async refreshToken ({ clientId, clientSecret, refreshToken }) {
+    return this.#withErrorHandling('provider.drive.token.refresh.error', async () => {
+      const { access_token: accessToken } = await getOauthClient().post('token', { form: { refresh_token: refreshToken, grant_type: 'refresh_token', client_id: clientId, client_secret: clientSecret } }).json()
+      return { accessToken }
+    })
+  }
+
   async #withErrorHandling (tag, fn) {
     return withProviderErrorHandling({
       fn,

+ 11 - 1
packages/@uppy/companion/src/server/provider/dropbox/index.js

@@ -24,6 +24,10 @@ const getClient = ({ token }) => got.extend({
   },
 })
 
+const getOauthClient = () => got.extend({
+  prefixUrl: 'https://api.dropboxapi.com/oauth2',
+})
+
 async function list ({ directory, query, token }) {
   if (query.cursor) {
     return getClient({ token }).post('files/list_folder/continue', { json: { cursor: query.cursor }, responseType: 'json' }).json()
@@ -52,7 +56,6 @@ class DropBox extends Provider {
   constructor (options) {
     super(options)
     this.authProvider = DropBox.authProvider
-    // needed for the thumbnails fetched via companion
     this.needsCookieAuth = true
   }
 
@@ -121,6 +124,13 @@ class DropBox extends Provider {
     })
   }
 
+  async refreshToken ({ clientId, clientSecret, refreshToken }) {
+    return this.#withErrorHandling('provider.dropbox.token.refresh.error', async () => {
+      const { access_token: accessToken } = await getOauthClient().post('token', { form: { refresh_token: refreshToken, grant_type: 'refresh_token', client_id: clientId, client_secret: clientSecret } }).json()
+      return { accessToken }
+    })
+  }
+
   async #withErrorHandling (tag, fn) {
     return withProviderErrorHandling({
       fn,

+ 2 - 0
packages/@uppy/companion/src/server/provider/error.d.ts

@@ -12,3 +12,5 @@ export class ProviderAuthError extends ProviderApiError {
 }
 
 export function errorToResponse(anyError: Error): { code: number, message: string }
+
+export function respondWithError(anyError: Error, res: any): boolean

+ 10 - 1
packages/@uppy/companion/src/server/provider/error.js

@@ -52,4 +52,13 @@ function errorToResponse (err) {
   return undefined
 }
 
-module.exports = { ProviderAuthError, ProviderApiError, errorToResponse }
+function respondWithError (err, res) {
+  const errResp = errorToResponse(err)
+  if (errResp) {
+    res.status(errResp.code).json({ message: errResp.message })
+    return true
+  }
+  return false
+}
+
+module.exports = { ProviderAuthError, ProviderApiError, errorToResponse, respondWithError }

+ 13 - 16
packages/@uppy/companion/src/server/provider/providerErrors.js

@@ -1,10 +1,8 @@
 const logger = require('../logger')
 const { ProviderApiError, ProviderAuthError } = require('./error')
 
-function convertProviderError ({ err, providerName, isAuthError = () => false, getJsonErrorMessage }) {
-  const { response } = err
-
-  function getErrorMessage () {
+async function withProviderErrorHandling ({ fn, tag, providerName, isAuthError = () => false, getJsonErrorMessage }) {
+  function getErrorMessage (response) {
     if (typeof response.body === 'object') {
       const message = getJsonErrorMessage(response.body)
       if (message != null) return message
@@ -17,22 +15,21 @@ function convertProviderError ({ err, providerName, isAuthError = () => false, g
     return `request to ${providerName} returned ${response.statusCode}`
   }
 
-  if (response) {
-    // @ts-ignore
-    if (isAuthError(response)) return new ProviderAuthError()
-
-    return new ProviderApiError(getErrorMessage(), response.statusCode)
-  }
-
-  return err
-}
-
-async function withProviderErrorHandling ({ fn, tag, providerName, isAuthError, getJsonErrorMessage }) {
   try {
     return await fn()
   } catch (err) {
-    const err2 = convertProviderError({ err, providerName, isAuthError, getJsonErrorMessage })
+    const { response } = err
+
+    let err2 = err
+
+    if (response) {
+      // @ts-ignore
+      if (isAuthError(response)) err2 = new ProviderAuthError()
+      else err2 = new ProviderApiError(getErrorMessage(response), response.statusCode)
+    }
+
     logger.error(err2, tag)
+
     throw err2
   }
 }

+ 18 - 7
packages/@uppy/companion/test/__tests__/callback.js

@@ -3,7 +3,7 @@ const mockOauthState = require('../mockoauthstate')()
 // eslint-disable-next-line import/order
 const request = require('supertest')
 const tokenService = require('../../src/server/helpers/jwt')
-const { getServer } = require('../mockserver')
+const { getServer, grantToken } = require('../mockserver')
 
 jest.mock('../../src/server/helpers/oauth-state', () => ({
   ...jest.requireActual('../../src/server/helpers/oauth-state'),
@@ -12,10 +12,10 @@ jest.mock('../../src/server/helpers/oauth-state', () => ({
 
 const authServer = getServer()
 const authData = {
-  dropbox: 'token value',
-  drive: 'token value',
+  dropbox: { accessToken: 'token value' },
+  drive: { accessToken: 'token value' },
 }
-const token = tokenService.generateEncryptedToken(authData, process.env.COMPANION_SECRET)
+const token = tokenService.generateEncryptedAuthToken(authData, process.env.COMPANION_SECRET)
 
 describe('test authentication callback', () => {
   test('authentication callback redirects to send-token url', () => {
@@ -27,14 +27,25 @@ describe('test authentication callback', () => {
       })
   })
 
-  test('the token gets sent via cookie and html', () => {
+  test('authentication callback sets cookie', () => {
+    console.log(process.env.COMPANION_SECRET)
+    return request(authServer)
+      .get('/dropbox/callback')
+      .expect(302)
+      .expect((res) => {
+        expect(res.header.location).toContain('http://localhost:3020/dropbox/send-token?uppyAuthToken=')
+        const authToken = decodeURIComponent(res.header['set-cookie'][0].split(';')[0].split('uppyAuthToken--dropbox=')[1])
+        const payload = tokenService.verifyEncryptedAuthToken(authToken, process.env.COMPANION_SECRET, 'dropbox')
+        expect(payload).toEqual({ dropbox: { accessToken: grantToken } })
+      })
+  })
+
+  test('the token gets sent via html', () => {
     // see mock ../../src/server/helpers/oauth-state above for state values
     return request(authServer)
       .get(`/dropbox/send-token?uppyAuthToken=${token}&state=state-with-newer-version`)
       .expect(200)
       .expect((res) => {
-        const authToken = res.header['set-cookie'][0].split(';')[0].split('uppyAuthToken--dropbox=')[1]
-        expect(authToken).toEqual(token)
         const body = `
     <!DOCTYPE html>
     <html>

+ 4 - 4
packages/@uppy/companion/test/__tests__/companion.js

@@ -32,11 +32,11 @@ const { getServer } = require('../mockserver')
 // todo don't share server between tests. rewrite to not use env variables
 const authServer = getServer({ COMPANION_CLIENT_SOCKET_CONNECT_TIMEOUT: '0' })
 const authData = {
-  dropbox: 'token value',
-  box: 'token value',
-  drive: 'token value',
+  dropbox: { accessToken: 'token value' },
+  box: { accessToken: 'token value' },
+  drive: { accessToken: 'token value' },
 }
-const token = tokenService.generateEncryptedToken(authData, process.env.COMPANION_SECRET)
+const token = tokenService.generateEncryptedAuthToken(authData, process.env.COMPANION_SECRET)
 const OAUTH_STATE = 'some-cool-nice-encrytpion'
 
 afterAll(() => {

+ 2 - 2
packages/@uppy/companion/test/__tests__/credentials.js

@@ -8,9 +8,9 @@ const { remoteZoomKey, remoteZoomSecret, remoteZoomVerificationToken } = require
 
 const authServer = getServer({ COMPANION_ZOOM_KEYS_ENDPOINT: 'http://localhost:2111/zoom-keys' })
 const authData = {
-  zoom: 'token value',
+  zoom: { accessToken: 'token value' },
 }
-const token = tokenService.generateEncryptedToken(authData, process.env.COMPANION_SECRET)
+const token = tokenService.generateEncryptedAuthToken(authData, process.env.COMPANION_SECRET)
 
 afterAll(() => {
   nock.cleanAll()

+ 3 - 9
packages/@uppy/companion/test/__tests__/preauth.js

@@ -1,14 +1,8 @@
 jest.mock('../../src/server/helpers/jwt', () => {
   return {
-    generateToken: () => {},
-    verifyToken: () => {},
-    generateEncryptedToken: () => {
-      return 'dummy token'
-    },
-    verifyEncryptedToken: () => {
-      return { payload: '' }
-    },
-    addToCookies: () => {},
+    generateEncryptedToken: () => 'dummy token',
+    verifyEncryptedToken: () => '',
+    addToCookiesIfNeeded: () => {},
     removeFromCookies: () => {},
   }
 })

+ 6 - 0
packages/@uppy/companion/test/__tests__/provider-manager.js

@@ -52,6 +52,9 @@ describe('Test Provider options', () => {
       authorize_url: 'https://www.dropbox.com/oauth2/authorize',
       access_url: 'https://api.dropbox.com/oauth2/token',
       callback: '/dropbox/callback',
+      custom_params: {
+        token_access_type: 'offline',
+      },
     })
 
     expect(grantConfig.box).toEqual({
@@ -73,6 +76,9 @@ describe('Test Provider options', () => {
         'https://www.googleapis.com/auth/drive.readonly',
       ],
       callback: '/drive/callback',
+      custom_params: {
+        access_type: 'offline',
+      },
     })
     expect(grantConfig.zoom).toEqual({
       key: 'zoom_key',

+ 2 - 2
packages/@uppy/companion/test/__tests__/providers.js

@@ -31,9 +31,9 @@ const AUTH_PROVIDERS = {
 }
 const authData = {}
 providerNames.forEach((provider) => {
-  authData[provider] = 'token value'
+  authData[provider] = { accessToken: 'token value' }
 })
-const token = tokenService.generateEncryptedToken(authData, process.env.COMPANION_SECRET)
+const token = tokenService.generateEncryptedAuthToken(authData, process.env.COMPANION_SECRET)
 
 const thisOrThat = (value1, value2) => {
   if (value1 !== undefined) {

+ 3 - 1
packages/@uppy/companion/test/mockserver.js

@@ -50,6 +50,8 @@ function updateEnv (env) {
 
 module.exports.setDefaultEnv = () => updateEnv(defaultEnv)
 
+module.exports.grantToken = 'fake token'
+
 module.exports.getServer = (extraEnv) => {
   const env = {
     ...defaultEnv,
@@ -69,7 +71,7 @@ module.exports.getServer = (extraEnv) => {
   authServer.use(session({ secret: 'grant', resave: true, saveUninitialized: true }))
   authServer.all('*/callback', (req, res, next) => {
     req.session.grant = {
-      response: { access_token: 'fake token' },
+      response: { access_token: module.exports.grantToken },
     }
     next()
   })

+ 34 - 0
packages/@uppy/core/src/UploaderPlugin.js

@@ -0,0 +1,34 @@
+import BasePlugin from './BasePlugin.js'
+
+export default class UploaderPlugin extends BasePlugin {
+  #queueRequestSocketToken
+
+  /** @protected */
+  setQueueRequestSocketToken (fn) {
+    this.#queueRequestSocketToken = fn
+  }
+
+  async uploadRemoteFile (file, options = {}) {
+    // TODO: we could rewrite this to use server-sent events instead of creating WebSockets.
+    try {
+      if (file.serverToken) {
+        return await this.connectToServerSocket(file)
+      }
+      const serverToken = await this.#queueRequestSocketToken(file).abortOn(options.signal)
+
+      if (!this.uppy.getState().files[file.id]) return undefined
+
+      this.uppy.setFileState(file.id, { serverToken })
+      return await this.connectToServerSocket(this.uppy.getFile(file.id))
+    } catch (err) {
+      if (err?.cause?.name === 'AbortError') {
+        // The file upload was aborted, it’s not an error
+        return undefined
+      }
+
+      this.uppy.setFileState(file.id, { serverToken: undefined })
+      this.uppy.emit('upload-error', file, err)
+      throw err
+    }
+  }
+}

+ 2 - 2
packages/@uppy/core/src/Uppy.js

@@ -1009,13 +1009,13 @@ class Uppy {
       errorHandler(error, file, response)
 
       if (typeof error === 'object' && error.message) {
-        const newError = new Error(error.message)
+        this.log(error.message, 'error')
+        const newError = new Error(this.i18n('failedToUpload', { file: file?.name }))
         newError.isUserFacing = true // todo maybe don't do this with all errors?
         newError.details = error.message
         if (error.details) {
           newError.details += ` ${error.details}`
         }
-        newError.message = this.i18n('failedToUpload', { file: file?.name })
         this.#informAndEmit([newError])
       } else {
         this.#informAndEmit([error])

+ 5 - 36
packages/@uppy/tus/src/index.js

@@ -1,4 +1,4 @@
-import BasePlugin from '@uppy/core/lib/BasePlugin.js'
+import UploaderPlugin from '@uppy/core/lib/UploaderPlugin.js'
 import * as tus from 'tus-js-client'
 import { Provider, RequestClient, Socket } from '@uppy/companion-client'
 import emitSocketProgress from '@uppy/utils/lib/emitSocketProgress'
@@ -52,13 +52,11 @@ const tusDefaultOptions = {
 /**
  * Tus resumable file uploader
  */
-export default class Tus extends BasePlugin {
+export default class Tus extends UploaderPlugin {
   static VERSION = packageJson.version
 
   #retryDelayIterator
 
-  #queueRequestSocketToken
-
   /**
    * @param {Uppy} uppy
    * @param {TusOptions} opts
@@ -102,7 +100,7 @@ export default class Tus extends BasePlugin {
     this.uploaderSockets = Object.create(null)
 
     this.handleResetProgress = this.handleResetProgress.bind(this)
-    this.#queueRequestSocketToken = this.requests.wrapPromiseFunction(this.#requestSocketToken, { priority: -1 })
+    this.setQueueRequestSocketToken(this.requests.wrapPromiseFunction(this.#requestSocketToken, { priority: -1 }))
   }
 
   handleResetProgress () {
@@ -455,36 +453,6 @@ export default class Tus extends BasePlugin {
     return res.token
   }
 
-  // NOTE! Keep this duplicated code in sync with other plugins
-  // TODO we should probably abstract this into a common function
-  /**
-   * @param {UppyFile} file for use with upload
-   * @returns {Promise<void>}
-   */
-  async #uploadRemote (file, options) {
-    this.resetUploaderReferences(file.id)
-
-    try {
-      if (file.serverToken) {
-        return await this.connectToServerSocket(file)
-      }
-      const serverToken = await this.#queueRequestSocketToken(file, options).abortOn(options?.signal)
-
-      if (!this.uppy.getState().files[file.id]) return undefined
-
-      this.uppy.setFileState(file.id, { serverToken })
-      return await this.connectToServerSocket(this.uppy.getFile(file.id))
-    } catch (err) {
-      if (err?.cause?.name !== 'AbortError') {
-        this.uppy.setFileState(file.id, { serverToken: undefined })
-        this.uppy.emit('upload-error', file, err)
-        throw err
-      }
-      // The file upload was aborted, it’s not an error
-      return undefined
-    }
-  }
-
   /**
    * See the comment on the upload() method.
    *
@@ -745,7 +713,8 @@ export default class Tus extends BasePlugin {
         }
         this.uppy.on('file-removed', removedHandler)
 
-        const uploadPromise = this.#uploadRemote(file, { signal: controller.signal })
+        this.resetUploaderReferences(file.id)
+        const uploadPromise = this.uploadRemoteFile(file, { signal: controller.signal })
 
         this.requests.wrapSyncFunction(() => {
           this.uppy.off('file-removed', removedHandler)

+ 4 - 31
packages/@uppy/xhr-upload/src/index.js

@@ -1,4 +1,4 @@
-import BasePlugin from '@uppy/core/lib/BasePlugin.js'
+import UploaderPlugin from '@uppy/core/lib/UploaderPlugin.js'
 import { nanoid } from 'nanoid/non-secure'
 import { Provider, RequestClient, Socket } from '@uppy/companion-client'
 import emitSocketProgress from '@uppy/utils/lib/emitSocketProgress'
@@ -46,12 +46,10 @@ function setTypeInBlob (file) {
   return dataWithUpdatedType
 }
 
-export default class XHRUpload extends BasePlugin {
+export default class XHRUpload extends UploaderPlugin {
   // eslint-disable-next-line global-require
   static VERSION = packageJson.version
 
-  #queueRequestSocketToken
-
   constructor (uppy, opts) {
     super(uppy, opts)
     this.type = 'uploader'
@@ -129,7 +127,7 @@ export default class XHRUpload extends BasePlugin {
     }
 
     this.uploaderEvents = Object.create(null)
-    this.#queueRequestSocketToken = this.requests.wrapPromiseFunction(this.#requestSocketToken, { priority: -1 })
+    this.setQueueRequestSocketToken(this.requests.wrapPromiseFunction(this.#requestSocketToken, { priority: -1 }))
   }
 
   getOptions (file) {
@@ -373,31 +371,6 @@ export default class XHRUpload extends BasePlugin {
     return res.token
   }
 
-  // NOTE! Keep this duplicated code in sync with other plugins
-  // TODO we should probably abstract this into a common function
-  async #uploadRemote (file, options) {
-    // TODO: we could rewrite this to use server-sent events instead of creating WebSockets.
-    try {
-      if (file.serverToken) {
-        return await this.connectToServerSocket(file)
-      }
-      const serverToken = await this.#queueRequestSocketToken(file, options).abortOn(options?.signal)
-
-      if (!this.uppy.getState().files[file.id]) return undefined
-
-      this.uppy.setFileState(file.id, { serverToken })
-      return await this.connectToServerSocket(this.uppy.getFile(file.id))
-    } catch (err) {
-      if (err?.cause?.name !== 'AbortError') {
-        this.uppy.setFileState(file.id, { serverToken: undefined })
-        this.uppy.emit('upload-error', file, err)
-        throw err
-      }
-      // The file upload was aborted, it’s not an error
-      return undefined
-    }
-  }
-
   async connectToServerSocket (file) {
     return new Promise((resolve, reject) => {
       const opts = this.getOptions(file)
@@ -603,7 +576,7 @@ export default class XHRUpload extends BasePlugin {
         }
         this.uppy.on('file-removed', removedHandler)
 
-        const uploadPromise = this.#uploadRemote(file, { signal: controller.signal })
+        const uploadPromise = this.uploadRemoteFile(file, { signal: controller.signal })
 
         this.requests.wrapSyncFunction(() => {
           this.uppy.off('file-removed', removedHandler)