fix(core): Do not add Authentication header when authentication type is body (#8201)

This commit is contained in:
कारतोफ्फेलस्क्रिप्ट™ 2024-01-08 12:38:24 +01:00 committed by GitHub
parent ccb2b076f8
commit ac1c642fdd
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
9 changed files with 117 additions and 94 deletions

View file

@ -1,6 +1,5 @@
/* eslint-disable @typescript-eslint/no-unsafe-return */
/* eslint-disable @typescript-eslint/no-unsafe-argument */
/* eslint-disable @typescript-eslint/no-explicit-any */
import * as qs from 'querystring';
import { Agent } from 'https';
@ -26,6 +25,7 @@ export interface ClientOAuth2Options {
clientId: string;
clientSecret?: string;
accessTokenUri: string;
authentication?: 'header' | 'body';
authorizationUri?: string;
redirectUri?: string;
scopes?: string[];

View file

@ -1,9 +1,12 @@
import type { ClientOAuth2, ClientOAuth2Options } from './ClientOAuth2';
import type { ClientOAuth2 } from './ClientOAuth2';
import type { ClientOAuth2Token, ClientOAuth2TokenData } from './ClientOAuth2Token';
import { DEFAULT_HEADERS } from './constants';
import type { Headers } from './types';
import { auth, expects, getRequestOptions } from './utils';
interface CredentialsFlowBody {
client_id?: string;
client_secret?: string;
grant_type: 'client_credentials';
scope?: string;
}
@ -19,10 +22,11 @@ export class CredentialsFlow {
/**
* Request an access token using the client credentials.
*/
async getToken(opts?: Partial<ClientOAuth2Options>): Promise<ClientOAuth2Token> {
const options = { ...this.client.options, ...opts };
async getToken(): Promise<ClientOAuth2Token> {
const options = { ...this.client.options };
expects(options, 'clientId', 'clientSecret', 'accessTokenUri');
const headers: Headers = { ...DEFAULT_HEADERS };
const body: CredentialsFlowBody = {
grant_type: 'client_credentials',
};
@ -31,15 +35,21 @@ export class CredentialsFlow {
body.scope = options.scopes.join(options.scopesSeparator ?? ' ');
}
const clientId = options.clientId;
const clientSecret = options.clientSecret;
if (options.authentication === 'body') {
body.client_id = clientId;
body.client_secret = clientSecret;
} else {
headers.Authorization = auth(clientId, clientSecret);
}
const requestOptions = getRequestOptions(
{
url: options.accessTokenUri,
method: 'POST',
headers: {
...DEFAULT_HEADERS,
// eslint-disable-next-line @typescript-eslint/naming-convention
Authorization: auth(options.clientId, options.clientSecret),
},
headers,
body,
},
options,

View file

@ -1,2 +1,3 @@
export { ClientOAuth2, ClientOAuth2Options, ClientOAuth2RequestObject } from './ClientOAuth2';
export { ClientOAuth2Token, ClientOAuth2TokenData } from './ClientOAuth2Token';
export type * from './types';

View file

@ -1 +1,19 @@
export type Headers = Record<string, string | string[]>;
export type OAuth2GrantType = 'pkce' | 'authorizationCode' | 'clientCredentials';
export interface OAuth2CredentialData {
clientId: string;
clientSecret?: string;
accessTokenUrl: string;
authentication?: 'header' | 'body';
authUrl?: string;
scope?: string;
authQueryParameters?: string;
grantType: OAuth2GrantType;
ignoreSSLIssues?: boolean;
oauthTokenData?: {
access_token: string;
refresh_token?: string;
};
}

View file

@ -1,4 +1,6 @@
import nock from 'nock';
import type { Headers } from '../src/types';
import type { ClientOAuth2Options } from '../src';
import { ClientOAuth2, ClientOAuth2Token } from '../src';
import * as config from './config';
@ -11,18 +13,24 @@ describe('CredentialsFlow', () => {
nock.restore();
});
beforeEach(() => jest.clearAllMocks());
describe('#getToken', () => {
const createAuthClient = (scopes?: string[]) =>
const createAuthClient = ({
scopes,
authentication,
}: Pick<ClientOAuth2Options, 'scopes' | 'authentication'> = {}) =>
new ClientOAuth2({
clientId: config.clientId,
clientSecret: config.clientSecret,
accessTokenUri: config.accessTokenUri,
authentication,
authorizationGrants: ['credentials'],
scopes,
});
const mockTokenCall = (requestedScope?: string) =>
nock(config.baseUrl)
const mockTokenCall = async ({ requestedScope }: { requestedScope?: string } = {}) => {
const nockScope = nock(config.baseUrl)
.post(
'/login/oauth/access_token',
({ scope, grant_type }) =>
@ -34,10 +42,19 @@ describe('CredentialsFlow', () => {
refresh_token: config.refreshToken,
scope: requestedScope,
});
return new Promise<{ headers: Headers; body: unknown }>((resolve) => {
nockScope.once('request', (req) => {
resolve({
headers: req.headers,
body: req.requestBodyBuffers.toString('utf-8'),
});
});
});
};
it('should request the token', async () => {
const authClient = createAuthClient(['notifications']);
mockTokenCall('notifications');
const authClient = createAuthClient({ scopes: ['notifications'] });
const requestPromise = mockTokenCall({ requestedScope: 'notifications' });
const user = await authClient.credentials.getToken();
@ -45,34 +62,62 @@ describe('CredentialsFlow', () => {
expect(user.accessToken).toEqual(config.accessToken);
expect(user.tokenType).toEqual('bearer');
expect(user.data.scope).toEqual('notifications');
const { headers, body } = await requestPromise;
expect(headers.authorization).toBe('Basic YWJjOjEyMw==');
expect(body).toEqual('grant_type=client_credentials&scope=notifications');
});
it('when scopes are undefined, it should not send scopes to an auth server', async () => {
const authClient = createAuthClient();
mockTokenCall();
const requestPromise = mockTokenCall();
const user = await authClient.credentials.getToken();
expect(user).toBeInstanceOf(ClientOAuth2Token);
expect(user.accessToken).toEqual(config.accessToken);
expect(user.tokenType).toEqual('bearer');
expect(user.data.scope).toEqual(undefined);
const { body } = await requestPromise;
expect(body).toEqual('grant_type=client_credentials');
});
it('when scopes is an empty array, it should send empty scope string to an auth server', async () => {
const authClient = createAuthClient([]);
mockTokenCall('');
const authClient = createAuthClient({ scopes: [] });
const requestPromise = mockTokenCall({ requestedScope: '' });
const user = await authClient.credentials.getToken();
expect(user).toBeInstanceOf(ClientOAuth2Token);
expect(user.accessToken).toEqual(config.accessToken);
expect(user.tokenType).toEqual('bearer');
expect(user.data.scope).toEqual('');
const { body } = await requestPromise;
expect(body).toEqual('grant_type=client_credentials&scope=');
});
it('should handle authentication = "header"', async () => {
const authClient = createAuthClient({ scopes: [] });
const requestPromise = mockTokenCall({ requestedScope: '' });
await authClient.credentials.getToken();
const { headers, body } = await requestPromise;
expect(headers?.authorization).toBe('Basic YWJjOjEyMw==');
expect(body).toEqual('grant_type=client_credentials&scope=');
});
it('should handle authentication = "body"', async () => {
const authClient = createAuthClient({ scopes: [], authentication: 'body' });
const requestPromise = mockTokenCall({ requestedScope: '' });
await authClient.credentials.getToken();
const { headers, body } = await requestPromise;
expect(headers?.authorization).toBe(undefined);
expect(body).toEqual('grant_type=client_credentials&scope=&client_id=abc&client_secret=123');
});
describe('#sign', () => {
it('should be able to sign a standard request object', async () => {
const authClient = createAuthClient(['notifications']);
mockTokenCall('notifications');
const authClient = createAuthClient({ scopes: ['notifications'] });
void mockTokenCall({ requestedScope: 'notifications' });
const token = await authClient.credentials.getToken();
const requestOptions = token.sign({
@ -99,8 +144,8 @@ describe('CredentialsFlow', () => {
});
it('should make a request to get a new access token', async () => {
const authClient = createAuthClient(['notifications']);
mockTokenCall('notifications');
const authClient = createAuthClient({ scopes: ['notifications'] });
void mockTokenCall({ requestedScope: 'notifications' });
const token = await authClient.credentials.getToken();
expect(token.accessToken).toEqual(config.accessToken);

View file

@ -1,4 +1,4 @@
import type { ClientOAuth2Options } from '@n8n/client-oauth2';
import type { ClientOAuth2Options, OAuth2CredentialData } from '@n8n/client-oauth2';
import { ClientOAuth2 } from '@n8n/client-oauth2';
import Csrf from 'csrf';
import { Response } from 'express';
@ -7,24 +7,11 @@ import * as qs from 'querystring';
import omit from 'lodash/omit';
import set from 'lodash/set';
import split from 'lodash/split';
import type { OAuth2GrantType } from 'n8n-workflow';
import { ApplicationError, jsonParse, jsonStringify } from 'n8n-workflow';
import { Authorized, Get, RestController } from '@/decorators';
import { OAuthRequest } from '@/requests';
import { AbstractOAuthController } from './abstractOAuth.controller';
interface OAuth2CredentialData {
clientId: string;
clientSecret?: string;
accessTokenUrl?: string;
authUrl?: string;
scope?: string;
authQueryParameters?: string;
authentication?: 'header' | 'body';
grantType: OAuth2GrantType;
ignoreSSLIssues?: boolean;
}
interface CsrfStateParam {
cid: string;
token: string;
@ -226,6 +213,7 @@ export class OAuth2CredentialController extends AbstractOAuthController {
clientSecret: credential.clientSecret ?? '',
accessTokenUri: credential.accessTokenUrl ?? '',
authorizationUri: credential.authUrl ?? '',
authentication: credential.authentication ?? 'header',
redirectUri: `${this.baseUrl}/callback`,
scopes: split(credential.scope ?? 'openid', ','),
scopesSeparator: credential.scope?.includes(',') ? ',' : ' ',

View file

@ -12,6 +12,7 @@ import type {
ClientOAuth2Options,
ClientOAuth2RequestObject,
ClientOAuth2TokenData,
OAuth2CredentialData,
} from '@n8n/client-oauth2';
import { ClientOAuth2 } from '@n8n/client-oauth2';
import type {
@ -103,7 +104,6 @@ import {
NodeHelpers,
NodeOperationError,
NodeSslError,
OAuth2GrantType,
WorkflowDataProxy,
createDeferredPromise,
deepCopy,
@ -140,7 +140,6 @@ import {
} from './Constants';
import { extractValue } from './ExtractValue';
import type { ExtendedValidationResult, IResponseError } from './Interfaces';
import { getClientCredentialsToken } from './OAuth2Helper';
import {
getAllWorkflowExecutionMetadata,
getWorkflowExecutionMetadata,
@ -1215,31 +1214,31 @@ export async function requestOAuth2(
oAuth2Options?: IOAuth2Options,
isN8nRequest = false,
) {
const credentials = await this.getCredentials(credentialsType);
const credentials = (await this.getCredentials(
credentialsType,
)) as unknown as OAuth2CredentialData;
// Only the OAuth2 with authorization code grant needs connection
if (
credentials.grantType === OAuth2GrantType.authorizationCode &&
credentials.oauthTokenData === undefined
) {
if (credentials.grantType === 'authorizationCode' && credentials.oauthTokenData === undefined) {
throw new ApplicationError('OAuth credentials not connected');
}
const oAuthClient = new ClientOAuth2({
clientId: credentials.clientId as string,
clientSecret: credentials.clientSecret as string,
accessTokenUri: credentials.accessTokenUrl as string,
clientId: credentials.clientId,
clientSecret: credentials.clientSecret,
accessTokenUri: credentials.accessTokenUrl,
scopes: (credentials.scope as string).split(' '),
ignoreSSLIssues: credentials.ignoreSSLIssues as boolean,
ignoreSSLIssues: credentials.ignoreSSLIssues,
authentication: credentials.authentication ?? 'header',
});
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 &&
credentials.grantType === 'clientCredentials' &&
(oauthTokenData === undefined || Object.keys(oauthTokenData).length === 0)
) {
const { data } = await getClientCredentialsToken(oAuthClient, credentials);
const { data } = await oAuthClient.credentials.getToken();
// Find the credentials
if (!node.credentials?.[credentialsType]) {
throw new ApplicationError('Node does not have credential type', {
@ -1249,12 +1248,13 @@ export async function requestOAuth2(
}
const nodeCredentials = node.credentials[credentialsType];
credentials.oauthTokenData = data;
// Save the refreshed token
await additionalData.credentialsHelper.updateCredentials(
nodeCredentials,
credentialsType,
Object.assign(credentials, { oauthTokenData: data }),
credentials as unknown as ICredentialDataDecryptedObject,
);
oauthTokenData = data;
@ -1296,7 +1296,7 @@ export async function requestOAuth2(
const tokenRefreshOptions: IDataObject = {};
if (oAuth2Options?.includeCredentialsOnRefreshOnBody) {
const body: IDataObject = {
client_id: credentials.clientId as string,
client_id: credentials.clientId,
...(credentials.grantType === 'authorizationCode' && {
client_secret: credentials.clientSecret as string,
}),
@ -1314,8 +1314,8 @@ export async function requestOAuth2(
);
// if it's OAuth2 with client credentials grant type, get a new token
// instead of refreshing it.
if (OAuth2GrantType.clientCredentials === credentials.grantType) {
newToken = await getClientCredentialsToken(token.client, credentials);
if (credentials.grantType === 'clientCredentials') {
newToken = await token.client.credentials.getToken();
} else {
newToken = await token.refresh(tokenRefreshOptions as unknown as ClientOAuth2Options);
}
@ -1335,7 +1335,7 @@ export async function requestOAuth2(
await additionalData.credentialsHelper.updateCredentials(
nodeCredentials,
credentialsType,
credentials,
credentials as unknown as ICredentialDataDecryptedObject,
);
const refreshedRequestOption = newToken.sign(requestOptions as ClientOAuth2RequestObject);
@ -1391,8 +1391,8 @@ export async function requestOAuth2(
// if it's OAuth2 with client credentials grant type, get a new token
// instead of refreshing it.
if (OAuth2GrantType.clientCredentials === credentials.grantType) {
newToken = await getClientCredentialsToken(token.client, credentials);
if (credentials.grantType === 'clientCredentials') {
newToken = await token.client.credentials.getToken();
} else {
newToken = await token.refresh(tokenRefreshOptions as unknown as ClientOAuth2Options);
}

View file

@ -1,22 +0,0 @@
import type { ICredentialDataDecryptedObject } from 'n8n-workflow';
import type { ClientOAuth2, ClientOAuth2Options, ClientOAuth2Token } from '@n8n/client-oauth2';
export const getClientCredentialsToken = async (
oAuth2Client: ClientOAuth2,
credentials: ICredentialDataDecryptedObject,
): Promise<ClientOAuth2Token> => {
const options = {};
if (credentials.authentication === 'body') {
Object.assign(options, {
headers: {
// eslint-disable-next-line @typescript-eslint/naming-convention
Authorization: '',
},
body: {
client_id: credentials.clientId as string,
client_secret: credentials.clientSecret as string,
},
});
}
return oAuth2Client.credentials.getToken(options as ClientOAuth2Options);
};

View file

@ -2124,23 +2124,6 @@ export interface IConnectedNode {
depth: number;
}
export const enum OAuth2GrantType {
pkce = 'pkce',
authorizationCode = 'authorizationCode',
clientCredentials = 'clientCredentials',
}
export interface IOAuth2Credentials {
grantType: 'authorizationCode' | 'clientCredentials' | 'pkce';
clientId: string;
clientSecret: string;
accessTokenUrl: string;
authUrl: string;
authQueryParameters: string;
authentication: 'body' | 'header';
scope: string;
oauthTokenData?: IDataObject;
}
export type PublicInstalledPackage = {
packageName: string;
installedVersion: string;