feat: v11 (#792)

BREAKING CHANGE: middleware only and (always) handles requests at `path` BREAKING CHANGE: Only accept string payloads for `webhooks.verify()` and `webhooks.verifyAndReceive()` BREAKING CHANGE: The middleware now expects only string payloads for `request.body` BREAKING CHANGE: Only accept string payloads for `webhooks.verify()` Modifiy the `module` field in `package.json` to point to the source ESM, and adds a `browser` field that points to the browser bundle Co-authored-by: Baoshan Sheng <sheng@icmd.org>
octokit · Apr 14, 2023 · 15e5422 · 15e5422
1 parent b2287bc
commit 15e5422
Show file tree

Hide file tree

Showing 16 changed files with 82 additions and 286 deletions.
diff --git a/README.md b/README.md
@@ -162,7 +162,7 @@ webhooks.sign(eventPayload);
           eventPayload
         </code>
         <em>
-          (Object)
+          (String)
         </em>
       </td>
       <td>
@@ -556,12 +556,19 @@ const webhooks = new Webhooks({
   secret: "mysecret",
 });
 
-const middleware = createNodeMiddleware(webhooks, { path: "/" });
-
-createServer(middleware).listen(3000);
-// can now receive user authorization callbacks at POST /
+const middleware = createNodeMiddleware(webhooks, { path: "/webhooks" });
+createServer(async (req, res) => {
+  // `middleware` returns `false` when `req` is unhandled (beyond `/webhooks`)
+  if (await middleware(req, res)) return;
+  res.writeHead(404);
+  res.end();
+}).listen(3000);
+// can now receive user authorization callbacks at POST /webhooks
 ```
 
+The middleware returned from `createNodeMiddleware` can also serve as an
+`Express.js` middleware directly.
+
 <table width="100%">
   <tbody valign="top">
     <tr>
@@ -597,32 +604,6 @@ createServer(middleware).listen(3000);
 
 Used for internal logging. Defaults to [`console`](https://developer.mozilla.org/en-US/docs/Web/API/console) with `debug` and `info` doing nothing.
 
-</td>
-    </tr>
-    <tr>
-      <td>
-        <code>onUnhandledRequest (deprecated)</code>
-        <em>
-          function
-        </em>
-      </td>
-      <td>
-
-Defaults to
-
-```js
-function onUnhandledRequest(request, response) {
-  response.writeHead(400, {
-    "content-type": "application/json",
-  });
-  response.end(
-    JSON.stringify({
-      error: error.message,
-    })
-  );
-}
-```
-
 </td>
     </tr>
   <tbody>

diff --git a/scripts/build.mjs b/scripts/build.mjs
@@ -74,9 +74,9 @@ async function main() {
         ...pkg,
         files: ["dist-*/**", "bin/**"],
         main: "dist-node/index.js",
-        module: "dist-web/index.js",
+        browser: "dist-web/index.js",
         types: "dist-types/index.d.ts",
-        source: "dist-src/index.js",
+        module: "dist-src/index.js",
         sideEffects: false,
       },
       null,

diff --git a/src/index.ts b/src/index.ts
@@ -1,7 +1,6 @@
 import { createLogger } from "./createLogger";
 import { createEventHandler } from "./event-handler/index";
-import { sign } from "./sign";
-import { verify } from "./verify";
+import { sign, verify } from "@octokit/webhooks-methods";
 import { verifyAndReceive } from "./verify-and-receive";
 import type {
   EmitterWebhookEvent,
@@ -13,27 +12,15 @@ import type {
   WebhookError,
   WebhookEventHandlerError,
   EmitterWebhookEventWithStringPayloadAndSignature,
-  EmitterWebhookEventWithSignature,
 } from "./types";
 
 export { createNodeMiddleware } from "./middleware/node/index";
 export { emitterEventNames } from "./generated/webhook-names";
 
-export type Iverify = {
-  /** @deprecated Passing a JSON payload object to `verify()` is deprecated and the functionality will be removed in a future release of `@octokit/webhooks`. */
-  (eventPayload: object, signature: string): Promise<boolean>;
-  (eventPayload: string, signature: string): Promise<boolean>;
-};
-export type IverifyAndReceive = {
-  /** @deprecated Passing a JSON payload object to `verifyAndReceive()` is deprecated and the functionality will be removed in a future release of `@octokit/webhooks`. */
-  (options: EmitterWebhookEventWithSignature): Promise<void>;
-  (options: EmitterWebhookEventWithStringPayloadAndSignature): Promise<void>;
-};
-
 // U holds the return value of `transform` function in Options
 class Webhooks<TTransformed = unknown> {
-  public sign: (payload: string | object) => Promise<string>;
-  public verify: Iverify;
+  public sign: (payload: string) => Promise<string>;
+  public verify: (eventPayload: string, signature: string) => Promise<boolean>;
   public on: <E extends EmitterWebhookEventName>(
     event: E | E[],
     callback: HandlerFunction<E, TTransformed>
@@ -45,7 +32,9 @@ class Webhooks<TTransformed = unknown> {
     callback: RemoveHandlerFunction<E, TTransformed>
   ) => void;
   public receive: (event: EmitterWebhookEvent) => Promise<void>;
-  public verifyAndReceive: IverifyAndReceive;
+  public verifyAndReceive: (
+    options: EmitterWebhookEventWithStringPayloadAndSignature
+  ) => Promise<void>;
 
   constructor(options: Options<TTransformed> & { secret: string }) {
     if (!options || !options.secret) {
@@ -60,31 +49,13 @@ class Webhooks<TTransformed = unknown> {
     };
 
     this.sign = sign.bind(null, options.secret);
-    this.verify = (eventPayload: object | string, signature: string) => {
-      if (typeof eventPayload === "object") {
-        console.warn(
-          "[@octokit/webhooks] Passing a JSON payload object to `verify()` is deprecated and the functionality will be removed in a future release of `@octokit/webhooks`"
-        );
-      }
-      return verify(options.secret, eventPayload, signature);
-    };
+    this.verify = verify.bind(null, options.secret);
     this.on = state.eventHandler.on;
     this.onAny = state.eventHandler.onAny;
     this.onError = state.eventHandler.onError;
     this.removeListener = state.eventHandler.removeListener;
     this.receive = state.eventHandler.receive;
-    this.verifyAndReceive = (
-      options:
-        | EmitterWebhookEventWithStringPayloadAndSignature
-        | EmitterWebhookEventWithSignature
-    ) => {
-      if (typeof options.payload === "object") {
-        console.warn(
-          "[@octokit/webhooks] Passing a JSON payload object to `verifyAndReceive()` is deprecated and the functionality will be removed in a future release of `@octokit/webhooks`"
-        );
-      }
-      return verifyAndReceive(state, options);
-    };
+    this.verifyAndReceive = verifyAndReceive.bind(null, state);
   }
 }
 

diff --git a/src/middleware/node/get-payload.ts b/src/middleware/node/get-payload.ts
@@ -1,4 +1,3 @@
-import type { WebhookEvent } from "@octokit/webhooks-types";
 // @ts-ignore to address #245
 import AggregateError from "aggregate-error";
 
@@ -12,20 +11,11 @@ import AggregateError from "aggregate-error";
 // }
 type IncomingMessage = any;
 
-export function getPayload(
-  request: IncomingMessage
-): Promise<WebhookEvent | string> {
+export function getPayload(request: IncomingMessage): Promise<string> {
   // If request.body already exists we can stop here
   // See https://github.com/octokit/webhooks.js/pull/23
 
-  if (request.body) {
-    if (typeof request.body !== "string") {
-      console.warn(
-        "[@octokit/webhooks] Passing the payload as a JSON object in `request.body` is deprecated and will be removed in a future release of `@octokit/webhooks`, please pass it as a a `string` instead."
-      );
-    }
-    return Promise.resolve(request.body as WebhookEvent | string);
-  }
+  if (request.body) return Promise.resolve(request.body);
 
   return new Promise((resolve, reject) => {
     let data = "";

diff --git a/src/middleware/node/index.ts b/src/middleware/node/index.ts
@@ -1,26 +1,17 @@
 import { createLogger } from "../../createLogger";
 import type { Webhooks } from "../../index";
 import { middleware } from "./middleware";
-import { onUnhandledRequestDefault } from "./on-unhandled-request-default";
 import type { MiddlewareOptions } from "./types";
 
 export function createNodeMiddleware(
   webhooks: Webhooks,
   {
     path = "/api/github/webhooks",
-    onUnhandledRequest = onUnhandledRequestDefault,
     log = createLogger(),
   }: MiddlewareOptions = {}
 ) {
-  const deprecateOnUnhandledRequest = (request: any, response: any) => {
-    console.warn(
-      "[@octokit/webhooks] `onUnhandledRequest()` is deprecated and will be removed in a future release of `@octokit/webhooks`"
-    );
-    return onUnhandledRequest(request, response);
-  };
   return middleware.bind(null, webhooks, {
     path,
-    onUnhandledRequest: deprecateOnUnhandledRequest,
     log,
   } as Required<MiddlewareOptions>);
 }
diff --git a/src/middleware/node/middleware.ts b/src/middleware/node/middleware.ts
@@ -11,14 +11,15 @@ import type { WebhookEventHandlerError } from "../../types";
 import type { MiddlewareOptions } from "./types";
 import { getMissingHeaders } from "./get-missing-headers";
 import { getPayload } from "./get-payload";
+import { onUnhandledRequestDefault } from "./on-unhandled-request-default";
 
 export async function middleware(
   webhooks: Webhooks,
   options: Required<MiddlewareOptions>,
   request: IncomingMessage,
   response: ServerResponse,
   next?: Function
-) {
+): Promise<boolean> {
   let pathname: string;
   try {
     pathname = new URL(request.url as string, "http://localhost").pathname;
@@ -31,17 +32,15 @@ export async function middleware(
         error: `Request URL could not be parsed: ${request.url}`,
       })
     );
-    return;
+    return true;
   }
 
-  const isUnknownRoute = request.method !== "POST" || pathname !== options.path;
-  const isExpressMiddleware = typeof next === "function";
-  if (isUnknownRoute) {
-    if (isExpressMiddleware) {
-      return next!();
-    } else {
-      return options.onUnhandledRequest(request, response);
-    }
+  if (pathname !== options.path) {
+    next?.();
+    return false;
+  } else if (request.method !== "POST") {
+    onUnhandledRequestDefault(request, response);
+    return true;
   }
 
   // Check if the Content-Type header is `application/json` and allow for charset to be specified in it
@@ -60,7 +59,7 @@ export async function middleware(
         error: `Unsupported "Content-Type" header value. Must be "application/json"`,
       })
     );
-    return;
+    return true;
   }
 
   const missingHeaders = getMissingHeaders(request).join(", ");
@@ -75,7 +74,7 @@ export async function middleware(
       })
     );
 
-    return;
+    return true;
   }
 
   const eventName = request.headers["x-github-event"] as WebhookEventName;
@@ -99,18 +98,19 @@ export async function middleware(
     await webhooks.verifyAndReceive({
       id: id,
       name: eventName as any,
-      payload: payload as any,
+      payload,
       signature: signatureSHA256,
     });
     clearTimeout(timeout);
 
-    if (didTimeout) return;
+    if (didTimeout) return true;
 
     response.end("ok\n");
+    return true;
   } catch (error) {
     clearTimeout(timeout);
 
-    if (didTimeout) return;
+    if (didTimeout) return true;
 
     const err = Array.from(error as WebhookEventHandlerError)[0];
     const errorMessage = err.message
@@ -125,5 +125,7 @@ export async function middleware(
         error: errorMessage,
       })
     );
+
+    return true;
   }
 }
diff --git a/src/middleware/node/types.ts b/src/middleware/node/types.ts
@@ -1,17 +1,6 @@
-// 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 type { Logger } from "../../createLogger";
 
 export type MiddlewareOptions = {
   path?: string;
   log?: Logger;
-  /** @deprecated `onUnhandledRequest()` is deprecated and will be removed in a future release of `@octokit/webhooks` */
-  onUnhandledRequest?: (
-    request: IncomingMessage,
-    response: ServerResponse
-  ) => void;
 };
diff --git a/src/sign.ts b/src/sign.ts
diff --git a/src/to-normalized-json-string.ts b/src/to-normalized-json-string.ts
diff --git a/src/types.ts b/src/types.ts
@@ -21,10 +21,6 @@ export type EmitterWebhookEventWithStringPayloadAndSignature = {
   payload: string;
   signature: string;
 };
-/** @deprecated */
-export type EmitterWebhookEventWithSignature = EmitterWebhookEvent & {
-  signature: string;
-};
 
 interface BaseWebhookEvent<TName extends WebhookEventName> {
   id: string;

diff --git a/src/verify-and-receive.ts b/src/verify-and-receive.ts
@@ -1,24 +1,18 @@
 import { verify } from "@octokit/webhooks-methods";
 
-import { toNormalizedJsonString } from "./to-normalized-json-string";
 import type {
   EmitterWebhookEventWithStringPayloadAndSignature,
-  EmitterWebhookEventWithSignature,
   State,
 } from "./types";
 
 export async function verifyAndReceive(
   state: State & { secret: string },
-  event:
-    | EmitterWebhookEventWithStringPayloadAndSignature
-    | EmitterWebhookEventWithSignature
+  event: EmitterWebhookEventWithStringPayloadAndSignature
 ): Promise<any> {
   // verify will validate that the secret is not undefined
   const matchesSignature = await verify(
     state.secret,
-    typeof event.payload === "object"
-      ? toNormalizedJsonString(event.payload)
-      : event.payload,
+    event.payload,
     event.signature
   );
 
@@ -35,9 +29,6 @@ export async function verifyAndReceive(
   return state.eventHandler.receive({
     id: event.id,
     name: event.name,
-    payload:
-      typeof event.payload === "string"
-        ? JSON.parse(event.payload)
-        : event.payload,
+    payload: JSON.parse(event.payload),
   });
 }