Skip to content

fix: require PKCE S256 on public OAuth clients#44

Open
nanookclaw wants to merge 1 commit into
Freedisch:mainfrom
nanookclaw:fix/enforce-pkce-public-clients
Open

fix: require PKCE S256 on public OAuth clients#44
nanookclaw wants to merge 1 commit into
Freedisch:mainfrom
nanookclaw:fix/enforce-pkce-public-clients

Conversation

@nanookclaw

Copy link
Copy Markdown

Problem

authorizeSubmit and autoAuthorize pass code_challenge through to issueCode without checking whether it is present. When a client omits the challenge, the Token handler's PKCE verification is silently skipped because the stored codeChallenge is empty. For public clients (no client secret), PKCE is the only proof-of-identity during code exchange.

Fix

Both authorization paths now reject requests where code_challenge is absent or code_challenge_method is not S256, returning 400 before any code is issued. This moves PKCE from opt-in to mandatory.

Closes #31

Reject authorization requests that omit code_challenge or use a
method other than S256. Without this, an attacker who intercepts the
auth code can exchange it for a token at /oauth/token with no proof
of client identity.

Closes Freedisch#31

Signed-off-by: Nanook <nanookclaw@users.noreply.github.com>
@vercel

vercel Bot commented May 10, 2026

Copy link
Copy Markdown

@nanookclaw is attempting to deploy a commit to the freedisch's projects Team on Vercel.

A member of the Team first needs to authorize it.

@Freedisch

Freedisch commented May 13, 2026

Copy link
Copy Markdown
Owner

Thanks for your contribution
Have you run some local tests to see if it breaks the auth ?

@nanookclaw

Copy link
Copy Markdown
Author

Ran the local Go test suite from the module root after re-checking this branch:

cd havril && go test ./...

Result: all packages pass; the repo currently has no Go test files, so this verifies compile/package integrity rather than exercising an auth integration test.

I also manually re-read the OAuth flow touched by this PR:

  • authorizeSubmit now rejects missing/non-S256 PKCE before validating the token or issuing a code.
  • autoAuthorize applies the same check before silent authorization.
  • Existing Claude/MCP clients that send code_challenge_method=S256 continue through the same issueCodeToken exchange path.

So this should not break the current Claude auth flow; it only rejects clients that omit PKCE or use a non-S256 method.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Security: PKCE not enforced on public OAuth clients

2 participants