From 3a38b3286713708caaad3eb9729efb8328f8b707 Mon Sep 17 00:00:00 2001 From: RomanDavydchuk Date: Mon, 15 Sep 2025 15:55:13 +0300 Subject: [PATCH] fix(Webhook Trigger Node): Duplicate webhook paths are not detected for methods other than GET (#19378) Co-authored-by: Michael Kret --- .../composables/useWorkflowHelpers.test.ts | 120 +++++++++++++++++- .../src/composables/useWorkflowHelpers.ts | 27 ++-- 2 files changed, 130 insertions(+), 17 deletions(-) diff --git a/packages/frontend/editor-ui/src/composables/useWorkflowHelpers.test.ts b/packages/frontend/editor-ui/src/composables/useWorkflowHelpers.test.ts index 1fb96ad7cb..b815f92827 100644 --- a/packages/frontend/editor-ui/src/composables/useWorkflowHelpers.test.ts +++ b/packages/frontend/editor-ui/src/composables/useWorkflowHelpers.test.ts @@ -312,7 +312,7 @@ describe('useWorkflowHelpers', () => { type: WEBHOOK_NODE_TYPE, webhookId: '1', parameters: { - method: 'GET', + httpMethod: 'GET', path: 'test-path', }, }, @@ -333,7 +333,7 @@ describe('useWorkflowHelpers', () => { }, trigger: { parameters: { - method: 'GET', + httpMethod: 'GET', path: 'test-path', }, type: 'n8n-nodes-base.webhook', @@ -417,7 +417,7 @@ describe('useWorkflowHelpers', () => { type: WEBHOOK_NODE_TYPE, webhookId: '1', parameters: { - method: 'GET', + httpMethod: 'GET', path: 'test-path', }, }, @@ -535,7 +535,7 @@ describe('useWorkflowHelpers', () => { { type: WEBHOOK_NODE_TYPE, parameters: { - method: 'GET', + httpMethod: 'GET', path: 'test-path', }, webhookId: 'test-webhook-id', @@ -560,7 +560,7 @@ describe('useWorkflowHelpers', () => { { type: WEBHOOK_NODE_TYPE, parameters: { - method: 'GET', + httpMethod: 'GET', path: '', }, webhookId: 'test-webhook-id', @@ -576,6 +576,116 @@ describe('useWorkflowHelpers', () => { }); findWebhookSpy.mockRestore(); }); + + it('should find conflicting webhook data with multiple methods', async () => { + const workflowHelpers = useWorkflowHelpers(); + uiStore.stateIsDirty = false; + vi.spyOn(workflowsStore, 'fetchWorkflow').mockResolvedValue({ + nodes: [ + { + type: WEBHOOK_NODE_TYPE, + webhookId: '1', + parameters: { + httpMethod: ['PUT', 'PATCH'], + path: 'test-path', + multipleMethods: true, + }, + }, + ], + } as unknown as IWorkflowDb); + vi.spyOn(apiWebhooks, 'findWebhook').mockResolvedValue({ + method: 'PATCH', + webhookPath: 'test-path', + node: 'Webhook 1', + workflowId: '456', + }); + expect(await workflowHelpers.checkConflictingWebhooks('123')).toEqual({ + conflict: { + method: 'PATCH', + node: 'Webhook 1', + webhookPath: 'test-path', + workflowId: '456', + }, + trigger: { + parameters: { + httpMethod: ['PUT', 'PATCH'], + path: 'test-path', + multipleMethods: true, + }, + type: 'n8n-nodes-base.webhook', + webhookId: '1', + }, + }); + }); + + it('should return null if no conflicts with multiple methods', async () => { + const workflowHelpers = useWorkflowHelpers(); + uiStore.stateIsDirty = false; + vi.spyOn(workflowsStore, 'fetchWorkflow').mockResolvedValue({ + nodes: [ + { + type: WEBHOOK_NODE_TYPE, + webhookId: '1', + parameters: { + httpMethod: ['PUT', 'PATCH'], + path: 'test-path', + multipleMethods: true, + }, + }, + ], + } as unknown as IWorkflowDb); + vi.spyOn(apiWebhooks, 'findWebhook').mockResolvedValue(null); + expect(await workflowHelpers.checkConflictingWebhooks('123')).toEqual(null); + }); + + it('should fallback to GET, POST if httpMethod is not set and multipleMethods is true', async () => { + const workflowHelpers = useWorkflowHelpers(); + uiStore.stateIsDirty = false; + vi.spyOn(workflowsStore, 'fetchWorkflow').mockResolvedValue({ + nodes: [ + { + type: WEBHOOK_NODE_TYPE, + webhookId: '1', + parameters: { + path: 'test-path', + multipleMethods: true, + }, + }, + ], + } as unknown as IWorkflowDb); + const spy = vi.spyOn(apiWebhooks, 'findWebhook'); + await workflowHelpers.checkConflictingWebhooks('123'); + expect(spy).toHaveBeenCalledWith(expect.anything(), { + method: 'GET', + path: 'test-path', + }); + expect(spy).toHaveBeenCalledWith(expect.anything(), { + method: 'POST', + path: 'test-path', + }); + }); + + it('should fallback to GET if httpMethod is not set and multipleMethods is false', async () => { + const workflowHelpers = useWorkflowHelpers(); + uiStore.stateIsDirty = false; + vi.spyOn(workflowsStore, 'fetchWorkflow').mockResolvedValue({ + nodes: [ + { + type: WEBHOOK_NODE_TYPE, + webhookId: '1', + parameters: { + path: 'test-path', + }, + }, + ], + } as unknown as IWorkflowDb); + const spy = vi.spyOn(apiWebhooks, 'findWebhook'); + await workflowHelpers.checkConflictingWebhooks('123'); + expect(spy).toHaveBeenCalledWith(expect.anything(), { + method: 'GET', + path: 'test-path', + }); + }); }); describe('executeData', () => { diff --git a/packages/frontend/editor-ui/src/composables/useWorkflowHelpers.ts b/packages/frontend/editor-ui/src/composables/useWorkflowHelpers.ts index e4b8c9a9ad..813b1abe8c 100644 --- a/packages/frontend/editor-ui/src/composables/useWorkflowHelpers.ts +++ b/packages/frontend/editor-ui/src/composables/useWorkflowHelpers.ts @@ -1017,11 +1017,14 @@ export function useWorkflowHelpers() { return workflow.nodes.some((node) => node.type.startsWith(packageName)); }; - function getMethod(trigger: INode) { + function getMethods(trigger: INode) { if (trigger.type === WEBHOOK_NODE_TYPE) { - return (trigger.parameters.method as string) ?? 'GET'; + if (trigger.parameters.multipleMethods === true) { + return (trigger.parameters.httpMethod as string[]) ?? ['GET', 'POST']; + } + return [(trigger.parameters.httpMethod as string) ?? 'GET']; } - return 'POST'; + return ['POST']; } function getWebhookPath(trigger: INode) { @@ -1053,17 +1056,17 @@ export function useWorkflowHelpers() { ); for (const trigger of triggers) { - const method = getMethod(trigger); - + const methods = getMethods(trigger); const path = getWebhookPath(trigger); + for (const method of methods) { + const conflict = await findWebhook(rootStore.restApiContext, { + path, + method, + }); - const conflict = await findWebhook(rootStore.restApiContext, { - path, - method, - }); - - if (conflict && conflict.workflowId !== workflowId) { - return { trigger, conflict }; + if (conflict && conflict.workflowId !== workflowId) { + return { trigger, conflict }; + } } }