Repository navigation
Conversation
The comment said a key implies its signing algorithm. An RSA key without alg does not imply RS256; RS256 is only the default when the keys do not say, because RS and PS are both possible. Say what the function returns. Co-authored-by: Mecatl <noreply@mecatl.dev>
A private_key_jwt delegate without a token_exchange block is refused since the trust-mode rule landed. Users upgrading from an earlier release may have such a client that relied on the implicit any behaviour, so the error now names the one-line change that restores it. Co-authored-by: Mecatl <noreply@mecatl.dev>
A broker that keeps one client_id across key rotation needs the pre-provisioned client fields on the operator API, not only in the RunConfig. Delegate clients on inboundGrants.tokenExchange.delegateClients now accept tokenEndpointAuthMethod, jwksRef, tokenEndpointAuthSigningAlg, grantTypes, redirectUris and a tokenExchange source policy. jwksRef is a ConfigMap key holding the public JWKS. The operator mounts it read-only under its own base path and sets jwks_file to match. Editing the ConfigMap does not restart the pod yet. Admission mirrors the runtime validator so the CRD cannot produce a config the auth server refuses. clientSecretRef is now optional in the schema: CEL requires it unless the method is private_key_jwt, where it and jwksRef swap places, and jwksRef.optional is rejected. grantTypes, when set, must include token exchange; redirectUris and refresh_token need authorization_code; tokenEndpointAuthSigningAlg needs private_key_jwt and a supported algorithm. A client that sets grantTypes or uses private_key_jwt must declare tokenExchange.selfIssuedSubjectTokens, any or restricted. Every delegate client, private_key_jwt included, still counts as confidential for the cleartext-transport rule. The deprecated top-level delegateClients shares the type, so the new fields, the CEL rules and the JWKS mount apply to it as well. effectiveDelegateClients picks the canonical list or falls back to the deprecated one, so a jwksRef there never leaves jwks_file pointing at a missing file. Co-authored-by: Mecatl <noreply@mecatl.dev>
A delegate client's jwksRef ConfigMap is mounted with subPath, so a running pod never sees an edit. Without a pod-template change and a reconcile on the edit, rotating the broker's key would need a manual restart. The embedded-auth ConfigMap checksum now also covers the JWKS bytes of the delegate clients the runtime config is built from. The section is hashed only when a jwksRef exists, behind its own marker, so the checksum of every config without one is byte-identical and no existing pod rolls on upgrade. A missing ConfigMap or key is reported as a terminal InvalidCABundle error through ValidateEmbeddedAuthServerCABundles, as CA bundles are. The ConfigMap indexes for MCPExternalAuthConfig and VirtualMCPServer include the JWKS ConfigMap names, so editing one enqueues the dependent reconciles. Co-authored-by: Mecatl <noreply@mecatl.dev>
The operator now rolls the auth server pod when a delegate client's JWKS ConfigMap is edited, but the architecture docs still said an edit needs a manual restart. Say what happens on an operator-managed edit, and keep the restart instruction for the RunConfig path, where the runner still reads jwks_file once at startup. Co-authored-by: Mecatl <noreply@mecatl.dev>
The delegate client JWKS feature depends on behaviour a fake API server cannot show: the auth server pod reading the mounted key at the path the operator sets, and a pod roll when the JWKS ConfigMap changes. This adds a kind suite on a VirtualMCPServer with a restricted private_key_jwt delegate that covers the mount and a matching-key exchange, a wrong key under a registered kid, a foreign subject token that restricted refuses, and a two-stage key rotation that waits for the roll before it checks which key is trusted. A retired kid is no longer registered, so the server refuses it as a malformed request (400 invalid_request), not as a wrong key under a registered kid (401 invalid_client). Co-authored-by: Mecatl <noreply@mecatl.dev>
CI lint failed on three findings in the delegate JWKS change: a field alignment that gci rejects in the deployment test, and two functions over the cyclomatic complexity limit. Split the VirtualMCPServer ConfigMap indexer into an authz helper and an auth server helper, and move the spec-level auth server checks into validateAuthServerSpec so the reconciler has one branch for them. The checks and their messages are unchanged. The kind test's exchange helper always received the same client ID, so it now uses a suite constant and the parameter is gone. Co-authored-by: Mecatl <noreply@mecatl.dev>
Lint still flagged two helpers in the delegate JWKS kind suite whose client ID argument always received the suite's delegate client. Both now use the suite constant, so the parameter is gone from each signature and its call sites. Co-authored-by: Mecatl <noreply@mecatl.dev>
Let delegate clients declared on the CRD enable refresh-token issuance through token exchange, as the RunConfig path already does. Require allowedSourceClients with exchangeRefreshTokens, matching the runtime validator, so admission rejects configs the runtime would refuse. Name the CRD fields in the architecture doc. Co-authored-by: Mecatl <noreply@mecatl.dev>
Exchange refresh tokens are only useful if the operator config reaches a running auth server, and nothing in the unit or envtest suites shows that. This adds a kind suite on a VirtualMCPServer with two restricted private_key_jwt delegates, one with exchangeRefreshTokens and one without. The cases cover: a token exchange that issues a refresh token with the configured 24h cap, which must reach the server (the grant's expires_at and the token's expires_in), a reused subject token refused by its jti, a refresh_token grant refused for a client that has only token exchange, a foreign or self subject token refused by the source policy, and the control delegate that issues an ordinary access token and no refresh token. The commit only issues tokens. Redeeming them is the derive work, so those cases are not here. Co-authored-by: Mecatl <noreply@mecatl.dev>
jhrozek
requested review from
ChrisJBurns,
JAORMX,
amirejaz,
blkt,
jerm-dro,
rdimitrov,
reyortiz3 and
tgrunnagle
as code owners
November 2, 2026 16:15
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## stable-client-v2-f-3 #7021 +/- ##
========================================================
- Coverage 80.48% 80.46% -0.02%
========================================================
Files 852 852
Lines 87170 87178 +8
========================================================
- Hits 70160 70152 -8
- Misses 17005 17021 +16
Partials 5 5 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
github-actions
Bot
added
size/XL
Contributor
Author
|
Please hold off on reviewing this one too, for the same reason as #7019. After the fix for "Configured delegate client with api_audience can exchange for API access tokens" (#7026) lands, this PR changes as follows:
I'll also add negative e2e cases: no access token for the input audience, and a subject token without |
jhrozek
force-pushed
the
stable-client-v2-f-3
branch
4 times, most recently
from
November 3, 2026 17:53
ca23b39 to
4ea59c0
Compare
Contributor
Author
|
Drafting this one too, for the reason I gave on the delegate client JWKS PR (Expose delegate client JWKS and token-exchange policy in the CRD): no deployment needs this in the CRD yet, and the shape is expensive to change once released. Two things are specific to this PR. The field produces a token nothing can redeem until Derive child exchange grants from refresh tokens lands, so for now it would give operators a switch that does nothing. And a few decisions that touch the field are still open, tracked in Follow-ups from the exchange refresh token review. The pushed branch is still the version from before the audience change. I'll update it when this is picked up again. |
jhrozek
marked this pull request as draft
November 3, 2026 18:43
This branch has not been deployed
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
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.
Closes #6897
Summary
Operators can configure a delegate client to issue exchange refresh tokens through the RFC 8693 token exchange only through the RunConfig today. The runtime handler already issues them (#7003), but the operator API cannot express the setting, so CRD users have no way to turn it on. This PR exposes it on the delegate client of
MCPExternalAuthConfigandVirtualMCPServer.What changed:
tokenExchange.exchangeRefreshTokenson the delegate client, withrefreshTokenMaxLifetimeandidJagMaxLifetimeas duration caps. CEL requiresapiAudienceandallowedSourceClientsalongside it, matching the runtime validator.The PR is stacked on #7019, which carries the delegate client CRD work this builds on. Retarget to
mainonce #7019 merges.Type of change
Test plan
VirtualMCPServer delegate client exchangeRefreshTokenspassed locally on a kind cluster with the operator built from this branch and the CRDs installed from this branch. 5 of 5 specs.task test): not run on this commit.task lint-fix): not run on this commit.API Compatibility
v1beta1API. The new fields are optional and additive.Does this introduce a user-facing change?
Yes. Before: a delegate client on the CRD cannot enable exchange refresh tokens. After:
tokenExchange.exchangeRefreshTokenscan be set on a delegate client, with a requiredapiAudienceandallowedSourceClients. The runtime then issues a refresh token through token exchange, capped by the configured lifetime.Tokens issued this way cannot be redeemed yet. Redeeming them is the derive work in #7007. Until that lands, the field produces a token nothing can use.
Special notes for reviewers
mainalso includes Expose delegate client JWKS and token-exchange policy in the CRD #7019's commits until that PR merges.docs/arch/17-token-exchange-delegation.mdshould say issuance only until Derive child exchange grants from refresh tokens #7007 lands. This PR updates them.