Translate identityFromToken from CRD to runtime config#5280
Closed
jhrozek wants to merge 2 commits into
Closed
Conversation
Add the operator-side and runtime-side translation that carries
IdentityFromToken from MCPExternalAuthConfig (v1beta1 CRD) through
the authserver run-config and into upstream.OAuth2Config so the
provider added in the previous commit actually receives the field.
Two translation points mirror the existing TokenResponseMapping
plumbing:
* cmd/thv-operator/pkg/controllerutil/authserver.go
CRD v1beta1.IdentityFromTokenConfig
-> authserver.IdentityFromTokenRunConfig (operator path)
* pkg/authserver/runner/embeddedauthserver.go
authserver.IdentityFromTokenRunConfig
-> upstream.IdentityFromTokenConfig (runtime path)
A new IdentityFromTokenRunConfig type lives next to
TokenResponseMappingRunConfig in pkg/authserver/config.go.
Tests cover the round-trip on both sides and the nil-config case
(no spurious allocation).
IdentityFromToken paths that use the `@upstreamjwt` modifier (e.g. `access_token|@upstreamjwt|sub` for upstreams that embed identity inside a JWS access token) require RegisterModifiers() to run before any extraction. Until now the call lived only in test TestMain functions, so production paths silently no-op'd and operators saw a generic "path not found" error. Add the call near the top of NewEmbeddedAuthServer. Protect the gjson.AddModifier write with a sync.Once so concurrent constructors (parallel test cases, thundering-herd restarts) do not race on the gjson global modifier map.
jhrozek
requested review from
ChrisJBurns,
JAORMX,
jerm-dro and
yrobla
as code owners
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #5280 +/- ##
==========================================
+ Coverage 68.33% 68.34% +0.01%
==========================================
Files 619 619
Lines 63325 63339 +14
==========================================
+ Hits 43274 43290 +16
+ Misses 16816 16815 -1
+ Partials 3235 3234 -1 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Contributor
Author
|
sorry claude got PR horny |
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
1 participant
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.
Summary
identityFromTokenstory (Cannot run a Snowflake MCP server behind ToolHive's MCPRemoteProxy #5150). The CRD field (Add identityFromToken to MCPExternalAuthConfig CRD #5269) and the OAuth2 provider integration (Wire identityFromToken into the OAuth2 upstream provider #5222) are both inmain; this PR wires them together by threading the field through the two operator-side translation points that sit between them.cmd/thv-operator/pkg/controllerutil/translates the v1beta1OAuth2UpstreamConfig.IdentityFromTokenblock into the authserver run-config. Mirrors the existingtokenResponseMappingplumbing exactly.pkg/authserver/runner/'s embedded-auth-server bootstrapper translates the run-config field into thepkg/authserver/upstream.OAuth2Configthe provider consumes. Same mechanical mirror.upstream.RegisterModifiers()is called once inNewEmbeddedAuthServerso@upstreamjwtgjson paths actually resolve in production deployments. Without this, modifier-bearing paths silently returned empty strings at runtime.Closes #5157
Type of change
Test plan
task test)task lint-fix)New tests cover the all-fields and nil cases for both translation points. The
embeddedauthserver_test.goadditions verify the runner-layer translation;authserver_test.gocovers the operator-side builder.API Compatibility
v1beta1API, OR theapi-break-allowedlabel is applied and the migration guidance is described above.Additive only — threads an existing CRD field through two translation layers.
Does this introduce a user-facing change?
Yes. Operators who configure
identityFromTokenon an OAuth2 upstream in aVirtualMCPServercan now deploy to a real cluster and have the embedded auth server honour the field. Previously, the CRD accepted the config but it was silently discarded before reaching the provider.Implementation plan
Approved implementation plan
Phase 4 of the Snowflake /
identityFromTokenstory (#5150):extractIdentityFromTokenResponseand the@upstreamjwtgjson modifier.IdentityFromTokenConfigCRD type onMCPExternalAuthConfigv1beta1.BaseOAuth2Provider.ExchangeCodeForIdentity. New priority chain.RegisterModifiers()bootstrap call.thv llm setupdoesn't append provider path prefix for Envoy AI Gateway #5158): YAML examples.Generated with Claude Code