fix(oauth): capture DPoP JKT in standard authorize flow and harden nil guards - #121
Open
haileyok wants to merge 3 commits into
Open
fix(oauth): capture DPoP JKT in standard authorize flow and harden nil guards#121haileyok wants to merge 3 commits into
haileyok wants to merge 3 commits into
Conversation
…l guards
The standard (non-PAR) authorize flow had an empty DPoP JKT block, so
DPoP-bound clients that didn't use PAR silently lost their DPoP binding.
This caused tokens to be issued as Bearer instead of DPoP, and a nil
pointer dereference when the token was used against protected endpoints.
Changes:
- handle_oauth_authorize.go: Validate DPoP proof header, pass it to
AuthenticateClient, and capture JKT for DPoP-bound clients (matching
the PAR handler). Return an error when dpop_jkt is required but not
provided.
- handle_oauth_par.go: Guard against nil dpopProof before accessing
.JKT to prevent nil dereference when a DPoP-bound client omits both
the proof header and the dpop_jkt parameter.
- middleware.go: Replace raw dereference of DpopJkt with the nil-safe
dpopJktMatches helper to prevent panics on non-DPoP-bound tokens.
- handle_oauth_token.go: Fix cnf claim to use RFC 9449 format
{"jkt": "..."} instead of a bare string, in both authorization_code
and refresh_token grants. Also fixes a *string vs string bug in the
refresh grant (the *& was a no-op).
- handle_well_known.go: Advertise granular scope resources (repo, rpc,
blob, account, identity, include) in scopes_supported metadata.
…bsent The previous hard-error for missing dpop_jkt broke DPoP-bound clients that use the standard authorize flow without sending dpop_jkt (e.g. pckt.blog). Instead of rejecting the request, the authorize and PAR endpoints now silently proceed without dpop_jkt. The token endpoint falls back to the DPoP proof presented at token exchange time to determine the effective JKT for the cnf claim, token type, and stored token parameters. This keeps the DPoP binding correct while maintaining interoperability with clients that don't send dpop_jkt in the authorize request.
The CORS config used AllowOrigins: ["*"] and AllowHeaders: ["*"] with AllowCredentials: true. Per the Fetch spec, wildcard values for origin and headers are invalid when credentials are allowed — browsers reject the preflight, silently blocking the actual request. This was preventing OAuth token exchanges from browser-based clients (like pckt.blog) that send credentials with their fetch requests. The preflight would be blocked, so POST /oauth/token never reached the server. Fix: use AllowOriginFunc to echo the specific Origin header, and omit AllowHeaders so Echo reflects the client's requested headers (both valid for credentialed requests).
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.
Problem
DPoP-bound clients (those with
dpop_bound_access_tokens: true) that use the standard (non-PAR) authorize flow — sending query params directly to/oauth/authorizeinstead of going through/oauth/par— silently lose their DPoP binding. This causes OAuth to fail: tokens are issued as Bearer instead of DPoP, and using them against protected endpoints triggers a nil pointer panic.Root Cause
The standard authorize handler (
handle_oauth_authorize.go) had an empty DPoP JKT block where the PAR handler (handle_oauth_par.go) actively captures the JKT. It also passednilas the proof toAuthenticateClient, so no DPoP validation ever happened.Changes
handle_oauth_authorize.go) — Validate DPoP proof header, pass toAuthenticateClient, capture JKT for DPoP-bound clients or error ifdpop_jktis required but missing.middleware.go) — Replace raw*DpopJktdereference with the nil-safedpopJktMatcheshelper.handle_oauth_par.go) — Guard against nildpopProofbefore accessing.JKT.cnfclaim format (handle_oauth_token.go) — Fix to RFC 9449{"jkt": "..."}object in both auth code and refresh grants; also fixes a*stringvsstringbug in refresh grant.scopes_supported(handle_well_known.go) — Add granular scope resources (repo, rpc, blob, account, identity, include).Testing
go vet ./...— cleango build ./...— cleango test ./...— all pass