mirror of
https://github.com/BerriAI/litellm.git
synced 2026-09-12 23:01:41 +00:00
feat(mcp): per-server fail-closed OAuth challenge at the v2 egress
When an authorization_code server has no usable per-user token, the arm returns a semantic
unauthorized and the graft builds the 401 where the full MCPServer is in hand: a relative,
per-server RFC 9728 resource_metadata pointer (/.well-known/oauth-protected-resource/mcp/{name})
that names the server's own authorization server, instead of the resolver's earlier root pointer
which resolved to the gateway's generic PRM. Relative, so it is correct behind a reverse proxy
without request context. The listing-phase 401 still emits the RFC 8414 authorization_uri form;
both now target the same server, so the remaining difference is cosmetic and unifies in a later PR.
This commit is contained in:
parent
8e8a5d4f1e
commit
40f4a00282
5 changed files with 80 additions and 32 deletions
|
|
@ -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,
|
||||
|
|
|
|||
|
|
@ -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}"'},
|
||||
)
|
||||
|
|
|
|||
|
|
@ -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,
|
||||
},
|
||||
)
|
||||
|
|
|
|||
|
|
@ -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"]
|
||||
|
|
|
|||
|
|
@ -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")
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue