Skip to content

Commit

Permalink
fix: don't respond to OPTIONS requests for non-oauth requests (#539)
Browse files Browse the repository at this point in the history
  • Loading branch information
APTy committed Jun 18, 2024
1 parent 55bd05c commit d4829f7
Show file tree
Hide file tree
Showing 2 changed files with 46 additions and 6 deletions.
13 changes: 7 additions & 6 deletions src/middleware/handle-request.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,13 @@ export async function handleRequest(
{ pathPrefix = "/api/github/oauth" }: HandlerOptions,
request: OctokitRequest,
): Promise<OctokitResponse | undefined> {
// request.url may include ?query parameters which we don't want for `route`
// hence the workaround using new URL()
let { pathname } = new URL(request.url as string, "http://localhost");
if (!pathname.startsWith(`${pathPrefix}/`)) {
return undefined;
}

if (request.method === "OPTIONS") {
return {
status: 200,
Expand All @@ -24,12 +31,6 @@ export async function handleRequest(
};
}

// request.url may include ?query parameters which we don't want for `route`
// hence the workaround using new URL()
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(" ");
Expand Down
39 changes: 39 additions & 0 deletions test/node-middleware.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,45 @@ describe("createNodeMiddleware(app)", () => {
server.close();

expect(response.status).toEqual(200);
expect(response.headers.get("access-control-allow-origin")).toEqual("*");
expect(response.headers.get("access-control-allow-methods")).toEqual("*");
expect(response.headers.get("access-control-allow-headers")).toEqual(
"Content-Type, User-Agent, Authorization",
);
});

it("doesn't overwrite pre-flight requests unrelated to github oauth", async () => {
const app = new OAuthApp({
clientId: "0123",
clientSecret: "0123secret",
});

const server = createServer((req, res) => {
if (req.url === "/health") {
res.writeHead(200, {
"Content-Type": "text/plain",
"Access-Control-Allow-Origin": "http://localhost:8080",
});
res.end("OK");
return;
}
createNodeMiddleware(app);
}).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}/health`, {
method: "OPTIONS",
});

server.close();

expect(response.status).toEqual(200);
expect(response.headers.get("access-control-allow-origin")).toEqual(
"http://localhost:8080",
);
expect(response.headers.get("access-control-allow-methods")).toEqual(null);
expect(response.headers.get("access-control-allow-headers")).toEqual(null);
});

it("GET /api/github/oauth/login", async () => {
Expand Down

0 comments on commit d4829f7

Please sign in to comment.