[jhrozek]

Summary

• Why: delegate clients could only be secret-authenticated, token-exchange-only clients. Stable pre-provisioned broker client for token exchange and ID-JAG redemption #6893 needs a pre-provisioned broker that keeps its client_id across key rotation, so the configuration has to accept an auth method, a JWKS file, grant types and redirect URIs.
• What:
  • delegate_clients[] accepts token_endpoint_auth_method, token_endpoint_auth_signing_alg, jwks_file, grant_types and redirect_uris. All are optional.
  • One validator in the registration package checks them and shares the DCR private_key_jwt key check. It runs on RunConfig (keys not loaded yet), on the resolved Config (keys loaded) before any client is registered, and when the client is built.
  • Redirect URIs follow the confidential-client rule: https, not loopback. refresh_token requires authorization_code, as in DCR.
  • Every delegate client still counts as confidential for the cleartext-transport check, private_key_jwt ones included (see the notes below). The transport errors now name both triggers (confidential registration or delegate clients) instead of only confidential registration.
  • Replacing a configured client now also logs the WARN from Let delegate clients change policy on restart #6918 when its grant types, redirect URIs or auth method change. A JWKS change is key rotation and stays silent.
• The runner does not load the JWKS file or pass the new fields yet. The next PR in the stack, Wire configured delegate clients into the runner #6953, wires them. The two PRs are meant to merge back-to-back, with no release in between.

Refs #6893

Type of change

• New feature

Test plan

• Unit tests (task test) — run on the packages this PR touches, not the whole repository (see below)
• Linting (task lint-fix) — golangci-lint on the same packages, no findings

Tests, lint and vet were run on the touched packages at the tip of the stack, which includes this PR. The generated docs and SDK files were regenerated and task docs-verify reports no drift. The validator has table-driven tests for each rule on the RunConfig, resolved Config and constructor paths. After review, go test ./pkg/authserver/... and the operator unit tests were rerun on this commit.

Does this introduce a user-facing change?

Yes, to the RunConfig schema and to two error and log messages.

• Before: a delegate client entry could only carry the secret, scopes and audiences. A client had to be token-exchange-only and secret-authenticated.
• After: the five optional fields above are accepted and validated. Invalid combinations (for example private_key_jwt with a client secret, or refresh_token without authorization_code) now fail validation at startup.
• Until Wire configured delegate clients into the runner #6953 lands, the runner ignores the new fields: grant_types, redirect_uris and an auth method pin are dropped (the client stays token-exchange-only), and a private_key_jwt client fails at startup with resolved client secret is empty. No configuration ends up with more access than it asked for. The operator CRD does not expose these fields.
• The cleartext-transport errors now start with confidential clients (allow_confidential_client_registration or delegate clients) ... instead of allow_confidential_client_registration ....
• The "configured client replaced with a different policy" WARN gains old_/new_grant_types, old_/new_token_endpoint_auth_method and old_/new_redirect_uris.

Special notes for reviewers

• Transport rule for private_key_jwt delegates. These clients send no reusable secret, but they still count as confidential. Unlike DCR private_key_jwt clients, which can only use token exchange, a delegate client can hold authorization_code and refresh_token, so codes and tokens would cross plain HTTP. jti replay checks and the 5-minute assertion cap already limit what a captured assertion is worth, so this is a conservative choice, not a known hole. It also matches the CRD CEL rules. It can be relaxed later if an in-cluster HTTP issuer is needed.
• registration.NewStaticDelegateClient now returns fosite.Client instead of *fosite.DefaultClient, because it returns a DefaultOpenIDConnectClient when an auth method is pinned. This is intended. Code that used the concrete type has to change; nothing in Stacklok Enterprise Platform calls it.
• The constructor keeps today's bare client for default inputs and never marks the result as DCR-issued.
• The OpenAPI and SDK files are generated from the field comments.

🤖 Generated with Claude Code

[codecov]

Codecov Report

❌ Patch coverage is 99.50495% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 80.23%. Comparing base (f4c4366) to head (fb1e6a6).

Files with missing lines	Patch %	Lines
pkg/authserver/server/registration/client.go	96.96%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #6952      +/-   ##
==========================================
+ Coverage   80.19%   80.23%   +0.04%     
==========================================
  Files         846      847       +1     
  Lines       85962    86097     +135     
==========================================
+ Hits        68933    69080     +147     
+ Misses      17024    17012      -12     
  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.

[ChrisJBurns]

• high · security — private_key_jwt-only delegate clients now allow a non-loopback cleartext HTTP issuer (pkg/authserver/config.go, pkg/authserver/server/provider.go)

  No secret is sent, but client assertions, auth codes (now that authorization_code is allowed) and issued tokens still cross cleartext HTTP. Captured assertions can be replayed until they expire. The CRD path still requires HTTPS, so the two paths now disagree.

• medium · correctness — Auth method fingerprint is read from the getter for live clients but the raw field for Redis rows (pkg/authserver/storage/redis.go, pkg/authserver/storage/types.go)

  fingerprintOfClient calls GetTokenEndpointAuthMethod(), and fosite's DefaultOpenIDConnectClient may return client_secret_basic when the field is empty. Redis storedClient uses the raw field. Any OIDC-typed row with an empty method could then show a spurious policy change or reserved-row conflict.

[tgrunnagle]

I went through the validator matrix, and coverage is thorough: every combination of auth method, secret and key state; key contents (private, symmetric, mismatched type); grant rules; redirect rules; transport at the RunConfig, Config and direct-params layers; and reconcile warnings on both backends.

@ChrisJBurns I looked into both of your findings. My notes are inline on config.go:2289 and types.go:1258. In short, the first is a fair policy question that only bites once the CRD gains an auth-method field, and the second doesn't hold for fosite v0.49.0.

Test gaps worth filling: a test that pins GetTokenEndpointAuthMethod() == "" for an empty-method OIDC client, and a reconcile test for a redirect URI change (see the policyChanged comment).

[jhrozek]

Thanks for both of these.

On the cleartext HTTP one, you were right, and I went back to strict: every delegate client counts as confidential again, private_key_jwt ones included. The replay risk is smaller than it sounds, since jtis are reserved atomically and assertions are capped at 5 minutes. But a delegate client can hold authorization_code and refresh_token, while a DCR private_key_jwt client is token-exchange-only. So codes and refresh tokens would have crossed plain HTTP, and that's not where I want the default to be. It also keeps the RunConfig path matching the CRD rules. There's a comment at the call site explaining why, so it can be relaxed later if we do need an in-cluster HTTP issuer. The errors also now say "allow_confidential_client_registration or delegate clients", so a key-only config doesn't get told it's sending secrets.

The fingerprint one doesn't hold for fosite v0.49.0. DefaultOpenIDConnectClient.GetTokenEndpointAuthMethod() returns the raw field. The client_secret_basic fallback is in fosite's authentication logic, not the getter. You might be thinking of GetGrantTypes/GetResponseTypes, which is why storedClient.fingerprint reads raw fields. buildStoredClient fills the stored method from the same getter, so the two can't disagree. I added TestBuildStoredClient_StoresRawTokenEndpointAuthMethod, which fails if a future fosite starts returning a default there.