diff --git a/src/middleware/node/middleware.ts b/src/middleware/node/middleware.ts index b4391260f..f7494aa9b 100644 --- a/src/middleware/node/middleware.ts +++ b/src/middleware/node/middleware.ts @@ -11,6 +11,25 @@ export async function middleware( request: IncomingMessage, response: ServerResponse ) { + // request.url mayb include ?query parameters which we don't want for `route` + // hence the workaround using new URL() + const { pathname } = new URL(request.url as string, "http://localhost"); + const route = [request.method, pathname].join(" "); + const routes = { + getLogin: `GET ${options.pathPrefix}/login`, + getCallback: `GET ${options.pathPrefix}/callback`, + createToken: `POST ${options.pathPrefix}/token`, + getToken: `GET ${options.pathPrefix}/token`, + patchToken: `PATCH ${options.pathPrefix}/token`, + deleteToken: `DELETE ${options.pathPrefix}/token`, + deleteGrant: `DELETE ${options.pathPrefix}/grant`, + }; + + if (!Object.values(routes).includes(route)) { + options.onUnhandledRequest(request, response); + return; + } + let parsedRequest; try { parsedRequest = await parseRequest(request); @@ -24,10 +43,10 @@ export async function middleware( }) ); } - const { route, headers, query, body } = parsedRequest; + const { headers, query, body } = parsedRequest; try { - if (route === `GET ${options.pathPrefix}/login`) { + if (route === routes.getLogin) { const url = app.getAuthorizationUrl({ state: query.state, scopes: query.scopes?.split(","), @@ -39,7 +58,7 @@ export async function middleware( return response.end(); } - if (route === `GET ${options.pathPrefix}/callback`) { + if (route === routes.getCallback) { if (query.error) { throw new Error( `[@octokit/oauth-app] ${query.error} ${query.error_description}` @@ -66,7 +85,7 @@ export async function middleware( return response.end(); } - if (route === `POST ${options.pathPrefix}/token`) { + if (route === routes.createToken) { // @ts-ignore body is guaraenteed to exist const { state: oauthState, code, redirectUrl } = body; @@ -88,7 +107,7 @@ export async function middleware( return response.end(JSON.stringify({ token, scopes })); } - if (route === `GET ${options.pathPrefix}/token`) { + if (route === routes.getToken) { const token = headers.authorization?.substr("token ".length); if (!token) { @@ -107,7 +126,7 @@ export async function middleware( return response.end(JSON.stringify(result)); } - if (route === `PATCH ${options.pathPrefix}/token`) { + if (route === routes.patchToken) { const token = headers.authorization?.substr("token ".length); if (!token) { @@ -126,7 +145,7 @@ export async function middleware( return response.end(JSON.stringify(result)); } - if (route === `DELETE ${options.pathPrefix}/token`) { + if (route === routes.deleteToken) { const token = headers.authorization?.substr("token ".length) as string; if (!token) { @@ -143,24 +162,21 @@ export async function middleware( return response.end(); } - if (route === `DELETE ${options.pathPrefix}/grant`) { - const token = headers.authorization?.substr("token ".length) as string; - - if (!token) { - throw new Error( - '[@octokit/oauth-app] "Authorization" header is required' - ); - } - - await app.deleteAuthorization({ - token, - }); + // route === routes.deleteGrant + const token = headers.authorization?.substr("token ".length) as string; - response.writeHead(204); - return response.end(); + if (!token) { + throw new Error( + '[@octokit/oauth-app] "Authorization" header is required' + ); } - options.onUnhandledRequest(request, response); + await app.deleteAuthorization({ + token, + }); + + response.writeHead(204); + return response.end(); } catch (error) { response.writeHead(400, { "content-type": "application/json", diff --git a/src/middleware/node/parse-request.ts b/src/middleware/node/parse-request.ts index 7baa03b0b..0fad90ce8 100644 --- a/src/middleware/node/parse-request.ts +++ b/src/middleware/node/parse-request.ts @@ -4,7 +4,6 @@ import { IncomingMessage } from "http"; import fromEntries from "fromentries"; type ParsedRequest = { - route: string; headers: { authorization?: string; }; @@ -28,17 +27,13 @@ type ParsedRequest = { export async function parseRequest( request: IncomingMessage ): Promise { - const { pathname, searchParams } = new URL( - request.url as string, - "http://localhost" - ); - const route = [request.method, pathname].join(" "); + const { searchParams } = new URL(request.url as string, "http://localhost"); const query = fromEntries(searchParams); const headers = request.headers; if (!["POST", "PATCH"].includes(request.method as string)) { - return { route, headers, query }; + return { headers, query }; } return new Promise((resolve, reject) => { @@ -48,10 +43,10 @@ export async function parseRequest( .on("data", (chunk) => bodyChunks.push(chunk)) .on("end", async () => { const bodyString = Buffer.concat(bodyChunks).toString(); - if (!bodyString) return resolve({ route, headers, query }); + if (!bodyString) return resolve({ headers, query }); try { - resolve({ route, headers, query, body: JSON.parse(bodyString) }); + resolve({ headers, query, body: JSON.parse(bodyString) }); } catch (error) { reject(error); } diff --git a/test/node-middleware.test.ts b/test/node-middleware.test.ts index 34bf1a52b..4f86edb0b 100644 --- a/test/node-middleware.test.ts +++ b/test/node-middleware.test.ts @@ -1,4 +1,4 @@ -import { createServer } from "http"; +import { createServer, IncomingMessage, ServerResponse } from "http"; import { URL } from "url"; import fetch from "node-fetch"; @@ -299,6 +299,47 @@ describe("getNodeMiddleware(app)", () => { }); }); + it("POST /unrelated", async () => { + expect.assertions(4); + + const app = new OAuthApp({ + clientId: "0123", + clientSecret: "0123secret", + }); + + const server = createServer( + getNodeMiddleware(app, { + onUnhandledRequest: (request, response) => { + expect(request.method).toEqual("POST"); + expect(request.url).toEqual("/unrelated"); + + // test that the request has not yet been consumed with .on("data") + expect(request.complete).toEqual(false); + + response.writeHead(200); + response.end(); + }, + }) + ).listen(); + // @ts-ignore complains about { port } although it's included in returned AddressInfo interface + const { port } = server.address(); + + const { status, headers } = await fetch( + `http://localhost:${port}/unrelated`, + { + method: "POST", + body: JSON.stringify({ ok: true }), + headers: { + "content-type": "application/json", + }, + } + ); + + server.close(); + + expect(status).toEqual(200); + }); + // errors it("GET /unknown", async () => {