fix(controller): recover MCP auth session from RequestExtra in tool handlers#1853
fix(controller): recover MCP auth session from RequestExtra in tool handlers#1853onematchfox wants to merge 1 commit into
RequestExtra in tool handlers#1853Conversation
RequestExtra in tool handlers
There was a problem hiding this comment.
Pull request overview
This PR addresses an auth-context propagation gap in the Go MCP Streamable HTTP handling path: tool handlers lose the original HTTP request context (and thus the auth.Session), so invoke_agent re-authenticates using the preserved request headers in CallToolRequest.Extra to ensure downstream A2A calls can forward the user’s JWT.
Changes:
- Recover
auth.SessioninsidehandleInvokeAgentby re-authenticating fromRequestExtra.Header, then attach it to the handler context. - Add unit tests validating Authorization propagation to the A2A backend when an MCP client supplies (or omits) an Authorization header.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| go/core/internal/mcp/mcp_handler.go | Re-authenticates from RequestExtra.Header to restore the session in tool-handler context for downstream A2A auth propagation. |
| go/core/internal/mcp/mcp_handler_test.go | Adds end-to-end-ish unit tests covering auth propagation behavior through MCP → A2A. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| // The Go MCP SDK detaches the HTTP request context when dispatching to | ||
| // tool handlers, so auth.AuthSessionFrom(ctx) returns nothing. Recover | ||
| // the auth session from the HTTP headers preserved in RequestExtra so | ||
| // that the A2A client's outbound request to the agent carries the user's JWT. | ||
| if extra := req.GetExtra(); extra != nil { | ||
| if session, err := h.authenticator.Authenticate(ctx, extra.Header, nil); err == nil { | ||
| ctx = auth.AuthSessionTo(ctx, session) | ||
| } | ||
| } |
There was a problem hiding this comment.
RequestExtra doesn't carry query params
… handlers The Go MCP SDK detaches the HTTP request context before dispatching to tool handlers. From the [SDK source](https://github.com/modelcontextprotocol/go-sdk/blob/v1.5.0/mcp/streamable.go#L485-L487): > // Pass req.Context() here, to allow middleware to add context values. > // The context is detached in the jsonrpc2 library when handling the > // long-running stream. This means the auth session placed by `AuthnMiddleware` is not visible via `auth.AuthSessionFrom(ctx)` in tool handlers. The SDK does preserve the original HTTP headers in [RequestExtra.Header](https://github.com/modelcontextprotocol/go-sdk/blob/v1.5.0/mcp/streamable.go#L1155-L1158) though. Re-authenticate from those headers at the top of handleInvokeAgent so the A2A client's outbound request to the agent carries the user's JWT. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> Signed-off-by: Brian Fox <878612+onematchfox@users.noreply.github.com>
supreme-gg-gg
left a comment
There was a problem hiding this comment.
I tried testing the MCP handler before and after the change, but both seem to work fine with propagating auth context to a2a agent invocations (trusted proxy mode)
I asked copilot to take a look at the mcp go code and he told me this:
The conclusion is wrong because “detached context” here does not mean context values are dropped.
The SDK comment says it passes req.Context() specifically “to allow middleware to add context values”:
streamable.go
Lines 485-488
// Pass req.Context() here, to allow middleware to add context values.
// The context is detached in the jsonrpc2 library when handling the
// long-running stream.
session, err := server.Connect(req.Context(), transport, connectOpts)
The actual detach wrapper preserves values:
conn.go
Lines 758-767
type notDone struct{ ctx context.Context }
func (ic notDone) Value(key any) any {
return ic.ctx.Value(key)
}
func (notDone) Done() <-chan struct{} { return nil }
func (notDone) Err() error { return nil }
func (notDone) Deadline() (time.Time, bool) { return time.Time{}, false }
Also in: https://github.com/modelcontextprotocol/go-sdk/blob/v1.5.0/internal/xcontext/xcontext.go#L14-L15
So AuthnMiddleware context values should remain visible to tool handlers. The current test was misleading because it mounted MCPHandler directly, bypassing the real /mcp auth middleware path.
If this has been a valid issue can you lmk how was this not working before the change or how did you set this up?
| mcpHandler, err := NewMCPHandler(nil, backend.server.URL, authProvider, 5*time.Second) | ||
| require.NoError(t, err) | ||
|
|
||
| mcpServer := httptest.NewServer(mcpHandler) |
There was a problem hiding this comment.
These tests were passing without your re-authenticate change if you add the auth middleware
mcpServer := httptest.NewServer(auth.AuthnMiddleware(authProvider)(mcpHandler))
The Go MCP SDK detaches the HTTP request context before dispatching to tool handlers. From the SDK source:
This means the auth session placed by
AuthnMiddlewareis not visible viaauth.AuthSessionFrom(ctx)in tool handlers.The SDK does preserve the original HTTP headers in RequestExtra.Header though.
Re-authenticate from those headers at the top of
handleInvokeAgentso the A2A client's outbound request to the agent carries the user's JWT.