mirror of
https://github.com/Abdulazizzn/n8n-enterprise-unlocked.git
synced 2025-12-16 17:46:45 +00:00
fix(Webhook Trigger Node): Duplicate webhook paths are not detected for methods other than GET (#19378)
Co-authored-by: Michael Kret <michael.k@radency.com>
This commit is contained in:
@@ -312,7 +312,7 @@ describe('useWorkflowHelpers', () => {
|
|||||||
type: WEBHOOK_NODE_TYPE,
|
type: WEBHOOK_NODE_TYPE,
|
||||||
webhookId: '1',
|
webhookId: '1',
|
||||||
parameters: {
|
parameters: {
|
||||||
method: 'GET',
|
httpMethod: 'GET',
|
||||||
path: 'test-path',
|
path: 'test-path',
|
||||||
},
|
},
|
||||||
},
|
},
|
||||||
@@ -333,7 +333,7 @@ describe('useWorkflowHelpers', () => {
|
|||||||
},
|
},
|
||||||
trigger: {
|
trigger: {
|
||||||
parameters: {
|
parameters: {
|
||||||
method: 'GET',
|
httpMethod: 'GET',
|
||||||
path: 'test-path',
|
path: 'test-path',
|
||||||
},
|
},
|
||||||
type: 'n8n-nodes-base.webhook',
|
type: 'n8n-nodes-base.webhook',
|
||||||
@@ -417,7 +417,7 @@ describe('useWorkflowHelpers', () => {
|
|||||||
type: WEBHOOK_NODE_TYPE,
|
type: WEBHOOK_NODE_TYPE,
|
||||||
webhookId: '1',
|
webhookId: '1',
|
||||||
parameters: {
|
parameters: {
|
||||||
method: 'GET',
|
httpMethod: 'GET',
|
||||||
path: 'test-path',
|
path: 'test-path',
|
||||||
},
|
},
|
||||||
},
|
},
|
||||||
@@ -535,7 +535,7 @@ describe('useWorkflowHelpers', () => {
|
|||||||
{
|
{
|
||||||
type: WEBHOOK_NODE_TYPE,
|
type: WEBHOOK_NODE_TYPE,
|
||||||
parameters: {
|
parameters: {
|
||||||
method: 'GET',
|
httpMethod: 'GET',
|
||||||
path: 'test-path',
|
path: 'test-path',
|
||||||
},
|
},
|
||||||
webhookId: 'test-webhook-id',
|
webhookId: 'test-webhook-id',
|
||||||
@@ -560,7 +560,7 @@ describe('useWorkflowHelpers', () => {
|
|||||||
{
|
{
|
||||||
type: WEBHOOK_NODE_TYPE,
|
type: WEBHOOK_NODE_TYPE,
|
||||||
parameters: {
|
parameters: {
|
||||||
method: 'GET',
|
httpMethod: 'GET',
|
||||||
path: '',
|
path: '',
|
||||||
},
|
},
|
||||||
webhookId: 'test-webhook-id',
|
webhookId: 'test-webhook-id',
|
||||||
@@ -576,6 +576,116 @@ describe('useWorkflowHelpers', () => {
|
|||||||
});
|
});
|
||||||
findWebhookSpy.mockRestore();
|
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', () => {
|
describe('executeData', () => {
|
||||||
|
|||||||
@@ -1017,11 +1017,14 @@ export function useWorkflowHelpers() {
|
|||||||
return workflow.nodes.some((node) => node.type.startsWith(packageName));
|
return workflow.nodes.some((node) => node.type.startsWith(packageName));
|
||||||
};
|
};
|
||||||
|
|
||||||
function getMethod(trigger: INode) {
|
function getMethods(trigger: INode) {
|
||||||
if (trigger.type === WEBHOOK_NODE_TYPE) {
|
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) {
|
function getWebhookPath(trigger: INode) {
|
||||||
@@ -1053,17 +1056,17 @@ export function useWorkflowHelpers() {
|
|||||||
);
|
);
|
||||||
|
|
||||||
for (const trigger of triggers) {
|
for (const trigger of triggers) {
|
||||||
const method = getMethod(trigger);
|
const methods = getMethods(trigger);
|
||||||
|
|
||||||
const path = getWebhookPath(trigger);
|
const path = getWebhookPath(trigger);
|
||||||
|
for (const method of methods) {
|
||||||
|
const conflict = await findWebhook(rootStore.restApiContext, {
|
||||||
|
path,
|
||||||
|
method,
|
||||||
|
});
|
||||||
|
|
||||||
const conflict = await findWebhook(rootStore.restApiContext, {
|
if (conflict && conflict.workflowId !== workflowId) {
|
||||||
path,
|
return { trigger, conflict };
|
||||||
method,
|
}
|
||||||
});
|
|
||||||
|
|
||||||
if (conflict && conflict.workflowId !== workflowId) {
|
|
||||||
return { trigger, conflict };
|
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
Reference in New Issue
Block a user