Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(core): Add Support for custom CORS origins for webhooks #7455

Merged
merged 6 commits into from
Nov 22, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
56 changes: 39 additions & 17 deletions packages/cli/src/ActiveWorkflowRunner.ts
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ import type {
WorkflowExecuteMode,
INodeType,
IWebhookData,
IHttpRequestMethods,
} from 'n8n-workflow';
import {
NodeHelpers,
Expand All @@ -39,6 +40,7 @@ import type {
IWebhookManager,
IWorkflowDb,
IWorkflowExecutionDataProcess,
WebhookAccessControlOptions,
WebhookRequest,
} from '@/Interfaces';
import * as ResponseHelper from '@/ResponseHelper';
Expand Down Expand Up @@ -137,26 +139,14 @@ export class ActiveWorkflowRunner implements IWebhookManager {
response: express.Response,
): Promise<IResponseCallbackData> {
const httpMethod = request.method;
let path = request.params.path;
const path = request.params.path;

this.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);
}

const webhook = await this.webhookService.findWebhook(httpMethod, path);

if (webhook === null) {
throw new ResponseHelper.NotFoundError(
webhookNotFoundErrorMessage(path, httpMethod),
WEBHOOK_PROD_UNREGISTERED_HINT,
);
}
const webhook = await this.findWebhook(path, httpMethod);

if (webhook.isDynamic) {
const pathElements = path.split('/').slice(1);
Expand Down Expand Up @@ -235,13 +225,45 @@ export class ActiveWorkflowRunner implements IWebhookManager {
});
}

/**
* Gets all request methods associated with a single webhook
*/
async getWebhookMethods(path: string) {
return this.webhookService.getWebhookMethods(path);
}

async findAccessControlOptions(path: string, httpMethod: IHttpRequestMethods) {
const webhook = await this.findWebhook(path, httpMethod);

const workflowData = await this.workflowRepository.findOne({
where: { id: webhook.workflowId },
select: ['nodes'],
});

const nodes = workflowData?.nodes;
const webhookNode = nodes?.find(
({ type, parameters, typeVersion }) =>
parameters?.path === path &&
(parameters?.httpMethod ?? 'GET') === httpMethod &&
'webhook' in this.nodeTypes.getByNameAndVersion(type, typeVersion),
);
return webhookNode?.parameters?.options as WebhookAccessControlOptions;
}

private async findWebhook(path: string, httpMethod: IHttpRequestMethods) {
// Remove trailing slash
if (path.endsWith('/')) {
path = path.slice(0, -1);
}

const webhook = await this.webhookService.findWebhook(httpMethod, path);
if (webhook === null) {
throw new ResponseHelper.NotFoundError(
webhookNotFoundErrorMessage(path, httpMethod),
WEBHOOK_PROD_UNREGISTERED_HINT,
);
}

return webhook;
}

/**
* Returns the ids of the currently active workflows from memory.
*/
Expand Down
12 changes: 12 additions & 0 deletions packages/cli/src/Interfaces.ts
Original file line number Diff line number Diff line change
Expand Up @@ -267,8 +267,20 @@ export type WaitingWebhookRequest = WebhookRequest & {
params: WebhookRequest['path'] & { suffix?: string };
};

export interface WebhookAccessControlOptions {
allowedOrigins?: string;
}

export interface IWebhookManager {
/** Gets all request methods associated with a webhook path*/
getWebhookMethods?: (path: string) => Promise<IHttpRequestMethods[]>;

/** Find the CORS options matching a path and method */
findAccessControlOptions?: (
path: string,
httpMethod: IHttpRequestMethods,
) => Promise<WebhookAccessControlOptions | undefined>;

executeWebhook(req: WebhookRequest, res: Response): Promise<IResponseCallbackData>;
}

Expand Down
26 changes: 21 additions & 5 deletions packages/cli/src/TestWebhooks.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,9 +15,11 @@ import type {
IResponseCallbackData,
IWebhookManager,
IWorkflowDb,
WebhookAccessControlOptions,
WebhookRequest,
} from '@/Interfaces';
import { Push } from '@/push';
import { NodeTypes } from '@/NodeTypes';
import * as ResponseHelper from '@/ResponseHelper';
import * as WebhookHelpers from '@/WebhookHelpers';
import { webhookNotFoundErrorMessage } from './utils';
Expand All @@ -38,8 +40,9 @@ export class TestWebhooks implements IWebhookManager {
} = {};

