From d6d940c6b205e586cce390ec2936cb8a9836d618 Mon Sep 17 00:00:00 2001 From: Mikael Finstad Date: Mon, 11 Nov 2024 17:03:26 +0800 Subject: [PATCH] Enable CSRF protection in grant (OAuth2) (#5504) * Enable CSRF protection in grant (OAuth2) I've been doing some testing and from what I can see, this is already supported in https://github.com/simov/grant (which companion uses for OAuth2), when enabling the `state` parameter. It seems to be working and it is checking the parameter when redirected back from the provider: https://github.com/simov/grant/blob/61fe48a8dac6aa4ec5764fadff0898b743b85588/lib/flow/oauth2.js#L72So * fix test --- packages/@uppy/companion/src/config/grant.js | 22 ++++++++++++------- .../test/__tests__/provider-manager.js | 6 +++++ 2 files changed, 20 insertions(+), 8 deletions(-) diff --git a/packages/@uppy/companion/src/config/grant.js b/packages/@uppy/companion/src/config/grant.js index ad4983c24d..32b81a513e 100644 --- a/packages/@uppy/companion/src/config/grant.js +++ b/packages/@uppy/companion/src/config/grant.js @@ -1,6 +1,4 @@ const google = { - transport: 'session', - // access_type: offline is needed in order to get refresh tokens. // prompt: 'consent' is needed because sometimes a user will get stuck in an authenticated state where we will // receive no refresh tokens from them. This seems to be happen when running on different subdomains. @@ -15,51 +13,59 @@ const google = { "scope_delimiter": " " } +const defaults = { + transport: 'session', + state: true, // Enable CSRF check +}; + // oauth configuration for provider services that are used. module.exports = () => { return { // we need separate auth providers because scopes are different, // and because it would be a too big rewrite to allow reuse of the same provider. googledrive: { + ...defaults, ...google, + state: true, callback: '/drive/callback', scope: ['https://www.googleapis.com/auth/drive.readonly'], }, googlephotos: { + ...defaults, ...google, callback: '/googlephotos/callback', scope: ['https://www.googleapis.com/auth/photoslibrary.readonly', 'https://www.googleapis.com/auth/userinfo.email'], // if name is needed, then add https://www.googleapis.com/auth/userinfo.profile too }, dropbox: { - transport: 'session', + ...defaults, 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', + ...defaults, authorize_url: 'https://account.box.com/api/oauth2/authorize', access_url: 'https://api.box.com/oauth2/token', callback: '/box/callback', }, instagram: { - transport: 'session', + ...defaults, callback: '/instagram/callback', }, facebook: { - transport: 'session', + ...defaults, scope: ['email', 'user_photos'], callback: '/facebook/callback', }, // for onedrive microsoft: { - transport: 'session', + ...defaults, scope: ['files.read.all', 'offline_access', 'User.Read'], callback: '/onedrive/callback', }, zoom: { - transport: 'session', + ...defaults, authorize_url: 'https://zoom.us/oauth/authorize', access_url: 'https://zoom.us/oauth/token', callback: '/zoom/callback', diff --git a/packages/@uppy/companion/test/__tests__/provider-manager.js b/packages/@uppy/companion/test/__tests__/provider-manager.js index f5dfe73462..ce28981401 100644 --- a/packages/@uppy/companion/test/__tests__/provider-manager.js +++ b/packages/@uppy/companion/test/__tests__/provider-manager.js @@ -41,6 +41,7 @@ describe('Test Provider options', () => { providerManager.addProviderOptions(getCompanionOptions(), grantConfig, getOauthProvider) expect(grantConfig.instagram).toEqual({ transport: 'session', + "state": true, callback: '/instagram/callback', redirect_uri: 'http://localhost:3020/instagram/redirect', key: '123456', @@ -53,6 +54,7 @@ describe('Test Provider options', () => { key: 'dropbox_key', secret: 'dropbox_secret', transport: 'session', + "state": true, redirect_uri: 'http://localhost:3020/dropbox/redirect', authorize_url: 'https://www.dropbox.com/oauth2/authorize', access_url: 'https://api.dropbox.com/oauth2/token', @@ -66,6 +68,7 @@ describe('Test Provider options', () => { key: 'box_key', secret: 'box_secret', transport: 'session', + "state": true, redirect_uri: 'http://localhost:3020/box/redirect', authorize_url: 'https://account.box.com/api/oauth2/authorize', access_url: 'https://api.box.com/oauth2/token', @@ -81,6 +84,7 @@ describe('Test Provider options', () => { key: 'google_key', secret: 'google_secret', transport: 'session', + "state": true, redirect_uri: 'http://localhost:3020/drive/redirect', scope: [ 'https://www.googleapis.com/auth/drive.readonly', @@ -101,6 +105,7 @@ describe('Test Provider options', () => { key: 'google_key', secret: 'google_secret', transport: 'session', + "state": true, redirect_uri: 'http://localhost:3020/googlephotos/redirect', scope: ['https://www.googleapis.com/auth/photoslibrary.readonly', 'https://www.googleapis.com/auth/userinfo.email'], callback: '/googlephotos/callback', @@ -114,6 +119,7 @@ describe('Test Provider options', () => { key: 'zoom_key', secret: 'zoom_secret', transport: 'session', + "state": true, authorize_url: 'https://zoom.us/oauth/authorize', redirect_uri: 'http://localhost:3020/zoom/redirect', access_url: 'https://zoom.us/oauth/token',