From e6baaf192e5411a380996000e3bf4926811e6cc3 Mon Sep 17 00:00:00 2001 From: Baoshan Sheng Date: Wed, 7 Sep 2022 10:45:13 +0800 Subject: [PATCH 1/2] refactor: deprecate onUnhandledRequest middleware option --- README.md | 6 +-- src/index.ts | 11 ++++++ src/middleware/aws-lambda/api-gateway-v2.ts | 12 ++++-- src/middleware/node/index.ts | 10 ++++- src/middleware/web-worker/index.ts | 10 ++++- test/deprecations.test.ts | 41 ++++++++++++++++++++- test/smoke.test.ts | 6 ++- 7 files changed, 84 insertions(+), 12 deletions(-) diff --git a/README.md b/README.md index c3898ed7d..ce887fc1b 100644 --- a/README.md +++ b/README.md @@ -939,7 +939,7 @@ All exposed paths will be prefixed with the provided prefix. Defaults to `"/api/ - options.onUnhandledRequest + options.onUnhandledRequest __deprecated__ function @@ -1029,7 +1029,7 @@ All exposed paths will be prefixed with the provided prefix. Defaults to `"/api/ - options.onUnhandledRequest + options.onUnhandledRequest __deprecated__ function @@ -1119,7 +1119,7 @@ All exposed paths will be prefixed with the provided prefix. Defaults to `"/api/ - options.onUnhandledRequest + options.onUnhandledRequest __deprecated__ function diff --git a/src/index.ts b/src/index.ts index a129bb2ba..353d4de5b 100644 --- a/src/index.ts +++ b/src/index.ts @@ -50,6 +50,17 @@ import type { Options, State, } from "./types"; + +// types required by external handlers (aws-lambda, etc) +export type { + HandlerOptions, + OctokitRequest, + OctokitResponse, +} from "./middleware/types"; + +// generic handlers +export { handleRequest } from "./middleware/handle-request"; + export { createNodeMiddleware } from "./middleware/node/index"; export { createCloudflareHandler, diff --git a/src/middleware/aws-lambda/api-gateway-v2.ts b/src/middleware/aws-lambda/api-gateway-v2.ts index 873c9ae05..7eea54ac2 100644 --- a/src/middleware/aws-lambda/api-gateway-v2.ts +++ b/src/middleware/aws-lambda/api-gateway-v2.ts @@ -4,7 +4,7 @@ import { handleRequest } from "../handle-request"; import { onUnhandledRequestDefault } from "../on-unhandled-request-default"; import { HandlerOptions } from "../types"; import { OAuthApp } from "../../index"; -import { Options, ClientType } from "../../types"; +import { ClientType, Options } from "../../types"; import type { APIGatewayProxyEventV2, APIGatewayProxyStructuredResultV2, @@ -22,16 +22,22 @@ export function createAWSLambdaAPIGatewayV2Handler( app: OAuthApp>, { pathPrefix, - onUnhandledRequest = onUnhandledRequestDefaultAWSAPIGatewayV2, + onUnhandledRequest, }: HandlerOptions & { onUnhandledRequest?: ( event: APIGatewayProxyEventV2 ) => Promise; } = {} ) { + if (onUnhandledRequest) { + app.octokit.log.warn( + "[@octokit/oauth-app] `onUnhandledRequest` is deprecated and will be removed from the next major version." + ); + } + onUnhandledRequest ??= onUnhandledRequestDefaultAWSAPIGatewayV2; return async function (event: APIGatewayProxyEventV2) { const request = parseRequest(event); const response = await handleRequest(app, { pathPrefix }, request); - return response ? sendResponse(response) : onUnhandledRequest(event); + return response ? sendResponse(response) : onUnhandledRequest!(event); }; } diff --git a/src/middleware/node/index.ts b/src/middleware/node/index.ts index 9bc05f71f..d152ede38 100644 --- a/src/middleware/node/index.ts +++ b/src/middleware/node/index.ts @@ -25,7 +25,7 @@ export function createNodeMiddleware( app: OAuthApp>, { pathPrefix, - onUnhandledRequest = onUnhandledRequestDefaultNode, + onUnhandledRequest, }: HandlerOptions & { onUnhandledRequest?: ( request: IncomingMessage, @@ -33,6 +33,12 @@ export function createNodeMiddleware( ) => void; } = {} ) { + if (onUnhandledRequest) { + app.octokit.log.warn( + "[@octokit/oauth-app] `onUnhandledRequest` is deprecated and will be removed from the next major version." + ); + } + onUnhandledRequest ??= onUnhandledRequestDefaultNode; return async function ( request: IncomingMessage, response: ServerResponse, @@ -50,7 +56,7 @@ export function createNodeMiddleware( } else if (typeof next === "function") { next(); } else { - onUnhandledRequest(request, response); + onUnhandledRequest!(request, response); } }; } diff --git a/src/middleware/web-worker/index.ts b/src/middleware/web-worker/index.ts index 68511336a..8e91975b3 100644 --- a/src/middleware/web-worker/index.ts +++ b/src/middleware/web-worker/index.ts @@ -18,11 +18,17 @@ export function createWebWorkerHandler>( app: OAuthApp, { pathPrefix, - onUnhandledRequest = onUnhandledRequestDefaultWebWorker, + onUnhandledRequest, }: HandlerOptions & { onUnhandledRequest?: (request: Request) => Response | Promise; } = {} ) { + if (onUnhandledRequest) { + app.octokit.log.warn( + "[@octokit/oauth-app] `onUnhandledRequest` is deprecated and will be removed from the next major version." + ); + } + onUnhandledRequest ??= onUnhandledRequestDefaultWebWorker; return async function (request: Request): Promise { const octokitRequest = parseRequest(request); const octokitResponse = await handleRequest( @@ -32,7 +38,7 @@ export function createWebWorkerHandler>( ); return octokitResponse ? sendResponse(octokitResponse) - : await onUnhandledRequest(request); + : await onUnhandledRequest!(request); }; } diff --git a/test/deprecations.test.ts b/test/deprecations.test.ts index 7d9c9e871..6b521895d 100644 --- a/test/deprecations.test.ts +++ b/test/deprecations.test.ts @@ -1,7 +1,13 @@ import { URL } from "url"; import * as nodeFetch from "node-fetch"; import fromEntries from "fromentries"; -import { createCloudflareHandler, OAuthApp } from "../src"; +import { + createAWSLambdaAPIGatewayV2Handler, + createCloudflareHandler, + createNodeMiddleware, + createWebWorkerHandler, + OAuthApp, +} from "../src"; import { Octokit } from "@octokit/core"; describe("deprecations", () => { @@ -52,4 +58,37 @@ describe("deprecations", () => { expect(url.searchParams.get("state")).toMatch(/^\w+$/); expect(url.searchParams.get("scope")).toEqual(null); }); + + it("`onUnhandledRequest` is deprecated and will be removed from the next major version", async () => { + const warn = jest.fn().mockResolvedValue(undefined); + const handleRequest = createAWSLambdaAPIGatewayV2Handler( + new OAuthApp({ + clientType: "github-app", + clientId: "client_id_123", + clientSecret: "client_secret_456", + Octokit: Octokit.defaults({ + log: { + debug: () => undefined, + info: () => undefined, + warn, + error: () => undefined, + }, + }), + }), + { + onUnhandledRequest: async (request) => { + return { + statusCode: 404, + headers: {}, + body: "", + }; + }, + } + ); + + expect(warn.mock.calls.length).toEqual(1); + expect(warn.mock.calls[0][0]).toEqual( + "[@octokit/oauth-app] `onUnhandledRequest` is deprecated and will be removed from the next major version." + ); + }); }); diff --git a/test/smoke.test.ts b/test/smoke.test.ts index 2cb6c4ec9..4d2264c5a 100644 --- a/test/smoke.test.ts +++ b/test/smoke.test.ts @@ -1,4 +1,4 @@ -import { OAuthApp } from "../src"; +import { handleRequest, OAuthApp } from "../src"; describe("Smoke test", () => { it("OAuthApp is a function", () => { @@ -12,4 +12,8 @@ describe("Smoke test", () => { it("OAuthApp.VERSION is set", () => { expect(OAuthApp.VERSION).toEqual("0.0.0-development"); }); + + it("handleRequest is a function", () => { + expect(typeof handleRequest).toEqual("function"); + }); }); From f14ff6dbfc9242016a8aaf4e713cd0a5240e64b6 Mon Sep 17 00:00:00 2001 From: Baoshan Sheng Date: Wed, 7 Sep 2022 11:05:02 +0800 Subject: [PATCH 2/2] refactor!: remove onUnhandledRequest middleware option BREAKING CHANGE: drop onUnhandledRequest middleware option support --- README.md | 75 --------------- src/index.ts | 7 +- src/middleware/README.md | 4 +- src/middleware/aws-lambda/api-gateway-v2.ts | 38 ++------ src/middleware/handle-request.ts | 30 +++--- src/middleware/node/index.ts | 47 ++-------- ...t-default.ts => unknown-route-response.ts} | 6 +- src/middleware/web-worker/index.ts | 52 ++--------- test/aws-lambda-api-gateway-v2.test.ts | 34 +++++-- test/deprecations.test.ts | 79 +--------------- test/node-middleware.test.ts | 93 +++++++++---------- test/smoke.test.ts | 15 ++- test/web-worker-handler.test.ts | 76 +++++++-------- 13 files changed, 166 insertions(+), 390 deletions(-) rename src/middleware/{on-unhandled-request-default.ts => unknown-route-response.ts} (55%) diff --git a/README.md b/README.md index ce887fc1b..fb61b8f32 100644 --- a/README.md +++ b/README.md @@ -937,31 +937,6 @@ All exposed paths will be prefixed with the provided prefix. Defaults to `"/api/ - - - options.onUnhandledRequest __deprecated__ - - - function - - - -Defaults to - -```js -function onUnhandledRequest(request, response) { - response.writeHead(404, { - "content-type": "application/json", - }); - response.end( - JSON.stringify({ - error: `Unknown route: ${request.method} ${request.url}`, - }) - ); -} -``` - - @@ -1025,31 +1000,6 @@ addEventListener("fetch", (event) => { All exposed paths will be prefixed with the provided prefix. Defaults to `"/api/github/oauth"` - - - - - options.onUnhandledRequest __deprecated__ - - - function - - Defaults to - -```js -function onUnhandledRequest(request) { - return new Response( - JSON.stringify({ - error: `Unknown route: ${request.method} ${request.url}`, - }), - { - status: 404, - headers: { "content-type": "application/json" }, - } - ); -} -``` - @@ -1115,31 +1065,6 @@ export const handler = createAWSLambdaAPIGatewayV2Handler(app, { All exposed paths will be prefixed with the provided prefix. Defaults to `"/api/github/oauth"` - - - - - options.onUnhandledRequest __deprecated__ - - - function - - Defaults to returns: - -```js -function onUnhandledRequest(request) { - return - { - status: 404, - headers: { "content-type": "application/json" }, - body: JSON.stringify({ - error: `Unknown route: [METHOD] [URL]`, - }) - } - ); -} -``` - diff --git a/src/index.ts b/src/index.ts index 353d4de5b..b38eb0bea 100644 --- a/src/index.ts +++ b/src/index.ts @@ -60,12 +60,11 @@ export type { // generic handlers export { handleRequest } from "./middleware/handle-request"; +export { unknownRouteResponse } from "./middleware/unknown-route-response"; export { createNodeMiddleware } from "./middleware/node/index"; -export { - createCloudflareHandler, - createWebWorkerHandler, -} from "./middleware/web-worker/index"; +export { sendResponse as sendNodeResponse } from "./middleware/node/send-response"; +export { createWebWorkerHandler } from "./middleware/web-worker/index"; export { createAWSLambdaAPIGatewayV2Handler } from "./middleware/aws-lambda/api-gateway-v2"; type Constructor = new (...args: any[]) => T; diff --git a/src/middleware/README.md b/src/middleware/README.md index d08c0ba08..5f148e838 100644 --- a/src/middleware/README.md +++ b/src/middleware/README.md @@ -7,11 +7,9 @@ The `middleware` directory contains the generic HTTP handler. Each sub-directory ``` middleware ├── handle-request.ts -├── on-unhandled-request-default.ts ├── types.ts ├── node/ ├── web-worker/ (Cloudflare Workers & Deno) -└── deno/ (to be implemented) ``` ## Generic HTTP Handler @@ -85,5 +83,5 @@ Implementing an HTTP handler/middleware for a certain environment involves three 2. Write a function to render an `OctokitResponse` object (e.g., as `ServerResponse` in Node.js). See [`node/send-response.ts`](node/send-response.ts) for reference. 3. Expose an HTTP handler/middleware in the dialect of the environment which performs three steps: 1. Parse the HTTP request using (1). - 2. Process the `OctokitRequest` object using `handleRequest`. If the request is not handled by `handleRequest` (the request does not match any predefined route), [`onUnhandledRequestDefault`](on-unhandled-request-default.ts) can be used to generate a `404` response consistently. + 2. Process the `OctokitRequest` object using `handleRequest`. 3. Render the `OctokitResponse` object using (2). diff --git a/src/middleware/aws-lambda/api-gateway-v2.ts b/src/middleware/aws-lambda/api-gateway-v2.ts index 7eea54ac2..d2a9a0844 100644 --- a/src/middleware/aws-lambda/api-gateway-v2.ts +++ b/src/middleware/aws-lambda/api-gateway-v2.ts @@ -1,43 +1,23 @@ import { parseRequest } from "./api-gateway-v2-parse-request"; import { sendResponse } from "./api-gateway-v2-send-response"; import { handleRequest } from "../handle-request"; -import { onUnhandledRequestDefault } from "../on-unhandled-request-default"; -import { HandlerOptions } from "../types"; -import { OAuthApp } from "../../index"; -import { ClientType, Options } from "../../types"; +import type { HandlerOptions } from "../types"; +import type { OAuthApp } from "../../index"; +import type { ClientType, Options } from "../../types"; import type { APIGatewayProxyEventV2, APIGatewayProxyStructuredResultV2, } from "aws-lambda"; -async function onUnhandledRequestDefaultAWSAPIGatewayV2( - event: APIGatewayProxyEventV2 -): Promise { - const request = parseRequest(event); - const response = onUnhandledRequestDefault(request); - return sendResponse(response); -} - export function createAWSLambdaAPIGatewayV2Handler( app: OAuthApp>, - { - pathPrefix, - onUnhandledRequest, - }: HandlerOptions & { - onUnhandledRequest?: ( - event: APIGatewayProxyEventV2 - ) => Promise; - } = {} + options: HandlerOptions = {} ) { - if (onUnhandledRequest) { - app.octokit.log.warn( - "[@octokit/oauth-app] `onUnhandledRequest` is deprecated and will be removed from the next major version." - ); - } - onUnhandledRequest ??= onUnhandledRequestDefaultAWSAPIGatewayV2; - return async function (event: APIGatewayProxyEventV2) { + return async function ( + event: APIGatewayProxyEventV2 + ): Promise { const request = parseRequest(event); - const response = await handleRequest(app, { pathPrefix }, request); - return response ? sendResponse(response) : onUnhandledRequest!(event); + const response = await handleRequest(app, options, request); + return response ? sendResponse(response) : undefined; }; } diff --git a/src/middleware/handle-request.ts b/src/middleware/handle-request.ts index 671133a3a..8359f2f7b 100644 --- a/src/middleware/handle-request.ts +++ b/src/middleware/handle-request.ts @@ -1,4 +1,5 @@ import { OAuthApp } from "../index"; +import { unknownRouteResponse } from "./unknown-route-response"; import { HandlerOptions, OctokitRequest, OctokitResponse } from "./types"; import { ClientType, Options } from "../types"; // @ts-ignore - requires esModuleInterop flag @@ -8,7 +9,7 @@ export async function handleRequest( app: OAuthApp>, { pathPrefix = "/api/github/oauth" }: HandlerOptions, request: OctokitRequest -): Promise { +): Promise { if (request.method === "OPTIONS") { return { status: 200, @@ -23,23 +24,28 @@ export async function handleRequest( // request.url may 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"); + let { pathname } = new URL(request.url as string, "http://localhost"); + if (!pathname.startsWith(`${pathPrefix}/`)) { + return undefined; + } + pathname = pathname.slice(pathPrefix.length + 1); + const route = [request.method, pathname].join(" "); const routes = { - getLogin: `GET ${pathPrefix}/login`, - getCallback: `GET ${pathPrefix}/callback`, - createToken: `POST ${pathPrefix}/token`, - getToken: `GET ${pathPrefix}/token`, - patchToken: `PATCH ${pathPrefix}/token`, - patchRefreshToken: `PATCH ${pathPrefix}/refresh-token`, - scopeToken: `POST ${pathPrefix}/token/scoped`, - deleteToken: `DELETE ${pathPrefix}/token`, - deleteGrant: `DELETE ${pathPrefix}/grant`, + getLogin: `GET login`, + getCallback: `GET callback`, + createToken: `POST token`, + getToken: `GET token`, + patchToken: `PATCH token`, + patchRefreshToken: `PATCH refresh-token`, + scopeToken: `POST token/scoped`, + deleteToken: `DELETE token`, + deleteGrant: `DELETE grant`, }; // handle unknown routes if (!Object.values(routes).includes(route)) { - return null; + return unknownRouteResponse(request); } let json: any; diff --git a/src/middleware/node/index.ts b/src/middleware/node/index.ts index d152ede38..efd86532a 100644 --- a/src/middleware/node/index.ts +++ b/src/middleware/node/index.ts @@ -6,57 +6,28 @@ type ServerResponse = any; import { parseRequest } from "./parse-request"; import { sendResponse } from "./send-response"; -import { onUnhandledRequestDefault } from "../on-unhandled-request-default"; import { handleRequest } from "../handle-request"; -import { OAuthApp } from "../../index"; -import { HandlerOptions } from "../types"; -import { ClientType, Options } from "../../types"; - -function onUnhandledRequestDefaultNode( - request: IncomingMessage, - response: ServerResponse -) { - const octokitRequest = parseRequest(request); - const octokitResponse = onUnhandledRequestDefault(octokitRequest); - sendResponse(octokitResponse, response); -} +import type { OAuthApp } from "../../index"; +import type { HandlerOptions } from "../types"; +import type { ClientType, Options } from "../../types"; export function createNodeMiddleware( app: OAuthApp>, - { - pathPrefix, - onUnhandledRequest, - }: HandlerOptions & { - onUnhandledRequest?: ( - request: IncomingMessage, - response: ServerResponse - ) => void; - } = {} + options: HandlerOptions = {} ) { - if (onUnhandledRequest) { - app.octokit.log.warn( - "[@octokit/oauth-app] `onUnhandledRequest` is deprecated and will be removed from the next major version." - ); - } - onUnhandledRequest ??= onUnhandledRequestDefaultNode; return async function ( request: IncomingMessage, response: ServerResponse, next?: Function ) { - const octokitRequest = parseRequest(request); - const octokitResponse = await handleRequest( - app, - { pathPrefix }, - octokitRequest - ); - + const octokitRequest = await parseRequest(request); + const octokitResponse = await handleRequest(app, options, octokitRequest); if (octokitResponse) { sendResponse(octokitResponse, response); - } else if (typeof next === "function") { - next(); + return true; } else { - onUnhandledRequest!(request, response); + next?.(); + return false; } }; } diff --git a/src/middleware/on-unhandled-request-default.ts b/src/middleware/unknown-route-response.ts similarity index 55% rename from src/middleware/on-unhandled-request-default.ts rename to src/middleware/unknown-route-response.ts index 14dd11757..12050c025 100644 --- a/src/middleware/on-unhandled-request-default.ts +++ b/src/middleware/unknown-route-response.ts @@ -1,8 +1,6 @@ -import { OctokitRequest, OctokitResponse } from "./types"; +import type { OctokitRequest } from "./types"; -export function onUnhandledRequestDefault( - request: OctokitRequest -): OctokitResponse { +export function unknownRouteResponse(request: OctokitRequest) { return { status: 404, headers: { "content-type": "application/json" }, diff --git a/src/middleware/web-worker/index.ts b/src/middleware/web-worker/index.ts index 8e91975b3..3e7462f4d 100644 --- a/src/middleware/web-worker/index.ts +++ b/src/middleware/web-worker/index.ts @@ -1,53 +1,17 @@ import { parseRequest } from "./parse-request"; import { sendResponse } from "./send-response"; import { handleRequest } from "../handle-request"; -import { onUnhandledRequestDefault } from "../on-unhandled-request-default"; -import { OAuthApp } from "../../index"; -import { HandlerOptions } from "../types"; -import { ClientType, Options } from "../../types"; - -async function onUnhandledRequestDefaultWebWorker( - request: Request -): Promise { - const octokitRequest = parseRequest(request); - const octokitResponse = onUnhandledRequestDefault(octokitRequest); - return sendResponse(octokitResponse); -} +import type { OAuthApp } from "../../index"; +import type { HandlerOptions } from "../types"; +import type { ClientType, Options } from "../../types"; export function createWebWorkerHandler>( app: OAuthApp, - { - pathPrefix, - onUnhandledRequest, - }: HandlerOptions & { - onUnhandledRequest?: (request: Request) => Response | Promise; - } = {} + options: HandlerOptions = {} ) { - if (onUnhandledRequest) { - app.octokit.log.warn( - "[@octokit/oauth-app] `onUnhandledRequest` is deprecated and will be removed from the next major version." - ); - } - onUnhandledRequest ??= onUnhandledRequestDefaultWebWorker; - return async function (request: Request): Promise { - const octokitRequest = parseRequest(request); - const octokitResponse = await handleRequest( - app, - { pathPrefix }, - octokitRequest - ); - return octokitResponse - ? sendResponse(octokitResponse) - : await onUnhandledRequest!(request); + return async function (request: Request): Promise { + const octokitRequest = await parseRequest(request); + const octokitResponse = await handleRequest(app, options, octokitRequest); + return octokitResponse ? sendResponse(octokitResponse) : undefined; }; } - -/** @deprecated */ -export function createCloudflareHandler( - ...args: Parameters -) { - args[0].octokit.log.warn( - "[@octokit/oauth-app] `createCloudflareHandler` is deprecated, use `createWebWorkerHandler` instead" - ); - return createWebWorkerHandler(...args); -} diff --git a/test/aws-lambda-api-gateway-v2.test.ts b/test/aws-lambda-api-gateway-v2.test.ts index 3209f1902..d40c72722 100644 --- a/test/aws-lambda-api-gateway-v2.test.ts +++ b/test/aws-lambda-api-gateway-v2.test.ts @@ -1,4 +1,4 @@ -import { OAuthApp, createAWSLambdaAPIGatewayV2Handler } from "../src/"; +import { createAWSLambdaAPIGatewayV2Handler, OAuthApp } from "../src/"; import { URL } from "url"; import { APIGatewayProxyEventV2 } from "aws-lambda"; @@ -21,7 +21,7 @@ describe("createAWSLambdaAPIGatewayV2Handler(app)", () => { createAWSLambdaAPIGatewayV2Handler(app); }); - it("fail-over to default unhandled request handler", async () => { + it("do not handle request with different prefix", async () => { const appMock = {}; const handleRequest = createAWSLambdaAPIGatewayV2Handler( appMock as unknown as OAuthApp @@ -32,6 +32,20 @@ describe("createAWSLambdaAPIGatewayV2Handler(app)", () => { rawPath: "/prod/unknown", } as APIGatewayProxyEventV2); + expect(response).toBeUndefined(); + }); + + it("fail-over to default unhandled request handler", async () => { + const appMock = {}; + const handleRequest = createAWSLambdaAPIGatewayV2Handler( + appMock as unknown as OAuthApp + ); + + const response = (await handleRequest({ + requestContext: { http: { method: "GET" }, stage: "prod" }, + rawPath: "/prod/api/github/oauth/unknown", + } as APIGatewayProxyEventV2))!; + expect(response.statusCode).toBe(404); }); @@ -39,10 +53,10 @@ describe("createAWSLambdaAPIGatewayV2Handler(app)", () => { const app = new OAuthApp({ clientId: "0123", clientSecret: "0123secret" }); const handleRequest = createAWSLambdaAPIGatewayV2Handler(app); - const response = await handleRequest({ + const response = (await handleRequest({ requestContext: { http: { method: "OPTIONS" }, stage: "prod" }, rawPath: "/prod/api/github/oauth/token", - } as APIGatewayProxyEventV2); + } as APIGatewayProxyEventV2))!; expect(response.statusCode).toStrictEqual(200); expect(response.headers!["access-control-allow-origin"]).toBe("*"); @@ -56,10 +70,10 @@ describe("createAWSLambdaAPIGatewayV2Handler(app)", () => { const app = new OAuthApp({ clientId: "0123", clientSecret: "0123secret" }); const handleRequest = createAWSLambdaAPIGatewayV2Handler(app); - const response = await handleRequest({ + const response = (await handleRequest({ requestContext: { http: { method: "GET" }, stage: "$default" }, rawPath: "/api/github/oauth/login", - } as APIGatewayProxyEventV2); + } as APIGatewayProxyEventV2))!; expect(response.statusCode).toBe(302); const url = new URL(response.headers!.location as string); @@ -74,10 +88,10 @@ describe("createAWSLambdaAPIGatewayV2Handler(app)", () => { const app = new OAuthApp({ clientId: "0123", clientSecret: "0123secret" }); const handleRequest = createAWSLambdaAPIGatewayV2Handler(app); - const response = await handleRequest({ + const response = (await handleRequest({ requestContext: { http: { method: "GET" }, stage: "prod" }, rawPath: "/prod/api/github/oauth/login", - } as APIGatewayProxyEventV2); + } as APIGatewayProxyEventV2))!; expect(response.statusCode).toBe(302); const url = new URL(response.headers!.location as string); @@ -92,11 +106,11 @@ describe("createAWSLambdaAPIGatewayV2Handler(app)", () => { const app = new OAuthApp({ clientId: "0123", clientSecret: "0123secret" }); const handleRequest = createAWSLambdaAPIGatewayV2Handler(app); - const response = await handleRequest({ + const response = (await handleRequest({ requestContext: { http: { method: "GET" }, stage: "prod" }, rawPath: "/prod/api/github/oauth/login", rawQueryString: "state=mystate123&scopes=one,two,three", - } as APIGatewayProxyEventV2); + } as APIGatewayProxyEventV2))!; expect(response.statusCode).toBe(302); const url = new URL(response.headers!.location as string); diff --git a/test/deprecations.test.ts b/test/deprecations.test.ts index 6b521895d..9afaa6400 100644 --- a/test/deprecations.test.ts +++ b/test/deprecations.test.ts @@ -1,14 +1,5 @@ -import { URL } from "url"; import * as nodeFetch from "node-fetch"; import fromEntries from "fromentries"; -import { - createAWSLambdaAPIGatewayV2Handler, - createCloudflareHandler, - createNodeMiddleware, - createWebWorkerHandler, - OAuthApp, -} from "../src"; -import { Octokit } from "@octokit/core"; describe("deprecations", () => { beforeAll(() => { @@ -22,73 +13,5 @@ describe("deprecations", () => { delete (global as any).Response; }); - it("createCloudflareHandler works but logs out deprecation message", async () => { - const warn = jest.fn().mockResolvedValue(undefined); - const handleRequest = createCloudflareHandler( - new OAuthApp({ - clientType: "github-app", - clientId: "client_id_123", - clientSecret: "client_secret_456", - Octokit: Octokit.defaults({ - log: { - debug: () => undefined, - info: () => undefined, - warn, - error: () => undefined, - }, - }), - }) - ); - - expect(warn.mock.calls.length).toEqual(1); - expect(warn.mock.calls[0][0]).toEqual( - "[@octokit/oauth-app] `createCloudflareHandler` is deprecated, use `createWebWorkerHandler` instead" - ); - - const request = new Request("/api/github/oauth/login"); - const { status, headers } = await handleRequest(request); - - expect(status).toEqual(302); - const url = new URL(headers.get("location") as string); - expect(url).toMatchObject({ - origin: "https://github.com", - pathname: "/login/oauth/authorize", - }); - expect(url.searchParams.get("client_id")).toEqual("client_id_123"); - expect(url.searchParams.get("state")).toMatch(/^\w+$/); - expect(url.searchParams.get("scope")).toEqual(null); - }); - - it("`onUnhandledRequest` is deprecated and will be removed from the next major version", async () => { - const warn = jest.fn().mockResolvedValue(undefined); - const handleRequest = createAWSLambdaAPIGatewayV2Handler( - new OAuthApp({ - clientType: "github-app", - clientId: "client_id_123", - clientSecret: "client_secret_456", - Octokit: Octokit.defaults({ - log: { - debug: () => undefined, - info: () => undefined, - warn, - error: () => undefined, - }, - }), - }), - { - onUnhandledRequest: async (request) => { - return { - statusCode: 404, - headers: {}, - body: "", - }; - }, - } - ); - - expect(warn.mock.calls.length).toEqual(1); - expect(warn.mock.calls[0][0]).toEqual( - "[@octokit/oauth-app] `onUnhandledRequest` is deprecated and will be removed from the next major version." - ); - }); + it("has no deprecations currently", async () => {}); }); diff --git a/test/node-middleware.test.ts b/test/node-middleware.test.ts index 28b2827ae..5fec802bc 100644 --- a/test/node-middleware.test.ts +++ b/test/node-middleware.test.ts @@ -1,4 +1,4 @@ -import { createServer } from "http"; +import { createServer, IncomingMessage } from "http"; import { URL } from "url"; import fetch from "node-fetch"; @@ -492,55 +492,18 @@ describe("createNodeMiddleware(app)", () => { }); }); - it("POST /unrelated", async () => { - expect.assertions(4); - - const app = new OAuthApp({ - clientId: "0123", - clientSecret: "0123secret", - }); - - const server = createServer( - createNodeMiddleware(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-expect-error 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 () => { const appMock = {}; - const server = createServer( - createNodeMiddleware(appMock as unknown as OAuthApp) - ).listen(); + const middleware = createNodeMiddleware(appMock as unknown as OAuthApp); + const requestListener = async (req: IncomingMessage, res: any) => { + if (!(await middleware(req, res))) { + res.writeHead(404, { "Content-Type": "text/plain" }); + res.write("Not found."); + res.end(); + } + }; + const server = createServer(requestListener).listen(); // @ts-expect-error complains about { port } although it's included in returned AddressInfo interface const { port } = server.address(); @@ -860,6 +823,42 @@ describe("createNodeMiddleware(app)", () => { server.close(); }); + it("???", async () => { + const app = express(); + + // app.all("/foo", (_request: any, response: any) => response.end("ok\n")); + app.use( + createNodeMiddleware( + new OAuthApp({ + clientId: "0123", + clientSecret: "0123secret", + }) + ) + ); + + const server = app.listen(); + + const { port } = server.address(); + + const response = await fetch(`http://localhost:${port}/test`, { + method: "POST", + body: "{}", + }); + + await expect(response.text()).resolves.toContain("Cannot POST /test"); + expect(response.status).toEqual(404); + + // const responseForFoo = await fetch(`http://localhost:${port}/foo`, { + // method: "POST", + // body: "{}", + // }); + + // await expect(responseForFoo.text()).resolves.toContain("ok\n"); + // expect(responseForFoo.status).toEqual(200); + + server.close(); + }); + it("express middleware no mount path no next", async () => { const app = express(); diff --git a/test/smoke.test.ts b/test/smoke.test.ts index 4d2264c5a..266026277 100644 --- a/test/smoke.test.ts +++ b/test/smoke.test.ts @@ -1,4 +1,9 @@ -import { handleRequest, OAuthApp } from "../src"; +import { + handleRequest, + OAuthApp, + sendNodeResponse, + unknownRouteResponse, +} from "../src"; describe("Smoke test", () => { it("OAuthApp is a function", () => { @@ -16,4 +21,12 @@ describe("Smoke test", () => { it("handleRequest is a function", () => { expect(typeof handleRequest).toEqual("function"); }); + + it("unknownRouteResponse is a function", () => { + expect(typeof unknownRouteResponse).toEqual("function"); + }); + + it("sendNodeResponse is a function", () => { + expect(typeof sendNodeResponse).toEqual("function"); + }); }); diff --git a/test/web-worker-handler.test.ts b/test/web-worker-handler.test.ts index ba26da54c..413e28fec 100644 --- a/test/web-worker-handler.test.ts +++ b/test/web-worker-handler.test.ts @@ -1,11 +1,7 @@ import { URL } from "url"; import * as nodeFetch from "node-fetch"; import fromEntries from "fromentries"; -import { - createCloudflareHandler, - createWebWorkerHandler, - OAuthApp, -} from "../src"; +import { createWebWorkerHandler, OAuthApp } from "../src"; import { Octokit } from "@octokit/core"; describe("createWebWorkerHandler(app)", () => { @@ -46,7 +42,7 @@ describe("createWebWorkerHandler(app)", () => { method: "OPTIONS", }); const response = await handleRequest(request); - expect(response.status).toStrictEqual(200); + expect(response!.status).toStrictEqual(200); }); it("GET /api/github/oauth/login", async () => { @@ -57,7 +53,7 @@ describe("createWebWorkerHandler(app)", () => { const handleRequest = createWebWorkerHandler(app); const request = new Request("/api/github/oauth/login"); - const { status, headers } = await handleRequest(request); + const { status, headers } = (await handleRequest(request))!; expect(status).toEqual(302); const url = new URL(headers.get("location") as string); @@ -79,7 +75,7 @@ describe("createWebWorkerHandler(app)", () => { const handleRequest = createWebWorkerHandler(app); const request = new Request("/api/github/oauth/login"); - const { status, headers } = await handleRequest(request); + const { status, headers } = (await handleRequest(request))!; expect(status).toEqual(302); const url = new URL(headers.get("location") as string); @@ -102,7 +98,7 @@ describe("createWebWorkerHandler(app)", () => { const request = new Request( "/api/github/oauth/login?state=mystate123&scopes=one,two,three" ); - const { status, headers } = await handleRequest(request); + const { status, headers } = (await handleRequest(request))!; expect(status).toEqual(302); const url = new URL(headers.get("location") as string); @@ -132,7 +128,7 @@ describe("createWebWorkerHandler(app)", () => { const request = new Request( "/api/github/oauth/callback?code=012345&state=state123" ); - const response = await handleRequest(request); + const response = (await handleRequest(request))!; expect(response.status).toEqual(200); expect(await response.text()).toMatch(/token123/); @@ -164,7 +160,7 @@ describe("createWebWorkerHandler(app)", () => { redirectUrl: "http://example.com", }), }); - const response = await handleRequest(request); + const response = (await handleRequest(request))!; expect(response.status).toEqual(201); expect(await response.json()).toStrictEqual({ @@ -198,7 +194,7 @@ describe("createWebWorkerHandler(app)", () => { authorization: "token token123", }, }); - const response = await handleRequest(request); + const response = (await handleRequest(request))!; expect(response.status).toEqual(200); expect(await response.json()).toStrictEqual({ @@ -231,7 +227,7 @@ describe("createWebWorkerHandler(app)", () => { method: "PATCH", headers: { authorization: "token token123" }, }); - const response = await handleRequest(request); + const response = (await handleRequest(request))!; expect(response.status).toEqual(200); expect(await response.json()).toStrictEqual({ @@ -269,7 +265,7 @@ describe("createWebWorkerHandler(app)", () => { permissions: { issues: "write" }, }), }); - const response = await handleRequest(request); + const response = (await handleRequest(request))!; expect(response.status).toEqual(200); expect(response.status).toEqual(200); @@ -320,7 +316,7 @@ describe("createWebWorkerHandler(app)", () => { headers: { authorization: "token token123" }, body: JSON.stringify({ refreshToken: "r1.refreshtoken123" }), }); - const response = await handleRequest(request); + const response = (await handleRequest(request))!; expect(await response.json()).toStrictEqual({ data: { id: 1 }, @@ -353,7 +349,7 @@ describe("createWebWorkerHandler(app)", () => { method: "PATCH", headers: { authorization: "token token123" }, }); - const response = await handleRequest(request); + const response = (await handleRequest(request))!; expect(response.status).toEqual(200); expect(await response.json()).toStrictEqual({ @@ -379,7 +375,7 @@ describe("createWebWorkerHandler(app)", () => { method: "DELETE", headers: { authorization: "token token123" }, }); - const response = await handleRequest(request); + const response = (await handleRequest(request))!; expect(response.status).toEqual(204); @@ -401,7 +397,7 @@ describe("createWebWorkerHandler(app)", () => { method: "DELETE", headers: { authorization: "token token123" }, }); - const response = await handleRequest(request); + const response = (await handleRequest(request))!; expect(response.status).toEqual(204); @@ -412,21 +408,11 @@ describe("createWebWorkerHandler(app)", () => { }); it("POST /unrelated", async () => { - expect.assertions(4); - const app = new OAuthApp({ clientId: "0123", clientSecret: "0123secret", }); - const handleRequest = createWebWorkerHandler(app, { - onUnhandledRequest: async (request: Request) => { - expect(request.method).toEqual("POST"); - expect(request.url).toEqual("/unrelated"); - const text = await request.text(); - expect(text).toEqual('{"ok":true}'); - return new Response(null, { status: 200 }); - }, - }); + const handleRequest = createWebWorkerHandler(app); const request = new Request("/unrelated", { method: "POST", @@ -435,9 +421,9 @@ describe("createWebWorkerHandler(app)", () => { "content-type": "application/json", }, }); - const { status } = await handleRequest(request); + const response = await handleRequest(request); - expect(status).toEqual(200); + expect(response).toBeUndefined(); }); // // errors @@ -449,8 +435,8 @@ describe("createWebWorkerHandler(app)", () => { ); const request = new Request("/unknown"); - const response = await handleRequest(request); - expect(response.status).toEqual(404); + const response = (await handleRequest(request))!; + expect(response).toBeUndefined(); }); it("GET /api/github/oauth/callback without code", async () => { @@ -460,7 +446,7 @@ describe("createWebWorkerHandler(app)", () => { ); const request = new Request("/api/github/oauth/callback"); - const response = await handleRequest(request); + const response = (await handleRequest(request))!; expect(response.status).toEqual(400); expect(await response.json()).toStrictEqual({ @@ -477,7 +463,7 @@ describe("createWebWorkerHandler(app)", () => { const request = new Request( "/api/github/oauth/callback?error=redirect_uri_mismatch&error_description=The+redirect_uri+MUST+match+the+registered+callback+URL+for+this+application.&error_uri=https://docs.github.com/en/developers/apps/troubleshooting-authorization-request-errors/%23redirect-uri-mismatch&state=xyz" ); - const response = await handleRequest(request); + const response = (await handleRequest(request))!; expect(response.status).toEqual(400); expect(await response.json()).toStrictEqual({ @@ -496,7 +482,7 @@ describe("createWebWorkerHandler(app)", () => { method: "POST", body: JSON.stringify({}), }); - const response = await handleRequest(request); + const response = (await handleRequest(request))!; expect(response.status).toEqual(400); expect(await response.json()).toStrictEqual({ @@ -514,7 +500,7 @@ describe("createWebWorkerHandler(app)", () => { method: "POST", body: "foo", }); - const response = await handleRequest(request); + const response = (await handleRequest(request))!; expect(response.status).toEqual(400); expect(await response.json()).toStrictEqual({ @@ -531,7 +517,7 @@ describe("createWebWorkerHandler(app)", () => { const request = new Request("/api/github/oauth/token", { headers: {}, }); - const response = await handleRequest(request); + const response = (await handleRequest(request))!; expect(response.status).toEqual(400); expect(await response.json()).toStrictEqual({ @@ -549,7 +535,7 @@ describe("createWebWorkerHandler(app)", () => { method: "PATCH", headers: {}, }); - const response = await handleRequest(request); + const response = (await handleRequest(request))!; expect(response.status).toEqual(400); expect(await response.json()).toStrictEqual({ @@ -567,7 +553,7 @@ describe("createWebWorkerHandler(app)", () => { method: "POST", headers: {}, }); - const response = await handleRequest(request); + const response = (await handleRequest(request))!; expect(response.status).toEqual(400); expect(await response.json()).toStrictEqual({ @@ -591,7 +577,7 @@ describe("createWebWorkerHandler(app)", () => { refreshToken: "r1.refreshtoken123", }), }); - const response = await handleRequest(request); + const response = (await handleRequest(request))!; expect(response.status).toEqual(400); expect(await response.json()).toStrictEqual({ @@ -615,7 +601,7 @@ describe("createWebWorkerHandler(app)", () => { authorization: "token token123", }, }); - const response = await handleRequest(request); + const response = (await handleRequest(request))!; expect(response.status).toEqual(400); expect(await response.json()).toStrictEqual({ @@ -633,7 +619,7 @@ describe("createWebWorkerHandler(app)", () => { method: "DELETE", headers: {}, }); - const response = await handleRequest(request); + const response = (await handleRequest(request))!; expect(response.status).toEqual(400); expect(await response.json()).toStrictEqual({ @@ -651,7 +637,7 @@ describe("createWebWorkerHandler(app)", () => { method: "DELETE", headers: {}, }); - const response = await handleRequest(request); + const response = (await handleRequest(request))!; expect(response.status).toEqual(400); expect(await response.json()).toStrictEqual({ @@ -669,7 +655,7 @@ describe("createWebWorkerHandler(app)", () => { ); const request = new Request("/test/login", { redirect: "manual" }); - const { status } = await handleRequest(request); + const { status } = (await handleRequest(request))!; expect(status).toEqual(302); });