Skip to content

Commit

Permalink
fix(Webhook Node): Fix URL params for webhooks (#6986)
Browse files Browse the repository at this point in the history
  • Loading branch information
netroy committed Aug 25, 2023
1 parent 87cf1d9 commit 596b569
Show file tree
Hide file tree
Showing 5 changed files with 93 additions and 26 deletions.
3 changes: 3 additions & 0 deletions packages/cli/src/ActiveWorkflowRunner.ts
Original file line number Diff line number Diff line change
Expand Up @@ -194,6 +194,9 @@ export class ActiveWorkflowRunner implements IWebhookManager {

Logger.debug(`Received webhook "${httpMethod}" for path "${path}"`);

// Reset request parameters
request.params = {} as WebhookRequest['params'];

// Remove trailing slash
if (path.endsWith('/')) {
path = path.slice(0, -1);
Expand Down
3 changes: 3 additions & 0 deletions packages/cli/src/TestWebhooks.ts
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,9 @@ export class TestWebhooks implements IWebhookManager {
const httpMethod = request.method;
let path = request.params.path;

// Reset request parameters
request.params = {} as WebhookRequest['params'];

// Remove trailing slash
if (path.endsWith('/')) {
path = path.slice(0, -1);
Expand Down
3 changes: 3 additions & 0 deletions packages/cli/src/WaitingWebhooks.ts
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,9 @@ export class WaitingWebhooks implements IWebhookManager {
const { path: executionId, suffix } = req.params;
Logger.debug(`Received waiting-webhook "${req.method}" for execution "${executionId}"`);

// Reset request parameters
req.params = {} as WaitingWebhookRequest['params'];

const execution = await this.executionRepository.findSingleExecution(executionId, {
includeData: true,
unflattenData: true,
Expand Down
104 changes: 81 additions & 23 deletions packages/cli/test/integration/webhooks.api.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import { InternalHooks } from '@/InternalHooks';
import { getLogger } from '@/Logger';
import { NodeTypes } from '@/NodeTypes';
import { Push } from '@/push';
import type { WorkflowEntity } from '@db/entities/WorkflowEntity';

import { mockInstance, initActiveWorkflowRunner } from './shared/utils';
import * as testDb from './shared/testDb';
Expand All @@ -22,47 +23,43 @@ describe('Webhook API', () => {

let agent: SuperAgentTest;

beforeAll(async () => {
await testDb.init();
});

afterAll(async () => {
await testDb.terminate();
});

describe('Content-Type support', () => {
beforeAll(async () => {
await testDb.init();

const node = new WebhookTestingNode();
const user = await testDb.createUser();
await testDb.createWorkflow(
{
active: true,
nodes: [
{
name: 'Webhook',
type: node.description.name,
typeVersion: 1,
parameters: {
httpMethod: 'POST',
path: 'abcd',
},
id: '74786112-fb73-4d80-bd9a-43982939b801',
webhookId: '5ccef736-be16-4d10-b7fb-feed7a61ff22',
position: [740, 420],
},
],
},
user,
);
await testDb.createWorkflow(createWebhookWorkflow(node), user);

const nodeTypes = mockInstance(NodeTypes);
nodeTypes.getByName.mockReturnValue(node);
nodeTypes.getByNameAndVersion.mockReturnValue(node);

await initActiveWorkflowRunner();

const server = new (class extends AbstractServer {})();
await server.start();
agent = testAgent(server.app);
});

afterAll(async () => {
await testDb.truncate(['Workflow']);
});

test('should handle JSON', async () => {
const response = await agent.post('/webhook/abcd').send({ test: true });
expect(response.statusCode).toEqual(200);
expect(response.body).toEqual({ type: 'application/json', body: { test: true } });
expect(response.body).toEqual({
type: 'application/json',
body: { test: true },
params: {},
});
});

test('should handle XML', async () => {
Expand All @@ -83,6 +80,7 @@ describe('Webhook API', () => {
inner: 'value',
},
},
params: {},
});
});

Expand All @@ -95,6 +93,7 @@ describe('Webhook API', () => {
expect(response.body).toEqual({
type: 'application/x-www-form-urlencoded',
body: { x: '5', y: 'str', z: 'false' },
params: {},
});
});

Expand All @@ -107,6 +106,7 @@ describe('Webhook API', () => {
expect(response.body).toEqual({
type: 'text/plain',
body: '{"key": "value"}',
params: {},
});
});

Expand All @@ -133,6 +133,44 @@ describe('Webhook API', () => {
});
});

describe('Params support', () => {
beforeAll(async () => {
const node = new WebhookTestingNode();
const user = await testDb.createUser();
await testDb.createWorkflow(createWebhookWorkflow(node, ':variable', 'PATCH'), user);

const nodeTypes = mockInstance(NodeTypes);
nodeTypes.getByName.mockReturnValue(node);
nodeTypes.getByNameAndVersion.mockReturnValue(node);

await initActiveWorkflowRunner();

const server = new (class extends AbstractServer {})();
await server.start();
agent = testAgent(server.app);
});

afterAll(async () => {
await testDb.truncate(['Workflow']);
});

test('should handle params', async () => {
const response = await agent
.patch('/webhook/5ccef736-be16-4d10-b7fb-feed7a61ff22/test')
.send({ test: true });
expect(response.statusCode).toEqual(200);
expect(response.body).toEqual({
type: 'application/json',
body: { test: true },
params: {
variable: 'test',
},
});

await agent.post('/webhook/abcd').send({ test: true }).expect(404);
});
});

class WebhookTestingNode implements INodeType {
description: INodeTypeDescription = {
displayName: 'Webhook Testing Node',
Expand Down Expand Up @@ -173,8 +211,28 @@ describe('Webhook API', () => {
webhookResponse: {
type: req.contentType,
body: req.body,
params: req.params,
},
};
}
}

const createWebhookWorkflow = (
node: WebhookTestingNode,
path = 'abcd',
httpMethod = 'POST',
): Partial<WorkflowEntity> => ({
active: true,
nodes: [
{
name: 'Webhook',
type: node.description.name,
typeVersion: 1,
parameters: { httpMethod, path },
id: '74786112-fb73-4d80-bd9a-43982939b801',
webhookId: '5ccef736-be16-4d10-b7fb-feed7a61ff22',
position: [740, 420],
},
],
});
});
6 changes: 3 additions & 3 deletions packages/nodes-base/nodes/Webhook/Webhook.node.ts
Original file line number Diff line number Diff line change
Expand Up @@ -127,7 +127,7 @@ export class Webhook extends Node {
const response: INodeExecutionData = {
json: {
headers: req.headers,
params: {},
params: req.params,
query: req.query,
body: req.body,
},
Expand Down Expand Up @@ -207,7 +207,7 @@ export class Webhook extends Node {
binary: {},
json: {
headers: req.headers,
params: {},
params: req.params,
query: req.query,
body: data,
},
Expand Down Expand Up @@ -263,7 +263,7 @@ export class Webhook extends Node {
binary: {},
json: {
headers: req.headers,
params: {},
params: req.params,
query: req.query,
body: {},
},
Expand Down

0 comments on commit 596b569

Please sign in to comment.