constructor(
private activeWebhooks: ActiveWebhooks,
private push: Push,
private readonly activeWebhooks: ActiveWebhooks,
private readonly push: Push,
private readonly nodeTypes: NodeTypes,
) {
activeWebhooks.testWebhooks = true;
}
Expand Down Expand Up @@ -161,9 +164,6 @@ export class TestWebhooks implements IWebhookManager {
});
}

/**
* Gets all request methods associated with a single test webhook
*/
async getWebhookMethods(path: string): Promise<IHttpRequestMethods[]> {
const webhookMethods = this.activeWebhooks.getWebhookMethods(path);
if (!webhookMethods.length) {
Expand All @@ -177,6 +177,22 @@ export class TestWebhooks implements IWebhookManager {
return webhookMethods;
}

async findAccessControlOptions(path: string, httpMethod: IHttpRequestMethods) {
const webhookKey = Object.keys(this.testWebhookData).find(
(key) => key.includes(path) && key.startsWith(httpMethod),
);
if (!webhookKey) return;

const { workflow } = this.testWebhookData[webhookKey];
const webhookNode = Object.values(workflow.nodes).find(
({ type, parameters, typeVersion }) =>
parameters?.path === path &&
(parameters?.httpMethod ?? 'GET') === httpMethod &&
'webhook' in this.nodeTypes.getByNameAndVersion(type, typeVersion),
);
return webhookNode?.parameters?.options as WebhookAccessControlOptions;
}

/**
* Checks if it has to wait for webhook data to execute the workflow.
* If yes it waits for it and resolves with the result of the workflow if not it simply resolves with undefined
Expand Down
23 changes: 22 additions & 1 deletion packages/cli/src/WebhookHelpers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,28 @@ export const webhookRequestHandler =
return ResponseHelper.sendErrorResponse(res, error as Error);
}
}
res.header('Access-Control-Allow-Origin', req.headers.origin);

const requestedMethod =
method === 'OPTIONS'
? (req.headers['access-control-request-method'] as IHttpRequestMethods)
: method;
if (webhookManager.findAccessControlOptions && requestedMethod) {
const options = await webhookManager.findAccessControlOptions(path, requestedMethod);
const { allowedOrigins } = options ?? {};

res.header(
'Access-Control-Allow-Origin',
!allowedOrigins || allowedOrigins === '*' ? req.headers.origin : allowedOrigins,
);

if (method === 'OPTIONS') {
res.header('Access-Control-Max-Age', '300');
const requestedHeaders = req.headers['access-control-request-headers'];
if (requestedHeaders?.length) {
res.header('Access-Control-Allow-Headers', requestedHeaders);
}
}
}
}

if (method === 'OPTIONS') {
Expand Down
140 changes: 140 additions & 0 deletions packages/cli/test/unit/WebhookHelpers.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,140 @@
import { type Response } from 'express';
import { mock } from 'jest-mock-extended';
import type { IHttpRequestMethods } from 'n8n-workflow';
import type { IWebhookManager, WebhookCORSRequest, WebhookRequest } from '@/Interfaces';
import { webhookRequestHandler } from '@/WebhookHelpers';

describe('WebhookHelpers', () => {
describe('webhookRequestHandler', () => {
const webhookManager = mock<Required<IWebhookManager>>();
const handler = webhookRequestHandler(webhookManager);

beforeEach(() => {
jest.resetAllMocks();
});

it('should throw for unsupported methods', async () => {
const req = mock<WebhookRequest | WebhookCORSRequest>({
method: 'CONNECT' as IHttpRequestMethods,
});
const res = mock<Response>();
res.status.mockReturnValue(res);

await handler(req, res);

expect(res.status).toHaveBeenCalledWith(500);
expect(res.json).toHaveBeenCalledWith({
code: 0,
message: 'The method CONNECT is not supported.',
});
});

describe('preflight requests', () => {
it('should handle missing header for requested method', async () => {
const req = mock<WebhookRequest | WebhookCORSRequest>({
method: 'OPTIONS',
headers: {
origin: 'https://example.com',
'access-control-request-method': undefined,
},
params: { path: 'test' },
});
const res = mock<Response>();
res.status.mockReturnValue(res);

webhookManager.getWebhookMethods.mockResolvedValue(['GET', 'PATCH']);

await handler(req, res);

expect(res.status).toHaveBeenCalledWith(204);
expect(res.header).toHaveBeenCalledWith(
'Access-Control-Allow-Methods',
'OPTIONS, GET, PATCH',
);
});

it('should handle default origin and max-age', async () => {
const req = mock<WebhookRequest | WebhookCORSRequest>({
method: 'OPTIONS',
headers: {
origin: 'https://example.com',
'access-control-request-method': 'GET',
},
params: { path: 'test' },
});
const res = mock<Response>();
res.status.mockReturnValue(res);

webhookManager.getWebhookMethods.mockResolvedValue(['GET', 'PATCH']);

await handler(req, res);

expect(res.status).toHaveBeenCalledWith(204);
expect(res.header).toHaveBeenCalledWith(
'Access-Control-Allow-Methods',
'OPTIONS, GET, PATCH',
);
expect(res.header).toHaveBeenCalledWith(
'Access-Control-Allow-Origin',
'https://example.com',
);
expect(res.header).toHaveBeenCalledWith('Access-Control-Max-Age', '300');
});

it('should handle wildcard origin', async () => {
const randomOrigin = (Math.random() * 10e6).toString(16);
const req = mock<WebhookRequest | WebhookCORSRequest>({
method: 'OPTIONS',
headers: {
origin: randomOrigin,
'access-control-request-method': 'GET',
},
params: { path: 'test' },
});
const res = mock<Response>();
res.status.mockReturnValue(res);

webhookManager.getWebhookMethods.mockResolvedValue(['GET', 'PATCH']);
webhookManager.findAccessControlOptions.mockResolvedValue({
allowedOrigins: '*',
});

await handler(req, res);

expect(res.status).toHaveBeenCalledWith(204);
expect(res.header).toHaveBeenCalledWith(
'Access-Control-Allow-Methods',
'OPTIONS, GET, PATCH',
);
expect(res.header).toHaveBeenCalledWith('Access-Control-Allow-Origin', randomOrigin);
});

it('should handle custom origin', async () => {
const req = mock<WebhookRequest | WebhookCORSRequest>({
method: 'OPTIONS',
headers: {
origin: 'https://example.com',
'access-control-request-method': 'GET',
},
params: { path: 'test' },
});
const res = mock<Response>();
res.status.mockReturnValue(res);

webhookManager.getWebhookMethods.mockResolvedValue(['GET', 'PATCH']);
webhookManager.findAccessControlOptions.mockResolvedValue({
allowedOrigins: 'https://test.com',
});

await handler(req, res);

expect(res.status).toHaveBeenCalledWith(204);
expect(res.header).toHaveBeenCalledWith(
'Access-Control-Allow-Methods',
'OPTIONS, GET, PATCH',
);
expect(res.header).toHaveBeenCalledWith('Access-Control-Allow-Origin', 'https://test.com');
});
});
});
});
10 changes: 8 additions & 2 deletions packages/cli/test/unit/webhooks.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,10 @@ describe('WebhookServer', () => {
const pathPrefix = config.getEnv(`endpoints.${key}`);
manager.getWebhookMethods.mockResolvedValueOnce(['GET']);

const response = await agent.options(`/${pathPrefix}/abcd`).set('origin', corsOrigin);
const response = await agent
.options(`/${pathPrefix}/abcd`)
.set('origin', corsOrigin)
.set('access-control-request-method', 'GET');
expect(response.statusCode).toEqual(204);
expect(response.body).toEqual({});
expect(response.headers['access-control-allow-origin']).toEqual(corsOrigin);
Expand All @@ -60,7 +63,10 @@ describe('WebhookServer', () => {
mockResponse({ test: true }, { key: 'value ' }),
);

const response = await agent.get(`/${pathPrefix}/abcd`).set('origin', corsOrigin);
const response = await agent
.get(`/${pathPrefix}/abcd`)
.set('origin', corsOrigin)
.set('access-control-request-method', 'GET');
expect(response.statusCode).toEqual(200);
expect(response.body).toEqual({ test: true });
expect(response.headers['access-control-allow-origin']).toEqual(corsOrigin);
Expand Down
1 change: 1 addition & 0 deletions packages/nodes-base/nodes/Webhook/Webhook.node.ts
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@ export class Webhook extends Node {
defaults: {
name: 'Webhook',
},
supportsCORS: true,
triggerPanel: {
header: '',
executionsHelp: {
Expand Down
3 changes: 2 additions & 1 deletion packages/workflow/src/Interfaces.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1594,7 +1594,8 @@ export interface INodeTypeDescription extends INodeTypeBaseDescription {
properties: INodeProperties[];
credentials?: INodeCredentialDescription[];
maxNodes?: number; // How many nodes of that type can be created in a workflow
polling?: boolean;
polling?: true | undefined;
supportsCORS?: true | undefined;
requestDefaults?: DeclarativeRestApiSettings.HttpRequestOptions;
requestOperations?: IN8nRequestOperations;
hooks?: {
Expand Down
Loading
Loading