Add SPIFFE client authentication dispatch

[jhrozek]

Summary

Part of the SPIFFE client-authentication epic (#6199). This is the coordination commit: it introduces the shared SPIFFEClientResolver seam and the fosite ClientAuthenticationStrategy dispatcher that both the X.509 (#6202) and JWT (#6203) credential arms resolve through, so the two arms cannot diverge into different authorization outcomes for the same association.

• Adds pkg/authserver/spiffe (moved out of package authserver to break an import cycle with the new dispatch code): SPIFFEAuthenticationMethod/SPIFFEAuthorizationPolicy (aliased back into authserver for backward compatibility), plus a request-context carrier for a claimed SPIFFE ID (context.go) and the normalized principal type (principal.go).
• Adds SPIFFEAssociationRegistry (pkg/authserver/spiffe_association_registry.go): an immutable lookup from a validated SPIFFE trust configuration to configured associations, resolving a SPIFFE ID + OAuth client ID + method to a NormalizedSPIFFEPrincipal. An empty client ID derives the configured client from the association; a supplied one must exactly own it.
• Adds newSPIFFEClientAuthenticationStrategy (pkg/authserver/server/spiffe_client_auth.go) and wires it into NewAuthorizationServer (provider.go): when a request carries the SPIFFE JWT assertion type or an ambient SPIFFE ID from context, dispatch there; otherwise fall through to the existing default strategy (or a caller-supplied override, now explicitly preserved).
• The JWT arm itself is not implemented here — the dispatcher's JWT-shaped requests fail closed with a generic invalid_request. X.509 is not implemented either. Both real implementations land in the follow-up PRs (Implement JWT-SVID OAuth client authentication (spiffe_jwt) #6203, Implement X.509-SVID OAuth client authentication (spiffe_x509) #6202) that stack on top of this one.

Type of change

• New feature

Test plan

• Unit tests (task test)

go test ./pkg/authserver/... passes. Manually verified go build ./... and go vet ./... are clean.

API Compatibility

This PR does not touch operator API surface (no CRD/v1beta1 changes).

Changes

File	Change
pkg/authserver/spiffe/context.go, principal.go	New: request-context carrier + normalized principal type
pkg/authserver/spiffe_association_registry.go	New: immutable SPIFFE association lookup/resolve
pkg/authserver/server/spiffe_client_auth.go	New: fosite ClientAuthenticationStrategy dispatcher
pkg/authserver/server/provider.go	Wire the dispatcher into NewAuthorizationServer, preserving a caller-supplied fallback strategy
pkg/authserver/server_impl.go	Construct the association registry and resolver from RunConfig
pkg/authserver/spiffe_trust.go	Minor type-alias adjustment for the moved spiffe package

Does this introduce a user-facing change?

No — this lands scaffolding only; neither credential arm is reachable yet.

Special notes for reviewers

This is the first of a stack of SPIFFE client-auth PRs (#6199). #6203 (JWT-SVID) stacks directly on top of this branch; #6202 (X.509-SVID) stacks on top of #6203. Please review/merge in that order.

🤖 Generated with Claude Code

[codecov]

Codecov Report

❌ Patch coverage is 94.79167% with 5 lines in your changes missing coverage. Please review.
✅ Project coverage is 78.84%. Comparing base (1843e3f) to head (a831a49).
⚠️ Report is 62 commits behind head on main.

Files with missing lines	Patch %	Lines
pkg/authserver/server_impl.go	75.00%	4 Missing ⚠️
pkg/authserver/spiffe_association_registry.go	97.95%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #6562      +/-   ##
==========================================
+ Coverage   78.69%   78.84%   +0.15%     
==========================================
  Files         777      780       +3     
  Lines       77115    77683     +568     
==========================================
+ Hits        60684    61251     +567     
- Misses      16426    16427       +1     
  Partials        5        5              

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[JAORMX]

Fresh panel review of head 5dee1fb7521e74127473258a63175a702a71f747 against main 5b669870017ee4b1ea39ea1b95a03f4c5b47aeab and #6199.

Blocking — repeated SPIFFE assertion types can bypass fail-closed dispatch

pkg/authserver/server/spiffe_client_auth.go:40 uses url.Values.Get, which considers only the first client_assertion_type. A request that includes a non-SPIFFE value first and urn:ietf:params:oauth:client-assertion-type:jwt-spiffe later falls through to the configured/default strategy rather than being rejected. The dispatcher must detect all assertion-type occurrences and reject duplicates or any SPIFFE-shaped assertion it cannot safely handle; add order-independent regression coverage. The later JWT implementation should similarly reject duplicate client_assertion values.

Blocking — the resolver contract's empty-client-ID path does not work

The PR states an empty client ID derives the configured client from the SPIFFE association, but pkg/authserver/spiffe_association_registry.go:109-114 unconditionally looks up byClientID[clientID], and pkg/authserver/server_impl.go:371-374 then loads storage with that original empty ID. Resolve the association first, enforce client ownership only when a client ID was supplied, and load principal.ClientID(); cover this through newSPIFFEClientResolver.

Non-blocking — defer unused public bundle-source configuration until it has a consumer

pkg/authserver/server/provider.go:128-136,209-217 exposes X.509/JWT bundle-source fields but this coordination PR only forwards them. Adding the dependencies beside the verifying implementation—or behind a proven narrow capability—avoids committing the public provider API before the two authentication arms establish its final contract.

The standards, reuse, and duplication panel members found no additional actionable findings. CI is green at this head.

[JAORMX]

Fresh review at 5dee1fb, scoped to the PR-owned range from its actual merge base 1843e3f (not the stacked #6563 changes).\n\nApproved. The code-review, security, architecture, and duplication/reuse panel found no merge blockers. The resolver seam keeps both future credential arms on the same association path; the configuration copy isolates caller-owned Fosite config; and unsupported SPIFFE inputs remain fail-closed.\n\nNon-blocking follow-up: pkg/authserver/spiffe/principal.go:17-23 says authserver.SPIFFEAuthenticationMethod remains an independent type requiring conversion, but pkg/authserver/spiffe_trust.go:47-49 makes it an alias. Please align the comment to avoid misleading the follow-up arms.\n\nExact-head CI is green; that status is independent of this approval.

[JAORMX]

Superseding my approval: a concurrent current-head JAORMX panel review (review 5154925759) identified two unresolved blockers in this exact head, which I have now re-verified.\n\n### Blocking — duplicate SPIFFE assertion types can bypass the fail-closed dispatch\n\npkg/authserver/server/spiffe_client_auth.go:40 reads only the first client_assertion_type via url.Values.Get. A request with a non-SPIFFE type followed by the SPIFFE JWT type falls through to the default strategy instead of being rejected. Detect all occurrences and reject ambiguous SPIFFE-shaped requests regardless of order; add regression coverage.\n\n### Blocking — optional client_id resolution is not implemented end-to-end\n\npkg/authserver/spiffe_association_registry.go:109-114 unconditionally requires byClientID[clientID], and pkg/authserver/server_impl.go:371-374 loads the original (possibly empty) ID rather than the resolved principal.ClientID(). This contradicts the stated optional/derived client-ID contract. Resolve the association first, check ownership only for a supplied client ID, then load principal.ClientID(); cover it through newSPIFFEClientResolver.\n\nExact-head CI remains green and is separate from this review decision.

[jhrozek]

@JAORMX thanks for digging back into this, replying to both points from the latest review.

On the duplicate client_assertion_type one: I traced through how the dispatcher is wired into fosite. It's installed as ClientAuthenticationStrategy, and when it falls through, fosite's own DefaultClientAuthenticationStrategy reads client_assertion_type off the exact same form.Get(...) call — same first-value-wins behavior on both sides. So there's no split-brain where the dispatcher sees one value and the fallback authenticates against another. And since both SPIFFE arms right now just return "not implemented" unconditionally, there's nothing to bypass into even in principle. I don't think this one holds up as a blocker, happy to add a duplicate-value test as cheap hygiene if you'd still like it, but I'd frame it as HPP hardening on the token endpoint generally rather than something specific to the SPIFFE dispatch.

On the empty/optional client_id resolution: you're right about the code as written, Resolve does require byClientID[clientID] to hit, and the resolver closure passes the original clientID through to stor.GetClient instead of the resolved principal's. That said, neither SPIFFE arm calls into the resolver yet, both return before reaching it, so this is currently dead code rather than something reachable today, and it fails closed either way. I'll fix it now anyway since it's cheap and better to get right before the arm that actually calls this lands: resolve the association first, only enforce ownership when a client ID was supplied, and use principal.ClientID() for the storage lookup.

Will push the fix shortly.

[jhrozek]

@JAORMX pushed the fix in c29c044.

Resolve now only enforces client-ID ownership when a client ID was actually supplied; an empty one derives the client entirely from the resolved association, and newSPIFFEClientResolver uses that resolved principal.ClientID() for the storage lookup instead of the original (possibly empty) parameter.

While going through this, a second-order issue came up: once the empty-client-ID path actually works, there's no ownership check left to catch an ambiguous match if two configured associations' patterns could both match the same concrete SPIFFE ID, resolution would depend on Go's map iteration order. NewSPIFFEAssociationRegistry now rejects overlapping patterns at construction (reusing the same overlap check NewSPIFFETrustConfig already applies one layer up, so this is largely belt-and-suspenders, but the registry didn't enforce its own invariant before).

Re the duplicate-assertion-type point from earlier, still think that one doesn't hold, the SPIFFE dispatcher and fosite's own default strategy read client_assertion_type through the identical form.Get(...) call, so there's no divergence to exploit, and both SPIFFE arms are unconditional-error stubs today regardless. Happy to add a duplicate-value rejection as general hygiene if you'd still like it, but wanted to flag the reasoning before doing so.

[JAORMX]

Fresh panel review of exact head c29c0449f13e3cf292948aaee7e0e84805e7e9f1, scoped to #6562’s PR-owned range from merge base 1843e3ff662856dcb0bfd68082355a46c6370c51 (excluding #6563 child changes).

Blocking — duplicate client_assertion_type values can downgrade a SPIFFE-shaped request

pkg/authserver/server/spiffe_client_auth.go:40 still uses url.Values.Get, so it inspects only the first client_assertion_type. A request containing a non-SPIFFE type first and urn:ietf:params:oauth:client-assertion-type:jwt-spiffe second falls through to defaultStrategy instead of being rejected. Fosite can then authenticate the first value (for example, a valid private-key JWT assertion), making dispatch order-dependent. That violates #6199’s fail-closed rule that missing or malformed SPIFFE credentials must not downgrade to another authentication method.

Inspect all occurrences and reject repeated/ambiguous assertion types (or route whenever any SPIFFE assertion type is present) before fallback; add order-independent regression coverage. The forthcoming JWT arm should likewise reject duplicate client_assertion values.

Resolved from my prior review: SPIFFEAssociationRegistry.Resolve now derives the configured client for an omitted client_id and checks ownership only when one was supplied (pkg/authserver/spiffe_association_registry.go:119-138); newSPIFFEClientResolver now loads principal.ClientID() (pkg/authserver/server_impl.go:371-375).

Non-blocking: the public bundle-source plumbing has no consumer yet, and pkg/authserver/spiffe/principal.go retains a stale independent-type/conversion comment despite the alias in spiffe_trust.go.

Exact-head CI is green; this is independent of the review decision.

[jhrozek]

@JAORMX pushed the fix in a831a49.

You were right on this one, I went and checked #6199 directly and it does state "missing or malformed SPIFFE credentials must not downgrade to another authentication method" as an explicit design constraint for this epic, so this isn't just a hypothetical ordering quirk, it's a real violation of that invariant. Also ran it by a SPIFFE-focused review for a second opinion, which confirmed the same thing: once SPIFFE-shaped material is present in a request, it should commit the request to that path rather than let parsing order decide, same posture as how mTLS-terminating proxies treat a peer cert.

The dispatcher now checks all client_assertion_type values via slices.Contains instead of form.Get's first-value-only read, so a SPIFFE-shaped value anywhere in the list routes into the SPIFFE arm regardless of position. Added a regression test for the SPIFFE-value-second case (the existing "takes precedence" test already covered SPIFFE-first).

[JAORMX]

Fresh panel review of exact head a831a492a2f3b613c7230719e86994954718541c, scoped to #6562’s PR-owned range from merge base 1843e3ff662856dcb0bfd68082355a46c6370c51 and #6199.

Approved. The previous blocking duplicate-assertion-type issue is resolved: pkg/authserver/server/spiffe_client_auth.go:44 examines every client_assertion_type, and the order-independent regression coverage is in pkg/authserver/server/spiffe_client_auth_test.go:57-68. The resolver now derives an omitted client ID from the association and loads the resolved client (pkg/authserver/server_impl.go:370-375).

The security, spec, standards, reuse, and duplication panel found no remaining merge blockers. The architecture panel again notes the currently unused public bundle-source plumbing in pkg/authserver/server/provider.go:128-136,209-217; this is safe dead scaffolding rather than a correctness or security regression, so it remains non-blocking follow-up work.

Current-head CI is still running; the completed checks are green, and that status is independent of this approval.