companion: override grant's default redirect_uri for consistent provider options (#2364)

* companion: override grant's default redirect_uri for consistent provider options

* companion[test]: passing tests

packages/@uppy/companion/src/server/controllers/oauth-redirect.js

@@ -9,6 +9,12 @@ const oAuthState = require('../helpers/oauth-state')
  * @param {object} res
  */
 module.exports = function oauthRedirect (req, res) {
+  const params = qs.stringify(req.query)
+  const authProvider = req.companion.provider.authProvider
+  if (!req.companion.options.server.oauthDomain) {
+    return res.redirect(`/connect/${authProvider}/callback?${params}`)
+  }
+
   const dynamic = (req.session.grant || {}).dynamic || {}
   const state = dynamic.state
   if (!state) {
@@ -18,9 +24,7 @@ module.exports = function oauthRedirect (req, res) {
   const handlerHostName = (new URL(handler)).host
 
   if (hasMatch(handlerHostName, req.companion.options.server.validHosts)) {
-    const providerName = req.companion.provider.authProvider
-    const params = qs.stringify(req.query)
-    const url = `${handler}/connect/${providerName}/callback?${params}`
+    const url = `${handler}/connect/${authProvider}/callback?${params}`
     return res.redirect(url)
   }
 

packages/@uppy/companion/src/server/provider/index.js

@@ -110,9 +110,10 @@ module.exports.addProviderOptions = (companionOptions, grantConfig) => {
       Object.assign(grantConfig[authProvider], provider.getExtraConfig())
 
       // override grant.js redirect uri with companion's custom redirect url
+      const isExternal = !!server.implicitPath
+      const redirectPath = `/${providerName}/redirect`
+      grantConfig[authProvider].redirect_uri = getURLBuilder(companionOptions)(redirectPath, isExternal)
       if (oauthDomain) {
-        const redirectPath = `/${providerName}/redirect`
-        const isExternal = !!server.implicitPath
         const fullRedirectPath = getURLBuilder(companionOptions)(redirectPath, isExternal, true)
         grantConfig[authProvider].redirect_uri = `${server.protocol}://${oauthDomain}${fullRedirectPath}`
       }

packages/@uppy/companion/test/__tests__/companion.js

@@ -30,7 +30,8 @@ jest.mock('../../src/server/helpers/oauth-state', () => {
 
 const request = require('supertest')
 const tokenService = require('../../src/server/helpers/jwt')
-const { authServer } = require('../mockserver')
+const { getServer } = require('../mockserver')
+const authServer = getServer()
 const authData = {
   dropbox: 'token value',
   drive: 'token value'
@@ -297,9 +298,12 @@ describe('connect to provider', () => {
   })
 })
 
-describe('handle oauth redirect', () => {
+describe('handle master oauth redirect', () => {
+  const serverWithMasterOauth = getServer({
+    COMPANION_OAUTH_DOMAIN: 'localhost:3040'
+  })
   test('redirect to a valid uppy instance', () => {
-    return request(authServer)
+    return request(serverWithMasterOauth)
       .get(`/dropbox/redirect?state=${OAUTH_STATE}`)
       .set('uppy-auth-token', token)
       .expect(302)
@@ -308,7 +312,7 @@ describe('handle oauth redirect', () => {
 
   test('do not redirect to invalid uppy instances', () => {
     const state = 'state-with-invalid-instance-url' // see mock ../../src/server/helpers/oauth-state above
-    return request(authServer)
+    return request(serverWithMasterOauth)
       .get(`/dropbox/redirect?state=${state}`)
       .set('uppy-auth-token', token)
       .expect(400)

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

@@ -29,6 +29,7 @@ describe('Test Provider options', () => {
     expect(grantConfig.instagram).toEqual({
       transport: 'session',
       callback: '/instagram/callback',
+      redirect_uri: 'http://localhost:3020/instagram/redirect',
       key: '123456',
       secret: 'instagram_secret',
       protocol: 'https',
@@ -39,6 +40,7 @@ describe('Test Provider options', () => {
       key: 'dropbox_key',
       secret: 'dropbox_secret',
       transport: 'session',
+      redirect_uri: 'http://localhost:3020/dropbox/redirect',
       authorize_url: 'https://www.dropbox.com/oauth2/authorize',
       access_url: 'https://api.dropbox.com/oauth2/token',
       callback: '/dropbox/callback'
@@ -48,6 +50,7 @@ describe('Test Provider options', () => {
       key: 'google_key',
       secret: 'google_secret',
       transport: 'session',
+      redirect_uri: 'http://localhost:3020/drive/redirect',
       scope: [
         'https://www.googleapis.com/auth/drive.readonly'
       ],

packages/@uppy/companion/test/mockserver.js

@@ -1,21 +1,31 @@
-const { app } = require('../src/standalone')
-
+/* global jest:false */
 const express = require('express')
 const session = require('express-session')
-const authServer = express()
 
-authServer.use(session({ secret: 'grant', resave: true, saveUninitialized: true }))
-authServer.all('*/callback', (req, res, next) => {
-  req.session.grant = {
-    response: { access_token: 'fake token' }
+module.exports.getServer = (env) => {
+  if (env) {
+    Object.keys(env).forEach((key) => {
+      process.env[key] = env[key]
+    })
   }
-  next()
-})
-authServer.all(['*/send-token', '*/redirect'], (req, res, next) => {
-  req.session.grant = { dynamic: { state: req.query.state || 'non-empty-value' } }
-  next()
-})
 
-authServer.use(app)
+  // delete from cache to force the server to reload companionOptions from the new env vars
+  jest.resetModules()
+  const { app } = require('../src/standalone')
+  const authServer = express()
+
+  authServer.use(session({ secret: 'grant', resave: true, saveUninitialized: true }))
+  authServer.all('*/callback', (req, res, next) => {
+    req.session.grant = {
+      response: { access_token: 'fake token' }
+    }
+    next()
+  })
+  authServer.all(['*/send-token', '*/redirect'], (req, res, next) => {
+    req.session.grant = { dynamic: { state: req.query.state || 'non-empty-value' } }
+    next()
+  })
 
-module.exports = { authServer }
+  authServer.use(app)
+  return authServer
+}