feat(sts): send RFC 8707 resource and audience on token exchange#2106
feat(sts): send RFC 8707 resource and audience on token exchange#2106QuentinBisson wants to merge 2 commits into
Conversation
|
Python companion: #2107 (same env vars across both runtimes). |
The token-propagation plugin hardcoded nil resource/audience on every STS token exchange, so issued tokens could not be scoped to a target backend. Read KAGENT_TOKEN_RESOURCE / KAGENT_TOKEN_AUDIENCE and pass them through to the exchange via a WithExchangeTarget option, omitting unset values. Signed-off-by: QuentinBisson <quentin@giantswarm.io>
a4d3268 to
6a887c7
Compare
There was a problem hiding this comment.
Pull request overview
This PR wires optional RFC 8707 resource and RFC 8693 audience parameters through the Go ADK STS token-exchange flow so exchanged tokens can be scoped to the intended backend (configured via new KAGENT_TOKEN_RESOURCE / KAGENT_TOKEN_AUDIENCE env vars).
Changes:
- Register
KAGENT_TOKEN_RESOURCEandKAGENT_TOKEN_AUDIENCEin the core env registry for discoverability. - Add an options pattern to the STS token propagation plugin and plumb resource/audience into
ExchangeTokenWithActorToken. - Read env vars at agent runtime startup and add a test asserting the form params are sent.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| go/core/pkg/env/kagent.go | Registers env vars for token-exchange resource/audience. |
| go/adk/pkg/sts/plugin.go | Adds WithExchangeTarget option and passes resource/audience into STS exchange calls. |
| go/adk/pkg/sts/plugin_test.go | Adds a test verifying resource/audience are present/absent on the exchange request. |
| go/adk/pkg/runner/adapter.go | Reads env vars and configures the STS plugin with WithExchangeTarget. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| p.resource, | ||
| p.audience, | ||
| "", // scope | ||
| "", // requestedTokenType | ||
| ) |
| var gotResource, gotAudience string | ||
| var srv *httptest.Server | ||
| srv = httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { | ||
| if r.URL.Path == "/.well-known/oauth-authorization-server" { | ||
| _ = json.NewEncoder(w).Encode(map[string]any{ | ||
| "issuer": srv.URL, | ||
| "token_endpoint": srv.URL + "/token", | ||
| }) | ||
| return | ||
| } | ||
| if r.URL.Path != "/token" { | ||
| http.NotFound(w, r) | ||
| return | ||
| } | ||
| if err := r.ParseForm(); err != nil { | ||
| t.Fatalf("ParseForm() error = %v", err) | ||
| } | ||
| gotResource = r.FormValue("resource") | ||
| gotAudience = r.FormValue("audience") | ||
| _ = json.NewEncoder(w).Encode(map[string]any{ | ||
| "access_token": "access-token", | ||
| "issued_token_type": string(TokenTypeJWT), | ||
| }) | ||
| })) |
| if gotResource != tt.wantResource { | ||
| t.Fatalf("resource = %q, want %q", gotResource, tt.wantResource) | ||
| } | ||
| if gotAudience != tt.wantAudience { | ||
| t.Fatalf("audience = %q, want %q", gotAudience, tt.wantAudience) | ||
| } |
Address review feedback on the resource/audience plumbing: - WithExchangeTarget takes string instead of any, so callers cannot pass a type the STS client silently drops; empty values are omitted. - The resource/audience test captures the token-exchange form over a channel and waits with a timeout, removing the data race and the unreliable t.Fatalf from the server handler goroutine. Signed-off-by: QuentinBisson <quentin@giantswarm.io>
|
Thanks — addressed in 7dccee6:
|
| resource any // RFC 8707 resource indicator passed to the STS exchange | ||
| audience any // RFC 8693 audience passed to the STS exchange |
There was a problem hiding this comment.
Why are these any, they can be string or []string?
| // Option configures a TokenPropagationPlugin. | ||
| type Option func(*TokenPropagationPlugin) | ||
|
|
||
| // WithExchangeTarget sets the RFC 8707 resource and RFC 8693 audience sent on | ||
| // token-exchange requests. Empty values are omitted from the request, so an | ||
| // unset target leaves the exchange unscoped. Values are single strings, which | ||
| // the STS client always serializes; multi-valued resources are out of scope. | ||
| func WithExchangeTarget(resource, audience string) Option { | ||
| return func(p *TokenPropagationPlugin) { | ||
| if resource != "" { | ||
| p.resource = resource | ||
| } | ||
| if audience != "" { | ||
| p.audience = audience | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
Do we really need to introduce an options pattern for 2 strings?
| // RFC 8707 resource / RFC 8693 audience scope the exchanged token to a | ||
| // backend. Empty values are omitted by WithExchangeTarget. | ||
| resource := strings.TrimSpace(os.Getenv("KAGENT_TOKEN_RESOURCE")) | ||
| audience := strings.TrimSpace(os.Getenv("KAGENT_TOKEN_AUDIENCE")) |
There was a problem hiding this comment.
Frankly I don't really like adding more env vars if we don't have to, they're hard to track. Can we add a token exchange section to our adk config file instead? This is for the Python impl as well
There was a problem hiding this comment.
Thanks for the review. I'll see what I can do :)
|
Thanks @EItanya. I agree on all three: I'll move off the env vars, drop the options pattern, and type the values as string (single value, with []string only if we later need multi-resource). One design question before I move it into the config, because the runtime constrains where it can sensibly live:
So there are two options for AgentConfig:
I'd lean toward (1) for this PR to keep it scoped. What do you think about introducing a new Does that split work for you, or would you prefer the per-server version here? |
What
The Go ADK token-propagation plugin (
go/adk/pkg/sts) always passednilresource and
nilaudience to the STS token exchange, so an issued tokencould not be scoped to a specific backend. This wires two optional inputs
through to the exchange:
KAGENT_TOKEN_RESOURCE— RFC 8707 resource indicator (the target backend).KAGENT_TOKEN_AUDIENCE— RFC 8693 audience, for STS servers that key on it.Both are read at agent-runtime startup and applied via a new
sts.WithExchangeTargetoption onNewTokenPropagationPlugin. Unset valuesare omitted from the request, so behaviour is unchanged when neither is set.
Why
Without a resource/audience, the STS returns a token whose audience is not the
MCP backend, so audience-validating backends reject it. RFC 8707 is the
standard way to scope an exchanged token to its intended resource. The STS
client already serialized
resource/audience; only the call site and theconfiguration plumbing were missing.
Notes
resource/audiencesent.go/core/pkg/envfor discoverability; afirst-class
AgentCRD field is intended as a follow-up.