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>
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #7019 +/- ##
==========================================
- Coverage 80.50% 80.48% -0.02%
==========================================
Files 852 852
Lines 87035 87170 +135
==========================================
+ Hits 70065 70160 +95
- Misses 16965 17005 +40
Partials 5 5 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
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>
|
Please hold off on reviewing this one for now. This PR exposes The fix lands first. After that I'll rework this PR to the new shape before it merges: |
Closes #6893
Summary
Operators can only configure a pre-provisioned delegate client for RFC 8693 token exchange through the RunConfig today. The operator CRD cannot express the key-based (
private_key_jwt) form that the auth server supports, so a broker that rotates its key has to edit the RunConfig by hand. This PR exposes the delegate client settings on the operator API and wires them through to the auth server.What changed:
MCPExternalAuthConfigandVirtualMCPServergainstokenEndpointAuthMethod,jwksRef(a ConfigMap key holding the public JWKS),tokenEndpointAuthSigningAlg,grantTypes,redirectUris, and atokenExchangepolicy block. Admission rules mirror the runtime validator for the cases CEL can express.jwksRefread-only and points the RunConfigjwks_fileat it.inboundGrants.tokenExchangelist is now accepted. The runtime validator previously counted only the deprecated top-level list, so such a server was rejected at reconcile.private_key_jwtdelegate without atoken_exchangeblock now names the one-line change that keeps the old behaviour.Type of change
Test plan
task test): run, exit 0, no failure output. The run includes the new validator test for canonical delegate clients.VirtualMCPServer delegate client jwksRefon a kind cluster built from this branch. 4 of 4 specs passed: the read-only mount with a matching-key exchange, a wrong key under a registered kid, a foreign subject token refused underrestricted, and a two-stage key rotation with a pod roll.task lint-fix): not run yet.The envtest CEL suite in
cmd/thv-operator/test-integration/mcp-external-auth/delegate_client_cel_test.gohas been written but not run. It is the check for the admission rules and should be run before merge.API Compatibility
v1beta1API. Every new field is optional. The only other change is that the deprecated top-leveldelegateClientsnow rejects the new fields. Those fields do not exist onmain, so no stored object is affected.Does this introduce a user-facing change?
Yes. Before: the operator CRD could not configure a
private_key_jwtdelegate client, and a VirtualMCPServer that configured delegates only oninboundGrants.tokenExchangewas rejected at reconcile. After: both work. Specifically:MCPExternalAuthConfigandVirtualMCPServer, documented in the CRD reference.delegateClientsrejects the new fields and points users atinboundGrants.tokenExchange.delegateClients.httpswith no fragment. A loopback redirect URI is refused when the operator reconciles, and the resource reports it in its status conditions. The API server does not refuse it at admission, because the operator has no admission webhook.Special notes for reviewers
private_key_jwtdelegates is already onmain(Require a trust mode for private_key_jwt delegates #7004). This PR only changes the error text.forceConfidentialRedirectUrisprecedent.ConfigMapKeyReftype rather thancorev1.ConfigMapKeySelector, sooptionalcannot be set and bothnameandkeyare required.deploy/charts/operator-crds/filesandtemplates, across bothMCPExternalAuthConfigandVirtualMCPServer.