refactor, and get access control working for production webhooks as well

packages/cli/src/ActiveWorkflowRunner.ts

@@ -23,6 +23,7 @@ import type {
 	WorkflowExecuteMode,
 	INodeType,
 	IWebhookData,
+	IHttpRequestMethods,
 } from 'n8n-workflow';
 import {
 	NodeHelpers,
@@ -137,40 +138,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,
-			);
-		}
-
-		if (webhook.isDynamic) {
-			const pathElements = path.split('/').slice(1);
-
-			// extracting params from path
-			// @ts-ignore
-			webhook.webhookPath.split('/').forEach((ele, index) => {
-				if (ele.startsWith(':')) {
-					// write params to req.params
-					// @ts-ignore
-					request.params[ele.slice(1)] = pathElements[index];
-				}
-			});
-		}
+		const webhook = await this.findWebhook(path, httpMethod);
 
 		const workflowData = await this.workflowRepository.findOne({
 			where: { id: webhook.workflowId },
@@ -235,13 +210,57 @@ export class ActiveWorkflowRunner implements IWebhookManager {
 		});
 	}
 
-	/**
-	 * Gets all request methods associated with a single webhook
-	 */
 	async getWebhookMethods(path: string) {
 		return this.webhookService.getWebhookMethods(path);
 	}
 
+	async getAccessControlOptions(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;
+		if (!nodes || !Object.keys(nodes).length) {
+			return null;
+		}
+
+		return WebhookHelpers.getAccessControlOptions(nodes, path, httpMethod);
+	}
+
+	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,
+			);
+		}
+
+		if (webhook.isDynamic) {
+			const pathElements = path.split('/').slice(1);
+
+			// extracting params from path
+			// @ts-ignore
+			webhook.webhookPath.split('/').forEach((ele, index) => {
+				if (ele.startsWith(':')) {
+					// write params to req.params
+					// @ts-ignore
+					request.params[ele.slice(1)] = pathElements[index];
+				}
+			});
+		}
+
+		return webhook;
+	}
+
 	/**
 	 * Returns the ids of the currently active workflows from memory.
 	 */

packages/cli/src/Interfaces.ts

@@ -23,6 +23,7 @@ import type {
 	INodeProperties,
 	IUserSettings,
 	IHttpRequestMethods,
+	WebhookAccessControlOptions,
 } from 'n8n-workflow';
 
 import type { ActiveWorkflowRunner } from '@/ActiveWorkflowRunner';
@@ -268,11 +269,14 @@ export type WaitingWebhookRequest = WebhookRequest & {
 };
 
 export interface IWebhookManager {
+	/** Gets all request methods associated with a webhook path*/
 	getWebhookMethods?: (path: string) => Promise<IHttpRequestMethods[]>;
+
+	/** Gets CORS access-control options associated with a single webhook */
 	getAccessControlOptions?: (
 		path: string,
 		httpMethod: IHttpRequestMethods,
-	) => Promise<IDataObject | null>;
+	) => Promise<WebhookAccessControlOptions | null>;
 	executeWebhook(req: WebhookRequest, res: Response): Promise<IResponseCallbackData>;
 }
 

packages/cli/src/TestWebhooks.ts

@@ -8,7 +8,6 @@ import type {
 	Workflow,
 	WorkflowActivateMode,
 	WorkflowExecuteMode,
-	IDataObject,
 } from 'n8n-workflow';
 
 import { ActiveWebhooks } from '@/ActiveWebhooks';
