From fa5cbe406174e65c816410ac560fd29108642bad Mon Sep 17 00:00:00 2001 From: Gergo Torcsvari Date: Mon, 27 Jun 2022 00:08:27 +0200 Subject: [PATCH] feat(middleware): original request passed down + regExp path match + better typed request and response --- README.md | 39 ++++-- src/middleware/node/get-missing-headers.ts | 3 +- src/middleware/node/get-payload.ts | 2 +- src/middleware/node/index.ts | 2 + src/middleware/node/middleware.ts | 24 +++- .../node/on-unhandled-request-default.ts | 4 +- src/middleware/node/types.ts | 4 +- src/types.ts | 5 + src/verify-and-receive.ts | 1 + test/integration/node-middleware.test.ts | 111 ++++++++++++++++++ 10 files changed, 177 insertions(+), 18 deletions(-) diff --git a/README.md b/README.md index 915d92835..8034dcce0 100644 --- a/README.md +++ b/README.md @@ -48,7 +48,7 @@ const webhooks = new Webhooks({ secret: "mysecret", }); -webhooks.onAny(({ id, name, payload }) => { +webhooks.onAny(({ id, name, payload, originalRequest }) => { console.log(name, "event received"); }); @@ -223,7 +223,7 @@ The `verify` method can be imported as static method from [`@octokit/webhooks-me ### webhooks.verifyAndReceive() ```js -webhooks.verifyAndReceive({ id, name, payload, signature }); +webhooks.verifyAndReceive({ id, name, payload, originalRequest, signature }); ``` @@ -316,7 +316,7 @@ eventHandler ### webhooks.receive() ```js -webhooks.receive({ id, name, payload }); +webhooks.receive({ id, name, payload, originalRequest }); ```
@@ -370,6 +370,8 @@ Returns a promise. Runs all handlers set with [`webhooks.on()`](#webhookson) in The `.receive()` method belongs to the `event-handler` module which can be used [standalone](src/event-handler/). +The `originalRequest` is an optional parameter, if it is set, it will be available in the `on` functions. + ### webhooks.on() ```js @@ -420,7 +422,7 @@ webhooks.on(eventNames, handler); Required. Method to be run each time the event with the passed name is received. the handler function can be an async function, throw an error or - return a Promise. The handler is called with an event object: {id, name, payload}. + return a Promise. The handler is called with an event object: {id, name, payload, originalRequest}. @@ -449,7 +451,7 @@ webhooks.onAny(handler); Required. Method to be run each time any event is received. the handler function can be an async function, throw an error or - return a Promise. The handler is called with an event object: {id, name, payload}. + return a Promise. The handler is called with an event object: {id, name, payload, originalRequest}. @@ -482,7 +484,7 @@ Asynchronous `error` event handler are not blocking the `.receive()` method from Required. Method to be run each time a webhook event handler throws an error or returns a promise that rejects. The handler function can be an async function, - return a Promise. The handler is called with an error object that has a .event property which has all the information on the event: {id, name, payload}. + return a Promise. The handler is called with an error object that has a .event property which has all the information on the event: {id, name, payload, originalRequest}. @@ -586,6 +588,29 @@ createServer(middleware).listen(3000); Custom path to match requests against. Defaults to /api/github/webhooks. + + + +
+ pathRegex + + RegEx + + + +Custom path matcher. If set, overrides the path option. +Can be used as; + +```js +const middleware = createNodeMiddleware( + webhooks, + { pathRegex: /^\/api\/github\/webhooks/ } +); +``` + +Test the regex before usage, the `g` and `y` flags [makes it stateful](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/RegExp/test)! + +
log @@ -721,7 +746,7 @@ A union of all possible events and event/action combinations supported by the ev ### `EmitterWebhookEvent` -The object that is emitted by `@octokit/webhooks` as an event; made up of an `id`, `name`, and `payload` properties. +The object that is emitted by `@octokit/webhooks` as an event; made up of an `id`, `name`, and `payload` properties, with an optional `originalRequest`. An optional generic parameter can be passed to narrow the type of the `name` and `payload` properties based on event names or event/action combinations, e.g. `EmitterWebhookEvent<"check_run" | "code_scanning_alert.fixed">`. ## License diff --git a/src/middleware/node/get-missing-headers.ts b/src/middleware/node/get-missing-headers.ts index d7d878d75..02f0c37f8 100644 --- a/src/middleware/node/get-missing-headers.ts +++ b/src/middleware/node/get-missing-headers.ts @@ -1,7 +1,8 @@ // remove type imports from http for Deno compatibility // see https://github.com/octokit/octokit.js/issues/24#issuecomment-817361886 // import { IncomingMessage } from "http"; -type IncomingMessage = any; + +import { IncomingMessage } from "./middleware"; const WEBHOOK_HEADERS = [ "x-github-event", diff --git a/src/middleware/node/get-payload.ts b/src/middleware/node/get-payload.ts index 956195826..df93c3b8c 100644 --- a/src/middleware/node/get-payload.ts +++ b/src/middleware/node/get-payload.ts @@ -1,6 +1,7 @@ import { WebhookEvent } from "@octokit/webhooks-types"; // @ts-ignore to address #245 import AggregateError from "aggregate-error"; +import { IncomingMessage } from "./middleware"; // remove type imports from http for Deno compatibility // see https://github.com/octokit/octokit.js/issues/24#issuecomment-817361886 @@ -10,7 +11,6 @@ import AggregateError from "aggregate-error"; // body?: WebhookEvent | unknown; // } // } -type IncomingMessage = any; export function getPayload(request: IncomingMessage): Promise { // If request.body already exists we can stop here diff --git a/src/middleware/node/index.ts b/src/middleware/node/index.ts index 06312ba3b..120b51cb5 100644 --- a/src/middleware/node/index.ts +++ b/src/middleware/node/index.ts @@ -8,12 +8,14 @@ export function createNodeMiddleware( webhooks: Webhooks, { path = "/api/github/webhooks", + pathRegex = undefined, onUnhandledRequest = onUnhandledRequestDefault, log = createLogger(), }: MiddlewareOptions = {} ) { return middleware.bind(null, webhooks, { path, + pathRegex, onUnhandledRequest, log, } as Required); diff --git a/src/middleware/node/middleware.ts b/src/middleware/node/middleware.ts index 95a25cf3e..bd6e537dd 100644 --- a/src/middleware/node/middleware.ts +++ b/src/middleware/node/middleware.ts @@ -1,8 +1,19 @@ // remove type imports from http for Deno compatibility // see https://github.com/octokit/octokit.js/issues/24#issuecomment-817361886 // import { IncomingMessage, ServerResponse } from "http"; -type IncomingMessage = any; -type ServerResponse = any; +export interface IncomingMessage { + headers: Record; + body?: any; + url?: string; + method?: string; + setEncoding(enc: string): void; + on(event: string, callback: (data: any) => void): void; +} +export interface ServerResponse { + set statusCode(value: number); + end(body: string): void; + writeHead(status: number, headers?: Record): void; +} import { WebhookEventName } from "@octokit/webhooks-types"; @@ -33,8 +44,10 @@ export async function middleware( ); return; } - - const isUnknownRoute = request.method !== "POST" || pathname !== options.path; + const pathMatch = options.pathRegex + ? options.pathRegex.test(pathname) + : pathname === options.path; + const isUnknownRoute = request.method !== "POST" || !pathMatch; const isExpressMiddleware = typeof next === "function"; if (isUnknownRoute) { if (isExpressMiddleware) { @@ -72,7 +85,7 @@ export async function middleware( didTimeout = true; response.statusCode = 202; response.end("still processing\n"); - }, 9000).unref(); + }, 9000); try { const payload = await getPayload(request); @@ -82,6 +95,7 @@ export async function middleware( name: eventName as any, payload: payload as any, signature: signatureSHA256, + originalRequest: request, }); clearTimeout(timeout); diff --git a/src/middleware/node/on-unhandled-request-default.ts b/src/middleware/node/on-unhandled-request-default.ts index 3664190fc..052e3741a 100644 --- a/src/middleware/node/on-unhandled-request-default.ts +++ b/src/middleware/node/on-unhandled-request-default.ts @@ -1,8 +1,8 @@ // remove type imports from http for Deno compatibility // see https://github.com/octokit/octokit.js/issues/24#issuecomment-817361886 // import { IncomingMessage, ServerResponse } from "http"; -type IncomingMessage = any; -type ServerResponse = any; + +import { IncomingMessage, ServerResponse } from "./middleware"; export function onUnhandledRequestDefault( request: IncomingMessage, diff --git a/src/middleware/node/types.ts b/src/middleware/node/types.ts index 81c4e0ede..01e24cfae 100644 --- a/src/middleware/node/types.ts +++ b/src/middleware/node/types.ts @@ -1,13 +1,13 @@ // remove type imports from http for Deno compatibility // see https://github.com/octokit/octokit.js/issues/24#issuecomment-817361886 // import { IncomingMessage, ServerResponse } from "http"; -type IncomingMessage = any; -type ServerResponse = any; import { Logger } from "../../createLogger"; +import { IncomingMessage, ServerResponse } from "./middleware"; export type MiddlewareOptions = { path?: string; + pathRegex?: RegExp; log?: Logger; onUnhandledRequest?: ( request: IncomingMessage, diff --git a/src/types.ts b/src/types.ts index a2eedbe46..eedb23e40 100644 --- a/src/types.ts +++ b/src/types.ts @@ -5,6 +5,7 @@ import type { } from "@octokit/webhooks-types"; import { Logger } from "./createLogger"; import type { emitterEventNames } from "./generated/webhook-names"; +import { IncomingMessage } from "./middleware/node/middleware"; export type EmitterWebhookEventName = typeof emitterEventNames[number]; export type EmitterWebhookEvent< @@ -12,6 +13,8 @@ export type EmitterWebhookEvent< > = TEmitterEvent extends `${infer TWebhookEvent}.${infer TAction}` ? BaseWebhookEvent> & { payload: { action: TAction }; + } & { + originalRequest?: IncomingMessage; } : BaseWebhookEvent>; @@ -20,6 +23,7 @@ export type EmitterWebhookEventWithStringPayloadAndSignature = { name: EmitterWebhookEventName; payload: string; signature: string; + originalRequest?: IncomingMessage; }; export type EmitterWebhookEventWithSignature = EmitterWebhookEvent & { @@ -30,6 +34,7 @@ interface BaseWebhookEvent { id: string; name: TName; payload: WebhookEventMap[TName]; + originalRequest?: IncomingMessage; } export interface Options { diff --git a/src/verify-and-receive.ts b/src/verify-and-receive.ts index 06a594497..15c7cc4b3 100644 --- a/src/verify-and-receive.ts +++ b/src/verify-and-receive.ts @@ -39,5 +39,6 @@ export async function verifyAndReceive( typeof event.payload === "string" ? JSON.parse(event.payload) : event.payload, + originalRequest: event.originalRequest, }); } diff --git a/test/integration/node-middleware.test.ts b/test/integration/node-middleware.test.ts index 6af202c1c..d1410fbb5 100644 --- a/test/integration/node-middleware.test.ts +++ b/test/integration/node-middleware.test.ts @@ -62,6 +62,117 @@ describe("createNodeMiddleware(webhooks)", () => { server.close(); }); + test("pathRegex match", async () => { + expect.assertions(7); + + const webhooks = new Webhooks({ + secret: "mySecret", + }); + + const server = createServer( + createNodeMiddleware(webhooks, { + pathRegex: /^\/api\/github\/webhooks/, + }) + ).listen(); + + // @ts-expect-error complains about { port } although it's included in returned AddressInfo interface + const { port } = server.address(); + + webhooks.on("push", (event) => { + expect(event.id).toBe("123e4567-e89b-12d3-a456-426655440000"); + }); + + const response1 = await fetch( + `http://localhost:${port}/api/github/webhooks/0001/testurl`, + { + method: "POST", + headers: { + "X-GitHub-Delivery": "123e4567-e89b-12d3-a456-426655440000", + "X-GitHub-Event": "push", + "X-Hub-Signature-256": signatureSha256, + }, + body: pushEventPayload, + } + ); + + expect(response1.status).toEqual(200); + await expect(response1.text()).resolves.toBe("ok\n"); + + const response2 = await fetch( + `http://localhost:${port}/api/github/webhooks/0001/testurl`, + { + method: "POST", + headers: { + "X-GitHub-Delivery": "123e4567-e89b-12d3-a456-426655440000", + "X-GitHub-Event": "push", + "X-Hub-Signature-256": signatureSha256, + }, + body: pushEventPayload, + } + ); + + expect(response2.status).toEqual(200); + await expect(response2.text()).resolves.toBe("ok\n"); + + const response3 = await fetch(`http://localhost:${port}/api/github/web`, { + method: "POST", + headers: { + "X-GitHub-Delivery": "123e4567-e89b-12d3-a456-426655440000", + "X-GitHub-Event": "push", + "X-Hub-Signature-256": signatureSha256, + }, + body: pushEventPayload, + }); + + expect(response3.status).toEqual(404); + + server.close(); + }); + + test("original request passed by as intented", async () => { + expect.assertions(6); + + const webhooks = new Webhooks({ + secret: "mySecret", + }); + + const server = createServer( + createNodeMiddleware(webhooks, { + pathRegex: /^\/api\/github\/webhooks/, + }) + ).listen(); + + // @ts-expect-error complains about { port } although it's included in returned AddressInfo interface + const { port } = server.address(); + + webhooks.on("push", (event) => { + expect(event.id).toBe("123e4567-e89b-12d3-a456-426655440000"); + const r = event.originalRequest; + expect(r).toBeDefined(); + expect(r?.headers["my-custom-header"]).toBe("customHeader"); + expect(r?.url).toBe(`/api/github/webhooks/0001/testurl`); + }); + + const response = await fetch( + `http://localhost:${port}/api/github/webhooks/0001/testurl`, + { + method: "POST", + headers: { + "X-GitHub-Delivery": "123e4567-e89b-12d3-a456-426655440000", + "X-GitHub-Event": "push", + "X-Hub-Signature-256": signatureSha256, + "my-custom-header": "customHeader", + }, + body: pushEventPayload, + } + ); + + expect(response.status).toEqual(200); + await expect(response.text()).resolves.toBe("ok\n"); + + server.close(); + }); + test("request.body already parsed (e.g. Lambda)", async () => { expect.assertions(3);