Skip to content

Commit 15bd969

Browse files
tommoorclaude
andauthored
fix: Handle trailing space on code challenge method (outline#12068)
* fix: Handle trailing space on code challenge method * Add tests for codeChallengeMethod whitespace trimming Addresses review feedback: adds test coverage for the trim behavior in saveAuthorizationCode, verifying trailing whitespace is stripped and whitespace-only input is treated as absent. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
1 parent 5e89016 commit 15bd969

File tree

3 files changed

+69
-2
lines changed

3 files changed

+69
-2
lines changed

app/scenes/Login/OAuthAuthorize.tsx

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -87,10 +87,11 @@ function Authorize() {
8787
redirect_uri: redirectUri,
8888
response_type: responseType,
8989
code_challenge: codeChallenge,
90-
code_challenge_method: codeChallengeMethod,
90+
code_challenge_method: rawCodeChallengeMethod,
9191
state,
9292
scope,
9393
} = Object.fromEntries(params);
94+
const codeChallengeMethod = rawCodeChallengeMethod?.trim();
9495
const [scopes] = useState(() => inputScopes(scope));
9596
const { error: clientError, data: response } = useRequest<{
9697
data: OAuthClient;

server/utils/oauth/OAuthInterface.test.ts

Lines changed: 66 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -221,6 +221,72 @@ describe("OAuthInterface", () => {
221221
});
222222
});
223223

224+
describe("#saveAuthorizationCode", () => {
225+
it("should trim trailing whitespace from codeChallengeMethod", async () => {
226+
const user = await buildUser();
227+
const oAuthClient = await buildOAuthClient({ teamId: user.teamId });
228+
229+
const code = {
230+
authorizationCode: randomUUID(),
231+
expiresAt: new Date(Date.now() + 60_000),
232+
redirectUri: oAuthClient.redirectUris[0],
233+
scope: [Scope.Read],
234+
codeChallenge: "abc123",
235+
codeChallengeMethod: "S256 ",
236+
};
237+
238+
await OAuthInterface.saveAuthorizationCode(
239+
code,
240+
{
241+
id: oAuthClient.clientId,
242+
databaseId: oAuthClient.id,
243+
grants: OAuthInterface.grants,
244+
},
245+
user
246+
);
247+
248+
const result = await OAuthInterface.getAuthorizationCode(
249+
code.authorizationCode
250+
);
251+
expect(result).not.toBe(false);
252+
if (result) {
253+
expect(result.codeChallengeMethod).toBe("S256");
254+
}
255+
});
256+
257+
it("should treat whitespace-only codeChallengeMethod as absent", async () => {
258+
const user = await buildUser();
259+
const oAuthClient = await buildOAuthClient({ teamId: user.teamId });
260+
261+
const code = {
262+
authorizationCode: randomUUID(),
263+
expiresAt: new Date(Date.now() + 60_000),
264+
redirectUri: oAuthClient.redirectUris[0],
265+
scope: [Scope.Read],
266+
codeChallenge: undefined,
267+
codeChallengeMethod: " ",
268+
};
269+
270+
await OAuthInterface.saveAuthorizationCode(
271+
code,
272+
{
273+
id: oAuthClient.clientId,
274+
databaseId: oAuthClient.id,
275+
grants: OAuthInterface.grants,
276+
},
277+
user
278+
);
279+
280+
const result = await OAuthInterface.getAuthorizationCode(
281+
code.authorizationCode
282+
);
283+
expect(result).not.toBe(false);
284+
if (result) {
285+
expect(result.codeChallengeMethod).toBeNull();
286+
}
287+
});
288+
});
289+
224290
describe("#validateScope", () => {
225291
it("should return empty array for empty scope", async () => {
226292
const result = await OAuthInterface.validateScope(user, client, []);

server/utils/oauth/OAuthInterface.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -288,7 +288,7 @@ export const OAuthInterface: RefreshTokenModel &
288288
scope,
289289
redirectUri,
290290
codeChallenge,
291-
codeChallengeMethod,
291+
codeChallengeMethod: codeChallengeMethod?.trim() || undefined,
292292
oauthClientId: client.databaseId,
293293
userId: user.id,
294294
grantId: (user as GrantUser).grantId || crypto.randomUUID(),

0 commit comments

Comments
 (0)