fix(mcp): surface tools/list 401 auth failures as a challenge on single-server routes (#31921)

A 401 while listing tools (a missing or expired per-user OAuth token, or an
upstream 401 for any auth_type) was swallowed to an empty tool list, so a
single-server client got a 200 with no tools and no WWW-Authenticate challenge
instead of a 401 it could re-authenticate against. Only oauth pass-through and
delegate-to-upstream oauth2 servers surfaced it; every other auth_type, and the
missing-token case for all of them, masked it.

The surface-vs-absorb decision now keys on the route, not the auth_type. An
upstream 401 in _fetch_tools_with_timeout becomes an MCPUpstreamAuthError
regardless of auth_type, and the per-user OAuth challenge raised during client
creation (a bare HTTPException 401 carrying a WWW-Authenticate header) is
converted to the same type in _get_tools_from_server. The challenge is scoped
to 401: a 403 (authenticated but forbidden, e.g. insufficient scope) is not a
re-auth signal and degrades to an empty list like any other non-auth error, and
the stdio-allowlist 403 (no challenge header) stays absorbed. The existing
routing then does the right thing: single-server routes turn the error into a
401 + WWW-Authenticate, while the multi-server aggregator absorbs it to an empty
list so one unauthenticated server does not fail the whole listing.

On the UI tools page, an OBO (per-user authorization_code) server now shows the
Authorize gate when the list call returns 401, not only when no credential row
exists. The backend already refreshes a still-refreshable token on the list
call, so a 401 means there is no valid token and none could be minted (expired
with no usable refresh token), which is exactly when the user must reauthorize.
This commit is contained in:
tin-berri 2026-07-02 18:05:32 -07:00 committed by GitHub
parent ef030235fd
commit b9df7fa705
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
6 changed files with 292 additions and 62 deletions

View file

@ -2130,7 +2130,7 @@ class MCPServerManager:
]
return tools
else:
tools = await self._fetch_tools_with_timeout(client, server.name, server=server)
tools = await self._fetch_tools_with_timeout(client, server.name)
self._remember_upstream_initialize_instructions(server, client)
prefixed_or_original_tools = self._create_prefixed_tools(tools, server, add_prefix=add_prefix)
@ -2142,6 +2142,17 @@ class MCPServerManager:
# client triggers the upstream OAuth flow. The multi-server
# aggregator catches this explicitly to keep absorbing.
raise
except HTTPException as e:
headers = e.headers or {}
www_authenticate = headers.get("WWW-Authenticate") or headers.get("www-authenticate")
if e.status_code == 401 and www_authenticate is not None:
raise MCPUpstreamAuthError(
status_code=401,
www_authenticate=www_authenticate,
server_name=server.name,
) from e
verbose_logger.warning(f"Failed to get tools from server {server.name}: {str(e)}")
return []
except Exception as e:
verbose_logger.warning(f"Failed to get tools from server {server.name}: {str(e)}")
return []
@ -2692,7 +2703,6 @@ class MCPServerManager:
self,
client: MCPClient,
server_name: str,
server: Optional[MCPServer] = None,
) -> List[MCPTool]:
"""
Fetch tools from MCP client with timeout and error handling.
@ -2700,38 +2710,27 @@ class MCPServerManager:
Uses anyio.fail_after() instead of asyncio.wait_for() to avoid conflicts
with the MCP SDK's anyio TaskGroup. See GitHub issue #20715 for details.
For OAuth pass-through and upstream-delegated OAuth2 MCP servers, an
upstream HTTP 401 is converted into :class:`MCPUpstreamAuthError`
instead of being swallowed to an empty tool list. That lets the
single-server HTTP routes surface a proper 401 + ``WWW-Authenticate``
challenge so standards-compliant MCP clients trigger the upstream
OAuth flow. Other servers keep today's swallow-and-log behaviour so
the multi-server ``/mcp`` aggregator doesn't get tainted by a single
bad server.
An upstream HTTP 401 is converted into :class:`MCPUpstreamAuthError`
instead of being swallowed to an empty tool list, regardless of the
server's auth_type. Callers route it by surface: the single-server HTTP
routes turn it into a 401 + ``WWW-Authenticate`` challenge so standards-
compliant MCP clients trigger the upstream OAuth flow, while the
multi-server ``/mcp`` aggregator absorbs it to an empty list so one
unauthenticated server doesn't fail the whole listing. Only a 401
(missing/invalid credential) drives the re-auth challenge; a 403
(authenticated but forbidden, e.g. insufficient scope) is not a re-auth
signal and, like other non-auth errors, returns an empty list.
Args:
client: MCP client instance
server_name: Name of the server for logging
server: Optional MCPServer; when upstream auth is delegated, auth
errors are re-raised as :class:`MCPUpstreamAuthError`.
Returns:
List of tools from the server
"""
should_surface_upstream_auth = bool(
server is not None
and (
server.is_oauth_passthrough
or (
server.auth_type == MCPAuth.oauth2
and getattr(server, "delegate_auth_to_upstream", False) is True
and not server.has_client_credentials
)
)
)
try:
with anyio.fail_after(MCP_TOOL_LISTING_TIMEOUT):
tools = await client.list_tools(raise_on_error=should_surface_upstream_auth)
tools = await client.list_tools(raise_on_error=True)
verbose_logger.debug(f"Tools from {server_name}: {tools}")
return tools
except TimeoutError:
@ -2744,16 +2743,15 @@ class MCPServerManager:
verbose_logger.warning(f"Connection error while listing tools from {server_name}: {str(e)}")
return []
except Exception as e:
if should_surface_upstream_auth:
auth_info = _extract_upstream_auth_failure(e)
if auth_info is not None:
status_code, www_authenticate = auth_info
verbose_logger.info(f"Upstream auth failure from MCP server {server_name}: HTTP {status_code}")
raise MCPUpstreamAuthError(
status_code=status_code,
www_authenticate=www_authenticate,
server_name=server_name,
) from e
auth_info = _extract_upstream_auth_failure(e)
if auth_info is not None and auth_info[0] == 401:
_, www_authenticate = auth_info
verbose_logger.info(f"Upstream auth failure from MCP server {server_name}: HTTP 401")
raise MCPUpstreamAuthError(
status_code=401,
www_authenticate=www_authenticate,
server_name=server_name,
) from e
verbose_logger.warning(f"Error listing tools from {server_name}: {str(e)}")
return []

