diff --git a/packages/cli/src/webhooks/waiting-webhooks.ts b/packages/cli/src/webhooks/waiting-webhooks.ts index 894a17cabea..9766d130c06 100644 --- a/packages/cli/src/webhooks/waiting-webhooks.ts +++ b/packages/cli/src/webhooks/waiting-webhooks.ts @@ -1,7 +1,7 @@ import { Logger } from '@n8n/backend-common'; import type { IExecutionResponse } from '@n8n/db'; import { ExecutionRepository } from '@n8n/db'; -import { Service } from '@n8n/di'; +import { Container, Service } from '@n8n/di'; import type express from 'express'; import { FORM_NODE_TYPE, @@ -25,6 +25,13 @@ import type { IWebhookManager, WaitingWebhookRequest, } from './webhook.types'; +import { + InstanceSettings, + WAITING_TOKEN_QUERY_PARAM, + prepareUrlForSigning, + generateUrlSignature, +} from 'n8n-core'; +import crypto from 'crypto'; /** * Service for handling the execution of webhooks of Wait nodes that use the @@ -82,6 +89,30 @@ export class WaitingWebhooks implements IWebhookManager { }); } + private getHmacSecret() { + return Container.get(InstanceSettings).hmacSignatureSecret; + } + + private validateSignatureInRequest(req: express.Request, secret: string) { + try { + const actualToken = req.query[WAITING_TOKEN_QUERY_PARAM]; + + if (typeof actualToken !== 'string') return false; + + const parsedUrl = new URL(req.url, `http://${req.headers.host}`); + parsedUrl.searchParams.delete(WAITING_TOKEN_QUERY_PARAM); + + const urlForSigning = prepareUrlForSigning(parsedUrl); + + const expectedToken = generateUrlSignature(urlForSigning, secret); + + const valid = crypto.timingSafeEqual(Buffer.from(actualToken), Buffer.from(expectedToken)); + return valid; + } catch (error) { + return false; + } + } + async executeWebhook( req: WaitingWebhookRequest, res: express.Response, @@ -97,6 +128,17 @@ export class WaitingWebhooks implements IWebhookManager { const execution = await this.getExecution(executionId); + if (execution && execution.data.validateSignature) { + const lastNodeExecuted = execution.data.resultData.lastNodeExecuted as string; + const lastNode = execution.workflowData.nodes.find((node) => node.name === lastNodeExecuted); + const shouldValidate = lastNode?.parameters.operation === SEND_AND_WAIT_OPERATION; + + if (shouldValidate && !this.validateSignatureInRequest(req, this.getHmacSecret())) { + res.status(401).json({ error: 'Invalid token' }); + return { noWebhookResponse: true }; + } + } + if (!execution) { throw new NotFoundError(`The execution "${executionId}" does not exist.`); } diff --git a/packages/core/nodes-testing/node-test-harness.ts b/packages/core/nodes-testing/node-test-harness.ts index b2a5515ff83..616d7785e05 100644 --- a/packages/core/nodes-testing/node-test-harness.ts +++ b/packages/core/nodes-testing/node-test-harness.ts @@ -210,6 +210,8 @@ export class NodeTestHarness { hooks.addHandler('workflowExecuteAfter', (fullRunData) => waitPromise.resolve(fullRunData)); const additionalData = mock({ + executionId: '1', + webhookWaitingBaseUrl: 'http://localhost/waiting-webhook', hooks, // Get from node.parameters currentNodeParameters: undefined, diff --git a/packages/core/src/constants.ts b/packages/core/src/constants.ts index 63fce74113e..ab32c0068de 100644 --- a/packages/core/src/constants.ts +++ b/packages/core/src/constants.ts @@ -19,3 +19,5 @@ export const CREDENTIAL_ERRORS = { INVALID_JSON: 'Decrypted credentials data is not valid JSON.', INVALID_DATA: 'Credentials data is not in a valid format.', }; + +export const WAITING_TOKEN_QUERY_PARAM = 'signature'; diff --git a/packages/core/src/execution-engine/node-execution-context/__tests__/node-execution-context.test.ts b/packages/core/src/execution-engine/node-execution-context/__tests__/node-execution-context.test.ts index 1837080cc86..6f5724b68c4 100644 --- a/packages/core/src/execution-engine/node-execution-context/__tests__/node-execution-context.test.ts +++ b/packages/core/src/execution-engine/node-execution-context/__tests__/node-execution-context.test.ts @@ -19,7 +19,11 @@ import { NodeExecutionContext } from '../node-execution-context'; class TestContext extends NodeExecutionContext {} describe('NodeExecutionContext', () => { - const instanceSettings = mock({ instanceId: 'abc123' }); + const instanceSettings = mock({ + instanceId: 'abc123', + encryptionKey: 'testEncryptionKey', + hmacSignatureSecret: 'testHmacSignatureSecret', + }); Container.set(InstanceSettings, instanceSettings); const node = mock(); @@ -363,4 +367,41 @@ describe('NodeExecutionContext', () => { expect(result).toEqual([node1]); }); }); + + describe('getSignedResumeUrl', () => { + beforeEach(() => { + jest.clearAllMocks(); + testContext = new TestContext( + workflow, + mock({ + id: 'node456', + }), + mock({ + executionId: '123', + webhookWaitingBaseUrl: 'http://localhost/waiting-webhook', + }), + mode, + { + validateSignature: true, + resultData: { runData: {} }, + }, + ); + nodeTypes.getByNameAndVersion.mockReturnValue(nodeType); + }); + it('should return a signed resume URL with no query parameters', () => { + const result = testContext.getSignedResumeUrl(); + + expect(result).toBe( + 'http://localhost/waiting-webhook/123/node456?signature=8e48dfd1107c1a736f70e7399493ffc50a2e8edd44f389c5f9c058da961682e7', + ); + }); + + it('should return a signed resume URL with query parameters', () => { + const result = testContext.getSignedResumeUrl({ approved: 'true' }); + + expect(result).toBe( + 'http://localhost/waiting-webhook/123/node456?approved=true&signature=11c5efc97a0d6f2ea9045dba6e397596cba29dc24adb44a9ebd3d1272c991e9b', + ); + }); + }); }); diff --git a/packages/core/src/execution-engine/node-execution-context/node-execution-context.ts b/packages/core/src/execution-engine/node-execution-context/node-execution-context.ts index 8356d241014..93cba9f99a5 100644 --- a/packages/core/src/execution-engine/node-execution-context/node-execution-context.ts +++ b/packages/core/src/execution-engine/node-execution-context/node-execution-context.ts @@ -30,12 +30,14 @@ import { ExpressionError, NodeHelpers, NodeOperationError, + UnexpectedError, } from 'n8n-workflow'; import { HTTP_REQUEST_AS_TOOL_NODE_TYPE, HTTP_REQUEST_NODE_TYPE, HTTP_REQUEST_TOOL_NODE_TYPE, + WAITING_TOKEN_QUERY_PARAM, } from '@/constants'; import { InstanceSettings } from '@/instance-settings'; @@ -44,6 +46,7 @@ import { ensureType } from './utils/ensure-type'; import { extractValue } from './utils/extract-value'; import { getAdditionalKeys } from './utils/get-additional-keys'; import { validateValueAgainstSchema } from './utils/validate-value-against-schema'; +import { generateUrlSignature, prepareUrlForSigning } from '../../utils/signature-helpers'; export abstract class NodeExecutionContext implements Omit { protected readonly instanceSettings = Container.get(InstanceSettings); @@ -200,6 +203,32 @@ export abstract class NodeExecutionContext implements Omit = {}) { + const { webhookWaitingBaseUrl, executionId } = this.additionalData; + + if (typeof executionId !== 'string') { + throw new UnexpectedError('Execution id is missing'); + } + + const baseURL = new URL(`${webhookWaitingBaseUrl}/${executionId}/${this.node.id}`); + + for (const [key, value] of Object.entries(parameters)) { + baseURL.searchParams.set(key, value); + } + + const urlForSigning = prepareUrlForSigning(baseURL); + + const token = generateUrlSignature(urlForSigning, this.instanceSettings.hmacSignatureSecret); + + baseURL.searchParams.set(WAITING_TOKEN_QUERY_PARAM, token); + + return baseURL.toString(); + } + getTimezone() { return this.workflow.timezone; } diff --git a/packages/core/src/execution-engine/node-execution-context/webhook-context.ts b/packages/core/src/execution-engine/node-execution-context/webhook-context.ts index fa8224defb6..1149716f133 100644 --- a/packages/core/src/execution-engine/node-execution-context/webhook-context.ts +++ b/packages/core/src/execution-engine/node-execution-context/webhook-context.ts @@ -24,7 +24,6 @@ import { getInputConnectionData } from './utils/get-input-connection-data'; import { getRequestHelperFunctions } from './utils/request-helper-functions'; import { returnJsonArray } from './utils/return-json-array'; import { getNodeWebhookUrl } from './utils/webhook-helper-functions'; - export class WebhookContext extends NodeExecutionContext implements IWebhookFunctions { readonly helpers: IWebhookFunctions['helpers']; diff --git a/packages/core/src/instance-settings/instance-settings.ts b/packages/core/src/instance-settings/instance-settings.ts index df453cba210..44a29c6c676 100644 --- a/packages/core/src/instance-settings/instance-settings.ts +++ b/packages/core/src/instance-settings/instance-settings.ts @@ -52,6 +52,8 @@ export class InstanceSettings { */ readonly instanceId: string; + readonly hmacSignatureSecret: string; + readonly instanceType: InstanceType; constructor( @@ -64,6 +66,7 @@ export class InstanceSettings { this.hostId = `${this.instanceType}-${this.isDocker ? os.hostname() : nanoid()}`; this.settings = this.loadOrCreate(); this.instanceId = this.generateInstanceId(); + this.hmacSignatureSecret = this.getOrGenerateHmacSignatureSecret(); } /** @@ -225,6 +228,14 @@ export class InstanceSettings { .digest('hex'); } + private getOrGenerateHmacSignatureSecret() { + const hmacSignatureSecretFromEnv = process.env.N8N_HMAC_SIGNATURE_SECRET; + if (hmacSignatureSecretFromEnv) return hmacSignatureSecretFromEnv; + + const { encryptionKey } = this; + return createHash('sha256').update(`hmac-signature:${encryptionKey}`).digest('hex'); + } + private save(settings: Settings) { this.settings = settings; writeFileSync(this.settingsFile, JSON.stringify(this.settings, null, '\t'), { diff --git a/packages/core/src/utils/__tests__/signature-helpers.test.ts b/packages/core/src/utils/__tests__/signature-helpers.test.ts new file mode 100644 index 00000000000..cc98c5d45f7 --- /dev/null +++ b/packages/core/src/utils/__tests__/signature-helpers.test.ts @@ -0,0 +1,26 @@ +import { generateUrlSignature } from '../signature-helpers'; + +describe('signature-helpers', () => { + const secret = 'test-secret'; + const baseUrl = 'http://localhost:5678'; + + describe('generateUrlSignature', () => { + it('should generate a signature token', () => { + const url = `${baseUrl}/webhook/abc`; + const token = generateUrlSignature(url, secret); + expect(token).toBe('fe7f1e4c11f875b2d24681e0b28d0bfed6d66381af5b0ab9633da2202a895243'); + }); + + it('should generate a different token for a different url', () => { + const url = `${baseUrl}/webhook/def`; + const token = generateUrlSignature(url, secret); + expect(token).toBe('ab8e72e7a0e47689596a6550283cbef9e2797b7370b0d6d99c89ee7c2394ea8f'); + }); + + it('should generate a different token for a different secret', () => { + const url = `${baseUrl}/webhook/abc`; + const token = generateUrlSignature(url, 'different-secret'); + expect(token).toBe('84a99b6950e12ffcf1fcf8e0fc0986c0c8a46df331932efd79b17e0c11801bd2'); + }); + }); +}); diff --git a/packages/core/src/utils/index.ts b/packages/core/src/utils/index.ts index 0744b656902..ea52a092a18 100644 --- a/packages/core/src/utils/index.ts +++ b/packages/core/src/utils/index.ts @@ -1 +1,2 @@ export * from './serialized-buffer'; +export * from './signature-helpers'; diff --git a/packages/core/src/utils/signature-helpers.ts b/packages/core/src/utils/signature-helpers.ts new file mode 100644 index 00000000000..fa8d8081a75 --- /dev/null +++ b/packages/core/src/utils/signature-helpers.ts @@ -0,0 +1,16 @@ +import crypto from 'crypto'; + +/** + * Generate signature token from url and secret + */ +export function generateUrlSignature(url: string, secret: string) { + const token = crypto.createHmac('sha256', secret).update(url).digest('hex'); + return token; +} + +/** + * Prepare url for signing + */ +export function prepareUrlForSigning(url: URL) { + return `${url.host}${url.pathname}${url.search}`; +} diff --git a/packages/nodes-base/nodes/Discord/test/v2/node/message/sendAndWait.test.ts b/packages/nodes-base/nodes/Discord/test/v2/node/message/sendAndWait.test.ts index 1a2ef8d8577..2e9fbdd4d11 100644 --- a/packages/nodes-base/nodes/Discord/test/v2/node/message/sendAndWait.test.ts +++ b/packages/nodes-base/nodes/Discord/test/v2/node/message/sendAndWait.test.ts @@ -57,6 +57,10 @@ describe('Test DiscordV2, message => sendAndWait', () => { mockExecuteFunctions.evaluateExpression.mockReturnValueOnce('http://localhost/waiting-webhook'); mockExecuteFunctions.evaluateExpression.mockReturnValueOnce('nodeID'); + mockExecuteFunctions.getSignedResumeUrl.mockReturnValue( + 'http://localhost/waiting-webhook/nodeID?approved=true&token=abc', + ); + const result = await discord.execute.call(mockExecuteFunctions); expect(result).toEqual([items]); @@ -74,7 +78,7 @@ describe('Test DiscordV2, message => sendAndWait', () => { label: 'Approve', style: 5, type: 2, - url: 'http://localhost/waiting-webhook/nodeID?approved=true', + url: 'http://localhost/waiting-webhook/nodeID?approved=true&token=abc', }, ], type: 1, diff --git a/packages/nodes-base/nodes/Discord/v2/helpers/utils.ts b/packages/nodes-base/nodes/Discord/v2/helpers/utils.ts index 695c832edd9..96055d2bc65 100644 --- a/packages/nodes-base/nodes/Discord/v2/helpers/utils.ts +++ b/packages/nodes-base/nodes/Discord/v2/helpers/utils.ts @@ -415,7 +415,7 @@ export function createSendAndWaitMessageBody(context: IExecuteFunctions) { type: 2, style: 5, label: option.label, - url: `${config.url}?approved=${option.value}`, + url: option.url, }; }), }, diff --git a/packages/nodes-base/nodes/EmailSend/test/v2/sendAndWait.operation.test.ts b/packages/nodes-base/nodes/EmailSend/test/v2/sendAndWait.operation.test.ts index 46b6bd2bbd1..8f30ff36016 100644 --- a/packages/nodes-base/nodes/EmailSend/test/v2/sendAndWait.operation.test.ts +++ b/packages/nodes-base/nodes/EmailSend/test/v2/sendAndWait.operation.test.ts @@ -44,8 +44,9 @@ describe('Test EmailSendV2, email => sendAndWait', () => { //getSendAndWaitConfig mockExecuteFunctions.getNodeParameter.mockReturnValueOnce('my message'); mockExecuteFunctions.getNodeParameter.mockReturnValueOnce('my subject'); - mockExecuteFunctions.evaluateExpression.mockReturnValueOnce('http://localhost/waiting-webhook'); - mockExecuteFunctions.evaluateExpression.mockReturnValueOnce('nodeID'); + mockExecuteFunctions.getSignedResumeUrl.mockReturnValue( + 'http://localhost/waiting-webhook/nodeID?approved=true&signature=abc', + ); mockExecuteFunctions.getNodeParameter.mockReturnValueOnce({}); // approvalOptions mockExecuteFunctions.getNodeParameter.mockReturnValueOnce({}); // options mockExecuteFunctions.getNodeParameter.mockReturnValueOnce('approval'); @@ -61,7 +62,9 @@ describe('Test EmailSendV2, email => sendAndWait', () => { expect(transporter.sendMail).toHaveBeenCalledWith({ from: 'from@mail.com', - html: expect.stringContaining('href="http://localhost/waiting-webhook/nodeID?approved=true"'), + html: expect.stringContaining( + 'href="http://localhost/waiting-webhook/nodeID?approved=true&signature=abc"', + ), subject: 'my subject', to: 'to@mail.com', }); diff --git a/packages/nodes-base/nodes/EmailSend/v2/sendAndWait.operation.ts b/packages/nodes-base/nodes/EmailSend/v2/sendAndWait.operation.ts index a8a7888a013..b28d40bf38a 100644 --- a/packages/nodes-base/nodes/EmailSend/v2/sendAndWait.operation.ts +++ b/packages/nodes-base/nodes/EmailSend/v2/sendAndWait.operation.ts @@ -30,7 +30,7 @@ export async function execute(this: IExecuteFunctions): Promise `*<${`${config.url}?approved=${option.value}`}|${option.label}>*`, + (option) => `*<${`${option.url}`}|${option.label}>*`, ); let text = `${config.message}\n\n\n${buttons.join(' ')}`; diff --git a/packages/nodes-base/nodes/Google/Chat/test/node/sendAndWait.operation.test.ts b/packages/nodes-base/nodes/Google/Chat/test/node/sendAndWait.operation.test.ts index a45fbb43f3b..fd4d42c0675 100644 --- a/packages/nodes-base/nodes/Google/Chat/test/node/sendAndWait.operation.test.ts +++ b/packages/nodes-base/nodes/Google/Chat/test/node/sendAndWait.operation.test.ts @@ -39,8 +39,9 @@ describe('Test GoogleChat, message => sendAndWait', () => { //getSendAndWaitConfig mockExecuteFunctions.getNodeParameter.mockReturnValueOnce('my message'); mockExecuteFunctions.getNodeParameter.mockReturnValueOnce('my subject'); - mockExecuteFunctions.evaluateExpression.mockReturnValueOnce('http://localhost/waiting-webhook'); - mockExecuteFunctions.evaluateExpression.mockReturnValueOnce('nodeID'); + mockExecuteFunctions.getSignedResumeUrl.mockReturnValue( + 'http://localhost/waiting-webhook/nodeID?approved=true&signature=abc', + ); mockExecuteFunctions.getNodeParameter.mockReturnValueOnce({}); // approvalOptions mockExecuteFunctions.getNodeParameter.mockReturnValueOnce({}); // options mockExecuteFunctions.getNodeParameter.mockReturnValueOnce('approval'); @@ -55,7 +56,7 @@ describe('Test GoogleChat, message => sendAndWait', () => { expect(mockExecuteFunctions.putExecutionToWait).toHaveBeenCalledTimes(1); expect(genericFunctions.googleApiRequest).toHaveBeenCalledWith('POST', '/v1/spaceID/messages', { - text: 'my message\n\n\n**\n\n_This_ _message_ _was_ _sent_ _automatically_ _with_ __', + text: 'my message\n\n\n**\n\n_This_ _message_ _was_ _sent_ _automatically_ _with_ __', }); }); }); diff --git a/packages/nodes-base/nodes/Microsoft/Outlook/test/v2/node/message/sendAndWait.test.ts b/packages/nodes-base/nodes/Microsoft/Outlook/test/v2/node/message/sendAndWait.test.ts index 6d304590b12..06e10558529 100644 --- a/packages/nodes-base/nodes/Microsoft/Outlook/test/v2/node/message/sendAndWait.test.ts +++ b/packages/nodes-base/nodes/Microsoft/Outlook/test/v2/node/message/sendAndWait.test.ts @@ -46,8 +46,9 @@ describe('Test MicrosoftOutlookV2, message => sendAndWait', () => { //getSendAndWaitConfig mockExecuteFunctions.getNodeParameter.mockReturnValueOnce('my message'); mockExecuteFunctions.getNodeParameter.mockReturnValueOnce('my subject'); - mockExecuteFunctions.evaluateExpression.mockReturnValueOnce('http://localhost/waiting-webhook'); - mockExecuteFunctions.evaluateExpression.mockReturnValueOnce('nodeID'); + mockExecuteFunctions.getSignedResumeUrl.mockReturnValue( + 'http://localhost/waiting-webhook/nodeID?approved=true&signature=abc', + ); mockExecuteFunctions.getNodeParameter.mockReturnValueOnce({}); // approvalOptions mockExecuteFunctions.getNodeParameter.mockReturnValueOnce({}); // options mockExecuteFunctions.getNodeParameter.mockReturnValueOnce('approval'); @@ -65,7 +66,7 @@ describe('Test MicrosoftOutlookV2, message => sendAndWait', () => { message: { body: { content: expect.stringContaining( - 'href="http://localhost/waiting-webhook/nodeID?approved=true"', + 'href="http://localhost/waiting-webhook/nodeID?approved=true&signature=abc"', ), contentType: 'html', }, diff --git a/packages/nodes-base/nodes/Microsoft/Outlook/v2/actions/message/sendAndWait.operation.ts b/packages/nodes-base/nodes/Microsoft/Outlook/v2/actions/message/sendAndWait.operation.ts index 9ac3e093301..d77b967923c 100644 --- a/packages/nodes-base/nodes/Microsoft/Outlook/v2/actions/message/sendAndWait.operation.ts +++ b/packages/nodes-base/nodes/Microsoft/Outlook/v2/actions/message/sendAndWait.operation.ts @@ -34,7 +34,7 @@ export async function execute(this: IExecuteFunctions, index: number, items: INo const config = getSendAndWaitConfig(this); const buttons: string[] = []; for (const option of config.options) { - buttons.push(createButton(config.url, option.label, option.value, option.style)); + buttons.push(createButton(option.url, option.label, option.style)); } let bodyContent: string; diff --git a/packages/nodes-base/nodes/Microsoft/Teams/test/v2/node/chatMessage/sendAndWait.test.ts b/packages/nodes-base/nodes/Microsoft/Teams/test/v2/node/chatMessage/sendAndWait.test.ts index 2a803b11c2e..15bc4079059 100644 --- a/packages/nodes-base/nodes/Microsoft/Teams/test/v2/node/chatMessage/sendAndWait.test.ts +++ b/packages/nodes-base/nodes/Microsoft/Teams/test/v2/node/chatMessage/sendAndWait.test.ts @@ -45,8 +45,9 @@ describe('Test MicrosoftTeamsV2, chatMessage => sendAndWait', () => { mockExecuteFunctions.getInstanceId.mockReturnValue('instanceId'); mockExecuteFunctions.getNode.mockReturnValue(mock({ typeVersion: 2 })); - mockExecuteFunctions.evaluateExpression.mockReturnValueOnce('http://localhost/waiting-webhook'); - mockExecuteFunctions.evaluateExpression.mockReturnValueOnce('nodeID'); + mockExecuteFunctions.getSignedResumeUrl.mockReturnValue( + 'http://localhost/waiting-webhook/nodeID?approved=true&signature=abc', + ); const result = await microsoftTeamsV2.execute.call(mockExecuteFunctions); @@ -60,7 +61,7 @@ describe('Test MicrosoftTeamsV2, chatMessage => sendAndWait', () => { { body: { content: - 'my message

Approve

This message was sent automatically with n8n', + 'my message

Approve

This message was sent automatically with n8n', contentType: 'html', }, }, diff --git a/packages/nodes-base/nodes/Microsoft/Teams/v2/actions/chatMessage/sendAndWait.operation.ts b/packages/nodes-base/nodes/Microsoft/Teams/v2/actions/chatMessage/sendAndWait.operation.ts index fe2f82dba07..ee0ad5a9f0f 100644 --- a/packages/nodes-base/nodes/Microsoft/Teams/v2/actions/chatMessage/sendAndWait.operation.ts +++ b/packages/nodes-base/nodes/Microsoft/Teams/v2/actions/chatMessage/sendAndWait.operation.ts @@ -23,9 +23,7 @@ export async function execute(this: IExecuteFunctions, i: number, instanceId: st const chatId = this.getNodeParameter('chatId', i, '', { extractValue: true }) as string; const config = getSendAndWaitConfig(this); - const buttons = config.options.map( - (option) => `${option.label}`, - ); + const buttons = config.options.map((option) => `${option.label}`); let content = `${config.message}

${buttons.join(' ')}`; diff --git a/packages/nodes-base/nodes/Slack/V2/GenericFunctions.ts b/packages/nodes-base/nodes/Slack/V2/GenericFunctions.ts index f9f5d2bbd0c..2050c54df87 100644 --- a/packages/nodes-base/nodes/Slack/V2/GenericFunctions.ts +++ b/packages/nodes-base/nodes/Slack/V2/GenericFunctions.ts @@ -301,7 +301,7 @@ export function createSendAndWaitMessageBody(context: IExecuteFunctions) { text: option.label, emoji: true, }, - url: `${config.url}?approved=${option.value}`, + url: option.url, }; }), }, diff --git a/packages/nodes-base/nodes/Slack/test/v2/utils.test.ts b/packages/nodes-base/nodes/Slack/test/v2/utils.test.ts index e56decf79f9..46dc377a2e1 100644 --- a/packages/nodes-base/nodes/Slack/test/v2/utils.test.ts +++ b/packages/nodes-base/nodes/Slack/test/v2/utils.test.ts @@ -9,6 +9,12 @@ describe('Slack Utility Functions', () => { beforeEach(() => { mockExecuteFunctions = mock(); mockExecuteFunctions.getNode.mockReturnValue({ name: 'Slack', typeVersion: 1 } as any); + mockExecuteFunctions.getSignedResumeUrl.mockReturnValueOnce( + 'http://localhost/waiting-webhook/nodeID?approved=true&signature=abc', + ); + mockExecuteFunctions.getSignedResumeUrl.mockReturnValueOnce( + 'http://localhost/waiting-webhook/nodeID?approved=false&signature=abc', + ); jest.clearAllMocks(); }); @@ -33,8 +39,6 @@ describe('Slack Utility Functions', () => { mockExecuteFunctions.getNodeParameter.mockReturnValueOnce('message'); mockExecuteFunctions.getNodeParameter.mockReturnValueOnce('subject'); - mockExecuteFunctions.evaluateExpression.mockReturnValueOnce('localhost'); - mockExecuteFunctions.evaluateExpression.mockReturnValueOnce('node123'); mockExecuteFunctions.getNodeParameter.mockReturnValueOnce({}); expect(createSendAndWaitMessageBody(mockExecuteFunctions)).toEqual({ @@ -70,7 +74,7 @@ describe('Slack Utility Functions', () => { type: 'plain_text', }, type: 'button', - url: 'localhost/node123?approved=true', + url: 'http://localhost/waiting-webhook/nodeID?approved=true&signature=abc', }, ], type: 'actions', @@ -86,8 +90,6 @@ describe('Slack Utility Functions', () => { mockExecuteFunctions.getNodeParameter.mockReturnValueOnce('message'); mockExecuteFunctions.getNodeParameter.mockReturnValueOnce('subject'); - mockExecuteFunctions.evaluateExpression.mockReturnValueOnce('localhost'); - mockExecuteFunctions.evaluateExpression.mockReturnValueOnce('node123'); mockExecuteFunctions.getNodeParameter.mockReturnValueOnce({ approvalType: 'double' }); expect(createSendAndWaitMessageBody(mockExecuteFunctions)).toEqual({ @@ -123,7 +125,7 @@ describe('Slack Utility Functions', () => { type: 'plain_text', }, type: 'button', - url: 'localhost/node123?approved=false', + url: 'http://localhost/waiting-webhook/nodeID?approved=false&signature=abc', }, { @@ -134,7 +136,7 @@ describe('Slack Utility Functions', () => { type: 'plain_text', }, type: 'button', - url: 'localhost/node123?approved=true', + url: 'http://localhost/waiting-webhook/nodeID?approved=true&signature=abc', }, ], type: 'actions', @@ -150,8 +152,6 @@ describe('Slack Utility Functions', () => { mockExecuteFunctions.getNodeParameter.mockReturnValueOnce('message'); mockExecuteFunctions.getNodeParameter.mockReturnValueOnce('subject'); - mockExecuteFunctions.evaluateExpression.mockReturnValueOnce('localhost'); - mockExecuteFunctions.evaluateExpression.mockReturnValueOnce('node123'); mockExecuteFunctions.getNodeParameter.mockReturnValueOnce({}); mockExecuteFunctions.getNode.mockReturnValue({ name: 'Slack', typeVersion: 2.3 } as any); @@ -187,7 +187,7 @@ describe('Slack Utility Functions', () => { type: 'plain_text', }, type: 'button', - url: 'localhost/node123?approved=true', + url: 'http://localhost/waiting-webhook/nodeID?approved=true&signature=abc', }, ], type: 'actions', @@ -203,8 +203,6 @@ describe('Slack Utility Functions', () => { mockExecuteFunctions.getNodeParameter.mockReturnValueOnce('message'); mockExecuteFunctions.getNodeParameter.mockReturnValueOnce('subject'); - mockExecuteFunctions.evaluateExpression.mockReturnValueOnce('localhost'); - mockExecuteFunctions.evaluateExpression.mockReturnValueOnce('node123'); mockExecuteFunctions.getNodeParameter.mockReturnValueOnce({ approvalType: 'double' }); mockExecuteFunctions.getNode.mockReturnValue({ name: 'Slack', typeVersion: 2.3 } as any); @@ -241,7 +239,7 @@ describe('Slack Utility Functions', () => { type: 'plain_text', }, type: 'button', - url: 'localhost/node123?approved=false', + url: 'http://localhost/waiting-webhook/nodeID?approved=false&signature=abc', }, { @@ -252,7 +250,7 @@ describe('Slack Utility Functions', () => { type: 'plain_text', }, type: 'button', - url: 'localhost/node123?approved=true', + url: 'http://localhost/waiting-webhook/nodeID?approved=true&signature=abc', }, ], type: 'actions', diff --git a/packages/nodes-base/nodes/Telegram/GenericFunctions.ts b/packages/nodes-base/nodes/Telegram/GenericFunctions.ts index d954e961e07..d2da6d2d515 100644 --- a/packages/nodes-base/nodes/Telegram/GenericFunctions.ts +++ b/packages/nodes-base/nodes/Telegram/GenericFunctions.ts @@ -278,7 +278,7 @@ export function createSendAndWaitMessageBody(context: IExecuteFunctions) { config.options.map((option) => { return { text: option.label, - url: `${config.url}?approved=${option.value}`, + url: option.url, }; }), ], diff --git a/packages/nodes-base/nodes/Telegram/tests/node/sendAndWait.operation.test.ts b/packages/nodes-base/nodes/Telegram/tests/node/sendAndWait.operation.test.ts index 552bbb2415b..dbc3e110994 100644 --- a/packages/nodes-base/nodes/Telegram/tests/node/sendAndWait.operation.test.ts +++ b/packages/nodes-base/nodes/Telegram/tests/node/sendAndWait.operation.test.ts @@ -42,8 +42,9 @@ describe('Test Telegram, message => sendAndWait', () => { //getSendAndWaitConfig mockExecuteFunctions.getNodeParameter.mockReturnValueOnce('my message'); mockExecuteFunctions.getNodeParameter.mockReturnValueOnce('my subject'); - mockExecuteFunctions.evaluateExpression.mockReturnValueOnce('http://localhost/waiting-webhook'); - mockExecuteFunctions.evaluateExpression.mockReturnValueOnce('nodeID'); + mockExecuteFunctions.getSignedResumeUrl.mockReturnValue( + 'http://localhost/waiting-webhook/nodeID?approved=true&signature=abc', + ); mockExecuteFunctions.getNodeParameter.mockReturnValueOnce({}); // approvalOptions mockExecuteFunctions.getNodeParameter.mockReturnValueOnce({}); // options mockExecuteFunctions.getNodeParameter.mockReturnValueOnce('approval'); @@ -63,7 +64,12 @@ describe('Test Telegram, message => sendAndWait', () => { parse_mode: 'Markdown', reply_markup: { inline_keyboard: [ - [{ text: 'Approve', url: 'http://localhost/waiting-webhook/nodeID?approved=true' }], + [ + { + text: 'Approve', + url: 'http://localhost/waiting-webhook/nodeID?approved=true&signature=abc', + }, + ], ], }, text: 'my message\n\n_This message was sent automatically with _[n8n](https://n8n.io/?utm_source=n8n-internal&utm_medium=powered_by&utm_campaign=n8n-nodes-base.telegram_instanceId)', diff --git a/packages/nodes-base/nodes/WhatsApp/GenericFunctions.ts b/packages/nodes-base/nodes/WhatsApp/GenericFunctions.ts index fd14a13e74c..b416a01f4de 100644 --- a/packages/nodes-base/nodes/WhatsApp/GenericFunctions.ts +++ b/packages/nodes-base/nodes/WhatsApp/GenericFunctions.ts @@ -113,7 +113,7 @@ export const createMessage = ( instanceId: string, ): IHttpRequestOptions => { const buttons = sendAndWaitConfig.options.map((option) => { - return `*${option.label}:*\n_${sendAndWaitConfig.url}?approved=${option.value}_\n\n`; + return `*${option.label}:*\n_${option.url}_\n\n`; }); let n8nAttribution: string = ''; diff --git a/packages/nodes-base/nodes/WhatsApp/tests/node/sendAndWait.test.ts b/packages/nodes-base/nodes/WhatsApp/tests/node/sendAndWait.test.ts index f3f46ca0210..720e155f892 100644 --- a/packages/nodes-base/nodes/WhatsApp/tests/node/sendAndWait.test.ts +++ b/packages/nodes-base/nodes/WhatsApp/tests/node/sendAndWait.test.ts @@ -39,8 +39,9 @@ describe('Test WhatsApp Business Cloud, sendAndWait operation', () => { mockExecuteFunctions.getInputData.mockReturnValue(items); mockExecuteFunctions.getInstanceId.mockReturnValue('instanceId'); - mockExecuteFunctions.evaluateExpression.mockReturnValueOnce('http://localhost/waiting-webhook'); - mockExecuteFunctions.evaluateExpression.mockReturnValueOnce('nodeID'); + mockExecuteFunctions.getSignedResumeUrl.mockReturnValue( + 'http://localhost/waiting-webhook/nodeID?approved=true&signature=abc', + ); const result = await whatsApp.customOperations.message.sendAndWait.call(mockExecuteFunctions); @@ -55,7 +56,7 @@ describe('Test WhatsApp Business Cloud, sendAndWait operation', () => { body: { messaging_product: 'whatsapp', text: { - body: 'my message\n\n*Approve:*\n_http://localhost/waiting-webhook/nodeID?approved=true_\n\n', + body: 'my message\n\n*Approve:*\n_http://localhost/waiting-webhook/nodeID?approved=true&signature=abc_\n\n', }, to: '22222', type: 'text', diff --git a/packages/nodes-base/nodes/WhatsApp/tests/utils.test.ts b/packages/nodes-base/nodes/WhatsApp/tests/utils.test.ts index 26359d1964f..2e81baa98d8 100644 --- a/packages/nodes-base/nodes/WhatsApp/tests/utils.test.ts +++ b/packages/nodes-base/nodes/WhatsApp/tests/utils.test.ts @@ -28,10 +28,17 @@ describe('createMessage', () => { const mockSendAndWaitConfig: SendAndWaitConfig = { title: '', message: 'Please approve an option:', - url: 'https://example.com/approve', options: [ - { label: 'Yes', value: 'yes', style: 'primary' }, - { label: 'No', value: 'no', style: 'secondary' }, + { + label: 'Yes', + style: 'primary', + url: 'http://localhost/waiting-webhook/nodeID?approved=true&signature=abc', + }, + { + label: 'No', + style: 'secondary', + url: 'http://localhost/waiting-webhook/nodeID?approved=false&signature=abc', + }, ], }; @@ -55,8 +62,8 @@ describe('createMessage', () => { text: { body: 'Please approve an option:\n\n' + - '*Yes:*\n_https://example.com/approve?approved=yes_\n\n' + - '*No:*\n_https://example.com/approve?approved=no_\n\n', + '*Yes:*\n_http://localhost/waiting-webhook/nodeID?approved=true&signature=abc_\n\n' + + '*No:*\n_http://localhost/waiting-webhook/nodeID?approved=false&signature=abc_\n\n', }, type: 'text', to: recipientPhone, @@ -68,12 +75,11 @@ describe('createMessage', () => { const singleOptionConfig: SendAndWaitConfig = { title: '', message: 'Choose an option:', - url: 'https://example.com/approve', options: [ { label: 'Confirm', - value: 'confirm', style: '', + url: 'http://localhost/waiting-webhook/nodeID?approved=true&signature=abc', }, ], }; @@ -92,7 +98,7 @@ describe('createMessage', () => { body: { messaging_product: 'whatsapp', text: { - body: 'Choose an option:\n\n*Confirm:*\n_https://example.com/approve?approved=confirm_\n\n', + body: 'Choose an option:\n\n*Confirm:*\n_http://localhost/waiting-webhook/nodeID?approved=true&signature=abc_\n\n', }, type: 'text', to: recipientPhone, diff --git a/packages/nodes-base/utils/sendAndWait/test/util.test.ts b/packages/nodes-base/utils/sendAndWait/test/util.test.ts index 62eb5c5b729..fd9feab08eb 100644 --- a/packages/nodes-base/utils/sendAndWait/test/util.test.ts +++ b/packages/nodes-base/utils/sendAndWait/test/util.test.ts @@ -62,25 +62,20 @@ describe('Send and Wait utils tests', () => { return params[parameterName]; }); - mockExecuteFunctions.evaluateExpression.mockImplementation((expression: string) => { - const expressions: { [key: string]: string } = { - '{{ $execution?.resumeUrl }}': 'http://localhost', - '{{ $nodeId }}': 'testNodeId', - }; - return expressions[expression]; - }); - + mockExecuteFunctions.getSignedResumeUrl.mockReturnValue( + 'http://localhost/waiting-webhook/nodeID?approved=true&signature=abc', + ); const config = getSendAndWaitConfig(mockExecuteFunctions); expect(config).toEqual({ + appendAttribution: undefined, title: 'Test subject', message: 'Test message', - url: 'http://localhost/testNodeId', options: [ { label: 'Approve', - value: 'true', style: 'primary', + url: 'http://localhost/waiting-webhook/nodeID?approved=true&signature=abc', }, ], }); @@ -102,13 +97,12 @@ describe('Send and Wait utils tests', () => { return params[parameterName]; }); - mockExecuteFunctions.evaluateExpression.mockImplementation((expression: string) => { - const expressions: { [key: string]: string } = { - '{{ $execution?.resumeUrl }}': 'http://localhost', - '{{ $nodeId }}': 'testNodeId', - }; - return expressions[expression]; - }); + mockExecuteFunctions.getSignedResumeUrl.mockReturnValueOnce( + 'http://localhost/waiting-webhook/nodeID?approved=true&signature=abc', + ); + mockExecuteFunctions.getSignedResumeUrl.mockReturnValueOnce( + 'http://localhost/waiting-webhook/nodeID?approved=false&signature=abc', + ); const config = getSendAndWaitConfig(mockExecuteFunctions); @@ -117,13 +111,13 @@ describe('Send and Wait utils tests', () => { expect.arrayContaining([ { label: 'Reject', - value: 'false', style: 'secondary', + url: 'http://localhost/waiting-webhook/nodeID?approved=false&signature=abc', }, { label: 'Approve', - value: 'true', style: 'primary', + url: 'http://localhost/waiting-webhook/nodeID?approved=true&signature=abc', }, ]), ); @@ -146,13 +140,7 @@ describe('Send and Wait utils tests', () => { return params[parameterName]; }); - mockExecuteFunctions.evaluateExpression.mockImplementation((expression: string) => { - const expressions: { [key: string]: string } = { - '{{ $execution?.resumeUrl }}': 'http://localhost', - '{{ $nodeId }}': 'testNodeId', - }; - return expressions[expression]; - }); + mockExecuteFunctions.getSignedResumeUrl.mockReturnValue('http://localhost/testNodeId'); }); it('should create a valid email object', () => { diff --git a/packages/nodes-base/utils/sendAndWait/utils.ts b/packages/nodes-base/utils/sendAndWait/utils.ts index c55fcc6efd0..d207b0ce7ce 100644 --- a/packages/nodes-base/utils/sendAndWait/utils.ts +++ b/packages/nodes-base/utils/sendAndWait/utils.ts @@ -34,8 +34,7 @@ import { escapeHtml } from '../utilities'; export type SendAndWaitConfig = { title: string; message: string; - url: string; - options: Array<{ label: string; value: string; style: string }>; + options: Array<{ label: string; url: string; style: string }>; appendAttribution?: boolean; }; @@ -480,8 +479,6 @@ export function getSendAndWaitConfig(context: IExecuteFunctions): SendAndWaitCon .replace(/\\n/g, '\n') .replace(/
/g, '\n'); const subject = escapeHtml(context.getNodeParameter('subject', 0, '') as string); - const resumeUrl = context.evaluateExpression('{{ $execution?.resumeUrl }}', 0) as string; - const nodeId = context.evaluateExpression('{{ $nodeId }}', 0) as string; const approvalOptions = context.getNodeParameter('approvalOptions.values', 0, {}) as { approvalType?: 'single' | 'double'; approveLabel?: string; @@ -495,18 +492,20 @@ export function getSendAndWaitConfig(context: IExecuteFunctions): SendAndWaitCon const config: SendAndWaitConfig = { title: subject, message, - url: `${resumeUrl}/${nodeId}`, options: [], appendAttribution: options?.appendAttribution as boolean, }; const responseType = context.getNodeParameter('responseType', 0, 'approval') as string; + context.setSignatureValidationRequired(); + const approvedSignedResumeUrl = context.getSignedResumeUrl({ approved: 'true' }); + if (responseType === 'freeText' || responseType === 'customForm') { const label = context.getNodeParameter('options.messageButtonLabel', 0, 'Respond') as string; config.options.push({ label, - value: 'true', + url: approvedSignedResumeUrl, style: 'primary', }); } else if (approvalOptions.approvalType === 'double') { @@ -514,15 +513,16 @@ export function getSendAndWaitConfig(context: IExecuteFunctions): SendAndWaitCon const buttonApprovalStyle = approvalOptions.buttonApprovalStyle || 'primary'; const disapproveLabel = escapeHtml(approvalOptions.disapproveLabel || 'Disapprove'); const buttonDisapprovalStyle = approvalOptions.buttonDisapprovalStyle || 'secondary'; + const disapprovedSignedResumeUrl = context.getSignedResumeUrl({ approved: 'false' }); config.options.push({ label: disapproveLabel, - value: 'false', + url: disapprovedSignedResumeUrl, style: buttonDisapprovalStyle, }); config.options.push({ label: approveLabel, - value: 'true', + url: approvedSignedResumeUrl, style: buttonApprovalStyle, }); } else { @@ -530,7 +530,7 @@ export function getSendAndWaitConfig(context: IExecuteFunctions): SendAndWaitCon const style = approvalOptions.buttonApprovalStyle || 'primary'; config.options.push({ label, - value: 'true', + url: approvedSignedResumeUrl, style, }); } @@ -538,12 +538,12 @@ export function getSendAndWaitConfig(context: IExecuteFunctions): SendAndWaitCon return config; } -export function createButton(url: string, label: string, approved: string, style: string) { +export function createButton(url: string, label: string, style: string) { let buttonStyle = BUTTON_STYLE_PRIMARY; if (style === 'secondary') { buttonStyle = BUTTON_STYLE_SECONDARY; } - return `${label}`; + return `${label}`; } export function createEmail(context: IExecuteFunctions) { @@ -560,7 +560,7 @@ export function createEmail(context: IExecuteFunctions) { const buttons: string[] = []; for (const option of config.options) { - buttons.push(createButton(config.url, option.label, option.value, option.style)); + buttons.push(createButton(option.url, option.label, option.style)); } let emailBody: string; if (config.appendAttribution !== false) { diff --git a/packages/workflow/src/interfaces.ts b/packages/workflow/src/interfaces.ts index 1c78b1a45d0..9347fbc36ff 100644 --- a/packages/workflow/src/interfaces.ts +++ b/packages/workflow/src/interfaces.ts @@ -889,6 +889,10 @@ export interface FunctionsBase { getRestApiUrl(): string; getInstanceBaseUrl(): string; getInstanceId(): string; + /** Get the waiting resume url signed with the signature token */ + getSignedResumeUrl(parameters?: Record): string; + /** Set requirement in the execution for signature token validation */ + setSignatureValidationRequired(): void; getChildNodes( nodeName: string, options?: { includeNodeParameters?: boolean }, @@ -2256,6 +2260,11 @@ export interface IRunExecutionData { waitingExecutionSource: IWaitingForExecutionSource | null; }; parentExecution?: RelatedExecution; + /** + * This is used to prevent breaking change + * for waiting executions started before signature validation was added + */ + validateSignature?: boolean; waitTill?: Date; pushRef?: string;