From 46509d307ac10eabbf76a4536b3304ec80bcd348 Mon Sep 17 00:00:00 2001 From: Ifedapo Olarewaju Date: Thu, 14 Mar 2019 16:12:01 +0100 Subject: [PATCH] set separate auth cookie for each provider fixes the issue were uppyAuthToken cookie gets overridden by the most recent provider token. As a result thumbnail endpoint may return 401 for previously authed providers --- .../src/server/controllers/logout.js | 6 +---- .../src/server/controllers/send-token.js | 2 +- .../@uppy/companion/src/server/helpers/jwt.js | 24 +++++++++++++++++-- .../@uppy/companion/src/server/middlewares.js | 2 +- .../companion/test/__tests__/companion.js | 2 +- 5 files changed, 26 insertions(+), 10 deletions(-) diff --git a/packages/@uppy/companion/src/server/controllers/logout.js b/packages/@uppy/companion/src/server/controllers/logout.js index 2e93f803d..7b962bafe 100644 --- a/packages/@uppy/companion/src/server/controllers/logout.js +++ b/packages/@uppy/companion/src/server/controllers/logout.js @@ -11,11 +11,7 @@ function logout (req, res) { if (req.uppy.providerTokens[providerName]) { delete req.uppy.providerTokens[providerName] - tokenService.addToCookies( - res, - tokenService.generateToken(req.uppy.providerTokens, req.uppy.options.secret), - req.uppy.options - ) + tokenService.removeFromCookies(res, req.uppy.options, req.uppy.provider.authProviderName) } if (session.grant) { diff --git a/packages/@uppy/companion/src/server/controllers/send-token.js b/packages/@uppy/companion/src/server/controllers/send-token.js index 335d0987c..07fe8272b 100644 --- a/packages/@uppy/companion/src/server/controllers/send-token.js +++ b/packages/@uppy/companion/src/server/controllers/send-token.js @@ -16,7 +16,7 @@ const oAuthState = require('../helpers/oauth-state') module.exports = function sendToken (req, res, next) { const uppyAuthToken = req.uppy.authToken // add the token to cookies for thumbnail/image requests - tokenService.addToCookies(res, uppyAuthToken, req.uppy.options) + tokenService.addToCookies(res, uppyAuthToken, req.uppy.options, req.uppy.provider.authProvider) const state = (req.session.grant || {}).state if (state) { diff --git a/packages/@uppy/companion/src/server/helpers/jwt.js b/packages/@uppy/companion/src/server/helpers/jwt.js index 33e542d75..36efc133d 100644 --- a/packages/@uppy/companion/src/server/helpers/jwt.js +++ b/packages/@uppy/companion/src/server/helpers/jwt.js @@ -29,8 +29,9 @@ module.exports.verifyToken = (token, secret) => { * @param {object} res * @param {string} token * @param {object=} uppyOptions + * @param {string} providerName */ -module.exports.addToCookies = (res, token, uppyOptions) => { +module.exports.addToCookies = (res, token, uppyOptions, providerName) => { const cookieOptions = { maxAge: 1000 * 60 * 60 * 24 * 30, // would expire after 30 days httpOnly: true @@ -40,5 +41,24 @@ module.exports.addToCookies = (res, token, uppyOptions) => { cookieOptions.domain = uppyOptions.cookieDomain } // send signed token to client. - res.cookie('uppyAuthToken', token, cookieOptions) + res.cookie(`uppyAuthToken--${providerName}`, token, cookieOptions) +} + +/** + * + * @param {object} res + * @param {object=} uppyOptions + * @param {string} providerName + */ +module.exports.removeFromCookies = (res, uppyOptions, providerName) => { + const cookieOptions = { + maxAge: 1000 * 60 * 60 * 24 * 30, // would expire after 30 days + httpOnly: true + } + + if (uppyOptions.cookieDomain) { + cookieOptions.domain = uppyOptions.cookieDomain + } + + res.clearCookie(`uppyAuthToken--${providerName}`, cookieOptions) } diff --git a/packages/@uppy/companion/src/server/middlewares.js b/packages/@uppy/companion/src/server/middlewares.js index 2842ca6fc..a98fda04d 100644 --- a/packages/@uppy/companion/src/server/middlewares.js +++ b/packages/@uppy/companion/src/server/middlewares.js @@ -38,6 +38,6 @@ exports.gentleVerifyToken = (req, res, next) => { } exports.cookieAuthToken = (req, res, next) => { - req.uppy.authToken = req.cookies.uppyAuthToken + req.uppy.authToken = req.cookies[`uppyAuthToken--${req.uppy.provider.authProvider}`] return next() } diff --git a/packages/@uppy/companion/test/__tests__/companion.js b/packages/@uppy/companion/test/__tests__/companion.js index 064d48cbb..6d0c46ed2 100644 --- a/packages/@uppy/companion/test/__tests__/companion.js +++ b/packages/@uppy/companion/test/__tests__/companion.js @@ -80,7 +80,7 @@ describe('test authentication', () => { .get(`/drive/send-token?uppyAuthToken=${token}`) .expect(200) .expect((res) => { - const authToken = res.header['set-cookie'][0].split(';')[0].split('uppyAuthToken=')[1] + const authToken = res.header['set-cookie'][0].split(';')[0].split('uppyAuthToken--google=')[1] expect(authToken).toEqual(token) // see mock ../../src/server/helpers/oauth-state above for http://localhost:3020 const body = `