diff --git a/packages/@n8n/client-oauth2/src/ClientOAuth2.ts b/packages/@n8n/client-oauth2/src/ClientOAuth2.ts index 288b91e4c..6074cf11c 100644 --- a/packages/@n8n/client-oauth2/src/ClientOAuth2.ts +++ b/packages/@n8n/client-oauth2/src/ClientOAuth2.ts @@ -10,28 +10,28 @@ import type { ClientOAuth2TokenData } from './ClientOAuth2Token'; import { ClientOAuth2Token } from './ClientOAuth2Token'; import { CodeFlow } from './CodeFlow'; import { CredentialsFlow } from './CredentialsFlow'; -import type { Headers, Query } from './types'; +import type { Headers } from './types'; export interface ClientOAuth2RequestObject { url: string; method: 'DELETE' | 'GET' | 'HEAD' | 'PATCH' | 'POST' | 'PUT'; body?: Record; - query?: Query; + query?: qs.ParsedUrlQuery; headers?: Headers; } export interface ClientOAuth2Options { clientId: string; - clientSecret: string; + clientSecret?: string; accessTokenUri: string; authorizationUri?: string; redirectUri?: string; scopes?: string[]; + scopesSeparator?: ',' | ' '; authorizationGrants?: string[]; state?: string; body?: Record; - query?: Query; - headers?: Headers; + query?: qs.ParsedUrlQuery; } class ResponseError extends Error { diff --git a/packages/@n8n/client-oauth2/src/ClientOAuth2Token.ts b/packages/@n8n/client-oauth2/src/ClientOAuth2Token.ts index 5c749a900..a532ccd86 100644 --- a/packages/@n8n/client-oauth2/src/ClientOAuth2Token.ts +++ b/packages/@n8n/client-oauth2/src/ClientOAuth2Token.ts @@ -2,7 +2,7 @@ /* eslint-disable @typescript-eslint/no-unsafe-assignment */ /* eslint-disable @typescript-eslint/naming-convention */ import type { ClientOAuth2, ClientOAuth2Options, ClientOAuth2RequestObject } from './ClientOAuth2'; -import { auth, getRequestOptions } from './utils'; +import { auth, expects, getRequestOptions } from './utils'; import { DEFAULT_HEADERS } from './constants'; export interface ClientOAuth2TokenData extends Record { @@ -69,6 +69,8 @@ export class ClientOAuth2Token { async refresh(opts?: ClientOAuth2Options): Promise { const options = { ...this.client.options, ...opts }; + expects(options, 'clientSecret'); + if (!this.refreshToken) throw new Error('No refresh token'); const requestOptions = getRequestOptions( diff --git a/packages/@n8n/client-oauth2/src/CodeFlow.ts b/packages/@n8n/client-oauth2/src/CodeFlow.ts index f25feb574..fceb71cdd 100644 --- a/packages/@n8n/client-oauth2/src/CodeFlow.ts +++ b/packages/@n8n/client-oauth2/src/CodeFlow.ts @@ -2,7 +2,7 @@ import * as qs from 'querystring'; import type { ClientOAuth2, ClientOAuth2Options } from './ClientOAuth2'; import type { ClientOAuth2Token, ClientOAuth2TokenData } from './ClientOAuth2Token'; import { DEFAULT_HEADERS, DEFAULT_URL_BASE } from './constants'; -import { auth, expects, getAuthError, getRequestOptions, sanitizeScope } from './utils'; +import { auth, expects, getAuthError, getRequestOptions } from './utils'; interface CodeFlowBody { code: string | string[]; @@ -22,27 +22,30 @@ export class CodeFlow { /** * Generate the uri for doing the first redirect. */ - getUri(opts?: ClientOAuth2Options): string { - const options = { ...this.client.options, ...opts }; + getUri(opts?: Partial): string { + const options: ClientOAuth2Options = { ...this.client.options, ...opts }; // Check the required parameters are set. expects(options, 'clientId', 'authorizationUri'); - const query: Record = { + const url = new URL(options.authorizationUri); + + const queryParams = { + ...options.query, client_id: options.clientId, redirect_uri: options.redirectUri, response_type: 'code', state: options.state, + ...(options.scopes ? { scope: options.scopes.join(options.scopesSeparator ?? ' ') } : {}), }; - if (options.scopes !== undefined) { - query.scope = sanitizeScope(options.scopes); + + for (const [key, value] of Object.entries(queryParams)) { + if (value !== null && value !== undefined) { + url.searchParams.append(key, value); + } } - if (options.authorizationUri) { - const sep = options.authorizationUri.includes('?') ? '&' : '?'; - return options.authorizationUri + sep + qs.stringify({ ...query, ...options.query }); - } - throw new TypeError('Missing authorization uri, unable to get redirect uri'); + return url.toString(); } /** diff --git a/packages/@n8n/client-oauth2/src/CredentialsFlow.ts b/packages/@n8n/client-oauth2/src/CredentialsFlow.ts index 3bc7c2ac3..1b6eb70e8 100644 --- a/packages/@n8n/client-oauth2/src/CredentialsFlow.ts +++ b/packages/@n8n/client-oauth2/src/CredentialsFlow.ts @@ -1,7 +1,7 @@ import type { ClientOAuth2, ClientOAuth2Options } from './ClientOAuth2'; import type { ClientOAuth2Token, ClientOAuth2TokenData } from './ClientOAuth2Token'; import { DEFAULT_HEADERS } from './constants'; -import { auth, expects, getRequestOptions, sanitizeScope } from './utils'; +import { auth, expects, getRequestOptions } from './utils'; interface CredentialsFlowBody { grant_type: 'client_credentials'; @@ -29,7 +29,7 @@ export class CredentialsFlow { }; if (options.scopes !== undefined) { - body.scope = sanitizeScope(options.scopes); + body.scope = options.scopes.join(options.scopesSeparator ?? ' '); } const requestOptions = getRequestOptions( diff --git a/packages/@n8n/client-oauth2/src/types.ts b/packages/@n8n/client-oauth2/src/types.ts index 906efcc43..f64fcc10c 100644 --- a/packages/@n8n/client-oauth2/src/types.ts +++ b/packages/@n8n/client-oauth2/src/types.ts @@ -1,2 +1 @@ export type Headers = Record; -export type Query = Record; diff --git a/packages/@n8n/client-oauth2/src/utils.ts b/packages/@n8n/client-oauth2/src/utils.ts index e3433e5cf..44317e5dc 100644 --- a/packages/@n8n/client-oauth2/src/utils.ts +++ b/packages/@n8n/client-oauth2/src/utils.ts @@ -2,17 +2,21 @@ /* eslint-disable @typescript-eslint/restrict-plus-operands */ /* eslint-disable @typescript-eslint/no-unsafe-assignment */ /* eslint-disable @typescript-eslint/no-explicit-any */ -import type { ClientOAuth2RequestObject } from './ClientOAuth2'; +import type { ClientOAuth2Options, ClientOAuth2RequestObject } from './ClientOAuth2'; import { ERROR_RESPONSES } from './constants'; /** * Check if properties exist on an object and throw when they aren't. */ -export function expects(obj: any, ...args: any[]) { - for (let i = 1; i < args.length; i++) { - const prop = args[i]; - if (obj[prop] === null) { - throw new TypeError('Expected "' + prop + '" to exist'); +export function expects( + obj: ClientOAuth2Options, + ...keys: Keys[] +): asserts obj is ClientOAuth2Options & { + [K in Keys]: NonNullable; +} { + for (const key of keys) { + if (obj[key] === null || obj[key] === undefined) { + throw new TypeError('Expected "' + key + '" to exist'); } } } @@ -47,13 +51,6 @@ function toString(str: string | null | undefined) { return str === null ? '' : String(str); } -/** - * Sanitize the scopes option to be a string. - */ -export function sanitizeScope(scopes: string[] | string): string { - return Array.isArray(scopes) ? scopes.join(' ') : toString(scopes); -} - /** * Create basic auth header. */ diff --git a/packages/@n8n/client-oauth2/test/CodeFlow.test.ts b/packages/@n8n/client-oauth2/test/CodeFlow.test.ts index 7b62e5ba1..f479769c7 100644 --- a/packages/@n8n/client-oauth2/test/CodeFlow.test.ts +++ b/packages/@n8n/client-oauth2/test/CodeFlow.test.ts @@ -29,7 +29,7 @@ describe('CodeFlow', () => { expect(githubAuth.code.getUri()).toEqual( `${config.authorizationUri}?client_id=abc&` + `redirect_uri=${encodeURIComponent(config.redirectUri)}&` + - 'response_type=code&state=&scope=notifications', + 'response_type=code&scope=notifications', ); }); @@ -46,7 +46,7 @@ describe('CodeFlow', () => { expect(authWithoutScopes.code.getUri()).toEqual( `${config.authorizationUri}?client_id=abc&` + `redirect_uri=${encodeURIComponent(config.redirectUri)}&` + - 'response_type=code&state=', + 'response_type=code', ); }); }); @@ -64,7 +64,7 @@ describe('CodeFlow', () => { expect(authWithEmptyScopes.code.getUri()).toEqual( `${config.authorizationUri}?client_id=abc&` + `redirect_uri=${encodeURIComponent(config.redirectUri)}&` + - 'response_type=code&state=&scope=', + 'response_type=code&scope=', ); }); @@ -81,7 +81,7 @@ describe('CodeFlow', () => { expect(authWithEmptyScopes.code.getUri()).toEqual( `${config.authorizationUri}?client_id=abc&` + `redirect_uri=${encodeURIComponent(config.redirectUri)}&` + - 'response_type=code&state=&scope=', + 'response_type=code&scope=', ); }); @@ -99,7 +99,7 @@ describe('CodeFlow', () => { expect(authWithParams.code.getUri()).toEqual( `${config.authorizationUri}?bar=qux&client_id=abc&` + `redirect_uri=${encodeURIComponent(config.redirectUri)}&` + - 'response_type=code&state=&scope=notifications', + 'response_type=code&scope=notifications', ); }); }); diff --git a/packages/cli/src/credentials/oauth2Credential.api.ts b/packages/cli/src/credentials/oauth2Credential.api.ts index 03ac4f0c5..e2cf481ce 100644 --- a/packages/cli/src/credentials/oauth2Credential.api.ts +++ b/packages/cli/src/credentials/oauth2Credential.api.ts @@ -3,6 +3,7 @@ import { ClientOAuth2 } from '@n8n/client-oauth2'; import Csrf from 'csrf'; import express from 'express'; import pkceChallenge from 'pkce-challenge'; +import * as qs from 'querystring'; import get from 'lodash/get'; import omit from 'lodash/omit'; import set from 'lodash/set'; @@ -121,15 +122,21 @@ oauth2CredentialController.get( }; const stateEncodedStr = Buffer.from(JSON.stringify(state)).toString('base64'); + const scopes = get(oauthCredentials, 'scope', 'openid') as string; const oAuthOptions: ClientOAuth2Options = { clientId: get(oauthCredentials, 'clientId') as string, clientSecret: get(oauthCredentials, 'clientSecret', '') as string, accessTokenUri: get(oauthCredentials, 'accessTokenUrl', '') as string, authorizationUri: get(oauthCredentials, 'authUrl', '') as string, redirectUri: `${getInstanceBaseUrl()}/${restEndpoint}/oauth2-credential/callback`, - scopes: split(get(oauthCredentials, 'scope', 'openid,') as string, ','), + scopes: split(scopes, ','), + scopesSeparator: scopes.includes(',') ? ',' : ' ', state: stateEncodedStr, }; + const authQueryParameters = get(oauthCredentials, 'authQueryParameters', '') as string; + if (authQueryParameters) { + oAuthOptions.query = qs.parse(authQueryParameters); + } await Container.get(ExternalHooks).run('oauth2.authenticate', [oAuthOptions]); @@ -162,26 +169,12 @@ oauth2CredentialController.get( // Update the credentials in DB await Db.collections.Credentials.update(req.query.id, newCredentialsData); - const authQueryParameters = get(oauthCredentials, 'authQueryParameters', '') as string; - let returnUri = oAuthObj.code.getUri(); - - // if scope uses comma, change it as the library always return then with spaces - if ((get(oauthCredentials, 'scope') as string).includes(',')) { - const data = returnUri.split('?')[1]; - const scope = get(oauthCredentials, 'scope') as string; - const percentEncoded = [data, `scope=${encodeURIComponent(scope)}`].join('&'); - returnUri = `${get(oauthCredentials, 'authUrl', '') as string}?${percentEncoded}`; - } - - if (authQueryParameters) { - returnUri += `&${authQueryParameters}`; - } - - LoggerProxy.verbose('OAuth2 authentication successful for new credential', { + LoggerProxy.verbose('OAuth2 authorization url created for credential', { userId: req.user.id, credentialId, }); - return returnUri; + + return oAuthObj.code.getUri(); }), ); @@ -263,13 +256,15 @@ oauth2CredentialController.get( let options: Partial = {}; + const scopes = get(oauthCredentials, 'scope', 'openid') as string; const oAuth2Parameters: ClientOAuth2Options = { clientId: get(oauthCredentials, 'clientId') as string, clientSecret: get(oauthCredentials, 'clientSecret', '') as string, accessTokenUri: get(oauthCredentials, 'accessTokenUrl', '') as string, authorizationUri: get(oauthCredentials, 'authUrl', '') as string, redirectUri: `${getInstanceBaseUrl()}/${restEndpoint}/oauth2-credential/callback`, - scopes: split(get(oauthCredentials, 'scope', 'openid,') as string, ','), + scopes: split(scopes, ','), + scopesSeparator: scopes.includes(',') ? ',' : ' ', }; if (oauthCredentials.grantType === 'pkce') {