fix(mcp): stop preemptively 401-challenging client_credentials (M2M) MCP servers - #33582
fix(mcp): stop preemptively 401-challenging client_credentials (M2M) MCP servers#33582tin-berri wants to merge 1 commit into
Conversation
…MCP servers
The connect-time challenge in _raise_preemptive_401_for_unauthenticated_servers raised the
interactive OAuth 401 for every oauth2 server without a per-request oauth header, including
client_credentials (M2M) servers for which the gateway mints its own upstream token, making
them unusable over both /{alias}/mcp and the /mcp aggregate. The challenge now fires only for
flows that need a user token, and the loop applies the same request-time oauth2_flow backstop
the listing and tool-call paths use, so a legacy null-flow M2M-shape row is classified at the
challenge gate exactly as egress classifies it
Greptile SummaryThis PR fixes a bug where the connect-time 401 challenge gate in
Confidence Score: 5/5The change is safe to merge — it removes a spurious 401 raise for a specific server class without touching any authentication logic for other flows. The inversion of the No files require special attention.
|
| Filename | Overview |
|---|---|
| litellm/proxy/_experimental/mcp_server/server.py | Fixes _raise_preemptive_401_for_unauthenticated_servers to skip the interactive-OAuth 401 challenge for M2M (client_credentials) servers by adding an early continue when needs_user_oauth_token is False, and normalizes legacy null-flow M2M-shaped rows via resolve_oauth2_flow_for_request before the challenge gate — matching what listing and tool-call paths already do. |
| tests/test_litellm/proxy/_experimental/mcp_server/test_mcp_stale_session.py | Adds a parametrized test covering both a stamped oauth2_flow="client_credentials" row and an unstamped M2M-shape row; verifies the session manager is reached and has_user_oauth_token is never consulted for either case. |
Reviews (1): Last reviewed commit: "fix(mcp): stop preemptively 401-challeng..." | Re-trigger Greptile
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
…e-401 gate Grafted from PR #33582 (closing as superseded by this PR): drives handle_streamable_http_mcp with real MCPServer objects, parametrized over a stamped client_credentials row and a legacy unstamped M2M-shape row; both must reach the session manager without the per-user token store being consulted
…e-401 gate Grafted from PR #33582 (closing as superseded by this PR): drives handle_streamable_http_mcp with real MCPServer objects, parametrized over a stamped client_credentials row and a legacy unstamped M2M-shape row; both must reach the session manager without the per-user token store being consulted
Relevant issues
Ships the product fix that PR #33263 documents as a known failure against current main: its
test_gateway_exchanges_client_credentials_and_sends_minted_tokene2e test is the protocol-level regression guard for this bug and goes green once this landsLinear ticket
Part of LIT-4263 (migrate client_credentials M2M into v2); this ships the connect-time gate fix that track requires
Pre-Submission checklist
Please complete all items before asking a LiteLLM maintainer to review your PR
@greptileaito re-request a review after pushing changes)Delays in PR merge?
If you're seeing a delay in your PR being merged, ping the LiteLLM Team on Slack (#pr-review).
Screenshots / Proof of Fix
Rig: a local stub serving an OAuth token endpoint (client_credentials grant, counts every mint) plus an MCP streamable-HTTP mount that 401s any request whose bearer is not a token that endpoint minted. A served MCP call is therefore proof the gateway ran the grant and attached the minted token. Proxy runs against local Postgres with
--port 4001Register the M2M server the way a production host does:
Before the fix, connecting over the MCP protocol path is dead on arrival on both routes; the gateway pushes the client into an interactive OAuth flow it can never complete, and the stub IdP records zero mint attempts:
After the fix, the same commands complete the full flow. initialize returns a session, tools list, a tool call round-trips, and the stub proves the gateway minted exactly one token and attached it (the caller's virtual key never crossed the gateway boundary):
The interactive flow is untouched; a per-user oauth2 server with no stored token still gets the challenge:
Legacy rows are covered too. Nulling the stored flow on an M2M-shape row (
UPDATE "LiteLLM_MCPServerTable" SET oauth2_flow = NULL WHERE alias = 'm2mlegacy') and restarting the proxy, initialize on/m2mlegacy/mcpreturns 200 instead of the spurious challengeType
🐛 Bug Fix
Changes
_raise_preemptive_401_for_unauthenticated_serversinlitellm/proxy/_experimental/mcp_server/server.pyraised the interactive OAuth 401 for everyoauth2server that received no per-request oauth header. The interactive logic (delegate challenge, stored-token check) was correctly nested underneeds_user_oauth_token, but the raise sat at the outer level, so aclient_credentials(M2M) server, for which the gateway mints its own upstream token and no user flow exists, fell through into a challenge it can never satisfy. Both the/{alias}/mcproute and the/mcpaggregate go through this function, so M2M servers were unusable over the MCP protocol pathTwo changes, one invariant: a server whose credential mode requires no user participation is never preemptively challenged for user auth, and the challenge classifies a server's flow exactly as egress does. First, the
needs_user_oauth_tokenbranch is inverted into an earlycontinue, so the 401 raise is only reachable for flows that actually need a user token. Second, the loop now appliesMCPServerManager.resolve_oauth2_flow_for_requestright after resolving the server, the same request-time backstop the listing and tool-call paths already apply, so a legacy null-flow row with the M2M field shape is treated as M2M at the challenge gate instead of being pushed into a user OAuth flow that egress would never look forThe regression test (
test_client_credentials_server_is_not_preemptively_challengedintests/test_litellm/proxy/_experimental/mcp_server/test_mcp_stale_session.py) driveshandle_streamable_http_mcpwith a realMCPServerobject, parametrized over a stampedoauth2_flow="client_credentials"row and an unstamped M2M-shape row; both must reach the session manager without the per-user token store ever being consulted. Both cases fail on the unfixed code (the stamped case with the exact 401 from the bug, the unstamped case by consulting the per-user store) and pass with the fix. The pre-existing tests pinning the interactive, delegate, and passthrough challenges all still passQA runbook
auth_type: oauth2,oauth2_flow: client_credentials, atoken_url, and client credentials for a real M2M upstream (any IdP-guarded MCP server works)initializeto/{alias}/mcpwithx-litellm-api-key: Bearer <key>; expect HTTP 200 and anmcp-session-idheader, not a 401 with a gatewayauthorization_urinotifications/initialized), thentools/listand atools/call; both must succeed and the upstream must receive the gateway-minted bearer, never your virtual key/mcp/route withx-mcp-servers: <alias>; expect 200oauth2_flow: authorization_codeand repeat step 2 against it without authorizing first; expect the 401 challenge withauthorization_uri(interactive flow unchanged)Final Attestation