fix(core): Fix getting webhook methods from path only when dynamic webhook path (#17803)

packages/cli/src/webhooks/__tests__/test-webhooks.test.ts

@@ -1,3 +1,4 @@
+import type { WorkflowEntity } from '@n8n/db';
 import { generateNanoId } from '@n8n/db';
 import type * as express from 'express';
 import { mock } from 'jest-mock-extended';
@@ -7,6 +8,7 @@ import type {
 	IWebhookData,
 	IWorkflowExecuteAdditionalData,
 	Workflow,
+	IHttpRequestMethods,
 } from 'n8n-workflow';
 import { v4 as uuid } from 'uuid';
 
@@ -195,11 +197,40 @@ describe('TestWebhooks', () => {
 	});
 
 	describe('getWebhookMethods()', () => {
+		beforeEach(() => {
+			registrations.toKey.mockImplementation(
+				(webhook: Pick<IWebhookData, 'webhookId' | 'httpMethod' | 'path'>) => {
+					const { webhookId, httpMethod, path: webhookPath } = webhook;
+					if (!webhookId) return [httpMethod, webhookPath].join('|');
+
+					let path = webhookPath;
+					if (path.startsWith(webhookId)) {
+						const cutFromIndex = path.indexOf('/') + 1;
+
+						path = path.slice(cutFromIndex);
+					}
+					return [httpMethod, webhookId, path.split('/').length].join('|');
+				},
+			);
+		});
+
 		test('should normalize trailing slash', async () => {
 			const METHOD = 'POST';
 			const PATH_WITH_SLASH = 'register/';
 			const PATH_WITHOUT_SLASH = 'register';
-			registrations.getAllKeys.mockResolvedValue([`${METHOD}|${PATH_WITHOUT_SLASH}`]);
+			const webhookData = {
+				httpMethod: METHOD as IHttpRequestMethods,
+				path: PATH_WITHOUT_SLASH,
+			} as IWebhookData;
+
+			registrations.getRegistrationsHash.mockImplementation(async () => {
+				return {
+					[registrations.toKey(webhookData)]: {
+						workflowEntity: mock<WorkflowEntity>(),
+						webhook: webhookData,
+					},
+				};
+			});
 
 			const resultWithSlash = await testWebhooks.getWebhookMethods(PATH_WITH_SLASH);
 			const resultWithoutSlash = await testWebhooks.getWebhookMethods(PATH_WITHOUT_SLASH);
@@ -207,5 +238,30 @@ describe('TestWebhooks', () => {
 			expect(resultWithSlash).toEqual([METHOD]);
 			expect(resultWithoutSlash).toEqual([METHOD]);
 		});
+
+		test('should return methods for webhooks with dynamic paths', async () => {
+			const METHOD = 'POST';
+			const PATH = '12345/register/:id';
+
+			const webhookData = {
+				webhookId: '12345',
+				httpMethod: METHOD as IHttpRequestMethods,
+				// Path for dynamic webhook does not contain webhookId
+				path: 'register/:id',
+			};
+
+			registrations.getRegistrationsHash.mockImplementation(async () => {
+				return {
+					[registrations.toKey(webhookData)]: {
+						workflowEntity: mock<WorkflowEntity>(),
+						webhook: webhookData as IWebhookData,
+					},
+				};
+			});
+
+			const result = await testWebhooks.getWebhookMethods(PATH);
+
+			expect(result).toEqual([METHOD]);
+		});
 	});
 });

packages/cli/src/webhooks/__tests__/webhook.service.test.ts

@@ -154,6 +154,21 @@ describe('WebhookService', () => {
 
 			expect(returnedMethods).toEqual([]);
 		});
+
+		test('should return dynamic webhook method when static search returns nothing', async () => {
+			const webhookId = uuid();
+			const dynamicPath = `${webhookId}/user/1`;
+			const mockDynamicWebhook = createWebhook('POST', 'user/:id', webhookId, 2);
+
+			// Mock static webhook search to return empty
+			webhookRepository.find.mockResolvedValue([]);
+			// Mock dynamic webhook search to return a webhook
+			webhookRepository.findBy.mockResolvedValue([mockDynamicWebhook]);
+
+			const returnedMethods = await webhookService.getWebhookMethods(dynamicPath);
+
+			expect(returnedMethods).toEqual(['POST']);
+		});
 	});
 
 	describe('deleteWorkflowWebhooks()', () => {

packages/cli/src/webhooks/test-webhook-registrations.service.ts

@@ -71,6 +71,10 @@ export class TestWebhookRegistrationsService {
 		return Object.values(hash);
 	}
 
+	async getRegistrationsHash() {
+		return await this.cacheService.getHash<TestWebhookRegistration>(this.cacheKey);
+	}
+
 	async deregisterAll() {
 		await this.cacheService.delete(this.cacheKey);
 	}

packages/cli/src/webhooks/test-webhooks.ts

@@ -200,13 +200,34 @@ export class TestWebhooks implements IWebhookManager {
 		if (timeout) clearTimeout(timeout);
 	}
 
+	async getWebhooksFromPath(rawPath: string) {
+		const path = removeTrailingSlash(rawPath);
+		const webhooks: IWebhookData[] = [];
+		const registrations = await this.registrations.getRegistrationsHash();
+
+		for (const httpMethod of ['GET', 'POST', 'PUT', 'DELETE', 'PATCH'] as IHttpRequestMethods[]) {
+			const key = this.registrations.toKey({ httpMethod, path });
+			let webhook = registrations?.[key]?.webhook;
+			if (!webhook) {
+				// check for dynamic webhooks
+				const [webhookId, ...segments] = path.split('/');
+				const key = this.registrations.toKey({ httpMethod, path, webhookId });
+				if (registrations?.[key]) {
+					webhook = this.getActiveWebhookFromRegistration(segments.join('/'), registrations?.[key]);
+				}
+			}
+			if (webhook) {
+				webhooks.push(webhook);
+			}
+		}
+		return webhooks;
+	}
+
 	async getWebhookMethods(rawPath: string) {
 		const path = removeTrailingSlash(rawPath);
-		const allKeys = await this.registrations.getAllKeys();
+		const webhooks = await this.getWebhooksFromPath(path);
 
-		const webhookMethods = allKeys
-			.filter((key) => key.includes(path))
-			.map((key) => key.split('|')[0] as IHttpRequestMethods);
+		const webhookMethods = webhooks.map((webhook) => webhook.httpMethod);
 
 		if (!webhookMethods.length) throw new WebhookNotFoundError({ path });
 
@@ -393,33 +414,31 @@ export class TestWebhooks implements IWebhookManager {
 		return foundWebhook;
 	}
 
-	async getActiveWebhook(httpMethod: IHttpRequestMethods, path: string, webhookId?: string) {
-		const key = this.registrations.toKey({ httpMethod, path, webhookId });
-
-		let webhook: IWebhookData | undefined;
-		let maxMatches = 0;
+	getActiveWebhookFromRegistration(
+		path: string,
+		registration: TestWebhookRegistration,
+	): IWebhookData | undefined {
 		const pathElementsSet = new Set(path.split('/'));
-		// check if static elements match in path
-		// if more results have been returned choose the one with the most static-route matches
-		const registration = await this.registrations.get(key);
-
-		if (!registration) return;
 
 		const { webhook: dynamicWebhook } = registration;
 
 		const staticElements = dynamicWebhook.path.split('/').filter((ele) => !ele.startsWith(':'));
 		const allStaticExist = staticElements.every((staticEle) => pathElementsSet.has(staticEle));
 
-		if (allStaticExist && staticElements.length > maxMatches) {
-			maxMatches = staticElements.length;
-			webhook = dynamicWebhook;
-		}
-		// handle routes with no static elements
-		else if (staticElements.length === 0 && !webhook) {
-			webhook = dynamicWebhook;
+		// webhook matches if all static elements exist or if there are no static elements
+		if ((allStaticExist && staticElements.length > 0) || staticElements.length === 0) {
+			return dynamicWebhook;
 		}
+		return undefined;
+	}
 
-		return webhook;
+	async getActiveWebhook(httpMethod: IHttpRequestMethods, path: string, webhookId?: string) {
+		const key = this.registrations.toKey({ httpMethod, path, webhookId });
+		const registration = await this.registrations.get(key);
+
+		if (!registration) return;
+
+		return this.getActiveWebhookFromRegistration(path, registration);
 	}
 
 	/**

packages/cli/src/webhooks/webhook.service.ts

@@ -57,7 +57,7 @@ export class WebhookService {
 			return dbStaticWebhook;
 		}
 
-		return await this.findDynamicWebhook(method, path);
+		return await this.findDynamicWebhook(path, method);
 	}
 
 	/**
@@ -71,7 +71,7 @@ export class WebhookService {
 	 * Find a matching webhook with one or more dynamic path segments, e.g. `<uuid>/user/:id/posts`.
 	 * It is mandatory for dynamic webhooks to have `<uuid>/` at the base.
 	 */
-	private async findDynamicWebhook(method: Method, path: string) {
+	private async findDynamicWebhook(path: string, method?: Method) {
 		const [uuidSegment, ...otherSegments] = path.split('/');
 
 		const dynamicWebhooks = await this.webhookRepository.findBy({
@@ -133,10 +133,19 @@ export class WebhookService {
 		return await this.webhookRepository.remove(webhooks);
 	}
 
-	async getWebhookMethods(path: string) {
-		return await this.webhookRepository
-			.find({ select: ['method'], where: { webhookPath: path } })
+	async getWebhookMethods(rawPath: string) {
+		// Try to find static webhooks first
+		const staticMethods = await this.webhookRepository
+			.find({ select: ['method'], where: { webhookPath: rawPath } })
 			.then((rows) => rows.map((r) => r.method));
+
+		if (staticMethods.length > 0) {
+			return staticMethods;
+		}
+
+		// Otherwise, try to find dynamic webhooks based on path only
+		const dynamicWebhooks = await this.findDynamicWebhook(rawPath);
+		return dynamicWebhooks ? [dynamicWebhooks.method] : [];
 	}
 
 	private isDynamicPath(rawPath: string) {