Skip to content

Commit

Permalink
fix: web worker handler should be generic to support both client types
Browse files Browse the repository at this point in the history
BREAKING CHANGE: rename `createCloudflareWorker` to `createWebWorkerHandler`
  • Loading branch information
baoshan committed Jun 22, 2022
1 parent 1c04f9b commit 0a6db47
Show file tree
Hide file tree
Showing 10 changed files with 82 additions and 85 deletions.
8 changes: 4 additions & 4 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -964,20 +964,20 @@ function onUnhandledRequest(request, response) {
</tbody>
</table>

### `createCloudflareHandler(app, options)`
### `createWebWorkerHandler(app, options)`

Event handler for Cloudflare workers.
Event handler for web worker environments (Cloudflare workers or Deno).

```js
// worker.js
import { OAuthApp, createCloudflareHandler } from "@octokit/oauth-app";
import { OAuthApp, createWebWorkerHandler } from "@octokit/oauth-app";
const app = new OAuthApp({
clientType: "oauth-app",
clientId: "1234567890abcdef1234",
clientSecret: "1234567890abcdef1234567890abcdef12345678",
});

const handleRequest = createCloudflareHandler(app, {
const handleRequest = createWebWorkerHandler(app, {
pathPrefix: "/api/github/oauth",
});

Expand Down
28 changes: 14 additions & 14 deletions src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,49 +9,49 @@ import {
GetUserOctokitWithStateInterface,
} from "./methods/get-user-octokit";
import {
getWebFlowAuthorizationUrlWithState,
GetWebFlowAuthorizationUrlInterface,
getWebFlowAuthorizationUrlWithState,
} from "./methods/get-web-flow-authorization-url";
import {
createTokenWithState,
CreateTokenInterface,
createTokenWithState,
} from "./methods/create-token";
import {
checkTokenWithState,
CheckTokenInterface,
checkTokenWithState,
} from "./methods/check-token";
import {
resetTokenWithState,
ResetTokenInterface,
resetTokenWithState,
} from "./methods/reset-token";
import {
refreshTokenWithState,
RefreshTokenInterface,
refreshTokenWithState,
} from "./methods/refresh-token";
import {
scopeTokenWithState,
ScopeTokenInterface,
scopeTokenWithState,
} from "./methods/scope-token";
import {
deleteTokenWithState,
DeleteTokenInterface,
deleteTokenWithState,
} from "./methods/delete-token";
import {
deleteAuthorizationWithState,
DeleteAuthorizationInterface,
deleteAuthorizationWithState,
} from "./methods/delete-authorization";

import {
Options,
import type {
AddEventHandler,
ClientType,
ClientTypeFromOptions,
ConstructorOptions,
OctokitTypeFromOptions,
ClientTypeFromOptions,
ClientType,
AddEventHandler,
Options,
State,
} from "./types";
export { createNodeMiddleware } from "./middleware/node/index";
export { createCloudflareHandler } from "./middleware/cloudflare/index";
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: 2 additions & 2 deletions src/middleware/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -10,15 +10,15 @@ middleware
├── on-unhandled-request-default.ts
├── types.ts
├── node/
├── cloudflare/ (to be implemented)
├── web-worker/ (Cloudflare Workers & Deno)
└── deno/ (to be implemented)
```

## Generic HTTP Handler

[`handleRequest`](handle-request.ts) function is an abstract HTTP handler which accepts an `OctokitRequest` and returns an `OctokitResponse` if the request matches any predefined route.

> Different environments (e.g., Node.js, Cloudflare, etc.) exposes different APIs when processing HTTP requests (e.g., [`IncomingMessage`](https://nodejs.org/api/http.html#http_class_http_incomingmessage) for Node.js, [`Request`](https://developer.mozilla.org/en-US/docs/Web/API/Request) for Cloudflare workers, etc.). Two HTTP-related types ([`OctokitRequest` and `OctokitResponse`](./types.ts)) are generalized to make an abstract HTTP handler possible.
> Different environments (e.g., Node.js, Cloudflare Workers, Deno, etc.) exposes different APIs when processing HTTP requests (e.g., [`IncomingMessage`](https://nodejs.org/api/http.html#http_class_http_incomingmessage) for Node.js, [`Request`](https://developer.mozilla.org/en-US/docs/Web/API/Request) for Cloudflare workers, etc.). Two HTTP-related types ([`OctokitRequest` and `OctokitResponse`](./types.ts)) are generalized to make an abstract HTTP handler possible.
To share the behavior and capability with the existing Node.js middleware (and be compatible with [OAuth user authentication strategy in the browser](https://github.com/octokit/auth-oauth-user-client.js)), it is better to implement your HTTP handler/middleware based on `handleRequest` function.

Expand Down
18 changes: 6 additions & 12 deletions src/middleware/handle-request.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { OAuthApp } from "../index";
import { OctokitRequest, OctokitResponse, HandlerOptions } from "./types";
import { HandlerOptions, OctokitRequest, OctokitResponse } from "./types";
import { ClientType, Options } from "../types";
// @ts-ignore - requires esModuleInterop flag
import fromEntries from "fromentries";
Expand Down Expand Up @@ -89,16 +89,13 @@ export async function handleRequest(
`[@octokit/oauth-app] ${query.error} ${query.error_description}`
);
}
if (!query.state || !query.code) {
throw new Error(
'[@octokit/oauth-app] Both "code" & "state" parameters are required'
);
if (!query.code) {
throw new Error('[@octokit/oauth-app] "code" parameter is required');
}

const {
authentication: { token },
} = await app.createToken({
state: query.state,
code: query.code,
});

Expand All @@ -114,16 +111,13 @@ export async function handleRequest(
}

if (route === routes.createToken) {
const { state: oauthState, code, redirectUrl } = json;
const { code, redirectUrl } = json;

if (!oauthState || !code) {
throw new Error(
'[@octokit/oauth-app] Both "code" & "state" parameters are required'
);
if (!code) {
throw new Error('[@octokit/oauth-app] "code" parameter is required');
}

const result = await app.createToken({
state: oauthState,
code,
redirectUrl,
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,19 +5,19 @@ import { onUnhandledRequestDefault } from "../on-unhandled-request-default";
import { OAuthApp } from "../../index";
import { HandlerOptions } from "../types";

async function onUnhandledRequestDefaultCloudflare(
async function onUnhandledRequestDefaultWebWorker(
request: Request
): Promise<Response> {
const octokitRequest = parseRequest(request);
const octokitResponse = onUnhandledRequestDefault(octokitRequest);
return sendResponse(octokitResponse);
}

export function createCloudflareHandler(
app: OAuthApp,
export function createWebWorkerHandler<T>(
app: OAuthApp<T>,
{
pathPrefix,
onUnhandledRequest = onUnhandledRequestDefaultCloudflare,
onUnhandledRequest = onUnhandledRequestDefaultWebWorker,
}: HandlerOptions & {
onUnhandledRequest?: (request: Request) => Response | Promise<Response>;
} = {}
Expand Down
File renamed without changes.
File renamed without changes.
5 changes: 0 additions & 5 deletions test/app.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -128,7 +128,6 @@ describe("app", () => {
client_id: "0123",
client_secret: "0123secret",
code: "code123",
state: "state123",
},
}
)
Expand Down Expand Up @@ -158,7 +157,6 @@ describe("app", () => {
app.on("token.created", onTokenCallback);

const result = await app.createToken({
state: "state123",
code: "code123",
});

Expand Down Expand Up @@ -195,7 +193,6 @@ describe("app", () => {
it("app.createToken(options) for device flow", async () => {
const mock = fetchMock
.sandbox()

.postOnce(
"https://github.com/login/device/code",
{
Expand Down Expand Up @@ -944,7 +941,6 @@ describe("app", () => {
client_id: "0123",
client_secret: "0123secret",
code: "code123",
state: "state123",
},
}
)
Expand Down Expand Up @@ -985,7 +981,6 @@ describe("app", () => {
app.on("token.created", onTokenCallback2);

await app.createToken({
state: "state123",
code: "code123",
});

Expand Down
12 changes: 4 additions & 8 deletions test/node-middleware.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import { createServer } from "http";
import { URL } from "url";

import fetch from "node-fetch";
import { OAuthApp, createNodeMiddleware } from "../src/";
import { createNodeMiddleware, OAuthApp } from "../src/";

// import without types
const express = require("express");
Expand Down Expand Up @@ -152,7 +152,6 @@ describe("createNodeMiddleware(app)", () => {

expect(appMock.createToken.mock.calls.length).toEqual(1);
expect(appMock.createToken.mock.calls[0][0]).toStrictEqual({
state: "state123",
code: "012345",
});
});
Expand Down Expand Up @@ -195,7 +194,6 @@ describe("createNodeMiddleware(app)", () => {

expect(appMock.createToken.mock.calls.length).toEqual(1);
expect(appMock.createToken.mock.calls[0][0]).toStrictEqual({
state: "state123",
code: "012345",
redirectUrl: "http://example.com",
});
Expand Down Expand Up @@ -554,7 +552,7 @@ describe("createNodeMiddleware(app)", () => {
expect(response.status).toEqual(404);
});

it("GET /api/github/oauth/callback without code or state", async () => {
it("GET /api/github/oauth/callback without code", async () => {
const appMock = {};

const server = createServer(
Expand All @@ -571,8 +569,7 @@ describe("createNodeMiddleware(app)", () => {

expect(response.status).toEqual(400);
expect(await response.json()).toStrictEqual({
error:
'[@octokit/oauth-app] Both "code" & "state" parameters are required',
error: '[@octokit/oauth-app] "code" parameter is required',
});
});

Expand Down Expand Up @@ -619,8 +616,7 @@ describe("createNodeMiddleware(app)", () => {

expect(response.status).toEqual(400);
expect(await response.json()).toStrictEqual({
error:
'[@octokit/oauth-app] Both "code" & "state" parameters are required',
error: '[@octokit/oauth-app] "code" parameter is required',
});
});

Expand Down

0 comments on commit 0a6db47

Please sign in to comment.