Skip to content

[4/15] Add PKCE and single-use state CSRF protection to the OAuth client flow - #157

Open
UnknownJoe796 wants to merge 2 commits into
version-5.3from
v53/oauth-pkce
Open

[4/15] Add PKCE and single-use state CSRF protection to the OAuth client flow#157
UnknownJoe796 wants to merge 2 commits into
version-5.3from
v53/oauth-pkce

Conversation

@UnknownJoe796

Copy link
Copy Markdown
Contributor

Adds PKCE and single-use state CSRF protection to the OAuth client flow.

Pkce.kt is new; OauthCallbackEndpoint now rejects reused or unknown state
values. OauthCallbackSecurityTest covers the happy path and the replay and
forgery cases.

@UnknownJoe796

Copy link
Copy Markdown
Contributor Author

I'll feel more comfortable with this when it tests against an external, official (if possible) OAuth server.

UnknownJoe796 and others added 2 commits July 29, 2026 17:08
The OAuth client flow previously round-tripped a caller-supplied `state`
through the provider without storing or validating it (TODO acknowledged
in-code), and used no PKCE.

- state/CSRF: the value sent to the provider is now an opaque 256-bit
  single-use nonce. At flow start a FlowRecord (caller state + PKCE
  verifier) is stored in the cache keyed by the nonce with a short TTL;
  the callback validates and consumes it via getAndRemove BEFORE any
  token exchange, so unknown/expired/replayed callbacks are rejected.
  The caller's app STATE is preserved server-side and still delivered to
  onAccess; it no longer leaks to the provider.
- PKCE (RFC 7636): S256 code_challenge on the auth redirect and
  code_verifier on token exchange, gated by a per-provider supportsPkce
  flag (default true). Verified against the RFC Appendix B test vector.
- Confirmed the redirect_uri (fixed callback URL) and final UI redirect
  (driven only by a server-signed Proof) are not attacker-controllable;
  documented rather than adding an unnecessary whitelist.

Also fixes two latent bugs uncovered here: a NPE on code.state!! and an
onError result that was computed then discarded (execution fell through).

Requires a cache: OauthCallbackEndpoint/OauthProofEndpoints take a
Runtime<Cache> and loginUrl is now suspend (source-breaking on the 5.x
line). Docs note the shared-cache requirement for multi-instance
deployments and the residual login-CSRF limitation of cookie-less state.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
(cherry picked from commit d0bc38c)
(cherry picked from commit 72e2eed)
@UnknownJoe796
UnknownJoe796 force-pushed the v53/security-headers branch from d0355f1 to ee913c8 Compare July 30, 2026 02:19
@UnknownJoe796
UnknownJoe796 changed the base branch from v53/security-headers to version-5.3 July 30, 2026 02:24
@UnknownJoe796

Copy link
Copy Markdown
Contributor Author

More tests is good, but I want this tested against externally managed code.

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.

1 participant