diff --git a/README.md b/README.md index e3fc6a7b..9efae419 100644 --- a/README.md +++ b/README.md @@ -191,7 +191,7 @@ webhooks.verify(eventPayload, signature); eventPayload - (Object or String) + (Object [deprecated] or String) @@ -262,7 +262,7 @@ webhooks.verifyAndReceive({ id, name, payload, signature }); payload - Object or String + Object (deprecated) or String @@ -601,7 +601,7 @@ Used for internal logging. Defaults to [`console`](https://developer.mozilla.org - onUnhandledRequest + onUnhandledRequest (deprecated) function diff --git a/src/index.ts b/src/index.ts index 08868a63..06fba3ab 100644 --- a/src/index.ts +++ b/src/index.ts @@ -19,13 +19,21 @@ import { 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; + (eventPayload: string, signature: string): Promise; +}; +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; + (options: EmitterWebhookEventWithStringPayloadAndSignature): Promise; +}; + // U holds the return value of `transform` function in Options class Webhooks { public sign: (payload: string | object) => Promise; - public verify: ( - eventPayload: string | object, - signature: string - ) => Promise; + public verify: Iverify; public on: ( event: E | E[], callback: HandlerFunction @@ -37,11 +45,7 @@ class Webhooks { callback: RemoveHandlerFunction ) => void; public receive: (event: EmitterWebhookEvent) => Promise; - public verifyAndReceive: ( - options: - | EmitterWebhookEventWithStringPayloadAndSignature - | EmitterWebhookEventWithSignature - ) => Promise; + public verifyAndReceive: IverifyAndReceive; constructor(options: Options & { secret: string }) { if (!options || !options.secret) { @@ -56,13 +60,31 @@ class Webhooks { }; this.sign = sign.bind(null, options.secret); - this.verify = verify.bind(null, options.secret); + this.verify = (eventPayload: object | string, signature: string) => { + if (typeof eventPayload === "object") { + console.error( + "[@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.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 = verifyAndReceive.bind(null, state); + this.verifyAndReceive = ( + options: + | EmitterWebhookEventWithStringPayloadAndSignature + | EmitterWebhookEventWithSignature + ) => { + if (typeof options.payload === "object") { + console.error( + "[@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); + }; } } diff --git a/src/middleware/node/index.ts b/src/middleware/node/index.ts index 06312ba3..46398af7 100644 --- a/src/middleware/node/index.ts +++ b/src/middleware/node/index.ts @@ -12,9 +12,15 @@ export function createNodeMiddleware( log = createLogger(), }: MiddlewareOptions = {} ) { + const deprecateOnUnhandledRequest = (request: any, response: any) => { + console.error( + "[@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, + onUnhandledRequest: deprecateOnUnhandledRequest, log, } as Required); } diff --git a/src/middleware/node/types.ts b/src/middleware/node/types.ts index 81c4e0ed..48aa66fa 100644 --- a/src/middleware/node/types.ts +++ b/src/middleware/node/types.ts @@ -9,6 +9,7 @@ import { 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 diff --git a/src/to-normalized-json-string.ts b/src/to-normalized-json-string.ts index d20c519d..ca551cae 100644 --- a/src/to-normalized-json-string.ts +++ b/src/to-normalized-json-string.ts @@ -1,5 +1,5 @@ /** - * GitHub sends its JSON with an indentation of 2 spaces and a line break at the end + * GitHub sends its JSON with no indentation and no line break at the end */ export function toNormalizedJsonString(payload: object) { const payloadString = JSON.stringify(payload); diff --git a/src/types.ts b/src/types.ts index a2eedbe4..138374d6 100644 --- a/src/types.ts +++ b/src/types.ts @@ -21,7 +21,7 @@ export type EmitterWebhookEventWithStringPayloadAndSignature = { payload: string; signature: string; }; - +/** @deprecated */ export type EmitterWebhookEventWithSignature = EmitterWebhookEvent & { signature: string; }; diff --git a/test/integration/node-middleware.test.ts b/test/integration/node-middleware.test.ts index d0d69f47..3ffeaabf 100644 --- a/test/integration/node-middleware.test.ts +++ b/test/integration/node-middleware.test.ts @@ -168,43 +168,6 @@ describe("createNodeMiddleware(webhooks)", () => { server.close(); }); - test("custom non-found handler", async () => { - const webhooks = new Webhooks({ - secret: "mySecret", - }); - - const server = createServer( - createNodeMiddleware(webhooks, { - onUnhandledRequest(_request, response) { - response.writeHead(404); - response.end("nope"); - }, - }) - ).listen(); - - // @ts-expect-error complains about { port } although it's included in returned AddressInfo interface - const { port } = server.address(); - - const response = await fetch( - `http://localhost:${port}/api/github/webhooks`, - { - method: "PUT", - headers: { - "X-GitHub-Delivery": "123e4567-e89b-12d3-a456-426655440000", - "X-GitHub-Event": "push", - "X-Hub-Signature-256": signatureSha256, - }, - body: "invalid", - } - ); - - expect(response.status).toEqual(404); - - await expect(response.text()).resolves.toEqual("nope"); - - server.close(); - }); - test("Handles missing headers", async () => { const webhooks = new Webhooks({ secret: "mySecret", diff --git a/test/integration/webhooks.test.ts b/test/integration/webhooks.test.ts index 97337f12..9316f9cb 100644 --- a/test/integration/webhooks.test.ts +++ b/test/integration/webhooks.test.ts @@ -2,7 +2,7 @@ import { readFileSync } from "fs"; import { sign } from "@octokit/webhooks-methods"; -import { Webhooks, EmitterWebhookEvent } from "../../src"; +import { Webhooks } from "../../src"; import { toNormalizedJsonString } from "../../src/to-normalized-json-string"; const pushEventPayloadString = readFileSync( @@ -32,16 +32,6 @@ describe("Webhooks", () => { await webhooks.sign(pushEventPayloadString); }); - test("webhooks.verify(payload, signature) with object payload", async () => { - const secret = "mysecret"; - const webhooks = new Webhooks({ secret }); - - await webhooks.verify( - JSON.parse(pushEventPayloadString), - await sign({ secret, algorithm: "sha256" }, pushEventPayloadString) - ); - }); - test("webhooks.verify(payload, signature) with object payload containing special characters", async () => { const secret = "mysecret"; const webhooks = new Webhooks({ secret }); @@ -84,7 +74,7 @@ describe("Webhooks", () => { test("webhooks.verifyAndReceive(event) with incorrect signature", async () => { const webhooks = new Webhooks({ secret: "mysecret" }); - const pingPayload = {} as EmitterWebhookEvent<"ping">["payload"]; + const pingPayload = "{}"; await expect(async () => webhooks.verifyAndReceive({ id: "1", diff --git a/test/unit/deprecation.test.ts b/test/unit/deprecation.test.ts index e14b277b..af7584aa 100644 --- a/test/unit/deprecation.test.ts +++ b/test/unit/deprecation.test.ts @@ -1,3 +1,90 @@ +import { createServer } from "http"; +import { readFileSync } from "fs"; + +import fetch from "node-fetch"; +import { sign } from "@octokit/webhooks-methods"; + +import { Webhooks, createNodeMiddleware } from "../../src"; + +const pushEventPayloadString = readFileSync( + "test/fixtures/push-payload.json", + "utf-8" +); +let signatureSha256: string; describe("Deprecations", () => { - test("there are currently no deprecations", () => {}); + beforeAll(async () => { + signatureSha256 = await sign( + { secret: "mySecret", algorithm: "sha256" }, + pushEventPayloadString + ); + }); + test("onUnhandledRequest", async () => { + const spy = jest.spyOn(console, "error"); + const webhooks = new Webhooks({ + secret: "mySecret", + }); + + const server = createServer( + createNodeMiddleware(webhooks, { + onUnhandledRequest(_request, response) { + response.writeHead(404); + response.end("nope"); + }, + }) + ).listen(); + + // @ts-expect-error complains about { port } although it's included in returned AddressInfo interface + const { port } = server.address(); + + await fetch(`http://localhost:${port}/api/github/webhooks`, { + method: "PUT", + headers: { + "X-GitHub-Delivery": "123e4567-e89b-12d3-a456-426655440000", + "X-GitHub-Event": "push", + "X-Hub-Signature-256": signatureSha256, + }, + body: "invalid", + }); + + expect(spy).toBeCalledWith( + "[@octokit/webhooks] `onUnhandledRequest()` is deprecated and will be removed in a future release of `@octokit/webhooks`" + ); + spy.mockClear(); + server.close(); + }); + + test("webhooks.verify(payload, signature) with object payload", async () => { + const spy = jest.spyOn(console, "error"); + const secret = "mysecret"; + const webhooks = new Webhooks({ secret }); + + await webhooks.verify( + JSON.parse(pushEventPayloadString), + await sign({ secret, algorithm: "sha256" }, pushEventPayloadString) + ); + expect(spy).toBeCalledWith( + "[@octokit/webhooks] Passing a JSON payload object to `verify()` is deprecated and the functionality will be removed in a future release of `@octokit/webhooks`" + ); + spy.mockClear(); + }); + + test("webhooks.verifyAndReceive(payload, signature) with object payload", async () => { + const spy = jest.spyOn(console, "error"); + const secret = "mysecret"; + const webhooks = new Webhooks({ secret }); + + await webhooks.verifyAndReceive({ + id: "123e456", + name: "push", + payload: JSON.parse(pushEventPayloadString), + signature: await sign( + { secret, algorithm: "sha256" }, + pushEventPayloadString + ), + }); + expect(spy).toBeCalledWith( + "[@octokit/webhooks] Passing a JSON payload object to `verifyAndReceive()` is deprecated and the functionality will be removed in a future release of `@octokit/webhooks`" + ); + spy.mockClear(); + }); });