@@ -162,9 +161,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) {
@@ -178,31 +174,17 @@ export class TestWebhooks implements IWebhookManager {
 		return webhookMethods;
 	}
 
-	/**
-	 * Gets all request methods associated with a single test webhook
-	 */
-	async getAccessControlOptions(path: string, httpMethod: string): Promise<IDataObject | null> {
-		const webhookWorkflow = Object.keys(this.testWebhookData).find(
+	async getAccessControlOptions(path: string, httpMethod: IHttpRequestMethods) {
+		const webhookKey = Object.keys(this.testWebhookData).find(
 			(key) => key.includes(path) && key.startsWith(httpMethod),
 		);
 
-		const nodes = webhookWorkflow ? this.testWebhookData[webhookWorkflow].workflow.nodes : {};
-
+		const nodes = webhookKey ? this.testWebhookData[webhookKey].workflow.nodes : {};
 		if (!Object.keys(nodes).length) {
 			return null;
 		}
 
-		const result = Object.values(nodes).find((node) => {
-			return (
-				node.type === 'n8n-nodes-base.webhook' &&
-				node.parameters?.path === path &&
-				node.parameters?.httpMethod === httpMethod
-			);
-		});
-
-		const { accessControl } = (result?.parameters?.options as IDataObject) || {};
-
-		return accessControl ? ((accessControl as IDataObject).values as IDataObject) : null;
+		return WebhookHelpers.getAccessControlOptions(Object.values(nodes), path, httpMethod);
 	}
 
 	/**

packages/cli/src/WebhookHelpers.ts

@@ -30,6 +30,7 @@ import type {
 	IWebhookResponseData,
 	IWorkflowDataProxyAdditionalKeys,
 	IWorkflowExecuteAdditionalData,
+	WebhookAccessControlOptions,
 	Workflow,
 	WorkflowExecuteMode,
 } from 'n8n-workflow';
@@ -75,6 +76,22 @@ export const WEBHOOK_METHODS: IHttpRequestMethods[] = [
 	'PUT',
 ];
 
+export function getAccessControlOptions(
+	nodes: INode[],
+	path: string,
+	httpMethod: IHttpRequestMethods,
+) {
+	const result = nodes.find(
+		({ type, parameters }) =>
+			type === 'n8n-nodes-base.webhook' &&
+			parameters?.path === path &&
+			(parameters?.httpMethod ?? 'GET') === httpMethod,
+	);
+
+	const { accessControl } = (result?.parameters as IDataObject) || {};
+	return (accessControl as WebhookAccessControlOptions) ?? null;
+}
+
 export const webhookRequestHandler =
 	(webhookManager: IWebhookManager) =>
 	async (req: WebhookRequest | WebhookCORSRequest, res: express.Response) => {
@@ -102,23 +119,16 @@ export const webhookRequestHandler =
 		}
 
 		if (method === 'OPTIONS') {
-			const controlRequestMethod = req.headers['access-control-request-method'];
-
-			if (webhookManager.getAccessControlOptions && controlRequestMethod) {
-				try {
-					const accessControlOptions = await webhookManager.getAccessControlOptions(
-						path,
-						controlRequestMethod as IHttpRequestMethods,
-					);
-
-					if (accessControlOptions) {
-						const { allowMethods, allowOrigin, maxAge } = accessControlOptions;
-
-						res.header('Access-Control-Allow-Methods', (allowMethods as string) || '*');
-						res.header('Access-Control-Allow-Origin', (allowOrigin as string) || '*');
-						res.header('Access-Control-Max-Age', String((maxAge as number) * 1000) || '60000');
-					}
-				} catch (error) {}
+			const requestedMethod = req.headers['access-control-request-method'] as IHttpRequestMethods;
+
+			if (webhookManager.getAccessControlOptions && requestedMethod) {
+				const accessControlOptions = await webhookManager.getAccessControlOptions(
+					path,
+					requestedMethod,
+				);
+				const { allowedOrigins, preflightMaxAge } = accessControlOptions ?? {};
+				res.header('Access-Control-Allow-Origin', allowedOrigins ?? '*');
+				res.header('Access-Control-Max-Age', String((preflightMaxAge ?? 60) * 1000));
 			}
 
 			return ResponseHelper.sendSuccessResponse(res, {}, true, 204);

packages/nodes-base/nodes/Webhook/Webhook.node.ts

@@ -18,6 +18,7 @@ import isbot from 'isbot';
 import { file as tmpFile } from 'tmp-promise';
 
 import {
+	accessControlProperties,
 	authenticationProperty,
 	credentialsProperty,
 	defaultWebhookDescription,
@@ -90,23 +91,19 @@ export class Webhook extends Node {
 			responseDataProperty,
 			responseBinaryPropertyNameProperty,
 			optionsProperty,
+			accessControlProperties,
 		],
 	};
 
 	async webhook(context: IWebhookFunctions): Promise<IWebhookResponseData> {
-		const options = context.getNodeParameter('options', {}) as {
-			binaryData: boolean;
-			ignoreBots: boolean;
-			rawBody: Buffer;
-			responseData?: string;
-			accessControl: IDataObject;
-		};
 		const req = context.getRequestObject();
 		const resp = context.getResponseObject();
 
+		const ignoreBots = context.getNodeParameter('options.ignoreBots', {}) as boolean;
 		try {
-			if (options.ignoreBots && isbot(req.headers['user-agent']))
+			if (ignoreBots && isbot(req.headers['user-agent'])) {
 				throw new WebhookAuthorizationError(403);
+			}
 			await this.validateAuth(context);
 		} catch (error) {
 			if (error instanceof WebhookAuthorizationError) {
@@ -117,22 +114,24 @@ export class Webhook extends Node {
 			throw error;
 		}
 
-		if (options.binaryData) {
+		const binaryData = context.getNodeParameter('options.binaryData', {}) as boolean;
+		if (binaryData) {
 			return this.handleBinaryData(context);
 		}
 
 		if (req.contentType === 'multipart/form-data') {
 			return this.handleFormData(context);
 		}
 
+		const rawBody = context.getNodeParameter('options.rawBody', {}) as boolean;
 		const response: INodeExecutionData = {
 			json: {
 				headers: req.headers,
 				params: req.params,
 				query: req.query,
 				body: req.body,
 			},
-			binary: options.rawBody
+			binary: rawBody
 				? {
 						data: {
 							data: req.rawBody.toString(BINARY_ENCODING),
@@ -142,8 +141,9 @@ export class Webhook extends Node {
 				: undefined,
 		};
 
+		const responseData = context.getNodeParameter('options.responseData', {}) as string;
 		return {
-			webhookResponse: options.responseData,
+			webhookResponse: responseData,
 			workflowData: [[response]],
 		};
 	}