View file

@ -76,9 +76,7 @@ async def test_fetch_tools_from_passthrough_raises_on_upstream_401():
mock_client.list_tools = AsyncMock(side_effect=upstream_error)
with pytest.raises(MCPUpstreamAuthError) as exc_info:
await manager._fetch_tools_with_timeout(
mock_client, passthrough_server.name, server=passthrough_server
)
await manager._fetch_tools_with_timeout(mock_client, passthrough_server.name)
assert exc_info.value.status_code == 401
assert exc_info.value.www_authenticate == (
@ -113,9 +111,7 @@ async def test_fetch_tools_from_delegated_oauth2_raises_on_upstream_401():
mock_client.list_tools = AsyncMock(side_effect=upstream_error)
with pytest.raises(MCPUpstreamAuthError) as exc_info:
await manager._fetch_tools_with_timeout(
mock_client, delegated_server.name, server=delegated_server
)
await manager._fetch_tools_with_timeout(mock_client, delegated_server.name)
assert exc_info.value.status_code == 401
assert exc_info.value.www_authenticate == (
@ -126,7 +122,10 @@ async def test_fetch_tools_from_delegated_oauth2_raises_on_upstream_401():
@pytest.mark.asyncio
async def test_fetch_tools_from_client_credentials_oauth2_keeps_swallow_behavior():
async def test_fetch_tools_from_client_credentials_oauth2_surfaces_upstream_401():
"""The auth_type carve-out was removed: a client_credentials (M2M) server now
surfaces an upstream 401 as MCPUpstreamAuthError too, instead of swallowing it
to an empty list, so single-server routes can return a 401 challenge."""
manager = MCPServerManager()
m2m_server = MCPServer(
server_id="oauth-m2m",
@ -150,12 +149,12 @@ async def test_fetch_tools_from_client_credentials_oauth2_keeps_swallow_behavior
mock_client = MagicMock()
mock_client.list_tools = AsyncMock(side_effect=upstream_error)
tools = await manager._fetch_tools_with_timeout(
mock_client, m2m_server.name, server=m2m_server
)
with pytest.raises(MCPUpstreamAuthError) as exc_info:
await manager._fetch_tools_with_timeout(mock_client, m2m_server.name)
assert tools == []
mock_client.list_tools.assert_awaited_with(raise_on_error=False)
assert exc_info.value.status_code == 401
assert exc_info.value.server_name == "m2m_docs"
mock_client.list_tools.assert_awaited_with(raise_on_error=True)
@pytest.mark.asyncio
@ -176,9 +175,7 @@ async def test_fetch_tools_from_passthrough_returns_tools_on_success():
mock_client = MagicMock()
mock_client.list_tools = AsyncMock(return_value=[tool])
tools = await manager._fetch_tools_with_timeout(
mock_client, passthrough_server.name, server=passthrough_server
)
tools = await manager._fetch_tools_with_timeout(mock_client, passthrough_server.name)
assert tools == [tool]
@ -238,8 +235,11 @@ def test_to_http_exception_skips_challenge_for_non_401_status():
@pytest.mark.asyncio
async def test_fetch_tools_from_gateway_managed_swallows_errors():
"""Regression guard: non-pass-through servers keep returning [] on errors."""
async def test_fetch_tools_from_gateway_managed_surfaces_upstream_401():
"""An oauth2 server that is neither pass-through nor delegate now surfaces an
upstream 401 as MCPUpstreamAuthError as well; the auth_type carve-out that
swallowed it to [] was removed. A missing upstream WWW-Authenticate is carried
through as None (the single-server route fabricates one from the gateway URL)."""
manager = MCPServerManager()
oauth2_server = MCPServer(
server_id="o1",
@ -260,11 +260,13 @@ async def test_fetch_tools_from_gateway_managed_swallows_errors():
mock_client = MagicMock()
mock_client.list_tools = AsyncMock(side_effect=upstream_error)
tools = await manager._fetch_tools_with_timeout(
mock_client, oauth2_server.name, server=oauth2_server
)
assert tools == []
mock_client.list_tools.assert_awaited_with(raise_on_error=False)
with pytest.raises(MCPUpstreamAuthError) as exc_info:
await manager._fetch_tools_with_timeout(mock_client, oauth2_server.name)
assert exc_info.value.status_code == 401
assert exc_info.value.www_authenticate is None
assert exc_info.value.server_name == "keycloak_whoami"
mock_client.list_tools.assert_awaited_with(raise_on_error=True)
def _http_server(server_id: str, name: str, **kwargs) -> MCPServer:

View file

@ -5228,5 +5228,139 @@ class TestCreateMcpClientV2Graft:
assert client._get_auth_headers()["Authorization"] == "Bearer hook-jwt"
def _upstream_status_error(status_code: int, challenge: str) -> httpx.HTTPStatusError:
request = httpx.Request("POST", "https://upstream.example/mcp")
response = httpx.Response(
status_code,
headers={"WWW-Authenticate": challenge},
request=request,
)
return httpx.HTTPStatusError(
"upstream rejected token", request=request, response=response
)
class TestMCPToolsListAuthSurfacing:
"""Regression: MCP tools/list 401 auth failures must surface as MCPUpstreamAuthError.
Previously a missing/expired per-user OAuth token, or an upstream 401 for any
non-carveout auth_type, was swallowed to an empty tool list, so a single-server
client saw a 200 with no tools instead of a 401 challenge. The listing helpers
now raise MCPUpstreamAuthError on a 401 regardless of auth_type; the single-server
routes turn it into a 401 + WWW-Authenticate while the aggregator absorbs it to an
empty list. Only a 401 challenges; a 403 (forbidden) degrades like any other error.
"""
@pytest.mark.asyncio
async def test_fetch_tools_with_timeout_surfaces_upstream_401(self):
from litellm.proxy._experimental.mcp_server.exceptions import (
MCPUpstreamAuthError,
)
manager = MCPServerManager()
challenge = 'Bearer resource_metadata="https://upstream.example/.well-known/oauth-protected-resource"'
client = MagicMock()
client.list_tools = AsyncMock(side_effect=_upstream_status_error(401, challenge))
with pytest.raises(MCPUpstreamAuthError) as exc_info:
await manager._fetch_tools_with_timeout(client, "static-key-server")
assert exc_info.value.status_code == 401
assert exc_info.value.www_authenticate == challenge
assert exc_info.value.server_name == "static-key-server"
@pytest.mark.asyncio
async def test_fetch_tools_with_timeout_absorbs_upstream_403(self):
"""Only a 401 drives the re-auth challenge. A 403 (authenticated but
forbidden, e.g. insufficient scope) is not a re-auth signal, so even
with a WWW-Authenticate header it degrades to an empty list rather than
surfacing a challenge."""
manager = MCPServerManager()
challenge = 'Bearer error="insufficient_scope", scope="read:tools"'
client = MagicMock()
client.list_tools = AsyncMock(side_effect=_upstream_status_error(403, challenge))
assert await manager._fetch_tools_with_timeout(client, "forbidden-server") == []
@pytest.mark.asyncio
async def test_fetch_tools_with_timeout_returns_empty_on_non_auth_error(self):
manager = MCPServerManager()
client = MagicMock()
client.list_tools = AsyncMock(side_effect=RuntimeError("upstream 500"))
assert await manager._fetch_tools_with_timeout(client, "srv") == []
@pytest.mark.asyncio
async def test_get_tools_from_server_surfaces_unusable_user_token(self):
from litellm.proxy._experimental.mcp_server.exceptions import (
MCPUpstreamAuthError,
)
manager = MCPServerManager()
server = MCPServer(
server_id="oauth-srv", name="oauth-srv", transport=MCPTransport.http
)
challenge = 'Bearer resource_metadata="/.well-known/oauth-protected-resource/mcp/oauth-srv"'
manager._create_mcp_client = AsyncMock(
side_effect=HTTPException(
status_code=401,
detail="Unauthorized",
headers={"WWW-Authenticate": challenge},
)
)
with pytest.raises(MCPUpstreamAuthError) as exc_info:
await manager._get_tools_from_server(server)
assert exc_info.value.status_code == 401
assert exc_info.value.www_authenticate == challenge
assert exc_info.value.server_name == "oauth-srv"
@pytest.mark.asyncio
async def test_get_tools_from_server_absorbs_non_challenge_http_error(self):
manager = MCPServerManager()
server = MCPServer(
server_id="stdio-srv", name="stdio-srv", transport=MCPTransport.http
)
manager._create_mcp_client = AsyncMock(
side_effect=HTTPException(
status_code=403,
detail="MCP stdio command 'foo' is not in the allowlist",
)
)
assert await manager._get_tools_from_server(server) == []
@pytest.mark.asyncio
async def test_aggregate_list_tools_absorbs_unauthenticated_server(self):
from litellm.proxy._experimental.mcp_server.exceptions import (
MCPUpstreamAuthError,
)
manager = MCPServerManager()
good = MCPServer(server_id="good", name="good", transport=MCPTransport.http)
bad = MCPServer(server_id="bad", name="bad", transport=MCPTransport.http)
manager.get_allowed_mcp_servers = AsyncMock(return_value=["good", "bad"])
manager.get_mcp_server_by_id = MagicMock(
side_effect=lambda server_id: {"good": good, "bad": bad}.get(server_id)
)
good_tool = MCPTool(name="good-do_thing", description="do thing", inputSchema={})
async def fake_get_tools(server, **kwargs):
if server.server_id == "bad":
raise MCPUpstreamAuthError(
status_code=401,
www_authenticate='Bearer realm="x"',
server_name="bad",
)
return [good_tool]
manager._get_tools_from_server = fake_get_tools
result = await manager.list_tools()
assert [t.name for t in result] == ["good-do_thing"]
if __name__ == "__main__":
pytest.main([__file__])

View file

@ -843,6 +843,76 @@ class TestListToolsRestAPI:
assert exc_info.value.status_code == upstream_status
assert exc_info.value.headers == {"www-authenticate": challenge}
async def test_aggregate_list_absorbs_one_server_auth_failure(self, monkeypatch):
"""The multi-server aggregate listing degrades a server whose upstream
rejects auth to an empty contribution and still returns the healthy
server's tools with a 200, rather than surfacing a 401."""
from litellm.proxy._experimental.mcp_server.exceptions import (
MCPUpstreamAuthError,
)
class StubServer:
def __init__(self, name):
self.alias = name
self.server_name = name
self.name = name
self.allowed_tools = None
self.mcp_info = {"server_name": name}
self.available_on_public_internet = True
good = StubServer("good")
bad = StubServer("bad")
async def fake_contexts(user_api_key_auth):
return [user_api_key_auth]
async def fake_get_allowed_mcp_servers(*args, **kwargs):
return ["good", "bad"]
async def fake_get_tools(server, *args, **kwargs):
if server.server_name == "bad":
raise MCPUpstreamAuthError(
status_code=401,
www_authenticate='Bearer realm="x"',
server_name="bad",
)
return ["good-tool"]
monkeypatch.setattr(
rest_endpoints,
"build_effective_auth_contexts",
fake_contexts,
raising=False,
)
monkeypatch.setattr(
rest_endpoints.global_mcp_server_manager,
"get_allowed_mcp_servers",
fake_get_allowed_mcp_servers,
raising=False,
)
monkeypatch.setattr(
rest_endpoints.global_mcp_server_manager,
"get_mcp_server_by_id",
lambda server_id: {"good": good, "bad": bad}.get(server_id),
raising=False,
)
monkeypatch.setattr(
rest_endpoints,
"_get_tools_for_single_server",
fake_get_tools,
raising=False,
)
request = _build_request(path="/mcp-rest/tools/list", method="GET")
result = await rest_endpoints.list_tool_rest_api(
request,
server_id=None,
user_api_key_dict=UserAPIKeyAuth(),
)
assert result["tools"] == ["good-tool"]
assert result["error"] is None
async def test_name_resolution_finds_server_by_uuid(self, monkeypatch):
"""When server_id is a name string, it should be resolved to its UUID
and used for the tools lookup when the UUID is in allowed_server_ids."""

View file

@ -131,6 +131,25 @@ describe("MCPToolsViewer auth gate routing", () => {
expect(screen.queryByText(GATE_TEXT)).not.toBeInTheDocument();
});
it("gates an OBO server whose stored token is expired and the list call 401s (refresh could not mint a token)", async () => {
// has_credential=true but the list call 401s: the server-side refresh could not
// produce a valid token (e.g. expired with no usable refresh token), so the user
// must reauthorize instead of seeing a dead empty list.
vi.mocked(getMCPOAuthUserCredentialStatus).mockResolvedValue(
credStatus({ has_credential: true, is_expired: true }),
);
vi.mocked(listMCPTools).mockResolvedValue({
tools: [],
error: "unauthorized",
status: 401,
} as unknown as Awaited<ReturnType<typeof listMCPTools>>);
renderViewer({ oauth2_flow: null, delegate_auth_to_upstream: false });
expect(await screen.findByText(GATE_TEXT)).toBeInTheDocument();
expect(screen.getByRole("button", { name: "Authorize" })).toBeInTheDocument();
});
it("does not gate an M2M server; lists with the LiteLLM key", async () => {
renderViewer({ oauth2_flow: "client_credentials", delegate_auth_to_upstream: false });

View file

@ -238,9 +238,14 @@ const MCPToolsViewer = ({
const toolsData = mcpToolsResponse?.tools || [];
const oboToolsError = mcpToolsError as (Error & { status?: number; response?: { status?: number } }) | null;
const oboTokenRejected = isObo && (oboToolsError?.status ?? oboToolsError?.response?.status) === 401;
// An auth gate replaces the tool list when the user must authenticate first:
// passthrough needs a browser token, OBO needs a stored DB credential.
const authGateActive = (isPassthrough && !oauthToken) || oboNeedsAuth;
// passthrough needs a browser token; OBO needs a stored DB credential or a
// still-valid one — a 401 from the list call means the backend has none even
// after attempting a refresh, so re-authorization is required.
const authGateActive = (isPassthrough && !oauthToken) || oboNeedsAuth || oboTokenRejected;
// Treat OBO credential-status loading as "tools loading" so the empty state
// doesn't flash before we know whether the user needs to authorize.
const toolsAreaLoading = isLoadingTools || oboStatusLoading;
@ -364,10 +369,12 @@ const MCPToolsViewer = ({
</div>
)}
{/* OBO auth gate only when no credential row exists for this user.
An existing-but-expired token is refreshed server-side on the
list call, so the gate never appears for a stored credential. */}
{oboNeedsAuth && (
{/* OBO auth gate shown when there is no credential row for this
user, or when the list call returns 401 (no valid token and the
server-side refresh could not mint one, e.g. an expired token
with no usable refresh token). A refreshable token is refreshed
on the list call and never trips this gate. */}
{(oboNeedsAuth || oboTokenRejected) && (
<div className="p-4 text-center bg-white border border-gray-200 rounded-lg">
<LockOutlined className="text-2xl text-gray-400 mb-2" />
<p className="text-xs font-medium text-gray-700 mb-1">Authentication required</p>