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

Remove onUnhandledRequest middleware option #341

Merged
merged 2 commits into from
Sep 10, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
75 changes: 0 additions & 75 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -937,31 +937,6 @@ All exposed paths will be prefixed with the provided prefix. Defaults to `"/api/

</td>
</tr>
<tr>
<th>
<code>options.onUnhandledRequest</code>
</th>
<th>
<code>function</code>
</th>
<td>

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}`,
})
);
}
```

</td></tr>
</tbody>
</table>

Expand Down Expand Up @@ -1025,31 +1000,6 @@ addEventListener("fetch", (event) => {

All exposed paths will be prefixed with the provided prefix. Defaults to `"/api/github/oauth"`

</td>
</tr>
<tr>
<th>
<code>options.onUnhandledRequest</code>
</th>
<th>
<code>function</code>
</th>
<td>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" },
}
);
}
```

</td>
</tr>
</tbody>
Expand Down Expand Up @@ -1115,31 +1065,6 @@ export const handler = createAWSLambdaAPIGatewayV2Handler(app, {

All exposed paths will be prefixed with the provided prefix. Defaults to `"/api/github/oauth"`

</td>
</tr>
<tr>
<th>
<code>options.onUnhandledRequest</code>
</th>
<th>
<code>function</code>
</th>
<td>Defaults to returns:

```js
function onUnhandledRequest(request) {
return
{
status: 404,
headers: { "content-type": "application/json" },
body: JSON.stringify({
error: `Unknown route: [METHOD] [URL]`,
})
}
);
}
```

</td>
</tr>
</tbody>
Expand Down
18 changes: 14 additions & 4 deletions src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -50,11 +50,21 @@ 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 { 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<T> = new (...args: any[]) => T;
Expand Down
4 changes: 1 addition & 3 deletions src/middleware/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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).
32 changes: 9 additions & 23 deletions src/middleware/aws-lambda/api-gateway-v2.ts
Original file line number Diff line number Diff line change
@@ -1,37 +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 { Options, ClientType } 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<APIGatewayProxyStructuredResultV2> {
const request = parseRequest(event);
const response = onUnhandledRequestDefault(request);
return sendResponse(response);
}

export function createAWSLambdaAPIGatewayV2Handler(
app: OAuthApp<Options<ClientType>>,
{
pathPrefix,
onUnhandledRequest = onUnhandledRequestDefaultAWSAPIGatewayV2,
}: HandlerOptions & {
onUnhandledRequest?: (
event: APIGatewayProxyEventV2
) => Promise<APIGatewayProxyStructuredResultV2>;
} = {}
options: HandlerOptions = {}
) {
return async function (event: APIGatewayProxyEventV2) {
return async function (
event: APIGatewayProxyEventV2
): Promise<APIGatewayProxyStructuredResultV2 | undefined> {
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;
};
}
30 changes: 18 additions & 12 deletions src/middleware/handle-request.ts
Original file line number Diff line number Diff line change
@@ -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
Expand All @@ -8,7 +9,7 @@ export async function handleRequest(
app: OAuthApp<Options<ClientType>>,
{ pathPrefix = "/api/github/oauth" }: HandlerOptions,
request: OctokitRequest
): Promise<OctokitResponse | null> {
): Promise<OctokitResponse | undefined> {
if (request.method === "OPTIONS") {
return {
status: 200,
Expand All @@ -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;
Expand Down
41 changes: 9 additions & 32 deletions src/middleware/node/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,51 +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<Options<ClientType>>,
{
pathPrefix,
onUnhandledRequest = onUnhandledRequestDefaultNode,
}: HandlerOptions & {
onUnhandledRequest?: (
request: IncomingMessage,
response: ServerResponse
) => void;
} = {}
options: HandlerOptions = {}
) {
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;
}
};
}
Original file line number Diff line number Diff line change
@@ -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" },
Expand Down
46 changes: 8 additions & 38 deletions src/middleware/web-worker/index.ts
Original file line number Diff line number Diff line change
@@ -1,47 +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<Response> {
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<T extends Options<ClientType>>(
app: OAuthApp<T>,
{
pathPrefix,
onUnhandledRequest = onUnhandledRequestDefaultWebWorker,
}: HandlerOptions & {
onUnhandledRequest?: (request: Request) => Response | Promise<Response>;
} = {}
options: HandlerOptions = {}
) {
return async function (request: Request): Promise<Response> {
const octokitRequest = parseRequest(request);
const octokitResponse = await handleRequest(
app,
{ pathPrefix },
octokitRequest
);
return octokitResponse
? sendResponse(octokitResponse)
: await onUnhandledRequest(request);
return async function (request: Request): Promise<Response | undefined> {
const octokitRequest = await parseRequest(request);
const octokitResponse = await handleRequest(app, options, octokitRequest);
return octokitResponse ? sendResponse(octokitResponse) : undefined;
};
}

/** @deprecated */
export function createCloudflareHandler<T>(
...args: Parameters<typeof createWebWorkerHandler>
) {
args[0].octokit.log.warn(
"[@octokit/oauth-app] `createCloudflareHandler` is deprecated, use `createWebWorkerHandler` instead"
);
return createWebWorkerHandler(...args);
}
Loading