diff --git a/README.md b/README.md index 1910373c7..75e2422ae 100644 --- a/README.md +++ b/README.md @@ -321,6 +321,7 @@ Middleware for Node's built in http server or [`express`](https://expressjs.com/ ```js const { App, createNodeMiddleware } = require("@octokit/app"); + const app = new App({ appId: 123, privateKey: "-----BEGIN PRIVATE KEY-----\n...", @@ -334,11 +335,20 @@ const app = new App({ }); const middleware = createNodeMiddleware(app); - -require("http").createServer(middleware).listen(3000); +require("http") + .createServer(async (req, res) => { + // `middleware` returns `false` when `req` is unhandled (beyond `/api/github`) + if (await middleware(req, res)) return; + res.writeHead(404); + res.end(); + }) + .listen(3000); // can now receive user authorization callbacks at /api/github/* ``` +The middleware returned from `createNodeMiddleware` can also serve as an +`Express.js` middleware directly. + @@ -391,31 +401,6 @@ Used for internal logging. Defaults to [`console`](https://developer.mozilla.org - - - -
- options.onUnhandledRequest - - function - - -Defaults to - -```js -function onUnhandledRequest(request, response) { - response.writeHead(400, { - "content-type": "application/json", - }); - response.end( - JSON.stringify({ - error: error.message, - }) - ); -} -``` - -
diff --git a/package-lock.json b/package-lock.json index 63e43f029..8e6e8a1bf 100644 --- a/package-lock.json +++ b/package-lock.json @@ -12,10 +12,10 @@ "@octokit/auth-app": "^4.0.0", "@octokit/auth-unauthenticated": "^3.0.0", "@octokit/core": "^4.0.0", - "@octokit/oauth-app": "^4.0.7", + "@octokit/oauth-app": "5.0.0-beta.1", "@octokit/plugin-paginate-rest": "^4.0.0", "@octokit/types": "^7.0.0", - "@octokit/webhooks": "^10.0.0" + "@octokit/webhooks": "11.0.0-beta.1" }, "devDependencies": { "@pika/pack": "^0.3.7", @@ -38,6 +38,43 @@ "node": ">= 14" } }, + "../oauth-app.js": { + "name": "@octokit/oauth-app", + "version": "0.0.0-development", + "extraneous": true, + "license": "MIT", + "dependencies": { + "@octokit/auth-oauth-app": "^5.0.0", + "@octokit/auth-oauth-user": "^2.0.0", + "@octokit/auth-unauthenticated": "^3.0.0", + "@octokit/core": "^4.0.0", + "@octokit/oauth-authorization-url": "^5.0.0", + "@octokit/oauth-methods": "^2.0.0", + "@types/aws-lambda": "^8.10.83", + "fromentries": "^1.3.1", + "universal-user-agent": "^6.0.0" + }, + "devDependencies": { + "@pika/pack": "^0.3.7", + "@pika/plugin-build-node": "^0.9.2", + "@pika/plugin-ts-standard-pkg": "^0.9.2", + "@types/jest": "^28.0.0", + "@types/node": "^16.0.0", + "@types/node-fetch": "^2.5.4", + "express": "^4.17.1", + "fetch-mock": "^9.0.0", + "jest": "^28.0.0", + "nock": "^13.0.0", + "node-fetch": "^2.6.0", + "prettier": "2.7.1", + "semantic-release-plugin-update-version-in-files": "^1.0.0", + "ts-jest": "^28.0.0", + "typescript": "^4.0.2" + }, + "engines": { + "node": ">= 14" + } + }, "node_modules/@ampproject/remapping": { "version": "2.2.0", "resolved": "https://registry.npmjs.org/@ampproject/remapping/-/remapping-2.2.0.tgz", @@ -2726,9 +2763,9 @@ } }, "node_modules/@octokit/oauth-app": { - "version": "4.0.8", - "resolved": "https://registry.npmjs.org/@octokit/oauth-app/-/oauth-app-4.0.8.tgz", - "integrity": "sha512-bPZNtV2KD8x9deUa+gLX1o6IhIMFwt6UlVZNJTnS9zSCbTpnmWzHhrZr86nM3fKOvU/KPyz40zJMGsoLtvaOgw==", + "version": "5.0.0-beta.1", + "resolved": "https://registry.npmjs.org/@octokit/oauth-app/-/oauth-app-5.0.0-beta.1.tgz", + "integrity": "sha512-s3gMhBPIc+CiQX2ktUAwqn6LelT7AUVp448cfnDSqlslbxOumKwbouTK4nrs0Qp3EKAFKyzo4QvWRY/ASRA50g==", "dependencies": { "@octokit/auth-oauth-app": "^5.0.0", "@octokit/auth-oauth-user": "^2.0.0", @@ -2824,9 +2861,9 @@ } }, "node_modules/@octokit/webhooks": { - "version": "10.1.5", - "resolved": "https://registry.npmjs.org/@octokit/webhooks/-/webhooks-10.1.5.tgz", - "integrity": "sha512-sQkxM6l9HdG1vsHFj2T/o8SnCPDDxovcs0rsSd4UR5jJFNPCPIBRmFNVHfM37nncLKuTwIpmMeePphNf1k6Waw==", + "version": "11.0.0-beta.1", + "resolved": "https://registry.npmjs.org/@octokit/webhooks/-/webhooks-11.0.0-beta.1.tgz", + "integrity": "sha512-wwOdpGPKI1TUHPBZxZRqC9eJDPZd/4gu8BRgLXCLULf6F0+FUyQ32UXudsGwNP+1PSJ3E4g8On4gVIUFnjZnsA==", "dependencies": { "@octokit/request-error": "^3.0.0", "@octokit/webhooks-methods": "^3.0.0", @@ -3121,9 +3158,9 @@ } }, "node_modules/@types/aws-lambda": { - "version": "8.10.102", - "resolved": "https://registry.npmjs.org/@types/aws-lambda/-/aws-lambda-8.10.102.tgz", - "integrity": "sha512-BT05v46n9KtSHa9SgGuOvm49eSruJ9utD8iNXpdpuUVYk8wOcqmm1LEzpNRkrXxD0CULc38sdLpk6q3Wa2WOwg==" + "version": "8.10.103", + "resolved": "https://registry.npmjs.org/@types/aws-lambda/-/aws-lambda-8.10.103.tgz", + "integrity": "sha512-mYWsrM5YPmnyJru7kMDX8RYSc486sDqVOP1kUdotthD3YjJ57iTBN3N7MMtL1qdVoPW2YmCnNnWscyidmPe6Gw==" }, "node_modules/@types/babel__core": { "version": "7.1.19", @@ -11214,9 +11251,9 @@ } }, "@octokit/oauth-app": { - "version": "4.0.8", - "resolved": "https://registry.npmjs.org/@octokit/oauth-app/-/oauth-app-4.0.8.tgz", - "integrity": "sha512-bPZNtV2KD8x9deUa+gLX1o6IhIMFwt6UlVZNJTnS9zSCbTpnmWzHhrZr86nM3fKOvU/KPyz40zJMGsoLtvaOgw==", + "version": "5.0.0-beta.1", + "resolved": "https://registry.npmjs.org/@octokit/oauth-app/-/oauth-app-5.0.0-beta.1.tgz", + "integrity": "sha512-s3gMhBPIc+CiQX2ktUAwqn6LelT7AUVp448cfnDSqlslbxOumKwbouTK4nrs0Qp3EKAFKyzo4QvWRY/ASRA50g==", "requires": { "@octokit/auth-oauth-app": "^5.0.0", "@octokit/auth-oauth-user": "^2.0.0", @@ -11291,9 +11328,9 @@ } }, "@octokit/webhooks": { - "version": "10.1.5", - "resolved": "https://registry.npmjs.org/@octokit/webhooks/-/webhooks-10.1.5.tgz", - "integrity": "sha512-sQkxM6l9HdG1vsHFj2T/o8SnCPDDxovcs0rsSd4UR5jJFNPCPIBRmFNVHfM37nncLKuTwIpmMeePphNf1k6Waw==", + "version": "11.0.0-beta.1", + "resolved": "https://registry.npmjs.org/@octokit/webhooks/-/webhooks-11.0.0-beta.1.tgz", + "integrity": "sha512-wwOdpGPKI1TUHPBZxZRqC9eJDPZd/4gu8BRgLXCLULf6F0+FUyQ32UXudsGwNP+1PSJ3E4g8On4gVIUFnjZnsA==", "requires": { "@octokit/request-error": "^3.0.0", "@octokit/webhooks-methods": "^3.0.0", @@ -11531,9 +11568,9 @@ } }, "@types/aws-lambda": { - "version": "8.10.102", - "resolved": "https://registry.npmjs.org/@types/aws-lambda/-/aws-lambda-8.10.102.tgz", - "integrity": "sha512-BT05v46n9KtSHa9SgGuOvm49eSruJ9utD8iNXpdpuUVYk8wOcqmm1LEzpNRkrXxD0CULc38sdLpk6q3Wa2WOwg==" + "version": "8.10.103", + "resolved": "https://registry.npmjs.org/@types/aws-lambda/-/aws-lambda-8.10.103.tgz", + "integrity": "sha512-mYWsrM5YPmnyJru7kMDX8RYSc486sDqVOP1kUdotthD3YjJ57iTBN3N7MMtL1qdVoPW2YmCnNnWscyidmPe6Gw==" }, "@types/babel__core": { "version": "7.1.19", diff --git a/package.json b/package.json index fa3ebd021..93b48bf5d 100644 --- a/package.json +++ b/package.json @@ -21,10 +21,10 @@ "@octokit/auth-app": "^4.0.0", "@octokit/auth-unauthenticated": "^3.0.0", "@octokit/core": "^4.0.0", - "@octokit/oauth-app": "^4.0.7", + "@octokit/oauth-app": "5.0.0-beta.1", "@octokit/plugin-paginate-rest": "^4.0.0", "@octokit/types": "^7.0.0", - "@octokit/webhooks": "^10.0.0" + "@octokit/webhooks": "11.0.0-beta.1" }, "devDependencies": { "@pika/pack": "^0.3.7", diff --git a/src/middleware/node/index.ts b/src/middleware/node/index.ts index f71d3bd79..7a152bdbb 100644 --- a/src/middleware/node/index.ts +++ b/src/middleware/node/index.ts @@ -4,20 +4,19 @@ type IncomingMessage = any; type ServerResponse = any; -import { createNodeMiddleware as oauthNodeMiddleware } from "@octokit/oauth-app"; +import { + createNodeMiddleware as oauthNodeMiddleware, + sendNodeResponse, + unknownRouteResponse, +} from "@octokit/oauth-app"; import { createNodeMiddleware as webhooksNodeMiddleware } from "@octokit/webhooks"; import { App } from "../../index"; -import { onUnhandledRequestDefault } from "./on-unhandled-request-default"; import { Options } from "../../types"; export type MiddlewareOptions = { pathPrefix?: string; log?: Options["log"]; - onUnhandledRequest?: ( - request: IncomingMessage, - response: ServerResponse - ) => void; }; function noop() {} @@ -37,7 +36,6 @@ export function createNodeMiddleware( ); const optionsWithDefaults = { - onUnhandledRequest: onUnhandledRequestDefault, pathPrefix: "/api/github", ...options, log, @@ -46,41 +44,41 @@ export function createNodeMiddleware( const webhooksMiddleware = webhooksNodeMiddleware(app.webhooks, { path: optionsWithDefaults.pathPrefix + "/webhooks", log, - onUnhandledRequest: optionsWithDefaults.onUnhandledRequest, }); const oauthMiddleware = oauthNodeMiddleware(app.oauth, { pathPrefix: optionsWithDefaults.pathPrefix + "/oauth", - onUnhandledRequest: optionsWithDefaults.onUnhandledRequest, }); - return middleware.bind(null, optionsWithDefaults, { + return middleware.bind( + null, + optionsWithDefaults.pathPrefix, webhooksMiddleware, - oauthMiddleware, - }); + oauthMiddleware + ); } export async function middleware( - options: Required, - { webhooksMiddleware, oauthMiddleware }: any, + pathPrefix: string, + webhooksMiddleware: any, + oauthMiddleware: any, request: IncomingMessage, response: ServerResponse, next?: Function -) { +): Promise { const { pathname } = new URL(request.url as string, "http://localhost"); - if (pathname === `${options.pathPrefix}/webhooks`) { - return webhooksMiddleware(request, response, next); - } - if (pathname.startsWith(`${options.pathPrefix}/oauth/`)) { - return oauthMiddleware(request, response, next); + if (pathname.startsWith(`${pathPrefix}/`)) { + if (pathname === `${pathPrefix}/webhooks`) { + webhooksMiddleware(request, response); + } else if (pathname.startsWith(`${pathPrefix}/oauth/`)) { + oauthMiddleware(request, response); + } else { + sendNodeResponse(unknownRouteResponse(request), response); + } + return true; + } else { + next?.(); + return false; } - - const isExpressMiddleware = typeof next === "function"; - if (isExpressMiddleware) { - // @ts-ignore `next` must be a function as we check two lines above - return next(); - } - - return options.onUnhandledRequest(request, response); } diff --git a/src/middleware/node/on-unhandled-request-default.ts b/src/middleware/node/on-unhandled-request-default.ts deleted file mode 100644 index 3664190fc..000000000 --- a/src/middleware/node/on-unhandled-request-default.ts +++ /dev/null @@ -1,19 +0,0 @@ -// 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 function onUnhandledRequestDefault( - request: IncomingMessage, - response: ServerResponse -) { - response.writeHead(404, { - "content-type": "application/json", - }); - response.end( - JSON.stringify({ - error: `Unknown route: ${request.method} ${request.url}`, - }) - ); -} diff --git a/test/node-middleware.test.ts b/test/node-middleware.test.ts index cdd45a1c8..a218f5112 100644 --- a/test/node-middleware.test.ts +++ b/test/node-middleware.test.ts @@ -36,30 +36,99 @@ x//0u+zd/R/QRUzLOw4N72/Hu+UG6MNt5iDZFCtapRaKt6OvSBwy8w== -----END RSA PRIVATE KEY-----`; describe("createNodeMiddleware()", () => { - test("unknown route", async () => { - const app = new App({ - appId: APP_ID, - privateKey: PRIVATE_KEY, - webhooks: { - secret: "mysecret", - }, - oauth: { - clientId: "", - clientSecret: "", - }, + test("unknown route (outside pathPrefix)", async () => { + const middleware = createNodeMiddleware({} as unknown as App); + const server = createServer(async (req, res) => { + if (!(await middleware(req, res))) { + res.writeHead(404, { "Content-Type": "text/plain" }); + res.write("Not found."); + res.end(); + } + }).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}/unknown-route`); + + expect(response.status).toEqual(404); + expect(response.headers.get("Content-Type")).toBe("text/plain"); + await expect(response.text()).resolves.toBe("Not found."); + + server.close(); + }); + + test("unknown route (within pathPrefix)", async () => { + const middleware = createNodeMiddleware({} as unknown as App); + const server = createServer(async (req, res) => { + if (!(await middleware(req, res))) { + res.writeHead(404, { "Content-Type": "text/plain" }); + res.write("Not found."); + res.end(); + } + }).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/unknown-route` + ); + + expect(response.status).toEqual(404); + expect(response.headers.get("Content-Type")).toBe("application/json"); + await expect(response.json()).resolves.toEqual({ + error: "Unknown route: GET /api/github/unknown-route", }); - const server = createServer(createNodeMiddleware(app)).listen(); + server.close(); + }); + + test("unknown route (within webhooks path)", async () => { + const middleware = createNodeMiddleware({} as unknown as App); + const server = createServer(async (req, res) => { + if (!(await middleware(req, res))) { + res.writeHead(404, { "Content-Type": "text/plain" }); + res.write("Not found."); + res.end(); + } + }).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}/unknown-route`); + const response = await fetch( + `http://localhost:${port}/api/github/webhooks` + ); expect(response.status).toEqual(404); - await expect(response.text()).resolves.toMatch( - /Unknown route: GET \/unknown-route/ + expect(response.headers.get("Content-Type")).toBe("application/json"); + await expect(response.json()).resolves.toEqual({ + error: "Unknown route: GET /api/github/webhooks", + }); + + server.close(); + }); + + test("unknown route (within oauth pathPrefix)", async () => { + const middleware = createNodeMiddleware({} as unknown as App); + const server = createServer(async (req, res) => { + if (!(await middleware(req, res))) { + res.writeHead(404, { "Content-Type": "text/plain" }); + res.write("Not found."); + res.end(); + } + }).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/oauth/unknown-route` ); + expect(response.status).toEqual(404); + expect(response.headers.get("Content-Type")).toBe("application/json"); + await expect(response.json()).resolves.toEqual({ + error: "Unknown route: GET /api/github/oauth/unknown-route", + }); + server.close(); });