[jhrozek]

Summary

• Why: the configured delegate client fields from the previous PR were validated but never reached the running server, so a pre-provisioned broker could not use private_key_jwt or log users in. Once a configured client can hold grants other than token exchange, two checks that assumed delegates were back-channel-only have to change.
• What:
  • The runner reads each jwks_file next to the client secret handling and passes keys, auth method, grant types and redirect URIs to the client constructor. A jwks_file with more than one key must give every key a unique kid, and the client must send the kid of its signing key, because fosite checks an assertion without a kid only against the first signing key.
  • Discovery advertises private_key_jwt when a configured client uses it, and the client_secret_* methods only when confidential DCR is on or a configured client authenticates with a secret. The cleartext-transport rule from Add configured delegate client metadata #6952 is unchanged: every delegate client, private_key_jwt ones included, counts as confidential.
  • /authorize treats every client without authorization_code as back-channel only and answers invalid_client, as for an unknown client, so a client's existence cannot be probed.
  • Delegate clients are built and checked as consent-exempt before they are written, so a configured login client can be listed there and a bad consent-exempt ID aborts startup without leaving delegate rows in storage.
  • /authorize checks the requested resource against the client's own audiences (its resource allowlist for SPIFFE clients), and ID-JAG redemption does the same, both with invalid_target. The check uses the server's configured audience matching strategy, the rule the authorization code callback and the refresh grant already apply. The server-level and client-level refusals return the same hint, so an unauthenticated caller cannot tell which one refused.
  • In ID-JAG, DCR and CIMD clients are not narrowed (their audience is a copy of the server's list from registration), and every other client is, including one with no audiences. A client that sets grant_types explicitly must list jwt-bearer to redeem an ID-JAG, or it gets unauthorized_client. A client that omits grant_types is not gated, for backward compatibility.
  • The removal test harness gets two configured login clients, so removing one is covered.
  • The delegate client integration tests share one embedded auth server helper.

#6952 has merged, so this branch is rebased onto main and contains only these four commits.

Refs #6893

Type of change

• New feature

Test plan

• Unit tests: go test -race with the Taskfile's flags on ./pkg/authserver/... and ./docs/server/..., and task sdk-test. All pass. task test runs the whole repository and cannot be scoped, so I did not run it.
• Linting (task lint-fix): not run on this tip, CI runs it.

Each of the four commits also passes go vet ./pkg/authserver/... on its own, and task docs regenerates nothing. The integration tests in pkg/authserver cover a configured client logging in, private_key_jwt authentication through the runner, key rotation, discovery for secret, key-only and mixed delegates, the audience checks, and the removal harness for configured login clients, including a refresh token coming back when a removed ID is added again (Redis). E2E tests were not run.

Does this introduce a user-facing change?

Yes.

• Before: configured delegate client fields were ignored at runtime. /authorize did not enforce a client's declared audiences.
• After:
  • /authorize enforces a client's declared audiences with the same rule as the callback. A client with no audiences, which could never complete the callback, now gets invalid_target at /authorize instead of failing after the upstream login.
  • A DCR client keeps the audience list the server had when it registered. After an operator adds an audience, an existing DCR client that asks for it now gets invalid_target at /authorize. Before, the same request failed after the upstream login with invalid_request. The client has to register again.
  • ID-JAG redemption checks the resource against the client's audiences. A client registered without audiences (for example by an embedder) could redeem an ID-JAG for any allowed resource before. It now gets invalid_target. DCR and CIMD clients are not narrowed there.
  • Delegate clients that omit grant_types are not gated on jwt-bearer, but their ID-JAG resource must now be in their audiences.
  • A client without authorization_code gets invalid_client from /authorize.
  • Discovery lists private_key_jwt (with the signing algorithms) when a configured client uses it. It no longer lists client_secret_basic and client_secret_post when every delegate uses private_key_jwt and confidential DCR is off.
  • The hint text of the server-level audience refusal changes at /authorize, /token and in token exchange.
  • A configured client's jwks_file with more than one key needs a unique kid on every key, or startup fails.

Special notes for reviewers

• The audience check matches like fosite does by default: scheme and host exactly, and a path that equals the client's audience path or extends it at a segment boundary. A delegate audience such as https://h/ therefore covers every child path that is also in the server's allowed audiences. The server-level check that runs first still requires an exact match.
• /authorize answers invalid_client with no redirect for a known client without authorization_code. RFC 6749 section 4.1.2.1 would use unauthorized_client, but that would show which IDs exist.
• The ID-JAG grant gate stays a per-replica list of client IDs built from each replica's own config, not a marker on the stored row. Changing the default for omitted grant_types to include jwt-bearer would change the stored policy of every default delegate, so each one would be replaced on upgrade and older binaries would refuse the row.
• HasStaticSecretClients has the name of a flag the review of Add configured delegate client metadata #6952 removed. That flag did two jobs (transport rule and discovery). This one only drives discovery, and the transport rule still uses HasStaticDelegateClients.
• The plain JWT-bearer grant is not gated by the jwt-bearer rule, because it can run without client authentication.
• Rollout: roll out the new binary before updating a RunConfig that uses the new delegate fields. A pod on the previous binary validates grant_types, redirect_uris, token_endpoint_auth_method and jwks_file but its runner drops them. If it restarts after the config changed, it replaces the stored login client with a token-exchange-only row until a new-binary pod restarts, so /authorize answers invalid_client for that client, and a client pinned to one secret method accepts both. A private_key_jwt client makes the old pod fail at startup. The CRD does not expose these fields, so only a hand-written RunConfig can hit this today.
• Found while reviewing, not changed here: Device-code tokens carry every client audience and ignore client audience limits #6982 (device-code tokens carry every client audience), ID-JAG grant ignores the assertion's resource claim and requires a form resource #6983 (ID-JAG ignores the assertion's resource claim and requires a form resource), Reconsider public-client redemption of ID-JAG assertions in the embedded AS #6984 (public clients can redeem ID-JAG assertions).
• Commit by commit: the helper move first (no behaviour change), then the runner wiring, then audience binding, then the test coverage.

🤖 Generated with Claude Code

[codecov]

Codecov Report

❌ Patch coverage is 98.11321% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 80.26%. Comparing base (1710945) to head (7cf6ed5).
⚠️ Report is 1 commits behind head on main.

Files with missing lines	Patch %	Lines
pkg/authserver/server_impl.go	93.93%	2 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #6953      +/-   ##
==========================================
+ Coverage   80.24%   80.26%   +0.02%     
==========================================
  Files         847      847              
  Lines       86097    86218     +121     
==========================================
+ Hits        69085    69204     +119     
- Misses      17007    17009       +2     
  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.

[tgrunnagle]

I checked the main claims against the code and fosite v0.49.0:

• Callback parity: fosite's HandleAuthorizeEndpointRequest checks client.GetAudience() against the requested audience, and the callback sets that to pending.Resource. The strategy is the same one provider.go:520 configures. So the new /authorize check moves an existing failure earlier and doesn't add a new one.
• Back-channel predicate: NewStaticDelegateClient always sets explicit grant types, and fosite defaults empty grant types to authorization_code. !Has(authorization_code) therefore doesn't misclassify legacy or empty-grant clients.
• Runner: run-config validation already rejects a secret reference combined with private_key_jwt, so skipping secret resolution hides nothing. validatePrivateKeyJWTKey rejects non-public keys and keys without use: sig, so a private JWK in jwks_file fails at startup.
• ID-JAG ordering: the grant gate and the audience check both run before the assertion is consumed.

Testing: coverage is strong. There are dual-backend integration tests for private_key_jwt login and refresh, the /authorize rejection, ID-JAG explicit grants, consent-exempt startup and key rotation. The discovery tests cover all combinations, and the audience tests include a parity test against fosite. Gaps worth filling:

• a test that a refused ID-JAG (missing grant, or resource outside the client's audiences) leaves the assertion unconsumed;
• [refresh_token]-only and empty-response-type cases in the back-channel test table, since that check itself changed;
• an integration test that pins the fail-open path for an embedder client with no audiences.

[ChrisJBurns]

• medium · correctness — Client audience check hardcodes fosite's default strategy instead of the configured one (pkg/authserver/server/audience.go, pkg/authserver/server/tokenexchange/handler.go)

  ValidateClientAudience calls fosite.DefaultAudienceMatchingStrategy, but the RFC 8693 path and fosite's callback use config.GetAudienceStrategy(ctx). If a custom strategy is configured, /authorize can disagree with the callback, which breaks the 'cannot fail later' claim.

The base branch was changed.

[jhrozek]

Pushed an update. Rebased onto main now that #6952 is merged, so the PR is the four commits only. Thanks both for the reviews. Replies to the inline threads are above; the rest:

@ChrisJBurns the audience strategy point is fixed in 5150c3d. The client audience check takes the strategy as a parameter, /authorize and ID-JAG pass the configured one, and the token exchange path calls the same function.

@tgrunnagle on the test gaps:

• A refused ID-JAG leaves the assertion unconsumed: TestIDJAGHandler_ClientGrantAndAudience asserts that for every refused case, including the new ones (client without audiences, configured strategy refuses).
• [refresh_token]-only and empty response types in the back-channel table: the "missing authorization-code grant" row uses [refresh_token], and the back-channel-only row has no response types.
• The fail-open integration test: no longer needed, since the fail-open is gone. There is a test that a client without audiences is refused.

Other things that changed since the first round, from our own review of the stack:

• The two audience refusals at /authorize now return the same hint text. They differed, so an unauthenticated caller who knew a client ID and redirect URI could tell which policy refused a resource.
• A multi-key jwks_file needs a unique kid on every key (fosite only checks a kid-less assertion against the first key, which breaks rotation).
• Discovery lists client_secret_* only when confidential DCR is on or a configured delegate uses a secret. The cleartext-transport rule from Add configured delegate client metadata #6952 is unchanged.
• New tests for a stale stored row losing to the built delegate, and for a removed login client's refresh token working again when the ID is added back (Redis).

Three things I found while reviewing that are not caused by this PR are filed separately: #6982, #6983, #6984. The PR description has the before/after list and a rollout note.

[ChrisJBurns]

Approving. I reviewed it for security, for correctness (with mutation checks), and against the Mecatl design and our merged broker client. No high or critical findings. go test -race ./pkg/authserver/... passes, and all 22 mutations were caught. The /authorize audience strategy fix is in. Mecatl's real assertion (typ: client-authentication+jwt, ES256, thumbprint kid, aud = issuer string, 60 s lifetime) is accepted against this branch.

None of the following block this PR. 1 and 2 come from fosite's existing behaviour rather than this change; 3 and 4 are about this change.

1. Client-authentication failures return 500. An iat slightly in the future, an expired assertion, or a lifetime over 5 minutes returns 500 with "error":"error". RFC 6749 §5.2 and the Mecatl design's step (5) say a failed client authentication is 401 invalid_client. As it stands, the client can't tell what went wrong.

2. An unknown kid returns 400 invalid_request (from fosite's findPublicKey), not 401 invalid_client. At session open, Mecatl reads an invalid_request with no reason as "the user's access token was refused" (the design reports a bare invalid_request at open as subject_invalid). So a key not yet registered during rotation would surface to users as a sign-in problem instead of a configuration fault. Could it map to 401 invalid_client?

3. A removed key can come back. registerDelegateClients overwrites the shared Redis row on every start. If a pod with old config restarts during a rollout (crash or eviction) after an operator has removed a compromised key, it writes the key back, and the key works cluster-wide until a new-config pod restarts. Suggested fix: store a config generation or fingerprint on the row and refuse to overwrite a newer one.

4. Test gap: no test pins the /authorize audience strategy to the configured one. Production uses fosite's default, so hard-coding fosite.DefaultAudienceMatchingStrategy would still pass every test. A handler test that injects a custom strategy would pin the fix.

Still open for #6893 after this PR, just for tracking:

• the token-exchange source policy (delegates still get blanket rights, so #6893's "unlisted client is refused" acceptance check fails);
• #6954 (skew and the 60 s maximum; it's 5 minutes today);
• the CRD fields;
• reloading on key edits (jwks_file is read only at startup).

Smaller notes:

• token_endpoint_auth_signing_alg defaults to RS256, so startup fails for an ES256 key unless it's set. Maybe derive it from the key.
• Discovery lists every supported algorithm even when every client pins one.

[jhrozek]

Thanks for the approval and for the careful review, @ChrisJBurns.

Point 4 (no test pins the strategy): I hard-coded fosite.DefaultAudienceMatchingStrategy at both call sites on 7cf6ed5 and ran the tests. TestAuthorizeHandler_ResourceSelection fails for /authorize, and TestIDJAGHandler_ClientGrantAndAudience fails for ID-JAG. Both have a "configured audience strategy is applied" row that injects fosite's exact-match strategy, which refuses a path-prefix resource that the default accepts.

Points 1 and 2 (500 and invalid_request): both are real and not specific to this PR. fosite returns the raw inner error from claim validation (which becomes a 500), an unknown kid is invalid_request there, and our own SetClientAssertionJWT returns a plain error for a lifetime over 5 minutes. It affects every private_key_jwt client, so I'm not changing it here. It is filed as #6995 and linked to #6954, since it is the same code path.

Point 3 (a removed key can come back): agreed, it is a real window for revoking a key. It is the rolling-update behavior docs/arch/11 describes for any policy, and the follow-up branch that rolls pods on JWKS edits makes it matter more, so I'm tracking it there. Serving keys from each replica's own config looks like the right shape. I'm leaving this PR as it is so the approval stays.

The signing algorithm derivation goes with the CRD branch, and I'm leaving the discovery algorithm list alone.