fix(a2a): forward authenticated user to managed agents - #2485
Conversation
There was a problem hiding this comment.
Pull request overview
This PR fixes the legacy controller → managed-runtime A2A hop so the managed runtime receives the controller-authenticated session principal (via x-user-id / X-User-Id) instead of defaulting to the runtime service identity, addressing the identity-loss bug described in #2467 on release/v0.10.x.
Changes:
- Update the upstream A2A client interceptor to set the outbound user identity from
session.Principal().User.IDand replace any preexisting user-id service parameter. - Add focused unit tests to verify authenticated-principal forwarding and non-escalation in the presence of
ShareContext.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| go/core/internal/a2a/client_interceptors.go | Forwards controller-authenticated user identity on outbound managed-runtime A2A requests. |
| go/core/internal/a2a/client_interceptors_test.go | Adds regression tests covering user forwarding, share-context behavior, and trace propagation. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| authenticatedUserID := session.Principal().User.ID | ||
| if authenticatedUserID != "" { | ||
| httpReq.Header.Set("X-User-Id", authenticatedUserID) | ||
| delete(req.ServiceParams, "x-user-id") | ||
| } |
| t.Errorf("X-User-Id: want %q, got %q", "caller-user", got) | ||
| } | ||
| } | ||
|
|
Dependency and forward-port noteThis is the controller-side counterpart to #2483 and should merge after that runtime boundary is present. #2484 is related but independently mergeable. A main-oriented prototype exists in erauner12#13, but it predates the authorization-boundary refinement made in this PR. A forward-port must use the authenticated session principal only, replace any caller-supplied |
There was a problem hiding this comment.
It seems that UnsecureAuthenticator and ProxyAuthenticator already sets x-user-id inside the UpstreamAuth method above this change. Which auth provider are you using that produced the missing x-user-id?
I could be wrong but I'm suspecting that the symptoms you observed could be caused by #2449 instead (and the behaviour you described seems close to what's in its issue #2443, which is also applicable to 0.10.x). Since even if x-user-id is propagated properly, according to that issue the task store is not storing the owner properly resulting in "task not found" during the next lookup.
Can you take another look at this and lmk which one is the actual root cause in your setup?
EDIT: I noticed you have #2483 open as well, the #2449 I quoted earlier is functionally the same as yours, but my question remains the same
|
Closing this after rechecking the shipped authentication paths on Both The observed task-resolution failure is accounted for by the runtime-context defect covered by #2483 and the equivalent main work in #2449. Rather than add redundant identity behavior at another layer, I’m withdrawing this PR. If provider-independent interceptor enforcement is desired later, it can be proposed separately as explicit hardening. Thanks for helping separate these concerns. |
Summary
Forward the controller-authenticated session principal on the legacy controller-to-managed-runtime A2A hop on
release/v0.10.x.Fixes #2467.
Problem
The controller authenticates the initiating caller, but its outbound managed-runtime request did not carry that principal. Runtime task resolution could therefore use the runtime service account instead of the owner established at the controller boundary.
Change
The existing upstream-auth interceptor now replaces any preexisting
x-user-idservice parameter with exactly one value derived fromsession.Principal().User.IDafter the controller's normal authentication step.Provider authentication and trace propagation remain unchanged.
Trust boundary
The interceptor does not copy a raw inbound identity header and does not infer share-owner authority. Even when a validated
ShareContextis present, the generic interceptor forwards only the authenticated session principal. Share-aware forwarding requires operation-specific task/session validation and is intentionally outside this fix.Tests
cd go && go test ./internal/a2a -run '^TestUpstreamAuthInterceptor' -count=1cd go && go test -race ./internal/a2a -run '^TestUpstreamAuthInterceptor' -count=1Coverage verifies authenticated-principal forwarding, replacement of a preexisting value, non-escalation in the presence of
ShareContext, and preservation of trace behavior.Scope and ordering
This PR changes only controller-side principal forwarding. Runtime identity ingestion is tracked in #2465 and should land first. Persistent exact
GetTaskis tracked separately in #2464.