Skip to content

Commit 7cf2144

Browse files
committed
fix: revert "fix: do not require state parameter to create a token (#288)"
This reverts commit f14c68e.
1 parent 330e4ed commit 7cf2144

File tree

6 files changed

+41
-15
lines changed

6 files changed

+41
-15
lines changed

README.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -442,6 +442,7 @@ For the web flow, you have to pass the `code` from URL redirect described in [st
442442

443443
```js
444444
const { token } = await app.createToken({
445+
state: "state123",
445446
code: "code123",
446447
});
447448
```

src/middleware/handle-request.ts

Lines changed: 12 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
import { OAuthApp } from "../index";
2-
import { HandlerOptions, OctokitRequest, OctokitResponse } from "./types";
2+
import { OctokitRequest, OctokitResponse, HandlerOptions } from "./types";
33
import { ClientType, Options } from "../types";
44
// @ts-ignore - requires esModuleInterop flag
55
import fromEntries from "fromentries";
@@ -89,13 +89,16 @@ export async function handleRequest(
8989
`[@octokit/oauth-app] ${query.error} ${query.error_description}`
9090
);
9191
}
92-
if (!query.code) {
93-
throw new Error('[@octokit/oauth-app] "code" parameter is required');
92+
if (!query.state || !query.code) {
93+
throw new Error(
94+
'[@octokit/oauth-app] Both "code" & "state" parameters are required'
95+
);
9496
}
9597

9698
const {
9799
authentication: { token },
98100
} = await app.createToken({
101+
state: query.state,
99102
code: query.code,
100103
});
101104

@@ -111,13 +114,16 @@ export async function handleRequest(
111114
}
112115

113116
if (route === routes.createToken) {
114-
const { code, redirectUrl } = json;
117+
const { state: oauthState, code, redirectUrl } = json;
115118

116-
if (!code) {
117-
throw new Error('[@octokit/oauth-app] "code" parameter is required');
119+
if (!oauthState || !code) {
120+
throw new Error(
121+
'[@octokit/oauth-app] Both "code" & "state" parameters are required'
122+
);
118123
}
119124

120125
const result = await app.createToken({
126+
state: oauthState,
121127
code,
122128
redirectUrl,
123129
});

src/middleware/web-worker/index.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -13,8 +13,8 @@ async function onUnhandledRequestDefaultWebWorker(
1313
return sendResponse(octokitResponse);
1414
}
1515

16-
export function createWebWorkerHandler<T>(
17-
app: OAuthApp<T>,
16+
export function createWebWorkerHandler(
17+
app: OAuthApp,
1818
{
1919
pathPrefix,
2020
onUnhandledRequest = onUnhandledRequestDefaultWebWorker,

test/app.test.ts

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -92,7 +92,9 @@ describe("app", () => {
9292
clientId: "0123",
9393
clientSecret: "0123secret",
9494
});
95-
const { url } = app.getWebFlowAuthorizationUrl({ state: "state123" });
95+
const { url } = app.getWebFlowAuthorizationUrl({
96+
state: "state123",
97+
});
9698
expect(url).toStrictEqual(
9799
"https://github.com/login/oauth/authorize?allow_signup=true&client_id=0123&state=state123"
98100
);
@@ -103,7 +105,9 @@ describe("app", () => {
103105
clientId: "lv1.0123",
104106
clientSecret: "0123secret",
105107
});
106-
const { url } = app.getWebFlowAuthorizationUrl({ state: "state123" });
108+
const { url } = app.getWebFlowAuthorizationUrl({
109+
state: "state123",
110+
});
107111
expect(url).toStrictEqual(
108112
"https://github.com/login/oauth/authorize?allow_signup=true&client_id=lv1.0123&state=state123"
109113
);
@@ -124,6 +128,7 @@ describe("app", () => {
124128
client_id: "0123",
125129
client_secret: "0123secret",
126130
code: "code123",
131+
state: "state123",
127132
},
128133
}
129134
)
@@ -153,6 +158,7 @@ describe("app", () => {
153158
app.on("token.created", onTokenCallback);
154159

155160
const result = await app.createToken({
161+
state: "state123",
156162
code: "code123",
157163
});
158164

@@ -189,6 +195,7 @@ describe("app", () => {
189195
it("app.createToken(options) for device flow", async () => {
190196
const mock = fetchMock
191197
.sandbox()
198+
192199
.postOnce(
193200
"https://github.com/login/device/code",
194201
{
@@ -937,6 +944,7 @@ describe("app", () => {
937944
client_id: "0123",
938945
client_secret: "0123secret",
939946
code: "code123",
947+
state: "state123",
940948
},
941949
}
942950
)
@@ -977,6 +985,7 @@ describe("app", () => {
977985
app.on("token.created", onTokenCallback2);
978986

979987
await app.createToken({
988+
state: "state123",
980989
code: "code123",
981990
});
982991

test/node-middleware.test.ts

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@ import { createServer } from "http";
22
import { URL } from "url";
33

44
import fetch from "node-fetch";
5-
import { createNodeMiddleware, OAuthApp } from "../src/";
5+
import { OAuthApp, createNodeMiddleware } from "../src/";
66

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

153153
expect(appMock.createToken.mock.calls.length).toEqual(1);
154154
expect(appMock.createToken.mock.calls[0][0]).toStrictEqual({
155+
state: "state123",
155156
code: "012345",
156157
});
157158
});
@@ -179,6 +180,7 @@ describe("createNodeMiddleware(app)", () => {
179180
method: "POST",
180181
body: JSON.stringify({
181182
code: "012345",
183+
state: "state123",
182184
redirectUrl: "http://example.com",
183185
}),
184186
}
@@ -193,6 +195,7 @@ describe("createNodeMiddleware(app)", () => {
193195

194196
expect(appMock.createToken.mock.calls.length).toEqual(1);
195197
expect(appMock.createToken.mock.calls[0][0]).toStrictEqual({
198+
state: "state123",
196199
code: "012345",
197200
redirectUrl: "http://example.com",
198201
});
@@ -568,7 +571,8 @@ describe("createNodeMiddleware(app)", () => {
568571

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

@@ -615,7 +619,8 @@ describe("createNodeMiddleware(app)", () => {
615619

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

test/web-worker-handler.test.ts

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -139,6 +139,7 @@ describe("createWebWorkerHandler(app)", () => {
139139

140140
expect(appMock.createToken.mock.calls.length).toEqual(1);
141141
expect(appMock.createToken.mock.calls[0][0]).toStrictEqual({
142+
state: "state123",
142143
code: "012345",
143144
});
144145
});
@@ -161,6 +162,7 @@ describe("createWebWorkerHandler(app)", () => {
161162
method: "POST",
162163
body: JSON.stringify({
163164
code: "012345",
165+
state: "state123",
164166
redirectUrl: "http://example.com",
165167
}),
166168
});
@@ -173,6 +175,7 @@ describe("createWebWorkerHandler(app)", () => {
173175

174176
expect(appMock.createToken.mock.calls.length).toEqual(1);
175177
expect(appMock.createToken.mock.calls[0][0]).toStrictEqual({
178+
state: "state123",
176179
code: "012345",
177180
redirectUrl: "http://example.com",
178181
});
@@ -464,7 +467,8 @@ describe("createWebWorkerHandler(app)", () => {
464467

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

@@ -500,7 +504,8 @@ describe("createWebWorkerHandler(app)", () => {
500504

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

0 commit comments

Comments
 (0)