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>
n8n-io · Nov 22, 2023 · 99a9ea4 · 99a9ea4
1 parent 96fd2c5
commit 99a9ea4
Show file tree

Hide file tree

Showing 9 changed files with 264 additions and 29 deletions.
diff --git a/packages/cli/src/ActiveWorkflowRunner.ts b/packages/cli/src/ActiveWorkflowRunner.ts
@@ -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.
 	 */

diff --git a/packages/cli/src/Interfaces.ts b/packages/cli/src/Interfaces.ts
@@ -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>;
 }
 

diff --git a/packages/cli/src/TestWebhooks.ts b/packages/cli/src/TestWebhooks.ts
@@ -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

diff --git a/packages/cli/src/WebhookHelpers.ts b/packages/cli/src/WebhookHelpers.ts
@@ -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') {

diff --git a/packages/cli/test/unit/WebhookHelpers.test.ts b/packages/cli/test/unit/WebhookHelpers.test.ts
@@ -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');
+			});
+		});
+	});
+});
diff --git a/packages/cli/test/unit/webhooks.test.ts b/packages/cli/test/unit/webhooks.test.ts
@@ -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);

diff --git a/packages/nodes-base/nodes/Webhook/Webhook.node.ts b/packages/nodes-base/nodes/Webhook/Webhook.node.ts
@@ -45,6 +45,7 @@ export class Webhook extends Node {
 		defaults: {
 			name: 'Webhook',
 		},
+		supportsCORS: true,
 		triggerPanel: {
 			header: '',
 			executionsHelp: {

diff --git a/packages/workflow/src/Interfaces.ts b/packages/workflow/src/Interfaces.ts
@@ -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?: {