OAuth2: bound state map with TTL, add PKCE S256 (M-7) #110

Merged

qwc merged 1 commit from feature/oauth2-state-ttl-pkce into main

2026-05-18 22:19:20 +02:00

qwc commented

2026-05-18 21:56:46 +02:00

Owner

Summary

Workstream item #15. Audit finding M-7 (Medium): two OAuth2 flow issues.

Unbounded state map. states map[string]bool grew on every /auth/oauth2 hit and was only pruned when the corresponding callback consumed the state. An unauthenticated attacker spamming the endpoint kept entries forever.
No PKCE. RFC 7636 protects against authorization code interception, especially relevant for public clients or sloppy redirect URIs.

Changes

State map: map[string]stateEntry where stateEntry { expiresAt time.Time; codeVerifier string }.
oauth2StateTTL = 10 * time.Minute.
sweepExpiredLocked drops aged entries on every GenerateAuthURL call. No janitor goroutine — the map is bounded by the number of in-flight issuances within any 10-minute window.
PKCE: GenerateAuthURL now generates an oauth2.GenerateVerifier() per state and appends S256ChallengeOption(verifier) to the auth URL. The verifier is stored in the stateEntry.
ValidateState(s) bool → ConsumeState(s) (verifier string, ok bool). Handler passes the verifier through to HandleCallback.
HandleCallback(ctx, code, codeVerifier) — verifier sent to the token endpoint via oauth2.VerifierOption (RFC 7636 §4.5).

Tests

New oauth2_csrf_test.go:

AuthCodeURL contains code_challenge= and code_challenge_method=S256
ConsumeState returns the verifier and removes the entry; second call fails
Expired entries are rejected even when the map still contains them (TTL check inside ConsumeState)
100 issuances → age all entries → 1 more issuance → sweep leaves only the new one

Existing tests updated for the new HandleCallback / ConsumeState signatures.

Test plan

go test ./... passes
New tests cover PKCE, ConsumeState, TTL, sweep
Manual: hit /auth/oauth2, observe code_challenge=...&code_challenge_method=S256 in the redirect URL
Manual: with a real IdP, end-to-end login still works (Keycloak, Auth0, etc.)
Manual: leave a login attempt unredeemed for >10 minutes; trying to use the state after that returns "Invalid or expired" on the login page

🤖 Generated with Claude Code

## Summary Workstream item #15. Audit finding **M-7 (Medium)**: two OAuth2 flow issues. 1. **Unbounded state map.** `states map[string]bool` grew on every `/auth/oauth2` hit and was only pruned when the corresponding callback consumed the state. An unauthenticated attacker spamming the endpoint kept entries forever. 2. **No PKCE.** RFC 7636 protects against authorization code interception, especially relevant for public clients or sloppy redirect URIs. ## Changes - **State map**: `map[string]stateEntry` where `stateEntry { expiresAt time.Time; codeVerifier string }`. - **`oauth2StateTTL = 10 * time.Minute`**. - **`sweepExpiredLocked`** drops aged entries on every `GenerateAuthURL` call. No janitor goroutine — the map is bounded by the number of in-flight issuances within any 10-minute window. - **PKCE**: `GenerateAuthURL` now generates an `oauth2.GenerateVerifier()` per state and appends `S256ChallengeOption(verifier)` to the auth URL. The verifier is stored in the `stateEntry`. - **`ValidateState(s) bool`** → **`ConsumeState(s) (verifier string, ok bool)`**. Handler passes the verifier through to `HandleCallback`. - **`HandleCallback(ctx, code, codeVerifier)`** — verifier sent to the token endpoint via `oauth2.VerifierOption` (RFC 7636 §4.5). ## Tests New `oauth2_csrf_test.go`: - AuthCodeURL contains `code_challenge=` and `code_challenge_method=S256` - `ConsumeState` returns the verifier and removes the entry; second call fails - Expired entries are rejected even when the map still contains them (TTL check inside `ConsumeState`) - 100 issuances → age all entries → 1 more issuance → sweep leaves only the new one Existing tests updated for the new `HandleCallback` / `ConsumeState` signatures. ## Test plan - [x] `go test ./...` passes - [x] New tests cover PKCE, ConsumeState, TTL, sweep - [ ] Manual: hit `/auth/oauth2`, observe `code_challenge=...&code_challenge_method=S256` in the redirect URL - [ ] Manual: with a real IdP, end-to-end login still works (Keycloak, Auth0, etc.) - [ ] Manual: leave a login attempt unredeemed for >10 minutes; trying to use the state after that returns "Invalid or expired" on the login page 🤖 Generated with [Claude Code](https://claude.com/claude-code)

qwc added 1 commit

2026-05-18 21:56:46 +02:00

OAuth2: bound state map with TTL; add PKCE S256 (M-7)

CI / test (pull_request) Successful in 1m20s

Details

CI / build (pull_request) Successful in 59s

Details

CI / docker (pull_request) Has been skipped

Details

08124ebb3f

Audit finding M-7. Two issues in the OAuth2 flow:

  1. states map[string]bool grew on every /auth/oauth2 hit and was only
     pruned when the corresponding callback consumed the state. An
     unauthenticated attacker spamming the endpoint kept entries forever.

  2. No PKCE (RFC 7636). A public-client or sloppy-redirect deployment
     was more exposed than necessary to authorization code interception.

Changes:

  - stateEntry { expiresAt time.Time; codeVerifier string }; the map is
    map[string]stateEntry now.
  - oauth2StateTTL = 10 minutes.
  - sweepExpiredLocked drops aged entries on every GenerateAuthURL call —
    no janitor goroutine, map is bounded by in-flight issuances in any
    one TTL window.
  - GenerateAuthURL also calls oauth2.GenerateVerifier + S256ChallengeOption
    so the auth URL carries code_challenge + code_challenge_method=S256.
  - ValidateState becomes ConsumeState(state) (verifier string, ok bool).
    The handler passes verifier to HandleCallback.
  - HandleCallback signature gains codeVerifier; passes it to Exchange via
    oauth2.VerifierOption (RFC 7636 §4.5).

New tests (oauth2_csrf_test.go) cover:
  - AuthCodeURL includes code_challenge + S256
  - ConsumeState returns the verifier and deletes the entry
  - Expired state entries are rejected by ConsumeState
  - 100 issuances + age-out + 1 more issuance → sweep leaves only 1

Existing tests updated for the new HandleCallback / ConsumeState
signatures.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

qwc merged commit e2610475da into main

2026-05-18 22:19:20 +02:00

qwc deleted branch feature/oauth2-state-ttl-pkce

2026-05-18 22:19:20 +02:00

qwc referenced this pull request from a commit

2026-05-18 22:19:20 +02:00

Merge pull request 'OAuth2: bound state map with TTL, add PKCE S256 (M-7)' (#110) from feature/oauth2-state-ttl-pkce into main

qwc referenced this pull request from a commit

2026-07-13 13:19:10 +02:00

Merge pull request 'OAuth2: bound state map with TTL, add PKCE S256 (M-7)' (#110) from feature/oauth2-state-ttl-pkce into main