refactor(auth): stateless OAuth2 state via securecookie - #38
Merged
Conversation
Drops the in-memory state map, the RWMutex protecting it and the cleanup goroutine. The OAuth2 `state` parameter is now a self-contained signed blob — `securecookie.Encode` packs (returnURL, expiry) into the string we hand to the IdP, `Decode` verifies the signature and pulls them back out on callback. No server-side bookkeeping. Removed: - states map[string]*StateData + statesMu sync.RWMutex - cleanupStates goroutine + stateCleanupEvery constant - StateData struct (replaced by tiny private stateBlob) - crypto/rand + encoding/base64 + sync imports Added: - github.com/gorilla/securecookie (battle-tested; the lib handles all the HMAC + AES details and the MaxAge bookkeeping) Trade-offs vs the in-memory map: - Pod restart: same behaviour. Both the in-memory map AND the process-random securecookie keys are lost on restart, so in-flight logins fail closed either way. Stable keys (env or k8s secret) is a one-line follow-up that would also let multiple replicas share state. - Replay within TTL: theoretically possible with the new code (no nonce store to detect reuse), where the old code deleted on first use. Mitigated by the IdP's own one-time-code semantics and the 10-min TTL. Acceptable for the threat model; can add a nonce later if needed. Stats: - oidc.go: 252 -> 234 lines (-18) - Combined with phases A+B: 655 -> 319 (-336, -51% of pre-refactor auth code)
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Phase C of the auth-code rationalisation. Drops the hand-rolled state map + RWMutex + cleanup goroutine; replaces them with a signed self-contained blob carried in the OAuth2
stateparameter itself.What's gone
states map[string]*StateDatastatesMu sync.RWMutexcleanupStatesgoroutine +stateCleanupEveryconstantcrypto/rand+encoding/base64+syncimportsWhat's new
github.com/gorilla/securecookie— battle-tested HMAC + AES helper.Encode("dploy-state", blob)returns the URL-safe signed string we use asstate=…;Decode(…)verifies + returns the blob. About 8 lines of integration.Trade-offs vs the in-memory map (same as today)
Stable keys (future)
Process-random keys mean restart loses keys. Promoting them to env vars or a k8s Secret is a one-line follow-up that would also let multiple replicas share state across restarts. Out of scope for this PR.
Stats
oidc.goBuild green, existing
internal/auth+internal/handlerstests pass.