diff --git a/packages/cli/src/AbstractServer.ts b/packages/cli/src/AbstractServer.ts index 38cc327831..676db8db0a 100644 --- a/packages/cli/src/AbstractServer.ts +++ b/packages/cli/src/AbstractServer.ts @@ -16,7 +16,7 @@ import { rawBodyReader, bodyParser, corsMiddleware } from '@/middlewares'; import { WaitingForms } from '@/WaitingForms'; import { TestWebhooks } from '@/webhooks/TestWebhooks'; import { WaitingWebhooks } from '@/webhooks/WaitingWebhooks'; -import { webhookRequestHandler } from '@/webhooks/WebhookHelpers'; +import { createWebhookHandlerFor } from '@/webhooks/WebhookRequestHandler'; import { ActiveWebhooks } from '@/webhooks/ActiveWebhooks'; import { generateHostInstanceId } from './databases/utils/generators'; import { Logger } from '@/Logger'; @@ -181,33 +181,32 @@ export abstract class AbstractServer { // Setup webhook handlers before bodyParser, to let the Webhook node handle binary data in requests if (this.webhooksEnabled) { - const activeWebhooks = Container.get(ActiveWebhooks); - + const activeWebhooksRequestHandler = createWebhookHandlerFor(Container.get(ActiveWebhooks)); // Register a handler for active forms - this.app.all(`/${this.endpointForm}/:path(*)`, webhookRequestHandler(activeWebhooks)); + this.app.all(`/${this.endpointForm}/:path(*)`, activeWebhooksRequestHandler); // Register a handler for active webhooks - this.app.all(`/${this.endpointWebhook}/:path(*)`, webhookRequestHandler(activeWebhooks)); + this.app.all(`/${this.endpointWebhook}/:path(*)`, activeWebhooksRequestHandler); // Register a handler for waiting forms this.app.all( `/${this.endpointFormWaiting}/:path/:suffix?`, - webhookRequestHandler(Container.get(WaitingForms)), + createWebhookHandlerFor(Container.get(WaitingForms)), ); // Register a handler for waiting webhooks this.app.all( `/${this.endpointWebhookWaiting}/:path/:suffix?`, - webhookRequestHandler(Container.get(WaitingWebhooks)), + createWebhookHandlerFor(Container.get(WaitingWebhooks)), ); } if (this.testWebhooksEnabled) { - const testWebhooks = Container.get(TestWebhooks); + const testWebhooksRequestHandler = createWebhookHandlerFor(Container.get(TestWebhooks)); // Register a handler - this.app.all(`/${this.endpointFormTest}/:path(*)`, webhookRequestHandler(testWebhooks)); - this.app.all(`/${this.endpointWebhookTest}/:path(*)`, webhookRequestHandler(testWebhooks)); + this.app.all(`/${this.endpointFormTest}/:path(*)`, testWebhooksRequestHandler); + this.app.all(`/${this.endpointWebhookTest}/:path(*)`, testWebhooksRequestHandler); } // Block bots from scanning the application diff --git a/packages/cli/src/webhooks/WebhookHelpers.ts b/packages/cli/src/webhooks/WebhookHelpers.ts index f090a7ea7e..1b8bc2b7e9 100644 --- a/packages/cli/src/webhooks/WebhookHelpers.ts +++ b/packages/cli/src/webhooks/WebhookHelpers.ts @@ -20,7 +20,6 @@ import type { IDataObject, IDeferredPromise, IExecuteData, - IHttpRequestMethods, IN8nHttpFullResponse, INode, IPinData, @@ -41,13 +40,7 @@ import { NodeHelpers, } from 'n8n-workflow'; -import type { - IWebhookResponseCallbackData, - IWebhookManager, - WebhookCORSRequest, - WebhookRequest, -} from './webhook.types'; -import * as ResponseHelper from '@/ResponseHelper'; +import type { IWebhookResponseCallbackData, WebhookRequest } from './webhook.types'; import * as WorkflowHelpers from '@/WorkflowHelpers'; import { WorkflowRunner } from '@/WorkflowRunner'; import * as WorkflowExecuteAdditionalData from '@/WorkflowExecuteAdditionalData'; @@ -62,97 +55,6 @@ import { UnprocessableRequestError } from '@/errors/response-errors/unprocessabl import type { Project } from '@/databases/entities/Project'; import type { IExecutionDb, IWorkflowDb, IWorkflowExecutionDataProcess } from '@/Interfaces'; -export const WEBHOOK_METHODS: IHttpRequestMethods[] = [ - 'DELETE', - 'GET', - 'HEAD', - 'PATCH', - 'POST', - 'PUT', -]; - -export const webhookRequestHandler = - (webhookManager: IWebhookManager) => - async (req: WebhookRequest | WebhookCORSRequest, res: express.Response) => { - const { path } = req.params; - const method = req.method; - - if (method !== 'OPTIONS' && !WEBHOOK_METHODS.includes(method)) { - return ResponseHelper.sendErrorResponse( - res, - new Error(`The method ${method} is not supported.`), - ); - } - - // Setup CORS headers only if the incoming request has an `origin` header - if ('origin' in req.headers) { - if (webhookManager.getWebhookMethods) { - try { - const allowedMethods = await webhookManager.getWebhookMethods(path); - res.header('Access-Control-Allow-Methods', ['OPTIONS', ...allowedMethods].join(', ')); - } catch (error) { - return ResponseHelper.sendErrorResponse(res, error as Error); - } - } - - 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 ?? {}; - - if (allowedOrigins && allowedOrigins !== '*' && allowedOrigins !== req.headers.origin) { - const originsList = allowedOrigins.split(','); - const defaultOrigin = originsList[0]; - - if (originsList.length === 1) { - res.header('Access-Control-Allow-Origin', defaultOrigin); - } - - if (originsList.includes(req.headers.origin as string)) { - res.header('Access-Control-Allow-Origin', req.headers.origin); - } else { - res.header('Access-Control-Allow-Origin', defaultOrigin); - } - } else { - res.header('Access-Control-Allow-Origin', req.headers.origin); - } - - 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') { - return ResponseHelper.sendSuccessResponse(res, {}, true, 204); - } - - let response: IWebhookResponseCallbackData; - try { - response = await webhookManager.executeWebhook(req, res); - } catch (error) { - return ResponseHelper.sendErrorResponse(res, error as Error); - } - - // Don't respond, if already responded - if (response.noWebhookResponse !== true) { - ResponseHelper.sendSuccessResponse( - res, - response.data, - true, - response.responseCode, - response.headers, - ); - } - }; - /** * Returns all the webhooks which should be created for the given workflow */ diff --git a/packages/cli/src/webhooks/WebhookRequestHandler.ts b/packages/cli/src/webhooks/WebhookRequestHandler.ts new file mode 100644 index 0000000000..289175ac05 --- /dev/null +++ b/packages/cli/src/webhooks/WebhookRequestHandler.ts @@ -0,0 +1,119 @@ +import type express from 'express'; +import type { IHttpRequestMethods } from 'n8n-workflow'; +import type { + IWebhookManager, + WebhookOptionsRequest, + WebhookRequest, +} from '@/webhooks/webhook.types'; +import * as ResponseHelper from '@/ResponseHelper'; + +const WEBHOOK_METHODS: IHttpRequestMethods[] = ['DELETE', 'GET', 'HEAD', 'PATCH', 'POST', 'PUT']; + +class WebhookRequestHandler { + constructor(private readonly webhookManager: IWebhookManager) {} + + /** + * Handles an incoming webhook request. Handles CORS and delegates the + * request to the webhook manager to execute the webhook. + */ + async handleRequest(req: WebhookRequest | WebhookOptionsRequest, res: express.Response) { + const method = req.method; + + if (method !== 'OPTIONS' && !WEBHOOK_METHODS.includes(method)) { + return ResponseHelper.sendErrorResponse( + res, + new Error(`The method ${method} is not supported.`), + ); + } + + // Setup CORS headers only if the incoming request has an `origin` header + if ('origin' in req.headers) { + const corsSetupError = await this.setupCorsHeaders(req, res); + if (corsSetupError) { + return ResponseHelper.sendErrorResponse(res, corsSetupError); + } + } + + if (method === 'OPTIONS') { + return ResponseHelper.sendSuccessResponse(res, {}, true, 204); + } + + try { + const response = await this.webhookManager.executeWebhook(req, res); + + // Don't respond, if already responded + if (response.noWebhookResponse !== true) { + ResponseHelper.sendSuccessResponse( + res, + response.data, + true, + response.responseCode, + response.headers, + ); + } + } catch (error) { + return ResponseHelper.sendErrorResponse(res, error as Error); + } + } + + private async setupCorsHeaders( + req: WebhookRequest | WebhookOptionsRequest, + res: express.Response, + ): Promise { + const method = req.method; + const { path } = req.params; + + if (this.webhookManager.getWebhookMethods) { + try { + const allowedMethods = await this.webhookManager.getWebhookMethods(path); + res.header('Access-Control-Allow-Methods', ['OPTIONS', ...allowedMethods].join(', ')); + } catch (error) { + return error as Error; + } + } + + const requestedMethod = + method === 'OPTIONS' + ? (req.headers['access-control-request-method'] as IHttpRequestMethods) + : method; + if (this.webhookManager.findAccessControlOptions && requestedMethod) { + const options = await this.webhookManager.findAccessControlOptions(path, requestedMethod); + const { allowedOrigins } = options ?? {}; + + if (allowedOrigins && allowedOrigins !== '*' && allowedOrigins !== req.headers.origin) { + const originsList = allowedOrigins.split(','); + const defaultOrigin = originsList[0]; + + if (originsList.length === 1) { + res.header('Access-Control-Allow-Origin', defaultOrigin); + } + + if (originsList.includes(req.headers.origin as string)) { + res.header('Access-Control-Allow-Origin', req.headers.origin); + } else { + res.header('Access-Control-Allow-Origin', defaultOrigin); + } + } else { + res.header('Access-Control-Allow-Origin', req.headers.origin); + } + + 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); + } + } + } + + return null; + } +} + +export function createWebhookHandlerFor(webhookManager: IWebhookManager) { + const handler = new WebhookRequestHandler(webhookManager); + + return async (req: WebhookRequest | WebhookOptionsRequest, res: express.Response) => { + await handler.handleRequest(req, res); + }; +} diff --git a/packages/cli/src/webhooks/__tests__/WebhookHelpers.test.ts b/packages/cli/src/webhooks/__tests__/WebhookHelpers.test.ts deleted file mode 100644 index 9f86fbc549..0000000000 --- a/packages/cli/src/webhooks/__tests__/WebhookHelpers.test.ts +++ /dev/null @@ -1,142 +0,0 @@ -import { type Response } from 'express'; -import { mock } from 'jest-mock-extended'; -import { randomString } from 'n8n-workflow'; -import type { IHttpRequestMethods } from 'n8n-workflow'; - -import type { IWebhookManager, WebhookCORSRequest, WebhookRequest } from '@/webhooks/webhook.types'; -import { webhookRequestHandler } from '@/webhooks/WebhookHelpers'; - -describe('WebhookHelpers', () => { - describe('webhookRequestHandler', () => { - const webhookManager = mock>(); - const handler = webhookRequestHandler(webhookManager); - - beforeEach(() => { - jest.resetAllMocks(); - }); - - it('should throw for unsupported methods', async () => { - const req = mock({ - method: 'CONNECT' as IHttpRequestMethods, - }); - const res = mock(); - 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({ - method: 'OPTIONS', - headers: { - origin: 'https://example.com', - 'access-control-request-method': undefined, - }, - params: { path: 'test' }, - }); - const res = mock(); - 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({ - method: 'OPTIONS', - headers: { - origin: 'https://example.com', - 'access-control-request-method': 'GET', - }, - params: { path: 'test' }, - }); - const res = mock(); - 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 = randomString(10); - const req = mock({ - method: 'OPTIONS', - headers: { - origin: randomOrigin, - 'access-control-request-method': 'GET', - }, - params: { path: 'test' }, - }); - const res = mock(); - 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({ - method: 'OPTIONS', - headers: { - origin: 'https://example.com', - 'access-control-request-method': 'GET', - }, - params: { path: 'test' }, - }); - const res = mock(); - 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'); - }); - }); - }); -}); diff --git a/packages/cli/src/webhooks/__tests__/WebhookRequestHandler.test.ts b/packages/cli/src/webhooks/__tests__/WebhookRequestHandler.test.ts new file mode 100644 index 0000000000..aff3102419 --- /dev/null +++ b/packages/cli/src/webhooks/__tests__/WebhookRequestHandler.test.ts @@ -0,0 +1,220 @@ +import { type Response } from 'express'; +import { mock } from 'jest-mock-extended'; +import { randomString } from 'n8n-workflow'; +import type { IHttpRequestMethods } from 'n8n-workflow'; + +import type { + IWebhookManager, + IWebhookResponseCallbackData, + WebhookOptionsRequest, + WebhookRequest, +} from '@/webhooks/webhook.types'; +import { createWebhookHandlerFor } from '@/webhooks/WebhookRequestHandler'; +import { ResponseError } from '@/errors/response-errors/abstract/response.error'; + +describe('WebhookRequestHandler', () => { + const webhookManager = mock>(); + const handler = createWebhookHandlerFor(webhookManager); + + beforeEach(() => { + jest.resetAllMocks(); + }); + + it('should throw for unsupported methods', async () => { + const req = mock({ + method: 'CONNECT' as IHttpRequestMethods, + }); + const res = mock(); + 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({ + method: 'OPTIONS', + headers: { + origin: 'https://example.com', + 'access-control-request-method': undefined, + }, + params: { path: 'test' }, + }); + const res = mock(); + 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({ + method: 'OPTIONS', + headers: { + origin: 'https://example.com', + 'access-control-request-method': 'GET', + }, + params: { path: 'test' }, + }); + const res = mock(); + 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 = randomString(10); + const req = mock({ + method: 'OPTIONS', + headers: { + origin: randomOrigin, + 'access-control-request-method': 'GET', + }, + params: { path: 'test' }, + }); + const res = mock(); + 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({ + method: 'OPTIONS', + headers: { + origin: 'https://example.com', + 'access-control-request-method': 'GET', + }, + params: { path: 'test' }, + }); + const res = mock(); + 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'); + }); + }); + + describe('webhook requests', () => { + it('should delegate the request to the webhook manager and send the response', async () => { + const req = mock({ + method: 'GET', + params: { path: 'test' }, + }); + + const res = mock(); + + const executeWebhookResponse: IWebhookResponseCallbackData = { + responseCode: 200, + data: {}, + headers: { + 'x-custom-header': 'test', + }, + }; + webhookManager.executeWebhook.mockResolvedValueOnce(executeWebhookResponse); + + await handler(req, res); + + expect(webhookManager.executeWebhook).toHaveBeenCalledWith(req, res); + expect(res.status).toHaveBeenCalledWith(200); + expect(res.header).toHaveBeenCalledWith({ + 'x-custom-header': 'test', + }); + expect(res.json).toHaveBeenCalledWith(executeWebhookResponse.data); + }); + + it('should send an error response if webhook execution throws', async () => { + class TestError extends ResponseError {} + const req = mock({ + method: 'GET', + params: { path: 'test' }, + }); + + const res = mock(); + res.status.mockReturnValue(res); + + webhookManager.executeWebhook.mockRejectedValueOnce( + new TestError('Test error', 500, 100, 'Test hint'), + ); + + await handler(req, res); + + expect(webhookManager.executeWebhook).toHaveBeenCalledWith(req, res); + expect(res.status).toHaveBeenCalledWith(500); + expect(res.json).toHaveBeenCalledWith({ + code: 100, + message: 'Test error', + hint: 'Test hint', + }); + }); + + test.each(['DELETE', 'GET', 'HEAD', 'PATCH', 'POST', 'PUT'])( + "should handle '%s' method", + async (method) => { + const req = mock({ + method, + params: { path: 'test' }, + }); + + const res = mock(); + + const executeWebhookResponse: IWebhookResponseCallbackData = { + responseCode: 200, + }; + webhookManager.executeWebhook.mockResolvedValueOnce(executeWebhookResponse); + + await handler(req, res); + + expect(webhookManager.executeWebhook).toHaveBeenCalledWith(req, res); + expect(res.status).toHaveBeenCalledWith(200); + expect(res.json).toHaveBeenCalledWith(executeWebhookResponse.data); + }, + ); + }); +}); diff --git a/packages/cli/src/webhooks/webhook.types.ts b/packages/cli/src/webhooks/webhook.types.ts index 7602ed1871..4d34b00d04 100644 --- a/packages/cli/src/webhooks/webhook.types.ts +++ b/packages/cli/src/webhooks/webhook.types.ts @@ -1,7 +1,7 @@ import type { Request, Response } from 'express'; import type { IDataObject, IHttpRequestMethods } from 'n8n-workflow'; -export type WebhookCORSRequest = Request & { method: 'OPTIONS' }; +export type WebhookOptionsRequest = Request & { method: 'OPTIONS' }; export type WebhookRequest = Request<{ path: string }> & { method: IHttpRequestMethods;