diff --git a/packages/nodes-base/nodes/Zendesk/ZendeskTrigger.node.ts b/packages/nodes-base/nodes/Zendesk/ZendeskTrigger.node.ts index 2514f9854bb..9c9ddba9d42 100644 --- a/packages/nodes-base/nodes/Zendesk/ZendeskTrigger.node.ts +++ b/packages/nodes-base/nodes/Zendesk/ZendeskTrigger.node.ts @@ -220,81 +220,63 @@ export class ZendeskTrigger implements INodeType { webhookMethods = { default: { + /** + * Checks whether the Zendesk webhook and trigger created by this workflow + * still exist. Returns true only when both are confirmed present, which + * prevents unnecessary recreation and avoids the scenario where a failed + * create() call causes clearWebhooks() to delete the Zendesk webhook. + * + * We intentionally do NOT delete triggers owned by other workflows. + * Duplicated n8n workflows share the same webhook URL, so cleaning up + * "unknown" triggers would silently destroy a sibling workflow's setup. + */ async checkExists(this: IHookFunctions): Promise { const webhookUrl = this.getNodeWebhookUrl('default') as string; const webhookData = this.getWorkflowStaticData('node'); - const conditions = this.getNodeParameter('conditions') as IDataObject; - let endpoint = ''; - const resultAll = [], - resultAny = []; - - const conditionsAll = conditions.all as [IDataObject]; - if (conditionsAll) { - for (const conditionAll of conditionsAll) { - const aux: IDataObject = {}; - aux.field = conditionAll.field; - aux.operator = conditionAll.operation; - if (conditionAll.operation !== 'changed' && conditionAll.operation !== 'not_changed') { - aux.value = conditionAll.value; - } else { - aux.value = null; - } - resultAll.push(aux); - } - } - - const conditionsAny = conditions.any as [IDataObject]; - if (conditionsAny) { - for (const conditionAny of conditionsAny) { - const aux: IDataObject = {}; - aux.field = conditionAny.field; - aux.operator = conditionAny.operation; - if (conditionAny.operation !== 'changed' && conditionAny.operation !== 'not_changed') { - aux.value = conditionAny.value; - } else { - aux.value = null; - } - resultAny.push(aux); - } - } - - // get all webhooks + // ── Step 1: find the Zendesk webhook by endpoint URL ──────────────── // https://developer.zendesk.com/api-reference/event-connectors/webhooks/webhooks/#list-webhooks - const { webhooks } = await zendeskApiRequest.call(this, 'GET', '/webhooks'); - for (const webhook of webhooks) { - if (webhook.endpoint === webhookUrl) { - webhookData.targetId = webhook.id; - break; - } - } + const webhooksResponse = await zendeskApiRequest.call(this, 'GET', '/webhooks'); + const webhooks: Array<{ id: string; endpoint: string }> = webhooksResponse.webhooks ?? []; - // no target was found - if (webhookData.targetId === undefined) { + const matchingWebhook = webhooks.find((w) => w.endpoint === webhookUrl); + + if (!matchingWebhook) { + // Webhook is gone — clear any stale references so create() starts fresh + delete webhookData.targetId; + delete webhookData.webhookId; return false; } - endpoint = '/triggers/active'; - const triggers = await zendeskApiRequestAllItems.call(this, 'triggers', 'GET', endpoint); + // Keep targetId in sync with Zendesk (the ID is stable, but be defensive) + webhookData.targetId = matchingWebhook.id; - for (const trigger of triggers) { - const toDeleteTriggers = []; - // this trigger belong to the current target - if (trigger.actions[0].value[0].toString() === webhookData.targetId?.toString()) { - toDeleteTriggers.push(trigger.id); - } - // delete all trigger attach to this target; - if (toDeleteTriggers.length !== 0) { - await zendeskApiRequest.call( - this, - 'DELETE', - '/triggers/destroy_many', - {}, - { ids: toDeleteTriggers.join(',') }, - ); - } + // ── Step 2: confirm our specific trigger still exists ──────────────── + // If we have no stored trigger ID this is a fresh activation (e.g. first + // run, or the workflow was duplicated). Fall through to create(). + if (webhookData.webhookId === undefined) { + return false; } + // https://developer.zendesk.com/api-reference/ticketing/business-rules/triggers/#list-triggers + const triggers: Array<{ + id: string | number; + actions: Array<{ field: string; value: string[] }>; + }> = await zendeskApiRequestAllItems.call(this, 'triggers', 'GET', '/triggers/active'); + + const ourTriggerExists = triggers.some( + (trigger) => + trigger.id.toString() === (webhookData.webhookId as string).toString() && + trigger.actions[0]?.field === 'notification_webhook' && + trigger.actions[0]?.value[0]?.toString() === matchingWebhook.id, + ); + + if (ourTriggerExists) { + return true; + } + + // Trigger is gone — clear the stale ID so create() makes a fresh one + delete webhookData.webhookId; return false; }, async create(this: IHookFunctions): Promise { @@ -400,13 +382,14 @@ export class ZendeskTrigger implements INodeType { // Fetch the signing secret for webhook signature verification // https://developer.zendesk.com/api-reference/event-connectors/webhooks/webhooks/#show-webhook-signing-secret - const signingSecretResponse = (await zendeskApiRequest.call( + const signingSecretResponse = await zendeskApiRequest.call( this, 'GET', - `/webhooks/${target.id as string}/signing_secret`, - )) as { signing_secret?: { secret?: string } }; - if (signingSecretResponse.signing_secret?.secret) { - webhookData.webhookSecret = signingSecretResponse.signing_secret.secret; + `/webhooks/${String(target.id)}/signing_secret`, + ); + const secret: string | undefined = signingSecretResponse?.signing_secret?.secret; + if (secret) { + webhookData.webhookSecret = secret; } ((bodyTrigger.trigger as IDataObject).actions as IDataObject[])[0].value = [ @@ -422,16 +405,40 @@ export class ZendeskTrigger implements INodeType { }, async delete(this: IHookFunctions): Promise { const webhookData = this.getWorkflowStaticData('node'); + let deletionSucceeded = true; try { await zendeskApiRequest.call(this, 'DELETE', `/triggers/${webhookData.webhookId}`); - await zendeskApiRequest.call(this, 'DELETE', `/webhooks/${webhookData.targetId}`); - } catch (error) { - return false; + + // Only delete the Zendesk webhook if no other triggers still reference it. + // A duplicated n8n workflow shares the same webhook URL and may have its + // own trigger pointing at the same Zendesk webhook — deleting the webhook + // would silently break the sibling workflow's incoming events. + const remainingTriggers: Array<{ + actions: Array<{ field: string; value: unknown[] }>; + }> = await zendeskApiRequestAllItems.call(this, 'triggers', 'GET', '/triggers/active'); + + const webhookStillInUse = remainingTriggers.some( + (trigger) => + trigger.actions[0]?.field === 'notification_webhook' && + String(trigger.actions[0]?.value[0]) === String(webhookData.targetId), + ); + + if (!webhookStillInUse) { + await zendeskApiRequest.call(this, 'DELETE', `/webhooks/${webhookData.targetId}`); + } + } catch { + // The remote resource may already be gone (e.g. deleted in Zendesk + // manually, or removed when a duplicated workflow was deactivated). + // We still fall through to the finally block so stale IDs are cleared, + // ensuring the next activation starts with a clean slate instead of + // trying to reuse a webhook that no longer exists. + deletionSucceeded = false; + } finally { + delete webhookData.webhookId; + delete webhookData.targetId; + delete webhookData.webhookSecret; } - delete webhookData.webhookId; - delete webhookData.targetId; - delete webhookData.webhookSecret; - return true; + return deletionSucceeded; }, }, }; diff --git a/packages/nodes-base/nodes/Zendesk/__tests__/ZendeskTrigger.node.test.ts b/packages/nodes-base/nodes/Zendesk/__tests__/ZendeskTrigger.node.test.ts index 742aae76bcd..72e58ed4f9d 100644 --- a/packages/nodes-base/nodes/Zendesk/__tests__/ZendeskTrigger.node.test.ts +++ b/packages/nodes-base/nodes/Zendesk/__tests__/ZendeskTrigger.node.test.ts @@ -1,13 +1,38 @@ +import type { IHookFunctions } from 'n8n-workflow'; import { ZendeskTrigger } from '../ZendeskTrigger.node'; import * as GenericFunctions from '../GenericFunctions'; import * as ZendeskTriggerHelpers from '../ZendeskTriggerHelpers'; +const WEBHOOK_URL = 'https://n8n.example.com/webhook/abc123'; +const ZENDESK_WEBHOOK_ID = 'zd-webhook-111'; +const ZENDESK_TRIGGER_ID = 'zd-trigger-222'; + +function makeCheckExistsContext(webhookData: Record = {}): IHookFunctions { + return { + getNodeWebhookUrl: () => WEBHOOK_URL, + getWorkflowStaticData: () => webhookData, + } as unknown as IHookFunctions; +} + +const matchingWebhook = { id: ZENDESK_WEBHOOK_ID, endpoint: WEBHOOK_URL }; + +const ownTrigger = { + id: ZENDESK_TRIGGER_ID, + actions: [{ field: 'notification_webhook', value: [ZENDESK_WEBHOOK_ID, '{}'] }], +}; + +const siblingTrigger = { + id: 'zd-trigger-sibling', + actions: [{ field: 'notification_webhook', value: [ZENDESK_WEBHOOK_ID, '{}'] }], +}; + describe('ZendeskTrigger Node', () => { describe('create webhook method', () => { let mockThis: any; let webhookData: Record; beforeEach(() => { + jest.clearAllMocks(); webhookData = {}; mockThis = { getNodeWebhookUrl: () => 'https://example.com/webhook', @@ -58,6 +83,31 @@ describe('ZendeskTrigger Node', () => { expect(apiRequestSpy).toHaveBeenCalledWith('GET', '/webhooks/webhook-789/signing_secret'); }); + + it('should reuse the existing Zendesk webhook and skip POST /webhooks when targetId is already stored', async () => { + webhookData.targetId = 'existing-webhook-456'; + const signingSecretResponse = { signing_secret: { secret: 'reused-secret' } }; + const createdTrigger = { id: 'trigger-789' }; + + const apiRequestSpy = jest + .spyOn(GenericFunctions, 'zendeskApiRequest') + .mockResolvedValueOnce(signingSecretResponse) // GET /webhooks/{id}/signing_secret + .mockResolvedValueOnce({ trigger: createdTrigger }); // POST /triggers + + const trigger = new ZendeskTrigger(); + const result = await trigger.webhookMethods.default.create.call(mockThis); + + expect(result).toBe(true); + expect(webhookData.targetId).toBe('existing-webhook-456'); + expect(webhookData.webhookSecret).toBe('reused-secret'); + expect(webhookData.webhookId).toBe('trigger-789'); + + // POST /webhooks must not be called when a webhook already exists + const postWebhookCalls = apiRequestSpy.mock.calls.filter( + ([method, path]) => method === 'POST' && path === '/webhooks', + ); + expect(postWebhookCalls).toHaveLength(0); + }); }); describe('delete webhook method', () => { @@ -65,6 +115,7 @@ describe('ZendeskTrigger Node', () => { let mockThis: any; beforeEach(() => { + jest.clearAllMocks(); webhookData = { webhookId: 'trigger-123', targetId: 'webhook-456', @@ -81,6 +132,7 @@ describe('ZendeskTrigger Node', () => { .spyOn(GenericFunctions, 'zendeskApiRequest') .mockResolvedValueOnce({}) // DELETE /triggers .mockResolvedValueOnce({}); // DELETE /webhooks + jest.spyOn(GenericFunctions, 'zendeskApiRequestAllItems').mockResolvedValueOnce([]); // no remaining triggers → webhook can be deleted const trigger = new ZendeskTrigger(); const result = await trigger.webhookMethods.default.delete.call(mockThis); @@ -101,6 +153,178 @@ describe('ZendeskTrigger Node', () => { expect(result).toBe(false); }); + + it('should clear static data even when the API call fails, so the next activation does not try to reuse a deleted webhook', async () => { + jest + .spyOn(GenericFunctions, 'zendeskApiRequest') + .mockRejectedValueOnce(new Error('404 Not Found')); + + const trigger = new ZendeskTrigger(); + await trigger.webhookMethods.default.delete.call(mockThis); + + // Stale IDs must be gone so the next activation does not try to reuse + // a Zendesk webhook that no longer exists (e.g. deleted via duplication). + expect(webhookData.webhookId).toBeUndefined(); + expect(webhookData.targetId).toBeUndefined(); + expect(webhookData.webhookSecret).toBeUndefined(); + }); + + it('should not delete the Zendesk webhook when a sibling trigger still references it, so the duplicate workflow keeps receiving events', async () => { + const apiRequestSpy = jest + .spyOn(GenericFunctions, 'zendeskApiRequest') + .mockResolvedValueOnce({}); // DELETE /triggers + jest.spyOn(GenericFunctions, 'zendeskApiRequestAllItems').mockResolvedValueOnce([ + // A sibling trigger still points at the same webhook + { + actions: [{ field: 'notification_webhook', value: [webhookData.targetId, '{}'] }], + }, + ]); + + const trigger = new ZendeskTrigger(); + const result = await trigger.webhookMethods.default.delete.call(mockThis); + + expect(result).toBe(true); + + // DELETE /webhooks must not have been called + const deleteWebhookCalls = apiRequestSpy.mock.calls.filter( + ([method, path]) => + method === 'DELETE' && typeof path === 'string' && path.startsWith('/webhooks'), + ); + expect(deleteWebhookCalls).toHaveLength(0); + }); + }); + + describe('checkExists webhook method', () => { + beforeEach(() => { + jest.clearAllMocks(); + }); + + it('should return false and clear stale IDs when the webhook is not found in Zendesk, so create() starts fresh instead of reusing a deleted webhook', async () => { + jest.spyOn(GenericFunctions, 'zendeskApiRequest').mockResolvedValueOnce({ + webhooks: [{ id: 'other-id', endpoint: 'https://other.example.com' }], + }); + + const webhookData: Record = { + targetId: 'stale-webhook-id', + webhookId: 'stale-trigger-id', + }; + const trigger = new ZendeskTrigger(); + + const result = await trigger.webhookMethods.default.checkExists.call( + makeCheckExistsContext(webhookData), + ); + + expect(result).toBe(false); + expect(webhookData.targetId).toBeUndefined(); + expect(webhookData.webhookId).toBeUndefined(); + }); + + it('should return false without fetching triggers on first activation, so create() registers a trigger for the existing webhook without an unnecessary API call', async () => { + jest + .spyOn(GenericFunctions, 'zendeskApiRequest') + .mockResolvedValueOnce({ webhooks: [matchingWebhook] }); + const allItemsSpy = jest.spyOn(GenericFunctions, 'zendeskApiRequestAllItems'); + + const webhookData: Record = {}; + const trigger = new ZendeskTrigger(); + + const result = await trigger.webhookMethods.default.checkExists.call( + makeCheckExistsContext(webhookData), + ); + + expect(result).toBe(false); + expect(webhookData.targetId).toBe(ZENDESK_WEBHOOK_ID); + // Triggers API must not be called — we know we need to create without checking + expect(allItemsSpy).not.toHaveBeenCalled(); + }); + + it('should return true when both the webhook and our trigger are confirmed in Zendesk, so create() is skipped and the existing setup is preserved', async () => { + jest + .spyOn(GenericFunctions, 'zendeskApiRequest') + .mockResolvedValueOnce({ webhooks: [matchingWebhook] }); + jest.spyOn(GenericFunctions, 'zendeskApiRequestAllItems').mockResolvedValueOnce([ownTrigger]); + + const webhookData: Record = { + targetId: ZENDESK_WEBHOOK_ID, + webhookId: ZENDESK_TRIGGER_ID, + }; + const trigger = new ZendeskTrigger(); + + const result = await trigger.webhookMethods.default.checkExists.call( + makeCheckExistsContext(webhookData), + ); + + expect(result).toBe(true); + expect(webhookData.targetId).toBe(ZENDESK_WEBHOOK_ID); + expect(webhookData.webhookId).toBe(ZENDESK_TRIGGER_ID); + }); + + it('should return false and clear the stale trigger ID when the webhook exists but our trigger is gone, so create() makes a new trigger without being blocked by the invalid ID', async () => { + jest + .spyOn(GenericFunctions, 'zendeskApiRequest') + .mockResolvedValueOnce({ webhooks: [matchingWebhook] }); + jest.spyOn(GenericFunctions, 'zendeskApiRequestAllItems').mockResolvedValueOnce([]); + + const webhookData: Record = { + targetId: ZENDESK_WEBHOOK_ID, + webhookId: ZENDESK_TRIGGER_ID, + }; + const trigger = new ZendeskTrigger(); + + const result = await trigger.webhookMethods.default.checkExists.call( + makeCheckExistsContext(webhookData), + ); + + expect(result).toBe(false); + expect(webhookData.targetId).toBe(ZENDESK_WEBHOOK_ID); // webhook still present + expect(webhookData.webhookId).toBeUndefined(); // stale trigger ID cleared + }); + + it("should not issue any DELETE calls for triggers it does not own, so a duplicated workflow cannot silently destroy the original workflow's Zendesk trigger", async () => { + const apiRequestSpy = jest + .spyOn(GenericFunctions, 'zendeskApiRequest') + .mockResolvedValueOnce({ webhooks: [matchingWebhook] }); + jest + .spyOn(GenericFunctions, 'zendeskApiRequestAllItems') + .mockResolvedValueOnce([siblingTrigger]); + + // Our stored webhookId is different from the sibling's trigger ID + const webhookData: Record = { + targetId: ZENDESK_WEBHOOK_ID, + webhookId: ZENDESK_TRIGGER_ID, + }; + const trigger = new ZendeskTrigger(); + + await trigger.webhookMethods.default.checkExists.call(makeCheckExistsContext(webhookData)); + + const deleteCalls = apiRequestSpy.mock.calls.filter(([method]) => method === 'DELETE'); + expect(deleteCalls).toHaveLength(0); + }); + + it('should not match a trigger whose action field is not notification_webhook, so unrelated Zendesk triggers with coincidental IDs are never treated as ours', async () => { + jest + .spyOn(GenericFunctions, 'zendeskApiRequest') + .mockResolvedValueOnce({ webhooks: [matchingWebhook] }); + jest.spyOn(GenericFunctions, 'zendeskApiRequestAllItems').mockResolvedValueOnce([ + // Same ID as our stored webhookId but wrong action type + { + id: ZENDESK_TRIGGER_ID, + actions: [{ field: 'notification_user', value: ['user@example.com'] }], + }, + ]); + + const webhookData: Record = { + targetId: ZENDESK_WEBHOOK_ID, + webhookId: ZENDESK_TRIGGER_ID, + }; + const trigger = new ZendeskTrigger(); + + const result = await trigger.webhookMethods.default.checkExists.call( + makeCheckExistsContext(webhookData), + ); + + expect(result).toBe(false); + }); }); describe('webhook method', () => {