Skip to content

fix: use independent state and verifier for PKCE (RFC 7636) - #24

Open
Steffen025 wants to merge 1 commit into
cemalturkcan:mainfrom
Steffen025:fix/pkce-separate-state
Open

fix: use independent state and verifier for PKCE (RFC 7636)#24
Steffen025 wants to merge 1 commit into
cemalturkcan:mainfrom
Steffen025:fix/pkce-separate-state

Conversation

@Steffen025

Copy link
Copy Markdown
Contributor

Fixes #16.

Problem

createAuthorizationRequest reuses the PKCE code_verifier as the OAuth state parameter. Per RFC 7636 and RFC 6749 these serve different purposes:

Parameter Purpose Visibility
state CSRF protection Visible in authorization URL and redirect callback
code_verifier Proof of possession Never leaves the client until token exchange

Reusing the same value means observing the state (browser history, proxy logs, shoulder surfing) also reveals the code_verifier, weakening the PKCE guarantee.

Fix

Generate a separate base64url(randomBytes(32)) for state:

  const verifier = base64url(randomBytes(32));
+ const state = base64url(randomBytes(32));

createAuthorizationRequest now returns { url, verifier, state }, and exchangeCodeForTokens accepts state as a separate parameter.

Changes

File What
src/pkce.ts Separate state generation, added state parameter to exchangeCodeForTokens
src/index.ts Destructure and pass state through the auth flow
tests/pkce.test.ts 3 new tests: independence, URL inclusion, verifier not exposed in URL

Tests

124 tests pass:

bun test v1.3.10
 124 pass
 0 fail
 167 expect() calls

Generate a separate random value for the OAuth state parameter instead
of reusing the PKCE code_verifier. Per RFC 7636 and RFC 6749, these
serve different purposes:

- state: CSRF protection, visible in the redirect URL
- code_verifier: proof of possession, never exposed until token exchange

Reusing the same value means observing the state (browser history,
proxy logs) reveals the verifier, weakening the PKCE guarantee.

Changes:
- createAuthorizationRequest now returns { url, verifier, state }
- exchangeCodeForTokens accepts state as a separate parameter
- index.ts passes state through from request to exchange
- Three new tests verify independence and URL inclusion

Fixes cemalturkcan#16.
@cemalturkcan

Copy link
Copy Markdown
Owner

One security issue still remains here:

This PR generates a separate state, but the callback flow never validates it before exchanging the code. parseCallbackCode() only extracts the code, and index.ts passes whatever the user pasted straight into exchangeCodeForTokens(). That means the flow still lacks the session/CSRF check that state is supposed to provide.

If you want the state fix to be complete, the callback input needs to include the full callback URL or query string so the returned state can be parsed and compared against the generated one before the token request is sent.

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 state parameter reused as code verifier

2 participants