From fc7261aca6485141fca95b6eccffe1f1a9a8c0c4 Mon Sep 17 00:00:00 2001 From: agobrech <45268029+agobrech@users.noreply.github.com> Date: Wed, 21 Jun 2023 10:54:32 +0200 Subject: [PATCH] feat(core): Add PKCE for OAuth2 (#6324) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * Remove authorization header when empty * Import pkce * Add OAuth2 with new grant type to Twitter * Add pkce logic auto assign authorization code if pkce not defined * Add pkce to ui and interfaces * Fix scopes for Oauth2 twitter * Deubg + pass it through header * Add debug console, add airtable cred * Remove all console.logs, make PKCE in th body only when it exists * Remove invalid character ~ * Remove more console.logs * remove body inside query * Remove useless grantype check * Hide oauth2 twitter waiting for overhaul * Remove redundant header removal * Remove more console.logs * Add comment for code verifier * Remove uneeded scopes * Restore client id in callback * Revert "Add OAuth2 with new grant type to Twitter" This reverts commit 1c3b331aa1974159d1ffe1a4fbf2050722f0f24c. * Remove oauth2 from twitter * Remove properties linked to oauth2 * Fix lodash imports * remove redundant check * remove redundant codeVerifier * patch pkce-challenge to avoid generating `code_verifier` with `~` * store `codeVerifier` on the DB like `csrfSecret` * remove unrelated changes --------- Co-authored-by: Marcus Co-authored-by: कारतोफ्फेलस्क्रिप्ट™ --- package.json | 3 +- packages/cli/package.json | 1 + .../src/credentials/oauth2Credential.api.ts | 25 +++++++-- packages/core/src/NodeExecuteFunctions.ts | 7 ++- .../CredentialEdit/CredentialEdit.vue | 3 +- .../AirtableOAuth2Api.credentials.ts | 52 +++++++++++++++++++ .../credentials/OAuth2Api.credentials.ts | 8 ++- .../nodes/Airtable/Airtable.node.ts | 13 +++++ packages/nodes-base/package.json | 1 + packages/workflow/src/Interfaces.ts | 3 +- patches/pkce-challenge@3.0.0.patch | 13 +++++ pnpm-lock.yaml | 13 +++++ 12 files changed, 130 insertions(+), 12 deletions(-) create mode 100644 packages/nodes-base/credentials/AirtableOAuth2Api.credentials.ts create mode 100644 patches/pkce-challenge@3.0.0.patch diff --git a/package.json b/package.json index df71eb1546..7cb908271c 100644 --- a/package.json +++ b/package.json @@ -94,7 +94,8 @@ "patchedDependencies": { "element-ui@2.15.12": "patches/element-ui@2.15.12.patch", "typedi@0.10.0": "patches/typedi@0.10.0.patch", - "@sentry/cli@2.17.0": "patches/@sentry__cli@2.17.0.patch" + "@sentry/cli@2.17.0": "patches/@sentry__cli@2.17.0.patch", + "pkce-challenge@3.0.0": "patches/pkce-challenge@3.0.0.patch" } } } diff --git a/packages/cli/package.json b/packages/cli/package.json index a338c2633a..487825b842 100644 --- a/packages/cli/package.json +++ b/packages/cli/package.json @@ -163,6 +163,7 @@ "passport-cookie": "^1.0.9", "passport-jwt": "^4.0.0", "pg": "^8.8.0", + "pkce-challenge": "^3.0.0", "picocolors": "^1.0.0", "posthog-node": "^2.2.2", "prom-client": "^13.1.0", diff --git a/packages/cli/src/credentials/oauth2Credential.api.ts b/packages/cli/src/credentials/oauth2Credential.api.ts index f0e74b53fe..2e31432f0b 100644 --- a/packages/cli/src/credentials/oauth2Credential.api.ts +++ b/packages/cli/src/credentials/oauth2Credential.api.ts @@ -2,6 +2,7 @@ import type { ClientOAuth2Options } from '@n8n/client-oauth2'; import { ClientOAuth2 } from '@n8n/client-oauth2'; import Csrf from 'csrf'; import express from 'express'; +import pkceChallenge from 'pkce-challenge'; import get from 'lodash/get'; import omit from 'lodash/omit'; import set from 'lodash/set'; @@ -142,6 +143,16 @@ oauth2CredentialController.get( ); decryptedDataOriginal.csrfSecret = csrfSecret; + if (oauthCredentials.grantType === 'pkce') { + const { code_verifier, code_challenge } = pkceChallenge(); + oAuthOptions.query = { + ...oAuthOptions.query, + code_challenge, + code_challenge_method: 'S256', + }; + decryptedDataOriginal.codeVerifier = code_verifier; + } + credentials.setData(decryptedDataOriginal, encryptionKey); const newCredentialsData = credentials.getDataToSave() as unknown as ICredentialsDb; @@ -189,7 +200,6 @@ oauth2CredentialController.get( try { // realmId it's currently just use for the quickbook OAuth2 flow const { code, state: stateEncoded } = req.query; - if (!code || !stateEncoded) { return renderCallbackError( res, @@ -265,12 +275,21 @@ oauth2CredentialController.get( if ((get(oauthCredentials, 'authentication', 'header') as string) === 'body') { options = { body: { - client_id: get(oauthCredentials, 'clientId') as string, - client_secret: get(oauthCredentials, 'clientSecret', '') as string, + ...(oauthCredentials.grantType === 'pkce' && { + code_verifier: decryptedDataOriginal.codeVerifier, + }), + ...(oauthCredentials.grantType === 'authorizationCode' && { + client_id: get(oauthCredentials, 'clientId') as string, + client_secret: get(oauthCredentials, 'clientSecret', '') as string, + }), }, }; // @ts-ignore delete oAuth2Parameters.clientSecret; + } else if (oauthCredentials.grantType === 'pkce') { + options = { + body: { code_verifier: decryptedDataOriginal.codeVerifier }, + }; } await Container.get(ExternalHooks).run('oauth2.callback', [oAuth2Parameters]); diff --git a/packages/core/src/NodeExecuteFunctions.ts b/packages/core/src/NodeExecuteFunctions.ts index efccf5ea9e..16f8f7b691 100644 --- a/packages/core/src/NodeExecuteFunctions.ts +++ b/packages/core/src/NodeExecuteFunctions.ts @@ -1104,14 +1104,12 @@ export async function requestOAuth2( }); let oauthTokenData = credentials.oauthTokenData as ClientOAuth2TokenData; - // if it's the first time using the credentials, get the access token and save it into the DB. if ( credentials.grantType === OAuth2GrantType.clientCredentials && (oauthTokenData === undefined || Object.keys(oauthTokenData).length === 0) ) { const { data } = await getClientCredentialsToken(oAuthClient, credentials); - // Find the credentials if (!node.credentials?.[credentialsType]) { throw new Error( @@ -1150,7 +1148,6 @@ export async function requestOAuth2( if (oAuth2Options?.keepBearer === false && typeof newRequestHeaders.Authorization === 'string') { newRequestHeaders.Authorization = newRequestHeaders.Authorization.split(' ')[1]; } - if (oAuth2Options?.keyToIncludeInAccessTokenHeader) { Object.assign(newRequestHeaders, { [oAuth2Options.keyToIncludeInAccessTokenHeader]: token.accessToken, @@ -1166,7 +1163,9 @@ export async function requestOAuth2( if (oAuth2Options?.includeCredentialsOnRefreshOnBody) { const body: IDataObject = { client_id: credentials.clientId as string, - client_secret: credentials.clientSecret as string, + ...(credentials.grantType === 'authorizationCode' && { + client_secret: credentials.clientSecret as string, + }), }; tokenRefreshOptions.body = body; tokenRefreshOptions.headers = { diff --git a/packages/editor-ui/src/components/CredentialEdit/CredentialEdit.vue b/packages/editor-ui/src/components/CredentialEdit/CredentialEdit.vue index 37202c4eee..02dcf89a8a 100644 --- a/packages/editor-ui/src/components/CredentialEdit/CredentialEdit.vue +++ b/packages/editor-ui/src/components/CredentialEdit/CredentialEdit.vue @@ -412,7 +412,8 @@ export default defineComponent({ return ( !!this.credentialTypeName && (((this.credentialTypeName === 'oAuth2Api' || this.parentTypes.includes('oAuth2Api')) && - this.credentialData.grantType === 'authorizationCode') || + (this.credentialData.grantType === 'authorizationCode' || + this.credentialData.grantType === 'pkce')) || this.credentialTypeName === 'oAuth1Api' || this.parentTypes.includes('oAuth1Api')) ); diff --git a/packages/nodes-base/credentials/AirtableOAuth2Api.credentials.ts b/packages/nodes-base/credentials/AirtableOAuth2Api.credentials.ts new file mode 100644 index 0000000000..65a2bcce48 --- /dev/null +++ b/packages/nodes-base/credentials/AirtableOAuth2Api.credentials.ts @@ -0,0 +1,52 @@ +import type { ICredentialType, INodeProperties } from 'n8n-workflow'; + +const scopes = ['schema.bases:read', 'data.records:read', 'data.records:write']; + +export class AirtableOAuth2Api implements ICredentialType { + name = 'airtableOAuth2Api'; + + extends = ['oAuth2Api']; + + displayName = 'Airtable OAuth2 API'; + + documentationUrl = 'airtable'; + + properties: INodeProperties[] = [ + { + displayName: 'Grant Type', + name: 'grantType', + type: 'hidden', + default: 'pkce', + }, + { + displayName: 'Authorization URL', + name: 'authUrl', + type: 'hidden', + default: 'https://airtable.com/oauth2/v1/authorize', + }, + { + displayName: 'Access Token URL', + name: 'accessTokenUrl', + type: 'hidden', + default: 'https://airtable.com/oauth2/v1/token', + }, + { + displayName: 'Scope', + name: 'scope', + type: 'hidden', + default: `${scopes.join(' ')}`, + }, + { + displayName: 'Auth URI Query Parameters', + name: 'authQueryParameters', + type: 'hidden', + default: '', + }, + { + displayName: 'Authentication', + name: 'authentication', + type: 'hidden', + default: 'header', + }, + ]; +} diff --git a/packages/nodes-base/credentials/OAuth2Api.credentials.ts b/packages/nodes-base/credentials/OAuth2Api.credentials.ts index d818ea3b67..22b0d8258c 100644 --- a/packages/nodes-base/credentials/OAuth2Api.credentials.ts +++ b/packages/nodes-base/credentials/OAuth2Api.credentials.ts @@ -23,6 +23,10 @@ export class OAuth2Api implements ICredentialType { name: 'Client Credentials', value: 'clientCredentials', }, + { + name: 'PKCE', + value: 'pkce', + }, ], default: 'authorizationCode', }, @@ -32,7 +36,7 @@ export class OAuth2Api implements ICredentialType { type: 'string', displayOptions: { show: { - grantType: ['authorizationCode'], + grantType: ['authorizationCode', 'pkce'], }, }, default: '', @@ -74,7 +78,7 @@ export class OAuth2Api implements ICredentialType { type: 'string', displayOptions: { show: { - grantType: ['authorizationCode'], + grantType: ['authorizationCode', 'pkce'], }, }, default: '', diff --git a/packages/nodes-base/nodes/Airtable/Airtable.node.ts b/packages/nodes-base/nodes/Airtable/Airtable.node.ts index 49c5491976..096b8bfe98 100644 --- a/packages/nodes-base/nodes/Airtable/Airtable.node.ts +++ b/packages/nodes-base/nodes/Airtable/Airtable.node.ts @@ -42,6 +42,15 @@ export class Airtable implements INodeType { }, }, }, + { + name: 'airtableOAuth2Api', + required: true, + displayOptions: { + show: { + authentication: ['airtableOAuth2Api'], + }, + }, + }, ], properties: [ { @@ -57,6 +66,10 @@ export class Airtable implements INodeType { name: 'Access Token', value: 'airtableTokenApi', }, + { + name: 'OAuth2', + value: 'airtableOAuth2Api', + }, ], default: 'airtableApi', }, diff --git a/packages/nodes-base/package.json b/packages/nodes-base/package.json index f4b52092a6..a916c8ccf5 100644 --- a/packages/nodes-base/package.json +++ b/packages/nodes-base/package.json @@ -39,6 +39,7 @@ "dist/credentials/AffinityApi.credentials.js", "dist/credentials/AgileCrmApi.credentials.js", "dist/credentials/AirtableApi.credentials.js", + "dist/credentials/AirtableOAuth2Api.credentials.js", "dist/credentials/AirtableTokenApi.credentials.js", "dist/credentials/Amqp.credentials.js", "dist/credentials/ApiTemplateIoApi.credentials.js", diff --git a/packages/workflow/src/Interfaces.ts b/packages/workflow/src/Interfaces.ts index c6e2ed3486..2a944ddbe7 100644 --- a/packages/workflow/src/Interfaces.ts +++ b/packages/workflow/src/Interfaces.ts @@ -1907,11 +1907,12 @@ export interface IConnectedNode { } export const enum OAuth2GrantType { + pkce = 'pkce', authorizationCode = 'authorizationCode', clientCredentials = 'clientCredentials', } export interface IOAuth2Credentials { - grantType: 'authorizationCode' | 'clientCredentials'; + grantType: 'authorizationCode' | 'clientCredentials' | 'pkce'; clientId: string; clientSecret: string; accessTokenUrl: string; diff --git a/patches/pkce-challenge@3.0.0.patch b/patches/pkce-challenge@3.0.0.patch new file mode 100644 index 0000000000..f3d9159ee9 --- /dev/null +++ b/patches/pkce-challenge@3.0.0.patch @@ -0,0 +1,13 @@ +diff --git a/dist/main.js b/dist/main.js +index 86be84f44210b26583e0a7f1732acd8b98a5e701..a2b05be6a45355704fedf43b51a34793580eaf6c 100644 +--- a/dist/main.js ++++ b/dist/main.js +@@ -42,7 +42,7 @@ $parcel$export(module.exports, "verifyChallenge", () => $f5bfd4ce37214f4f$export + * @param size The desired length of the string + * @returns The random string + */ function $f5bfd4ce37214f4f$var$random(size) { +- const mask = "abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789-._~"; ++ const mask = "abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789-._"; + let result = ""; + const randomUints = $f5bfd4ce37214f4f$var$getRandomValues(size); + for(let i = 0; i < size; i++){ \ No newline at end of file diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index 5e122940ba..8082eaafff 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -32,6 +32,9 @@ patchedDependencies: element-ui@2.15.12: hash: prckukfdop5sl2her6de25cod4 path: patches/element-ui@2.15.12.patch + pkce-challenge@3.0.0: + hash: dypouzb3lve7vncq25i5fuanki + path: patches/pkce-challenge@3.0.0.patch typedi@0.10.0: hash: 62r6bc2crgimafeyruodhqlgo4 path: patches/typedi@0.10.0.patch @@ -383,6 +386,9 @@ importers: picocolors: specifier: ^1.0.0 version: 1.0.0 + pkce-challenge: + specifier: ^3.0.0 + version: 3.0.0(patch_hash=dypouzb3lve7vncq25i5fuanki) posthog-node: specifier: ^2.2.2 version: 2.2.2 @@ -18092,6 +18098,13 @@ packages: engines: {node: '>= 6'} dev: true + /pkce-challenge@3.0.0(patch_hash=dypouzb3lve7vncq25i5fuanki): + resolution: {integrity: sha512-sQ8sJJJuLhA5pFnoxayMCrFnBMNj7DDpa+TWxOXl4B24oXHlVSADi/3Bowm66QuzWkBuF6DhmaelCdlC2JKwsg==} + dependencies: + crypto-js: 4.1.1 + dev: false + patched: true + /pkg-dir@3.0.0: resolution: {integrity: sha512-/E57AYkoeQ25qkxMj5PBOVgF8Kiu/h7cYS30Z5+R7WaiCCBfLq58ZI/dSeaEKb9WVJV5n/03QwrN3IeWIFllvw==} engines: {node: '>=6'}