mirror of
https://github.com/BerriAI/litellm.git
synced 2026-09-13 23:11:40 +00:00
fix(mcp): keep user token in tools preview for interactive OAuth servers
The /mcp-rest/test/tools/list preview inferred the OAuth2 flow with its own copy of the M2M detection that guessed client_credentials whenever client_id, client_secret and token_url were all present. Interactive/OBO OAuth servers legitimately have all three (e.g. the hosted Slack MCP, GitHub Enterprise), so they were mislabeled M2M; the preview then dropped the user's forwarded token and tried a client_credentials fetch, which interactive flows do not support, so the Connection Status and Tool Configuration tabs failed before save while the saved server worked. Resolve the flow through the canonical _resolve_oauth2_flow instead, which only infers M2M when there is no authorization_url. Interactive servers keep the forwarded user token; genuine M2M servers (no authorization_url) are unchanged.
This commit is contained in:
parent
5a1c7839be
commit
eb5e54a05f
2 changed files with 65 additions and 7 deletions
|
|
@ -7,7 +7,6 @@ from typing import (
|
|||
Callable,
|
||||
Dict,
|
||||
List,
|
||||
Literal,
|
||||
Optional,
|
||||
Set,
|
||||
Tuple,
|
||||
|
|
@ -991,12 +990,17 @@ if MCP_AVAILABLE:
|
|||
try:
|
||||
client_id, client_secret, scopes = _extract_credentials(request)
|
||||
|
||||
_oauth2_flow: Optional[
|
||||
Literal["client_credentials", "authorization_code"]
|
||||
] = request.oauth2_flow or (
|
||||
"client_credentials"
|
||||
if client_id and client_secret and request.token_url
|
||||
else None
|
||||
# Match load-time flow resolution: only treat this as M2M when there
|
||||
# is no authorization_url, so interactive/OBO OAuth servers keep the
|
||||
# user's forwarded token instead of having it dropped for a
|
||||
# client_credentials token fetch.
|
||||
_oauth2_flow = global_mcp_server_manager._resolve_oauth2_flow(
|
||||
auth_type=request.auth_type,
|
||||
oauth2_flow=request.oauth2_flow,
|
||||
token_url=request.token_url,
|
||||
authorization_url=request.authorization_url,
|
||||
client_id=client_id,
|
||||
client_secret=client_secret,
|
||||
)
|
||||
# client_credentials requires token_url to fetch a token; without it the
|
||||
# incoming auth header would be dropped with nothing to replace it.
|
||||
|
|
|
|||
|
|
@ -270,6 +270,60 @@ class TestExecuteWithMcpClient:
|
|||
or "Authorization" not in captured["extra_headers"]
|
||||
)
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_interactive_oauth_with_creds_keeps_user_token(self, monkeypatch):
|
||||
"""Regression: an interactive OAuth server that has client_id/secret/token_url
|
||||
AND an authorization_url (e.g. the hosted Slack MCP) must NOT be mistaken
|
||||
for M2M. The user's forwarded token must reach the MCP client, not be
|
||||
dropped for a client_credentials fetch."""
|
||||
captured: dict = {}
|
||||
|
||||
def fake_build_stdio_env(server, raw_headers):
|
||||
return None
|
||||
|
||||
async def fake_create_client(*args, **kwargs):
|
||||
captured["server"] = kwargs.get("server")
|
||||
captured["extra_headers"] = kwargs.get("extra_headers")
|
||||
return object()
|
||||
|
||||
monkeypatch.setattr(
|
||||
rest_endpoints.global_mcp_server_manager,
|
||||
"_build_stdio_env",
|
||||
fake_build_stdio_env,
|
||||
raising=False,
|
||||
)
|
||||
monkeypatch.setattr(
|
||||
rest_endpoints.global_mcp_server_manager,
|
||||
"_create_mcp_client",
|
||||
fake_create_client,
|
||||
raising=False,
|
||||
)
|
||||
|
||||
async def ok_operation(client):
|
||||
return {"status": "ok"}
|
||||
|
||||
payload = NewMCPServerRequest(
|
||||
server_name="slack",
|
||||
url="https://mcp.slack.com/mcp",
|
||||
auth_type=MCPAuth.oauth2,
|
||||
authorization_url="https://slack.com/oauth/v2_user/authorize",
|
||||
token_url="https://slack.com/api/oauth.v2.user.access",
|
||||
credentials={
|
||||
"client_id": "123.456",
|
||||
"client_secret": "my-secret",
|
||||
},
|
||||
)
|
||||
|
||||
result = await rest_endpoints._execute_with_mcp_client(
|
||||
payload,
|
||||
ok_operation,
|
||||
oauth2_headers={"Authorization": "Bearer user-token"},
|
||||
)
|
||||
|
||||
assert result["status"] == "ok"
|
||||
assert captured["server"].has_client_credentials is False
|
||||
assert captured["extra_headers"]["Authorization"] == "Bearer user-token"
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_catches_exception_group(self, monkeypatch):
|
||||
"""MCP SDK's anyio TaskGroup raises BaseExceptionGroup which does not
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue