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: createNodeMiddleware() #509

Merged
merged 21 commits into from
Mar 30, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
21 commits
Select commit Hold shift + click to select a range
4973ecc
refactor: move current `createMiddleware` implementation to `src/midd…
gr2m Mar 26, 2021
c80480c
docs(README): `createNodeMiddleware()`. Remove obsolete documentation…
gr2m Mar 26, 2021
5a33184
WIP
gr2m Mar 27, 2021
7e8813d
adapt for #514
gr2m Mar 29, 2021
d615cf9
test: make `createNodeMiddleware()` tests pass
gr2m Mar 29, 2021
7921a10
refactor: address https://github.com/octokit/webhooks.js/pull/509\#di…
gr2m Mar 29, 2021
1579c59
refactor: address https://github.com/octokit/webhooks.js/pull/509\#di…
gr2m Mar 29, 2021
bd1358e
refactor: address https://github.com/octokit/webhooks.js/pull/509\#di…
gr2m Mar 29, 2021
c5ca127
test: address https://github.com/octokit/webhooks.js/pull/509\#discus…
gr2m Mar 29, 2021
ec736ac
test: address https://github.com/octokit/webhooks.js/pull/509\#discus…
gr2m Mar 29, 2021
6d2b594
make it work with node 10 & 12
gr2m Mar 29, 2021
107de04
build(dev-deps): express
gr2m Mar 29, 2021
89f202c
build(package): lock file
gr2m Mar 29, 2021
dc800a2
correctly handle usage with express. No need to check for correct rou…
gr2m Mar 29, 2021
f000384
test: 100% coverage
gr2m Mar 29, 2021
3ade8b3
TypeScript: use types of `Console` for log methods
gr2m Mar 29, 2021
7c4b8ba
feat: `createNodeMiddleware(express, { log })` option
gr2m Mar 29, 2021
7176f3a
remove @types/express to workaround stupid TypeScript errors
gr2m Mar 29, 2021
35b96d2
test: workaround requiring types for express
gr2m Mar 29, 2021
a41def2
feat: deprecate `path` option and `webhooks.middleware`
gr2m Mar 29, 2021
4ee5a6b
docs(README): remove `webhooks.middleware` from API toc
gr2m Mar 29, 2021
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
736 changes: 391 additions & 345 deletions README.md

Large diffs are not rendered by default.

464 changes: 464 additions & 0 deletions package-lock.json

Large diffs are not rendered by default.

3 changes: 3 additions & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -33,10 +33,13 @@
"@types/jest": "^26.0.9",
"@types/json-schema": "^7.0.7",
"@types/node": "^14.0.14",
"@types/node-fetch": "^2.5.8",
"@types/prettier": "^2.0.0",
"axios": "^0.21.0",
"express": "^4.17.1",
"get-port": "^5.0.0",
"jest": "^26.2.2",
"node-fetch": "^2.6.1",
"prettier": "^2.0.1",
"prettier-plugin-packagejson": "^2.2.9",
"semantic-release": "^17.0.0",
Expand Down
8 changes: 4 additions & 4 deletions src/createLogger.ts
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
export interface Logger {
debug: (message: string) => unknown;
info: (message: string) => unknown;
warn: (message: string) => unknown;
error: (message: string) => unknown;
debug: (...data: any[]) => void;
info: (...data: any[]) => void;
warn: (...data: any[]) => void;
error: (...data: any[]) => void;
}

export const createLogger = (logger?: Partial<Logger>): Logger => ({
Expand Down
39 changes: 31 additions & 8 deletions src/index.ts
Original file line number Diff line number Diff line change
@@ -1,9 +1,9 @@
import { IncomingMessage, ServerResponse } from "http";
import { createLogger } from "./createLogger";
import { createEventHandler } from "./event-handler/index";
import { createMiddleware } from "./middleware/index";
import { middleware } from "./middleware/middleware";
import { verifyAndReceive } from "./middleware/verify-and-receive";
import { createMiddleware } from "./middleware-legacy/index";
import { middleware } from "./middleware-legacy/middleware";
import { verifyAndReceive } from "./middleware-legacy/verify-and-receive";
import { sign } from "./sign/index";
import {
EmitterWebhookEvent,
Expand All @@ -16,6 +16,8 @@ import {
} from "./types";
import { verify } from "./verify/index";

export { createNodeMiddleware } from "./middleware/node/index";

// U holds the return value of `transform` function in Options
class Webhooks<TTransformed = unknown> {
public sign: (payload: string | object) => string;
Expand All @@ -31,14 +33,18 @@ class Webhooks<TTransformed = unknown> {
callback: HandlerFunction<E, TTransformed>
) => void;
public receive: (event: EmitterWebhookEvent) => Promise<void>;
public verifyAndReceive: (
options: EmitterWebhookEvent & { signature: string }
) => Promise<void>;

/**
* @deprecated use `createNodeMiddleware(webhooks)` instead
*/
public middleware: (
request: IncomingMessage,
response: ServerResponse,
next?: (err?: any) => void
) => void | Promise<void>;
public verifyAndReceive: (
options: EmitterWebhookEvent & { signature: string }
) => Promise<void>;

constructor(options: Options<TTransformed>) {
if (!options || !options.secret) {
Expand All @@ -53,21 +59,38 @@ class Webhooks<TTransformed = unknown> {
log: createLogger(options.log),
};

if ("path" in options) {
state.log.warn(
"[@octokit/webhooks] `path` option is deprecated and will be removed in a future release of `@octokit/webhooks`. Please use `createNodeMiddleware(webhooks, { path })` instead"
);
}

this.sign = sign.bind(null, options.secret);
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.middleware = middleware.bind(null, state);
this.verifyAndReceive = verifyAndReceive.bind(null, state);

this.middleware = function deprecatedMiddleware(
request: IncomingMessage,
response: ServerResponse,
next?: (err?: any) => void
) {
state.log.warn(
"[@octokit/webhooks] `webhooks.middleware` is deprecated and will be removed in a future release of `@octokit/webhooks`. Please use `createNodeMiddleware(webhooks)` instead"
);
return middleware(state, request, response, next);
};
}
}

/** @deprecated `createWebhooksApi()` is deprecated and will be removed in a future release of `@octokit/webhooks`, please use the `Webhooks` class instead */
const createWebhooksApi = <TTransformed>(options: Options<TTransformed>) => {
console.error(
const log = createLogger(options.log);
log.warn(
Comment on lines -70 to +93
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This isn't strictly related to this PR, but I get the changes

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yeah sorry for that, you are totally right, I've been lazy creating separate PRs for that. Same with the formatting changes in the README, I've to show octokit on Wednesday, I'm in a bit of a rush

"[@octokit/webhooks] `createWebhooksApi()` is deprecated and will be removed in a future release of `@octokit/webhooks`, please use the `Webhooks` class instead"
);
return new Webhooks<TTransformed>(options);
Expand Down
12 changes: 6 additions & 6 deletions src/middleware/README.md → src/middleware-legacy/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -3,15 +3,15 @@
If you only need the middleware with access to the `.sign()`, `.verify()` or the receiver’s `.receive()` method, you can use the webhooks middleware directly

```js
const { createMiddleware } = require('@octokit/webhooks')
const { createMiddleware } = require("@octokit/webhooks");
const middleware = createMiddleware({
secret: 'mysecret',
path: '/github-webhooks'
})
secret: "mysecret",
path: "/github-webhooks",
});

middleware.on('installation', asyncInstallationHook)
middleware.on("installation", asyncInstallationHook);

require('http').createServer(middleware).listen(3000)
require("http").createServer(middleware).listen(3000);
```

## API
Expand Down
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
12 changes: 12 additions & 0 deletions src/middleware/node/get-missing-headers.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
import { IncomingMessage } from "http";

const WEBHOOK_HEADERS = [
"x-github-event",
"x-hub-signature-256",
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I intentionally left out the x-hub-signature header here, we want to nudge users to use best practices. GHES support is good enough for the new header

"x-github-delivery",
];

// https://docs.github.com/en/developers/webhooks-and-events/webhook-events-and-payloads#delivery-headers
export function getMissingHeaders(request: IncomingMessage) {
return WEBHOOK_HEADERS.filter((header) => !(header in request.headers));
}
36 changes: 36 additions & 0 deletions src/middleware/node/get-payload.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
import { WebhookEvent } from "@octokit/webhooks-definitions/schema";
// @ts-ignore to address #245
import AggregateError from "aggregate-error";
import { IncomingMessage } from "http";

declare module "http" {
interface IncomingMessage {
body?: WebhookEvent;
}
}

export function getPayload(request: IncomingMessage): Promise<WebhookEvent> {
// If request.body already exists we can stop here
// See https://github.com/octokit/webhooks.js/pull/23

if (request.body) return Promise.resolve(request.body);

return new Promise((resolve, reject) => {
let data = "";

request.setEncoding("utf8");

// istanbul ignore next
request.on("error", (error) => reject(new AggregateError([error])));
request.on("data", (chunk) => (data += chunk));
request.on("end", () => {
try {
resolve(JSON.parse(data));
} catch (error) {
error.message = "Invalid JSON";
error.status = 400;
reject(new AggregateError([error]));
}
});
});
}
20 changes: 20 additions & 0 deletions src/middleware/node/index.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
import { createLogger } from "../../createLogger";
import { Webhooks } from "../../index";
import { middleware } from "./middleware";
import { onUnhandledRequestDefault } from "./on-unhandled-request-default";
import { MiddlewareOptions } from "./types";

export function createNodeMiddleware(
webhooks: Webhooks,
{
path = "/api/github/webhooks",
onUnhandledRequest = onUnhandledRequestDefault,
log = createLogger(),
}: MiddlewareOptions = {}
) {
return middleware.bind(null, webhooks, {
path,
onUnhandledRequest,
log,
} as Required<MiddlewareOptions>);
}
81 changes: 81 additions & 0 deletions src/middleware/node/middleware.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,81 @@
import { IncomingMessage, ServerResponse } from "http";

import { WebhookEventName } from "@octokit/webhooks-definitions/schema";

import { Webhooks } from "../../index";
import { WebhookEventHandlerError } from "../../types";
import { MiddlewareOptions } from "./types";
import { onUnhandledRequestDefault } from "./on-unhandled-request-default";
import { getMissingHeaders } from "./get-missing-headers";
import { getPayload } from "./get-payload";

export async function middleware(
webhooks: Webhooks,
options: Required<MiddlewareOptions>,
request: IncomingMessage,
response: ServerResponse,
next?: Function
) {
const { pathname } = new URL(request.url as string, "http://localhost");

const isUnknownRoute = request.method !== "POST" || pathname !== options.path;
const isExpressMiddleware = typeof next === "function";
if (!isExpressMiddleware && isUnknownRoute) {
options.log.debug(`not found: ${request.method} ${request.url}`);
return onUnhandledRequestDefault(request, response);
}

const missingHeaders = getMissingHeaders(request).join(", ");

if (missingHeaders) {
response.writeHead(400, {
"content-type": "application/json",
});
response.end(
JSON.stringify({
error: `Required headers missing: ${missingHeaders}`,
})
);

return;
}

const eventName = request.headers["x-github-event"] as WebhookEventName;
const signatureSHA256 = request.headers["x-hub-signature-256"] as string;
const id = request.headers["x-github-delivery"] as string;

options.log.debug(`${eventName} event received (id: ${id})`);

// GitHub will abort the request if it does not receive a response within 10s
// See https://github.com/octokit/webhooks.js/issues/185
let didTimeout = false;
const timeout = setTimeout(() => {
didTimeout = true;
response.statusCode = 202;
response.end("still processing\n");
}, 9000).unref();

try {
const payload = await getPayload(request);

await webhooks.verifyAndReceive({
id: id,
name: eventName as any,
payload: payload as any,
signature: signatureSHA256,
});
clearTimeout(timeout);

if (didTimeout) return;

response.end("ok\n");
} catch (error) {
clearTimeout(timeout);

if (didTimeout) return;

const statusCode = Array.from(error as WebhookEventHandlerError)[0].status;
response.statusCode = typeof statusCode !== "undefined" ? statusCode : 500;
response.end(error.toString());
}
}
15 changes: 15 additions & 0 deletions src/middleware/node/on-unhandled-request-default.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
import { IncomingMessage, ServerResponse } from "http";

export function onUnhandledRequestDefault(
request: IncomingMessage,
response: ServerResponse
) {
response.writeHead(404, {
"content-type": "application/json",
});
response.end(
JSON.stringify({
error: `Unknown route: ${request.method} ${request.url}`,
})
);
}
12 changes: 12 additions & 0 deletions src/middleware/node/types.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
import { IncomingMessage, ServerResponse } from "http";

import { Logger } from "../../createLogger";

export type MiddlewareOptions = {
path?: string;
log?: Logger;
onUnhandledRequest?: (
request: IncomingMessage,
response: ServerResponse
) => void;
};
1 change: 1 addition & 0 deletions src/verify/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ export function verify(

const signatureBuffer = Buffer.from(signature);
const algorithm = getAlgorithm(signature);

const verificationBuffer = Buffer.from(
sign({ secret, algorithm }, eventPayload)
);
Expand Down
2 changes: 1 addition & 1 deletion test/integration/middleware-test.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import { EventEmitter } from "events";
import { Buffer } from "buffer";
import { createMiddleware } from "../../src/middleware";
import { createMiddleware } from "../../src/middleware-legacy";

enum RequestMethodType {
POST = "POST",
Expand Down
Loading