feat(core): Add Support for custom CORS origins for webhooks (#7455)

node-850
https://community.n8n.io/t/add-ability-to-set-cors-allow-list-in-n8n-webhooks/7610
https://community.n8n.io/t/configure-cors-pre-flight-request-option-method-in-the-roadmap/32189

---------

Co-authored-by: कारतोफ्फेलस्क्रिप्ट™ <aditya@netroy.in>
This commit is contained in:
Michael Kret 2023-11-22 18:49:56 +02:00 committed by GitHub
parent 96fd2c51bd
commit 99a9ea497a
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
9 changed files with 264 additions and 29 deletions

View file

@ -23,6 +23,7 @@ import type {
WorkflowExecuteMode,
INodeType,
IWebhookData,
IHttpRequestMethods,
} from 'n8n-workflow';
import {
NodeHelpers,
@ -39,6 +40,7 @@ import type {
IWebhookManager,
IWorkflowDb,
IWorkflowExecutionDataProcess,
WebhookAccessControlOptions,
WebhookRequest,
} from '@/Interfaces';
import * as ResponseHelper from '@/ResponseHelper';
@ -137,26 +139,14 @@ export class ActiveWorkflowRunner implements IWebhookManager {
response: express.Response,
): Promise<IResponseCallbackData> {
const httpMethod = request.method;
let path = request.params.path;
const path = request.params.path;
this.logger.debug(`Received webhook "${httpMethod}" for path "${path}"`);
// Reset request parameters
request.params = {} as WebhookRequest['params'];
// Remove trailing slash
if (path.endsWith('/')) {
path = path.slice(0, -1);
}
const webhook = await this.webhookService.findWebhook(httpMethod, path);
if (webhook === null) {
throw new ResponseHelper.NotFoundError(
webhookNotFoundErrorMessage(path, httpMethod),
WEBHOOK_PROD_UNREGISTERED_HINT,
);
}
const webhook = await this.findWebhook(path, httpMethod);
if (webhook.isDynamic) {
const pathElements = path.split('/').slice(1);
@ -235,13 +225,45 @@ export class ActiveWorkflowRunner implements IWebhookManager {
});
}
/**
* Gets all request methods associated with a single webhook
*/
async getWebhookMethods(path: string) {
return this.webhookService.getWebhookMethods(path);
}
async findAccessControlOptions(path: string, httpMethod: IHttpRequestMethods) {
const webhook = await this.findWebhook(path, httpMethod);
const workflowData = await this.workflowRepository.findOne({
where: { id: webhook.workflowId },
select: ['nodes'],
});
const nodes = workflowData?.nodes;
const webhookNode = nodes?.find(
({ type, parameters, typeVersion }) =>
parameters?.path === path &&
(parameters?.httpMethod ?? 'GET') === httpMethod &&
'webhook' in this.nodeTypes.getByNameAndVersion(type, typeVersion),
);
return webhookNode?.parameters?.options as WebhookAccessControlOptions;
}
private async findWebhook(path: string, httpMethod: IHttpRequestMethods) {
// Remove trailing slash
if (path.endsWith('/')) {
path = path.slice(0, -1);
}
const webhook = await this.webhookService.findWebhook(httpMethod, path);
if (webhook === null) {
throw new ResponseHelper.NotFoundError(
webhookNotFoundErrorMessage(path, httpMethod),
WEBHOOK_PROD_UNREGISTERED_HINT,
);
}
return webhook;
}
/**
* Returns the ids of the currently active workflows from memory.
*/

View file

@ -267,8 +267,20 @@ export type WaitingWebhookRequest = WebhookRequest & {
params: WebhookRequest['path'] & { suffix?: string };
};
export interface WebhookAccessControlOptions {
allowedOrigins?: string;
}
export interface IWebhookManager {
/** Gets all request methods associated with a webhook path*/
getWebhookMethods?: (path: string) => Promise<IHttpRequestMethods[]>;
/** Find the CORS options matching a path and method */
findAccessControlOptions?: (
path: string,
httpMethod: IHttpRequestMethods,
) => Promise<WebhookAccessControlOptions | undefined>;
executeWebhook(req: WebhookRequest, res: Response): Promise<IResponseCallbackData>;
}

View file

@ -15,9 +15,11 @@ import type {
IResponseCallbackData,
IWebhookManager,
IWorkflowDb,
WebhookAccessControlOptions,
WebhookRequest,
} from '@/Interfaces';
import { Push } from '@/push';
import { NodeTypes } from '@/NodeTypes';
import * as ResponseHelper from '@/ResponseHelper';
import * as WebhookHelpers from '@/WebhookHelpers';
import { webhookNotFoundErrorMessage } from './utils';
@ -38,8 +40,9 @@ export class TestWebhooks implements IWebhookManager {
} = {};
constructor(
private activeWebhooks: ActiveWebhooks,
private push: Push,
private readonly activeWebhooks: ActiveWebhooks,
private readonly push: Push,
private readonly nodeTypes: NodeTypes,
) {
activeWebhooks.testWebhooks = true;
}
@ -161,9 +164,6 @@ export class TestWebhooks implements IWebhookManager {
});
}
/**
* Gets all request methods associated with a single test webhook
*/
async getWebhookMethods(path: string): Promise<IHttpRequestMethods[]> {
const webhookMethods = this.activeWebhooks.getWebhookMethods(path);
if (!webhookMethods.length) {
@ -177,6 +177,22 @@ export class TestWebhooks implements IWebhookManager {
return webhookMethods;
}
async findAccessControlOptions(path: string, httpMethod: IHttpRequestMethods) {
const webhookKey = Object.keys(this.testWebhookData).find(
(key) => key.includes(path) && key.startsWith(httpMethod),
);
if (!webhookKey) return;
const { workflow } = this.testWebhookData[webhookKey];
const webhookNode = Object.values(workflow.nodes).find(
({ type, parameters, typeVersion }) =>
parameters?.path === path &&
(parameters?.httpMethod ?? 'GET') === httpMethod &&
'webhook' in this.nodeTypes.getByNameAndVersion(type, typeVersion),
);
return webhookNode?.parameters?.options as WebhookAccessControlOptions;
}
/**
* Checks if it has to wait for webhook data to execute the workflow.
* If yes it waits for it and resolves with the result of the workflow if not it simply resolves with undefined

View file

@ -98,7 +98,28 @@ export const webhookRequestHandler =
return ResponseHelper.sendErrorResponse(res, error as Error);
}
}
res.header('Access-Control-Allow-Origin', req.headers.origin);
const requestedMethod =
method === 'OPTIONS'
? (req.headers['access-control-request-method'] as IHttpRequestMethods)
: method;
if (webhookManager.findAccessControlOptions && requestedMethod) {
const options = await webhookManager.findAccessControlOptions(path, requestedMethod);
const { allowedOrigins } = options ?? {};
res.header(
'Access-Control-Allow-Origin',
!allowedOrigins || allowedOrigins === '*' ? req.headers.origin : allowedOrigins,
);
if (method === 'OPTIONS') {
res.header('Access-Control-Max-Age', '300');
const requestedHeaders = req.headers['access-control-request-headers'];
if (requestedHeaders?.length) {
res.header('Access-Control-Allow-Headers', requestedHeaders);
}
}
}
}
if (method === 'OPTIONS') {

View file

@ -0,0 +1,140 @@
import { type Response } from 'express';
import { mock } from 'jest-mock-extended';
import type { IHttpRequestMethods } from 'n8n-workflow';
import type { IWebhookManager, WebhookCORSRequest, WebhookRequest } from '@/Interfaces';
import { webhookRequestHandler } from '@/WebhookHelpers';
describe('WebhookHelpers', () => {
describe('webhookRequestHandler', () => {
const webhookManager = mock<Required<IWebhookManager>>();
const handler = webhookRequestHandler(webhookManager);
beforeEach(() => {
jest.resetAllMocks();
});
it('should throw for unsupported methods', async () => {
const req = mock<WebhookRequest | WebhookCORSRequest>({
method: 'CONNECT' as IHttpRequestMethods,
});
const res = mock<Response>();
res.status.mockReturnValue(res);
await handler(req, res);
expect(res.status).toHaveBeenCalledWith(500);
expect(res.json).toHaveBeenCalledWith({
code: 0,
message: 'The method CONNECT is not supported.',
});
});
describe('preflight requests', () => {
it('should handle missing header for requested method', async () => {
const req = mock<WebhookRequest | WebhookCORSRequest>({
method: 'OPTIONS',
headers: {
origin: 'https://example.com',
'access-control-request-method': undefined,
},
params: { path: 'test' },
});
const res = mock<Response>();
res.status.mockReturnValue(res);
webhookManager.getWebhookMethods.mockResolvedValue(['GET', 'PATCH']);
await handler(req, res);
expect(res.status).toHaveBeenCalledWith(204);
expect(res.header).toHaveBeenCalledWith(
'Access-Control-Allow-Methods',
'OPTIONS, GET, PATCH',
);
});
it('should handle default origin and max-age', async () => {
const req = mock<WebhookRequest | WebhookCORSRequest>({
method: 'OPTIONS',
headers: {
origin: 'https://example.com',
'access-control-request-method': 'GET',
},
params: { path: 'test' },
});
const res = mock<Response>();
res.status.mockReturnValue(res);
webhookManager.getWebhookMethods.mockResolvedValue(['GET', 'PATCH']);
await handler(req, res);
expect(res.status).toHaveBeenCalledWith(204);
expect(res.header).toHaveBeenCalledWith(
'Access-Control-Allow-Methods',
'OPTIONS, GET, PATCH',
);
expect(res.header).toHaveBeenCalledWith(
'Access-Control-Allow-Origin',
'https://example.com',
);
expect(res.header).toHaveBeenCalledWith('Access-Control-Max-Age', '300');
});
it('should handle wildcard origin', async () => {
const randomOrigin = (Math.random() * 10e6).toString(16);
const req = mock<WebhookRequest | WebhookCORSRequest>({
method: 'OPTIONS',
headers: {
origin: randomOrigin,
'access-control-request-method': 'GET',
},
params: { path: 'test' },
});
const res = mock<Response>();
res.status.mockReturnValue(res);
webhookManager.getWebhookMethods.mockResolvedValue(['GET', 'PATCH']);
webhookManager.findAccessControlOptions.mockResolvedValue({
allowedOrigins: '*',
});
await handler(req, res);
expect(res.status).toHaveBeenCalledWith(204);
expect(res.header).toHaveBeenCalledWith(
'Access-Control-Allow-Methods',
'OPTIONS, GET, PATCH',
);
expect(res.header).toHaveBeenCalledWith('Access-Control-Allow-Origin', randomOrigin);
});
it('should handle custom origin', async () => {
const req = mock<WebhookRequest | WebhookCORSRequest>({
method: 'OPTIONS',
headers: {
origin: 'https://example.com',
'access-control-request-method': 'GET',
},
params: { path: 'test' },
});
const res = mock<Response>();
res.status.mockReturnValue(res);
webhookManager.getWebhookMethods.mockResolvedValue(['GET', 'PATCH']);
webhookManager.findAccessControlOptions.mockResolvedValue({
allowedOrigins: 'https://test.com',
});
await handler(req, res);
expect(res.status).toHaveBeenCalledWith(204);
expect(res.header).toHaveBeenCalledWith(
'Access-Control-Allow-Methods',
'OPTIONS, GET, PATCH',
);
expect(res.header).toHaveBeenCalledWith('Access-Control-Allow-Origin', 'https://test.com');
});
});
});
});

View file

@ -46,7 +46,10 @@ describe('WebhookServer', () => {
const pathPrefix = config.getEnv(`endpoints.${key}`);
manager.getWebhookMethods.mockResolvedValueOnce(['GET']);
const response = await agent.options(`/${pathPrefix}/abcd`).set('origin', corsOrigin);
const response = await agent
.options(`/${pathPrefix}/abcd`)
.set('origin', corsOrigin)
.set('access-control-request-method', 'GET');
expect(response.statusCode).toEqual(204);
expect(response.body).toEqual({});
expect(response.headers['access-control-allow-origin']).toEqual(corsOrigin);
@ -60,7 +63,10 @@ describe('WebhookServer', () => {
mockResponse({ test: true }, { key: 'value ' }),
);
const response = await agent.get(`/${pathPrefix}/abcd`).set('origin', corsOrigin);
const response = await agent
.get(`/${pathPrefix}/abcd`)
.set('origin', corsOrigin)
.set('access-control-request-method', 'GET');
expect(response.statusCode).toEqual(200);
expect(response.body).toEqual({ test: true });
expect(response.headers['access-control-allow-origin']).toEqual(corsOrigin);

View file

@ -45,6 +45,7 @@ export class Webhook extends Node {
defaults: {
name: 'Webhook',
},
supportsCORS: true,
triggerPanel: {
header: '',
executionsHelp: {

View file

@ -1594,7 +1594,8 @@ export interface INodeTypeDescription extends INodeTypeBaseDescription {
properties: INodeProperties[];
credentials?: INodeCredentialDescription[];
maxNodes?: number; // How many nodes of that type can be created in a workflow
polling?: boolean;
polling?: true | undefined;
supportsCORS?: true | undefined;
requestDefaults?: DeclarativeRestApiSettings.HttpRequestOptions;
requestOperations?: IN8nRequestOperations;
hooks?: {

View file

@ -236,7 +236,7 @@ export const cronNodeOptions: INodePropertyCollection[] = [
},
];
const specialNodeParameters: INodeProperties[] = [
const commonPollingParameters: INodeProperties[] = [
{
displayName: 'Poll Times',
name: 'pollTimes',
@ -252,12 +252,28 @@ const specialNodeParameters: INodeProperties[] = [
},
];
const commonCORSParameters: INodeProperties[] = [
{
displayName: 'Allowed Origins (CORS)',
name: 'allowedOrigins',
type: 'string',
default: '*',
description: 'The origin(s) to allow cross-origin non-preflight requests from in a browser',
},
];
/**
* Apply special parameters which should be added to nodeTypes depending on their type or configuration
*/
export function applySpecialNodeParameters(nodeType: INodeType): void {
if (nodeType.description.polling === true) {
nodeType.description.properties.unshift(...specialNodeParameters);
const { properties, polling, supportsCORS } = nodeType.description;
if (polling) {
properties.unshift(...commonPollingParameters);
}
if (nodeType.webhook && supportsCORS) {
const optionsProperty = properties.find(({ name }) => name === 'options');
if (optionsProperty) optionsProperty.options!.push(...commonCORSParameters);
else properties.push(...commonCORSParameters);
}
}