diff --git a/litellm/proxy/_experimental/mcp_server/mcp_server_manager.py b/litellm/proxy/_experimental/mcp_server/mcp_server_manager.py index 2f8831be0ea..569280f7ef7 100644 --- a/litellm/proxy/_experimental/mcp_server/mcp_server_manager.py +++ b/litellm/proxy/_experimental/mcp_server/mcp_server_manager.py @@ -63,6 +63,7 @@ from litellm.proxy._experimental.mcp_server.outbound_credentials import ( ) from litellm.proxy._experimental.mcp_server.outbound_credentials.adapter import ( raise_public, + raise_user_oauth_challenge, to_server_spec, to_subject, ) @@ -2064,6 +2065,10 @@ class MCPServerManager: ): resolved_auth = None case Error(err): + if err.tag == "unauthorized": + # The arm signals a missing per-user token semantically; raise the + # per-server OAuth challenge here, where the full MCPServer is in hand. + raise_user_oauth_challenge(server) raise_public(err) return MCPClient( server_url=server_url, diff --git a/litellm/proxy/_experimental/mcp_server/outbound_credentials/adapter.py b/litellm/proxy/_experimental/mcp_server/outbound_credentials/adapter.py index f119cbd416b..b266c5beb4d 100644 --- a/litellm/proxy/_experimental/mcp_server/outbound_credentials/adapter.py +++ b/litellm/proxy/_experimental/mcp_server/outbound_credentials/adapter.py @@ -158,3 +158,25 @@ def raise_public(error: CredError) -> NoReturn: case "not_implemented": raise HTTPException(status_code=501, detail=error.summary) assert_never(error.tag) + + +def raise_user_oauth_challenge(server: MCPServer) -> NoReturn: + """Raise the 401 an ``authorization_code`` server returns at egress when the user has no token. + + Points at the server's RFC 9728 Protected Resource Metadata (``resource_metadata``), which names + the upstream authorization server the client must complete OAuth with. The URL is per-server and + relative, so it resolves against the caller's own host (correct even behind a reverse proxy) + without needing request context. The listing-phase 401 still emits the RFC 8414 ``authorization_uri`` + form pending the format unification; both target the same server, so the difference is cosmetic. + """ + from litellm.proxy.utils import get_server_root_path # noqa: PLC0415 + + root = get_server_root_path() + prefix = "" if root == "/" else root + name = server.alias or server.server_name or server.name or server.server_id + resource_metadata = f"/.well-known/oauth-protected-resource{prefix}/mcp/{name}" + raise HTTPException( + status_code=401, + detail="Unauthorized", + headers={"www-authenticate": f'Bearer resource_metadata="{resource_metadata}"'}, + ) diff --git a/litellm/proxy/_experimental/mcp_server/outbound_credentials/resolver.py b/litellm/proxy/_experimental/mcp_server/outbound_credentials/resolver.py index ceb709e140d..9163bfdedaa 100644 --- a/litellm/proxy/_experimental/mcp_server/outbound_credentials/resolver.py +++ b/litellm/proxy/_experimental/mcp_server/outbound_credentials/resolver.py @@ -14,7 +14,6 @@ that each land in a follow-up PR with their seam. Pure v2: no imports from v1. from __future__ import annotations - import httpx from typing_extensions import assert_never @@ -86,7 +85,11 @@ class UpstreamCredentialProvider: case AuthorizationCodeConfig(): token = await self._authz_token(subject, server) if token is None: - return Error(_oauth_challenge(server.server_id)) + return Error( + CredError.of_unauthorized( + "Authorization required: complete the OAuth flow for this server." + ) + ) return Ok( StaticHeaderAuth( f"Bearer {token.access_token}", header_name="Authorization" @@ -132,27 +135,3 @@ def _not_implemented(kind: AuthSpecKind) -> Result[httpx.Auth, CredError]: return Error( CredError.of_not_implemented(f"{kind.value}: resolver arm not implemented yet") ) - - -_OAUTH_WWW_AUTHENTICATE = ( - 'Bearer resource_metadata="/.well-known/oauth-protected-resource"' -) - - -def _oauth_challenge(server_id: str) -> CredError: - """The 401 an authorization_code server returns when the user has no usable token. - - Carries the RFC 9728 ``WWW-Authenticate`` challenge that drives the OAuth flow, plus an - ``authorization_required`` body. The exact body is reconciled with v1 when the v1-backed token - source lands. - """ - message = "Authorization required: complete the OAuth flow for this server." - return CredError.of_unauthorized( - message, - www_authenticate=_OAUTH_WWW_AUTHENTICATE, - body={ - "error": "authorization_required", - "server_id": server_id, - "message": message, - }, - ) diff --git a/tests/test_litellm/proxy/_experimental/mcp_server/outbound_credentials/test_adapter.py b/tests/test_litellm/proxy/_experimental/mcp_server/outbound_credentials/test_adapter.py index e660462b9bc..060e117892b 100644 --- a/tests/test_litellm/proxy/_experimental/mcp_server/outbound_credentials/test_adapter.py +++ b/tests/test_litellm/proxy/_experimental/mcp_server/outbound_credentials/test_adapter.py @@ -7,12 +7,14 @@ maps each CredError onto its HTTP status. These pin the parity-critical mapping import base64 from types import SimpleNamespace +from unittest.mock import patch import pytest from fastapi import HTTPException from litellm.proxy._experimental.mcp_server.outbound_credentials.adapter import ( raise_public, + raise_user_oauth_challenge, to_server_spec, to_subject, ) @@ -178,3 +180,43 @@ def test_raise_public_plain_unauthorized_has_no_challenge(): assert exc.status_code == 401 assert exc.detail == "unauthorized: nope" assert exc.headers is None + + +_ROOT_PATH = "litellm.proxy.utils.get_server_root_path" + + +def test_raise_user_oauth_challenge_points_at_per_server_prm(): + with patch(_ROOT_PATH, return_value="/"), pytest.raises(HTTPException) as exc_info: + raise_user_oauth_challenge(_server(alias="my-srv")) + exc = exc_info.value + assert exc.status_code == 401 + assert ( + exc.headers["www-authenticate"] + == 'Bearer resource_metadata="/.well-known/oauth-protected-resource/mcp/my-srv"' + ) + + +def test_raise_user_oauth_challenge_includes_server_root_path(): + with ( + patch(_ROOT_PATH, return_value="/api/v1"), + pytest.raises(HTTPException) as exc_info, + ): + raise_user_oauth_challenge(_server(alias="my-srv")) + assert ( + exc_info.value.headers["www-authenticate"] + == 'Bearer resource_metadata="/.well-known/oauth-protected-resource/api/v1/mcp/my-srv"' + ) + + +@pytest.mark.parametrize( + "kwargs, expected_name", + [ + ({"alias": "a", "server_name": "sn"}, "a"), # alias wins + ({"server_name": "sn"}, "sn"), # then server_name + ({}, "n"), # then the name field (server_id is the last fallback) + ], +) +def test_raise_user_oauth_challenge_name_fallback(kwargs, expected_name): + with patch(_ROOT_PATH, return_value="/"), pytest.raises(HTTPException) as exc_info: + raise_user_oauth_challenge(_server(**kwargs)) + assert f'/mcp/{expected_name}"' in exc_info.value.headers["www-authenticate"] diff --git a/tests/test_litellm/proxy/_experimental/mcp_server/outbound_credentials/test_resolver.py b/tests/test_litellm/proxy/_experimental/mcp_server/outbound_credentials/test_resolver.py index b1d14c639c6..89e4397c3bc 100644 --- a/tests/test_litellm/proxy/_experimental/mcp_server/outbound_credentials/test_resolver.py +++ b/tests/test_litellm/proxy/_experimental/mcp_server/outbound_credentials/test_resolver.py @@ -111,7 +111,7 @@ async def test_authorization_code_emits_bearer_for_a_stored_token(): @pytest.mark.asyncio -async def test_authorization_code_without_token_is_unauthorized_with_challenge(): +async def test_authorization_code_without_token_is_semantically_unauthorized(): result = await UpstreamCredentialProvider( oauth_token_store=_FakeTokenStore({}) ).resolve_credentials( @@ -119,14 +119,14 @@ async def test_authorization_code_without_token_is_unauthorized_with_challenge() ) assert isinstance(result, Error) assert result.error.tag == "unauthorized" - challenge = result.error.unauthorized - assert challenge.www_authenticate is not None - assert challenge.body is not None - assert challenge.body["error"] == "authorization_required" + # Semantic only: the per-server challenge is built at the edge, not in the request-free arm. + assert "Authorization required" in result.error.unauthorized.detail + assert result.error.unauthorized.www_authenticate is None + assert result.error.unauthorized.body is None @pytest.mark.asyncio -async def test_authorization_code_store_unavailable_surfaces_the_challenge(): +async def test_authorization_code_store_unavailable_is_unauthorized(): class _Unavailable: async def fetch(self, user_id: str, server_id: str): raise TokenStoreUnavailable("down")