diff --git a/packages/nodes-base/credentials/TrelloApi.credentials.ts b/packages/nodes-base/credentials/TrelloApi.credentials.ts index 1623eaa1ac3..d905d573185 100644 --- a/packages/nodes-base/credentials/TrelloApi.credentials.ts +++ b/packages/nodes-base/credentials/TrelloApi.credentials.ts @@ -33,9 +33,11 @@ export class TrelloApi implements ICredentialType { { displayName: 'OAuth Secret', name: 'oauthSecret', - type: 'hidden', + type: 'string', typeOptions: { password: true }, default: '', + description: + 'Used to verify webhook authenticity. Found under the API Key tab at trello.com/power-ups/admin.', }, ]; diff --git a/packages/nodes-base/nodes/Trello/TrelloTrigger.node.ts b/packages/nodes-base/nodes/Trello/TrelloTrigger.node.ts index 26fc9c9c7c8..6f34979afb3 100644 --- a/packages/nodes-base/nodes/Trello/TrelloTrigger.node.ts +++ b/packages/nodes-base/nodes/Trello/TrelloTrigger.node.ts @@ -8,8 +8,7 @@ import { } from 'n8n-workflow'; import { apiRequest } from './GenericFunctions'; - -// import { createHmac } from 'crypto'; +import { verifySignature } from './TrelloTriggerHelpers'; export class TrelloTrigger implements INodeType { description: INodeTypeDescription = { @@ -147,22 +146,17 @@ export class TrelloTrigger implements INodeType { }; } + const isSignatureValid = await verifySignature.call(this); + if (!isSignatureValid) { + const res = this.getResponseObject(); + res.status(401).send('Unauthorized').end(); + return { + noWebhookResponse: true, + }; + } + const bodyData = this.getBodyData(); - // TODO: Check why that does not work as expected even though it gets done as described - // https://developers.trello.com/page/webhooks - - //const credentials = await this.getCredentials('trelloApi'); - // // Check if the request is valid - // const headerData = this.getHeaderData() as IDataObject; - // const webhookUrl = this.getNodeWebhookUrl('default'); - // const checkContent = JSON.stringify(bodyData) + webhookUrl; - // const computedSignature = createHmac('sha1', credentials.oauthSecret as string).update(checkContent).digest('base64'); - // if (headerData['x-trello-webhook'] !== computedSignature) { - // // Signature is not valid so ignore call - // return {}; - // } - return { workflowData: [this.helpers.returnJsonArray(bodyData)], }; diff --git a/packages/nodes-base/nodes/Trello/TrelloTriggerHelpers.ts b/packages/nodes-base/nodes/Trello/TrelloTriggerHelpers.ts new file mode 100644 index 00000000000..b3fccaa37c4 --- /dev/null +++ b/packages/nodes-base/nodes/Trello/TrelloTriggerHelpers.ts @@ -0,0 +1,38 @@ +import { createHmac } from 'crypto'; +import type { IWebhookFunctions } from 'n8n-workflow'; + +import { verifySignature as verifySignatureGeneric } from '../../utils/webhook-signature-verification'; + +export async function verifySignature(this: IWebhookFunctions): Promise { + const credential = await this.getCredentials('trelloApi'); + const req = this.getRequestObject(); + const secret = credential.oauthSecret; + + return verifySignatureGeneric({ + getExpectedSignature: () => { + if (!secret || typeof secret !== 'string' || !req.rawBody) { + return null; + } + + const callbackURL = this.getNodeWebhookUrl('default'); + const rawBody = Buffer.isBuffer(req.rawBody) + ? req.rawBody.toString('utf-8') + : typeof req.rawBody === 'string' + ? req.rawBody + : null; + + if (!rawBody) { + return null; + } + + return createHmac('sha1', secret) + .update(rawBody + callbackURL) + .digest('base64'); + }, + skipIfNoExpectedSignature: !secret || typeof secret !== 'string', + getActualSignature: () => { + const sig = req.header('x-trello-webhook'); + return typeof sig === 'string' ? sig : null; + }, + }); +} diff --git a/packages/nodes-base/nodes/Trello/test/TrelloTrigger.node.test.ts b/packages/nodes-base/nodes/Trello/test/TrelloTrigger.node.test.ts new file mode 100644 index 00000000000..1aac008c84e --- /dev/null +++ b/packages/nodes-base/nodes/Trello/test/TrelloTrigger.node.test.ts @@ -0,0 +1,76 @@ +import { TrelloTrigger } from '../TrelloTrigger.node'; + +jest.mock('../TrelloTriggerHelpers', () => ({ + verifySignature: jest.fn(), +})); + +import { verifySignature } from '../TrelloTriggerHelpers'; + +const mockedVerifySignature = jest.mocked(verifySignature); + +describe('TrelloTrigger', () => { + let trelloTrigger: TrelloTrigger; + let mockWebhookFunctions: any; + let mockRes: any; + + beforeEach(() => { + jest.clearAllMocks(); + trelloTrigger = new TrelloTrigger(); + + mockRes = { + status: jest.fn().mockReturnThis(), + send: jest.fn().mockReturnThis(), + end: jest.fn().mockReturnThis(), + }; + + mockWebhookFunctions = { + getWebhookName: jest.fn().mockReturnValue('default'), + getBodyData: jest.fn().mockReturnValue({ action: { type: 'createCard' } }), + getResponseObject: jest.fn().mockReturnValue(mockRes), + helpers: { + returnJsonArray: jest.fn().mockImplementation((data) => [{ json: data }]), + }, + }; + }); + + describe('webhook', () => { + it('should respond 200 to setup HEAD request', async () => { + mockWebhookFunctions.getWebhookName.mockReturnValue('setup'); + + const result = await trelloTrigger.webhook.call(mockWebhookFunctions); + + expect(mockRes.status).toHaveBeenCalledWith(200); + expect(mockRes.end).toHaveBeenCalled(); + expect(result).toEqual({ noWebhookResponse: true }); + }); + + it('should return workflow data when signature is valid', async () => { + mockedVerifySignature.mockResolvedValue(true); + + const result = await trelloTrigger.webhook.call(mockWebhookFunctions); + + expect(result).toEqual({ + workflowData: [[{ json: { action: { type: 'createCard' } } }]], + }); + }); + + it('should return 401 when signature is invalid', async () => { + mockedVerifySignature.mockResolvedValue(false); + + const result = await trelloTrigger.webhook.call(mockWebhookFunctions); + + expect(mockRes.status).toHaveBeenCalledWith(401); + expect(mockRes.send).toHaveBeenCalledWith('Unauthorized'); + expect(mockRes.end).toHaveBeenCalled(); + expect(result).toEqual({ noWebhookResponse: true }); + }); + + it('should trigger workflow when no secret is configured', async () => { + mockedVerifySignature.mockResolvedValue(true); + + const result = await trelloTrigger.webhook.call(mockWebhookFunctions); + + expect(result).toHaveProperty('workflowData'); + }); + }); +}); diff --git a/packages/nodes-base/nodes/Trello/test/TrelloTriggerHelpers.test.ts b/packages/nodes-base/nodes/Trello/test/TrelloTriggerHelpers.test.ts new file mode 100644 index 00000000000..ec6a814276d --- /dev/null +++ b/packages/nodes-base/nodes/Trello/test/TrelloTriggerHelpers.test.ts @@ -0,0 +1,114 @@ +import { createHmac } from 'crypto'; + +import { verifySignature } from '../TrelloTriggerHelpers'; + +describe('TrelloTriggerHelpers', () => { + let mockWebhookFunctions: any; + const testSecret = 'test-trello-oauth-secret'; + const testBody = '{"action":{"type":"createCard"},"model":{"id":"abc123"}}'; + const testCallbackUrl = 'https://n8n.example.com/webhook/trello'; + const testSignature = createHmac('sha1', testSecret) + .update(testBody + testCallbackUrl) + .digest('base64'); + + beforeEach(() => { + jest.clearAllMocks(); + + mockWebhookFunctions = { + getCredentials: jest.fn().mockResolvedValue({ + oauthSecret: testSecret, + }), + getRequestObject: jest.fn().mockReturnValue({ + header: jest.fn().mockImplementation((header: string) => { + if (header === 'x-trello-webhook') return testSignature; + return null; + }), + rawBody: testBody, + }), + getNodeWebhookUrl: jest.fn().mockReturnValue(testCallbackUrl), + getNode: jest.fn().mockReturnValue({ name: 'Trello Trigger' }), + }; + }); + + describe('verifySignature', () => { + it('should return true when no OAuth secret is configured', async () => { + mockWebhookFunctions.getCredentials.mockResolvedValue({ + apiKey: 'test-key', + apiToken: 'test-token', + }); + + const result = await verifySignature.call(mockWebhookFunctions); + + expect(result).toBe(true); + }); + + it('should return true when OAuth secret is empty string', async () => { + mockWebhookFunctions.getCredentials.mockResolvedValue({ + oauthSecret: '', + }); + + const result = await verifySignature.call(mockWebhookFunctions); + + expect(result).toBe(true); + }); + + it('should return true when signature is valid', async () => { + const result = await verifySignature.call(mockWebhookFunctions); + + expect(result).toBe(true); + }); + + it('should return false when signature is invalid', async () => { + mockWebhookFunctions.getRequestObject.mockReturnValue({ + header: jest.fn().mockImplementation((header: string) => { + if (header === 'x-trello-webhook') return 'invalidsignature'; + return null; + }), + rawBody: testBody, + }); + + const result = await verifySignature.call(mockWebhookFunctions); + + expect(result).toBe(false); + }); + + it('should return false when signature header is missing', async () => { + mockWebhookFunctions.getRequestObject.mockReturnValue({ + header: jest.fn().mockReturnValue(null), + rawBody: testBody, + }); + + const result = await verifySignature.call(mockWebhookFunctions); + + expect(result).toBe(false); + }); + + it('should handle rawBody as Buffer', async () => { + const bodyBuffer = Buffer.from(testBody); + const bufferSignature = createHmac('sha1', testSecret) + .update(testBody + testCallbackUrl) + .digest('base64'); + + mockWebhookFunctions.getRequestObject.mockReturnValue({ + header: jest.fn().mockImplementation((header: string) => { + if (header === 'x-trello-webhook') return bufferSignature; + return null; + }), + rawBody: bodyBuffer, + }); + + const result = await verifySignature.call(mockWebhookFunctions); + + expect(result).toBe(true); + }); + + it('should include callback URL in signature computation', async () => { + const differentCallbackUrl = 'https://different.example.com/webhook'; + mockWebhookFunctions.getNodeWebhookUrl.mockReturnValue(differentCallbackUrl); + + const result = await verifySignature.call(mockWebhookFunctions); + + expect(result).toBe(false); + }); + }); +});