From cde05cbb318e33e36932d66ba9ba225cbaf1df44 Mon Sep 17 00:00:00 2001 From: ifedapoolarewaju Date: Thu, 6 Feb 2020 15:32:14 +0100 Subject: [PATCH 1/3] companion: only set cookies for providers that need it fixes #1967 --- .../src/server/controllers/send-token.js | 6 ++++-- .../companion/src/server/provider/Provider.js | 1 + .../src/server/provider/drive/adapter.js | 2 +- .../src/server/provider/drive/index.js | 20 +++++++------------ .../src/server/provider/dropbox/index.js | 2 ++ 5 files changed, 15 insertions(+), 16 deletions(-) diff --git a/packages/@uppy/companion/src/server/controllers/send-token.js b/packages/@uppy/companion/src/server/controllers/send-token.js index d5dde169e5..9b54046c15 100644 --- 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 index 0a935e9660..d18fd68434 100644 --- 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 index e6b4c9ab18..128ef32712 100644 --- 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 index 98e6d777d7..3355f86359 100644 --- a/packages/@uppy/companion/src/server/provider/drive/index.js +++ b/packages/@uppy/companion/src/server/provider/drive/index.js @@ -78,7 +78,6 @@ class Drive extends Provider { const returnData = this.adaptData( filesResponse.body, sharedDrives && sharedDrives.body, - options.companion, directory, query ) @@ -114,16 +113,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 onedrive 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) { @@ -151,14 +145,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 index 3fc117e773..b6519816f1 100644 --- a/packages/@uppy/companion/src/server/provider/dropbox/index.js +++ b/packages/@uppy/companion/src/server/provider/dropbox/index.js @@ -24,6 +24,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 () { From deec61bc4e550e617ac5e1db37bf076f671d6c1a Mon Sep 17 00:00:00 2001 From: ifedapoolarewaju Date: Thu, 6 Feb 2020 15:38:02 +0100 Subject: [PATCH 2/3] comments corrected --- packages/@uppy/companion/src/server/provider/drive/index.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/@uppy/companion/src/server/provider/drive/index.js b/packages/@uppy/companion/src/server/provider/drive/index.js index 3355f86359..5b9bdd6344 100644 --- a/packages/@uppy/companion/src/server/provider/drive/index.js +++ b/packages/@uppy/companion/src/server/provider/drive/index.js @@ -114,7 +114,7 @@ class Drive extends Provider { } thumbnail (_, done) { - // not implementing this because a public thumbnail from onedrive will be used instead + // 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) From c0c5b3edfb55dd49458596a27fa6b9f29fb5cc53 Mon Sep 17 00:00:00 2001 From: ifedapoolarewaju Date: Thu, 6 Feb 2020 15:57:22 +0100 Subject: [PATCH 3/3] companion: fix tests --- packages/@uppy/companion/test/__tests__/companion.js | 4 ++-- packages/@uppy/companion/test/mockserver.js | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/packages/@uppy/companion/test/__tests__/companion.js b/packages/@uppy/companion/test/__tests__/companion.js index 166f588e14..e03bf6fa7b 100644 --- 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 = ` diff --git a/packages/@uppy/companion/test/mockserver.js b/packages/@uppy/companion/test/mockserver.js index 7e24609434..e908381a39 100644 --- 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() })