diff --git a/cypress/e2e/45-ai-assistant.cy.ts b/cypress/e2e/45-ai-assistant.cy.ts index 98b3b450cf..21caea250e 100644 --- a/cypress/e2e/45-ai-assistant.cy.ts +++ b/cypress/e2e/45-ai-assistant.cy.ts @@ -443,7 +443,7 @@ describe('AI Assistant Credential Help', () => { aiAssistant.getters.credentialEditAssistantButton().should('not.exist'); credentialsModal.getters.credentialAuthTypeRadioButtons().eq(1).click(); - credentialsModal.getters.credentialInputs().should('have.length', 1); + credentialsModal.getters.credentialInputs().should('have.length', 3); aiAssistant.getters.credentialEditAssistantButton().should('exist'); }); diff --git a/packages/nodes-base/credentials/SlackApi.credentials.ts b/packages/nodes-base/credentials/SlackApi.credentials.ts index 7643f6c8d9..b8eb9bb85b 100644 --- a/packages/nodes-base/credentials/SlackApi.credentials.ts +++ b/packages/nodes-base/credentials/SlackApi.credentials.ts @@ -21,6 +21,27 @@ export class SlackApi implements ICredentialType { default: '', required: true, }, + { + displayName: 'Signature Secret', + name: 'signatureSecret', + type: 'string', + typeOptions: { password: true }, + default: '', + description: + 'The signature secret is used to verify the authenticity of requests sent by Slack.', + }, + { + displayName: + 'We strongly recommend setting up a signing secret to ensure the authenticity of requests.', + name: 'notice', + type: 'notice', + default: '', + displayOptions: { + show: { + signatureSecret: [''], + }, + }, + }, ]; authenticate: IAuthenticateGeneric = { diff --git a/packages/nodes-base/nodes/Slack/SlackTrigger.node.ts b/packages/nodes-base/nodes/Slack/SlackTrigger.node.ts index 0ee1d03178..448d8a1c55 100644 --- a/packages/nodes-base/nodes/Slack/SlackTrigger.node.ts +++ b/packages/nodes-base/nodes/Slack/SlackTrigger.node.ts @@ -13,7 +13,7 @@ import { NodeConnectionTypes, } from 'n8n-workflow'; -import { downloadFile, getChannelInfo, getUserInfo } from './SlackTriggerHelpers'; +import { downloadFile, getChannelInfo, getUserInfo, verifySignature } from './SlackTriggerHelpers'; import { slackApiRequestAllItems } from './V2/GenericFunctions'; export class SlackTrigger implements INodeType { @@ -53,7 +53,7 @@ export class SlackTrigger implements INodeType { }, { displayName: - 'Set up a webhook in your Slack app to enable this node. More info', + 'Set up a webhook in your Slack app to enable this node. More info. We also recommend setting up a signing secret to ensure the authenticity of requests.', name: 'notice', type: 'notice', default: '', @@ -321,8 +321,17 @@ export class SlackTrigger implements INodeType { const binaryData: IBinaryKeyData = {}; const watchWorkspace = this.getNodeParameter('watchWorkspace', false) as boolean; let eventChannel: string = ''; - // Check if the request is a challenge request + + if (!(await verifySignature.call(this))) { + const res = this.getResponseObject(); + res.status(401).send('Unauthorized').end(); + return { + noWebhookResponse: true, + }; + } + if (req.body.type === 'url_verification') { + // Check if the request is a challenge request const res = this.getResponseObject(); res.status(200).json({ challenge: req.body.challenge }).end(); diff --git a/packages/nodes-base/nodes/Slack/SlackTriggerHelpers.ts b/packages/nodes-base/nodes/Slack/SlackTriggerHelpers.ts index f5281dd29d..7957cad6cd 100644 --- a/packages/nodes-base/nodes/Slack/SlackTriggerHelpers.ts +++ b/packages/nodes-base/nodes/Slack/SlackTriggerHelpers.ts @@ -1,6 +1,8 @@ import type { IHttpRequestOptions, IWebhookFunctions } from 'n8n-workflow'; import { NodeOperationError } from 'n8n-workflow'; +import { createHmac, timingSafeEqual } from 'crypto'; + import { slackApiRequest } from './V2/GenericFunctions'; export async function getUserInfo(this: IWebhookFunctions, userId: string): Promise { @@ -78,3 +80,57 @@ export async function downloadFile(this: IWebhookFunctions, url: string): Promis } return response; } + +export async function verifySignature(this: IWebhookFunctions): Promise { + const credential = await this.getCredentials('slackApi'); + if (!credential?.signatureSecret) { + return true; // No signature secret provided, skip verification + } + + const req = this.getRequestObject(); + + const signature = req.header('x-slack-signature'); + const timestamp = req.header('x-slack-request-timestamp'); + if (!signature || !timestamp) { + return false; + } + + const currentTime = Math.floor(Date.now() / 1000); + const timestampNum = parseInt(timestamp, 10); + if (isNaN(timestampNum) || Math.abs(currentTime - timestampNum) > 60 * 5) { + return false; + } + + try { + if (typeof credential.signatureSecret !== 'string') { + return false; + } + + if (!req.rawBody) { + return false; + } + + const hmac = createHmac('sha256', credential.signatureSecret); + + if (Buffer.isBuffer(req.rawBody)) { + hmac.update(`v0:${timestamp}:`); + hmac.update(req.rawBody); + } else { + const rawBodyString = + typeof req.rawBody === 'string' ? req.rawBody : JSON.stringify(req.rawBody); + hmac.update(`v0:${timestamp}:${rawBodyString}`); + } + + const computedSignature = `v0=${hmac.digest('hex')}`; + + const computedBuffer = Buffer.from(computedSignature); + const providedBuffer = Buffer.from(signature); + + return ( + computedBuffer.length === providedBuffer.length && + timingSafeEqual(computedBuffer, providedBuffer) + ); + } catch (error) { + return false; + } +} diff --git a/packages/nodes-base/nodes/Slack/test/SlackTriggerHelper.test.ts b/packages/nodes-base/nodes/Slack/test/SlackTriggerHelper.test.ts new file mode 100644 index 0000000000..db4dcf1494 --- /dev/null +++ b/packages/nodes-base/nodes/Slack/test/SlackTriggerHelper.test.ts @@ -0,0 +1,162 @@ +import { createHmac, timingSafeEqual } from 'crypto'; +import { verifySignature } from '../SlackTriggerHelpers'; + +// Mock crypto module +jest.mock('crypto', () => ({ + createHmac: jest.fn().mockReturnValue({ + update: jest.fn().mockReturnThis(), + digest: jest + .fn() + .mockReturnValue('a2114d57b48eac39b9ad189dd8316235a7b4a8d21a10bd27519666489c69b503'), + }), + timingSafeEqual: jest.fn(), +})); + +describe('SlackTriggerHelpers', () => { + let mockWebhookFunctions: any; + const testSignatureSecret = 'xyzz0WbapA4vBCDEFasx0q6G'; + const testTimestamp = '1531420618'; + const testBody = + 'token=xyzz0WbapA4vBCDEFasx0q6G&team_id=T1DC2JH3J&team_domain=testteamnow&channel_id=G8PSS9T3V&channel_name=foobar&user_id=U2CERLKJA&user_name=roadrunner&command=%2Fwebhook-collect&text=&response_url=https%3A%2F%2Fhooks.slack.com%2Fcommands%2FT1DC2JH3J%2F397700885554%2F96rGlfmibIGlgcZRskXaIFfN&trigger_id=398738663015.47445629121.803a0bc887a14d10d2c447fce8b6703c'; + const testSignature = 'v0=a2114d57b48eac39b9ad189dd8316235a7b4a8d21a10bd27519666489c69b503'; + + beforeEach(() => { + jest.clearAllMocks(); + + // Mock Date.now() to return a fixed timestamp + const fixedDate = new Date(parseInt(testTimestamp, 10) * 1000); + jest.spyOn(Date, 'now').mockImplementation(() => fixedDate.getTime()); + + mockWebhookFunctions = { + getCredentials: jest.fn(), + getRequestObject: jest.fn(), + getNode: jest.fn().mockReturnValue({ name: 'Slack Trigger' }), + }; + + // Default mock return values + mockWebhookFunctions.getRequestObject.mockReturnValue({ + header: jest.fn().mockImplementation((header) => { + if (header === 'x-slack-signature') return testSignature; + if (header === 'x-slack-request-timestamp') return testTimestamp; + return null; + }), + rawBody: testBody, + }); + }); + + describe('verifySignature', () => { + it('should return true when no credentials are provided', async () => { + mockWebhookFunctions.getCredentials.mockResolvedValue(null); + + const result = await verifySignature.call(mockWebhookFunctions); + + expect(result).toBe(true); + expect(mockWebhookFunctions.getCredentials).toHaveBeenCalledWith('slackApi'); + }); + + it('should return true when no signature secret is provided', async () => { + mockWebhookFunctions.getCredentials.mockResolvedValue({ + apiToken: 'test-token', + }); + + const result = await verifySignature.call(mockWebhookFunctions); + + expect(result).toBe(true); + expect(mockWebhookFunctions.getCredentials).toHaveBeenCalledWith('slackApi'); + }); + + it('should return false when signature header is missing', async () => { + mockWebhookFunctions.getCredentials.mockResolvedValue({ + signatureSecret: testSignatureSecret, + }); + + mockWebhookFunctions.getRequestObject.mockReturnValue({ + header: jest.fn().mockImplementation((header) => { + if (header === 'x-slack-request-timestamp') return testTimestamp; + return null; + }), + rawBody: testBody, + }); + + const result = await verifySignature.call(mockWebhookFunctions); + + expect(result).toBe(false); + }); + + it('should return false when timestamp header is missing', async () => { + mockWebhookFunctions.getCredentials.mockResolvedValue({ + signatureSecret: testSignatureSecret, + }); + + mockWebhookFunctions.getRequestObject.mockReturnValue({ + header: jest.fn().mockImplementation((header) => { + if (header === 'x-slack-signature') return testSignature; + return null; + }), + rawBody: testBody, + }); + + const result = await verifySignature.call(mockWebhookFunctions); + + expect(result).toBe(false); + }); + + it('should return false when timestamp is too old', async () => { + mockWebhookFunctions.getCredentials.mockResolvedValue({ + signatureSecret: testSignatureSecret, + }); + + // Mock Date.now() to return a timestamp that's more than 5 minutes after the request timestamp + const futureDate = new Date((parseInt(testTimestamp, 10) + 301) * 1000); + jest.spyOn(Date, 'now').mockImplementation(() => futureDate.getTime()); + + const result = await verifySignature.call(mockWebhookFunctions); + + expect(result).toBe(false); + }); + + it('should return true when signature is valid', async () => { + mockWebhookFunctions.getCredentials.mockResolvedValue({ + signatureSecret: testSignatureSecret, + }); + + // Mock the timingSafeEqual to return true for valid signature + (timingSafeEqual as jest.Mock).mockReturnValue(true); + + const result = await verifySignature.call(mockWebhookFunctions); + + expect(result).toBe(true); + expect(createHmac).toHaveBeenCalledWith('sha256', testSignatureSecret); + expect(timingSafeEqual).toHaveBeenCalled(); + }); + + it('should return false when signature is invalid', async () => { + mockWebhookFunctions.getCredentials.mockResolvedValue({ + signatureSecret: testSignatureSecret, + }); + + // Mock the timingSafeEqual to return false for invalid signature + (timingSafeEqual as jest.Mock).mockReturnValue(false); + + const result = await verifySignature.call(mockWebhookFunctions); + + expect(result).toBe(false); + expect(createHmac).toHaveBeenCalledWith('sha256', testSignatureSecret); + expect(timingSafeEqual).toHaveBeenCalled(); + }); + + it('should correctly build the signature string with the provided timestamp', async () => { + mockWebhookFunctions.getCredentials.mockResolvedValue({ + signatureSecret: testSignatureSecret, + }); + + await verifySignature.call(mockWebhookFunctions); + + expect(createHmac).toHaveBeenCalledWith('sha256', testSignatureSecret); + const mockHmac = createHmac('sha256', testSignatureSecret); + + // Verify that update was called with the expected string (using the timestamp from the request) + expect(mockHmac.update).toHaveBeenCalledWith(`v0:${testTimestamp}:${testBody}`); + }); + }); +});