Merge pull request #2055 from transloadit/conditional-cookie

companion: only set cookies for providers that need it
transloadit · Feb 13, 2020 · 6433056 · 6433056
2 parents 1205b9c + d7b92cc
commit 6433056
Show file tree

Hide file tree

Showing 7 changed files with 18 additions and 19 deletions.
diff --git a/packages/@uppy/companion/src/server/controllers/send-token.js b/packages/@uppy/companion/src/server/controllers/send-token.js
@@ -16,8 +16,10 @@ const versionCmp = require('../helpers/version')
  */
 module.exports = function sendToken (req, res, next) {
   const uppyAuthToken = req.companion.authToken
-  // add the token to cookies for thumbnail/image requests
-  tokenService.addToCookies(res, uppyAuthToken, req.companion.options, req.companion.provider.authProvider)
+  // 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 dynamic = (req.session.grant || {}).dynamic || {}
   const state = dynamic.state

diff --git a/packages/@uppy/companion/src/server/provider/Provider.js b/packages/@uppy/companion/src/server/provider/Provider.js
@@ -7,6 +7,7 @@ class Provider {
    * @param {object} options
    */
   constructor (options) {
+    this.needsCookieAuth = false
     return this
   }
 

diff --git a/packages/@uppy/companion/src/server/provider/drive/adapter.js b/packages/@uppy/companion/src/server/provider/drive/adapter.js
@@ -64,7 +64,7 @@ exports.getItemModifiedDate = (item) => {
 }
 
 exports.getItemThumbnailUrl = (item) => {
-  return `/drive/thumbnail/${exports.getItemRequestPath(item)}`
+  return item.thumbnailLink
 }
 
 exports.isSharedDrive = (item) => {

diff --git a/packages/@uppy/companion/src/server/provider/drive/index.js b/packages/@uppy/companion/src/server/provider/drive/index.js
@@ -81,7 +81,6 @@ class Drive extends Provider {
           const returnData = this.adaptData(
             filesResponse.body,
             sharedDrives && sharedDrives.body,
-            options.companion,
             directory,
             query
           )
@@ -117,16 +116,11 @@ class Drive extends Provider {
       })
   }
 
-  thumbnail ({ id, token }, done) {
-    return this.stats({ id, token }, (err, resp, body) => {
-      if (err || resp.statusCode !== 200) {
-        err = this._error(err, resp)
-        logger.error(err, 'provider.drive.thumbnail.error')
-        return done(err)
-      }
-
-      done(null, body.thumbnailLink ? request(body.thumbnailLink) : null)
-    })
+  thumbnail (_, done) {
+    // not implementing this because a public thumbnail from googledrive will be used instead
+    const err = new Error('call to thumbnail is not implemented')
+    logger.error(err, 'provider.drive.thumbnail.error')
+    return done(err)
   }
 
   size ({ id, token }, done) {
@@ -154,14 +148,14 @@ class Drive extends Provider {
       })
   }
 
-  adaptData (res, sharedDrivesResp, companion, directory, query) {
+  adaptData (res, sharedDrivesResp, directory, query) {
     const adaptItem = (item) => ({
       isFolder: adapter.isFolder(item),
       icon: adapter.getItemIcon(item),
       name: adapter.getItemName(item),
       mimeType: adapter.getMimeType(item),
       id: adapter.getItemId(item),
-      thumbnail: companion.buildURL(adapter.getItemThumbnailUrl(item), true),
+      thumbnail: adapter.getItemThumbnailUrl(item),
       requestPath: adapter.getItemRequestPath(item),
       modifiedDate: adapter.getItemModifiedDate(item),
       size: adapter.getItemSize(item),

diff --git a/packages/@uppy/companion/src/server/provider/dropbox/index.js b/packages/@uppy/companion/src/server/provider/dropbox/index.js
@@ -27,6 +27,8 @@ class DropBox extends Provider {
     super(options)
     this.authProvider = options.provider = DropBox.authProvider
     this.client = purest(options)
+    // needed for the thumbnails fetched via companion
+    this.needsCookieAuth = true
   }
 
   static get authProvider () {

diff --git a/packages/@uppy/companion/test/__tests__/companion.js b/packages/@uppy/companion/test/__tests__/companion.js
@@ -94,10 +94,10 @@ describe('test authentication', () => {
   test('the token gets sent via cookie and html', () => {
     // see mock ../../src/server/helpers/oauth-state above for state values
     return request(authServer)
-      .get(`/drive/send-token?uppyAuthToken=${token}&state=state-with-newer-version`)
+      .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--google=')[1]
+        const authToken = res.header['set-cookie'][0].split(';')[0].split('uppyAuthToken--dropbox=')[1]
         expect(authToken).toEqual(token)
         const body = `
     <!DOCTYPE html>

diff --git a/packages/@uppy/companion/test/mockserver.js b/packages/@uppy/companion/test/mockserver.js
@@ -11,7 +11,7 @@ authServer.all('*/callback', (req, res, next) => {
   }
   next()
 })
-authServer.all('/drive/send-token', (req, res, next) => {
+authServer.all('*/send-token', (req, res, next) => {
   req.session.grant = { dynamic: { state: req.query.state || 'non-empty-value' } }
   next()
 })
-Original file line number
+Diff line change
@@ Expand Up / @@ -11,7 +11,7 @@ authServer.all('*/callback', (req, res, next) => { @@
       }
       next()
     })
-    authServer.all('/drive/send-token', (req, res, next) => {
+    authServer.all('*/send-token', (req, res, next) => {
       req.session.grant = { dynamic: { state: req.query.state || 'non-empty-value' } }
       next()
     })
@@ Expand Down @@