fix: do not require state parameter to create a token (#288)

octokit · Jun 22, 2022 · f14c68e · f14c68e
1 parent 1c04f9b
commit f14c68e
Show file tree

Hide file tree

Showing 6 changed files with 32 additions and 42 deletions.
diff --git a/README.md b/README.md
@@ -442,7 +442,6 @@ For the web flow, you have to pass the `code` from URL redirect described in [st
 
 ```js
 const { token } = await app.createToken({
-  state: "state123",
   code: "code123",
 });
 ```

diff --git a/src/middleware/cloudflare/index.ts b/src/middleware/cloudflare/index.ts
@@ -13,8 +13,8 @@ async function onUnhandledRequestDefaultCloudflare(
   return sendResponse(octokitResponse);
 }
 
-export function createCloudflareHandler(
-  app: OAuthApp,
+export function createCloudflareHandler<T>(
+  app: OAuthApp<T>,
   {
     pathPrefix,
     onUnhandledRequest = onUnhandledRequestDefaultCloudflare,

diff --git a/src/middleware/handle-request.ts b/src/middleware/handle-request.ts
@@ -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";
@@ -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,
       });
 
@@ -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,
       });

diff --git a/test/app.test.ts b/test/app.test.ts
@@ -92,9 +92,7 @@ describe("app", () => {
       clientId: "0123",
       clientSecret: "0123secret",
     });
-    const { url } = app.getWebFlowAuthorizationUrl({
-      state: "state123",
-    });
+    const { url } = app.getWebFlowAuthorizationUrl({ state: "state123" });
     expect(url).toStrictEqual(
       "https://github.com/login/oauth/authorize?allow_signup=true&client_id=0123&state=state123"
     );
@@ -105,9 +103,7 @@ describe("app", () => {
       clientId: "lv1.0123",
       clientSecret: "0123secret",
     });
-    const { url } = app.getWebFlowAuthorizationUrl({
-      state: "state123",
-    });
+    const { url } = app.getWebFlowAuthorizationUrl({ state: "state123" });
     expect(url).toStrictEqual(
       "https://github.com/login/oauth/authorize?allow_signup=true&client_id=lv1.0123&state=state123"
     );
@@ -128,7 +124,6 @@ describe("app", () => {
             client_id: "0123",
             client_secret: "0123secret",
             code: "code123",
-            state: "state123",
           },
         }
       )
@@ -158,7 +153,6 @@ describe("app", () => {
     app.on("token.created", onTokenCallback);
 
     const result = await app.createToken({
-      state: "state123",
       code: "code123",
     });
 
@@ -195,7 +189,6 @@ describe("app", () => {
   it("app.createToken(options) for device flow", async () => {
     const mock = fetchMock
       .sandbox()
-
       .postOnce(
         "https://github.com/login/device/code",
         {
@@ -944,7 +937,6 @@ describe("app", () => {
             client_id: "0123",
             client_secret: "0123secret",
             code: "code123",
-            state: "state123",
           },
         }
       )
@@ -985,7 +977,6 @@ describe("app", () => {
     app.on("token.created", onTokenCallback2);
 
     await app.createToken({
-      state: "state123",
       code: "code123",
     });
 

diff --git a/test/cloudflare-handler.test.ts b/test/cloudflare-handler.test.ts
@@ -1,7 +1,7 @@
 import { URL } from "url";
 import * as nodeFetch from "node-fetch";
 import fromEntries from "fromentries";
-import { OAuthApp, createCloudflareHandler } from "../src/";
+import { createCloudflareHandler, OAuthApp } from "../src/";
 
 describe("createCloudflareHandler(app)", () => {
   beforeAll(() => {
@@ -15,6 +15,22 @@ describe("createCloudflareHandler(app)", () => {
     delete (global as any).Response;
   });
 
+  it("support both oauth-app and github-app", () => {
+    const oauthApp = new OAuthApp({
+      clientType: "oauth-app",
+      clientId: "0123",
+      clientSecret: "0123secret",
+    });
+    createCloudflareHandler(oauthApp);
+
+    const githubApp = new OAuthApp({
+      clientType: "github-app",
+      clientId: "0123",
+      clientSecret: "0123secret",
+    });
+    createCloudflareHandler(githubApp);
+  });
+
   it("allow pre-flight requests", async () => {
     const app = new OAuthApp({
       clientId: "0123",
@@ -118,7 +134,6 @@ describe("createCloudflareHandler(app)", () => {
 
     expect(appMock.createToken.mock.calls.length).toEqual(1);
     expect(appMock.createToken.mock.calls[0][0]).toStrictEqual({
-      state: "state123",
       code: "012345",
     });
   });
@@ -141,7 +156,6 @@ describe("createCloudflareHandler(app)", () => {
       method: "POST",
       body: JSON.stringify({
         code: "012345",
-        state: "state123",
         redirectUrl: "http://example.com",
       }),
     });
@@ -154,7 +168,6 @@ describe("createCloudflareHandler(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",
     });
@@ -446,8 +459,7 @@ describe("createCloudflareHandler(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',
     });
   });
 
@@ -483,8 +495,7 @@ describe("createCloudflareHandler(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',
     });
   });
 

diff --git a/test/node-middleware.test.ts b/test/node-middleware.test.ts
@@ -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");
@@ -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",
     });
   });
@@ -180,7 +179,6 @@ describe("createNodeMiddleware(app)", () => {
         method: "POST",
         body: JSON.stringify({
           code: "012345",
-          state: "state123",
           redirectUrl: "http://example.com",
         }),
       }
@@ -195,7 +193,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",
     });
@@ -571,8 +568,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',
     });
   });
 
@@ -619,8 +615,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',
     });
   });