mirror of
https://github.com/BerriAI/litellm.git
synced 2026-10-08 03:08:45 +00:00
fix(mcp): remove auth gate from OAuth broker authorize and token endpoints
Browser-initiated OAuth flows cannot send an API key, so requiring
user_api_key_auth on /server/oauth/{id}/authorize and /server/oauth/{id}/token
caused a 401 for all end users. Remove the dependency from both endpoints and
make user_api_key_dict optional in _get_cached_temporary_mcp_server_or_404 so
unauthenticated OAuth browser flows skip the admin-view gate.
Add regression tests:
- unit tests for loopback validation, state round-trip, and token validation
- respx HTTP integration tests covering the full authorize → callback → token flow
- Playwright E2E: Layer 1 directly asserts /authorize returns !401 without an API key; Layer 2 asserts the full UI OAuth form flow succeeds
- extend test-mcp.yml CI job to run both new test files
Co-authored-by: Cursor <cursoragent@cursor.com>
This commit is contained in:
parent
93d8375cbc
commit
fe309ea0b0
7 changed files with 605 additions and 25 deletions
7
.github/workflows/test-mcp.yml
vendored
7
.github/workflows/test-mcp.yml
vendored
|
|
@ -44,3 +44,10 @@ jobs:
|
||||||
- name: Run MCP tests
|
- name: Run MCP tests
|
||||||
run: |
|
run: |
|
||||||
uv run --no-sync pytest tests/mcp_tests -x -vv -n 4 --cov=litellm --cov-report=xml --durations=5
|
uv run --no-sync pytest tests/mcp_tests -x -vv -n 4 --cov=litellm --cov-report=xml --durations=5
|
||||||
|
|
||||||
|
- name: Run MCP OAuth broker tests (unit + respx HTTP flow)
|
||||||
|
run: |
|
||||||
|
uv run --no-sync pytest \
|
||||||
|
tests/test_litellm/proxy/_experimental/mcp_server/test_mcp_oauth_security_unit.py \
|
||||||
|
tests/test_litellm/proxy/_experimental/mcp_server/test_mcp_oauth_flow_http_respx.py \
|
||||||
|
-v --tb=short
|
||||||
|
|
|
||||||
|
|
@ -1449,7 +1449,7 @@ if MCP_AVAILABLE:
|
||||||
|
|
||||||
async def _get_cached_temporary_mcp_server_or_404(
|
async def _get_cached_temporary_mcp_server_or_404(
|
||||||
server_id: str,
|
server_id: str,
|
||||||
user_api_key_dict: UserAPIKeyAuth,
|
user_api_key_dict: Optional[UserAPIKeyAuth] = None,
|
||||||
request: Optional[Request] = None,
|
request: Optional[Request] = None,
|
||||||
) -> MCPServer:
|
) -> MCPServer:
|
||||||
server = await get_cached_temporary_mcp_server(server_id)
|
server = await get_cached_temporary_mcp_server(server_id)
|
||||||
|
|
@ -1475,8 +1475,11 @@ if MCP_AVAILABLE:
|
||||||
# callers are unrestricted; non-admins must have the server in their
|
# callers are unrestricted; non-admins must have the server in their
|
||||||
# allowed-servers set. Temporary cached servers come from the
|
# allowed-servers set. Temporary cached servers come from the
|
||||||
# admin-only `/server/oauth/session` setup flow and are not exposed
|
# admin-only `/server/oauth/session` setup flow and are not exposed
|
||||||
# to non-admins.
|
# to non-admins. Unauthenticated OAuth browser flows omit the key and
|
||||||
if not _user_has_admin_view(user_api_key_dict):
|
# skip this gate (same as pre-broker-auth behavior on authorize/token).
|
||||||
|
if user_api_key_dict is not None and not _user_has_admin_view(
|
||||||
|
user_api_key_dict
|
||||||
|
):
|
||||||
if resolved_from_temp_cache:
|
if resolved_from_temp_cache:
|
||||||
raise HTTPException(
|
raise HTTPException(
|
||||||
status_code=status.HTTP_403_FORBIDDEN,
|
status_code=status.HTTP_403_FORBIDDEN,
|
||||||
|
|
@ -1497,12 +1500,10 @@ if MCP_AVAILABLE:
|
||||||
@router.get(
|
@router.get(
|
||||||
"/server/oauth/{server_id}/authorize",
|
"/server/oauth/{server_id}/authorize",
|
||||||
include_in_schema=False,
|
include_in_schema=False,
|
||||||
dependencies=[Depends(user_api_key_auth)],
|
|
||||||
)
|
)
|
||||||
async def mcp_authorize(
|
async def mcp_authorize(
|
||||||
request: Request,
|
request: Request,
|
||||||
server_id: str,
|
server_id: str,
|
||||||
user_api_key_dict: UserAPIKeyAuth = Depends(user_api_key_auth),
|
|
||||||
client_id: Optional[str] = None,
|
client_id: Optional[str] = None,
|
||||||
redirect_uri: str = Query(...),
|
redirect_uri: str = Query(...),
|
||||||
state: str = "",
|
state: str = "",
|
||||||
|
|
@ -1512,7 +1513,7 @@ if MCP_AVAILABLE:
|
||||||
scope: Optional[str] = None,
|
scope: Optional[str] = None,
|
||||||
):
|
):
|
||||||
mcp_server = await _get_cached_temporary_mcp_server_or_404(
|
mcp_server = await _get_cached_temporary_mcp_server_or_404(
|
||||||
server_id, user_api_key_dict, request=request
|
server_id, request=request
|
||||||
)
|
)
|
||||||
# Use the server's stored client_id when the caller doesn't supply one
|
# Use the server's stored client_id when the caller doesn't supply one
|
||||||
resolved_client_id = mcp_server.client_id or client_id or ""
|
resolved_client_id = mcp_server.client_id or client_id or ""
|
||||||
|
|
@ -1542,12 +1543,10 @@ if MCP_AVAILABLE:
|
||||||
@router.post(
|
@router.post(
|
||||||
"/server/oauth/{server_id}/token",
|
"/server/oauth/{server_id}/token",
|
||||||
include_in_schema=False,
|
include_in_schema=False,
|
||||||
dependencies=[Depends(user_api_key_auth)],
|
|
||||||
)
|
)
|
||||||
async def mcp_token(
|
async def mcp_token(
|
||||||
request: Request,
|
request: Request,
|
||||||
server_id: str,
|
server_id: str,
|
||||||
user_api_key_dict: UserAPIKeyAuth = Depends(user_api_key_auth),
|
|
||||||
grant_type: str = Form(...),
|
grant_type: str = Form(...),
|
||||||
code: Optional[str] = Form(None),
|
code: Optional[str] = Form(None),
|
||||||
redirect_uri: Optional[str] = Form(None),
|
redirect_uri: Optional[str] = Form(None),
|
||||||
|
|
@ -1558,7 +1557,7 @@ if MCP_AVAILABLE:
|
||||||
scope: Optional[str] = Form(None),
|
scope: Optional[str] = Form(None),
|
||||||
):
|
):
|
||||||
mcp_server = await _get_cached_temporary_mcp_server_or_404(
|
mcp_server = await _get_cached_temporary_mcp_server_or_404(
|
||||||
server_id, user_api_key_dict, request=request
|
server_id, request=request
|
||||||
)
|
)
|
||||||
resolved_client_id = mcp_server.client_id or client_id or ""
|
resolved_client_id = mcp_server.client_id or client_id or ""
|
||||||
if not resolved_client_id:
|
if not resolved_client_id:
|
||||||
|
|
|
||||||
|
|
@ -0,0 +1,277 @@
|
||||||
|
"""
|
||||||
|
HTTP-level integration tests for MCP discoverable OAuth (authorize → callback → token).
|
||||||
|
|
||||||
|
Uses ASGITransport + httpx and mocks the upstream IdP with respx.
|
||||||
|
"""
|
||||||
|
|
||||||
|
from __future__ import annotations
|
||||||
|
|
||||||
|
import urllib.parse
|
||||||
|
from typing import Iterator
|
||||||
|
from unittest.mock import patch
|
||||||
|
|
||||||
|
import httpx
|
||||||
|
import litellm
|
||||||
|
import pytest
|
||||||
|
from fastapi import FastAPI
|
||||||
|
from httpx import ASGITransport
|
||||||
|
|
||||||
|
from litellm.types.mcp import MCPTransport
|
||||||
|
from litellm.types.mcp import MCPAuth
|
||||||
|
from litellm.types.mcp_server.mcp_server_manager import MCPServer
|
||||||
|
|
||||||
|
|
||||||
|
@pytest.fixture(autouse=True)
|
||||||
|
def mock_mcp_client_ip() -> Iterator[None]:
|
||||||
|
with patch(
|
||||||
|
"litellm.proxy._experimental.mcp_server.discoverable_endpoints.IPAddressUtils.get_mcp_client_ip",
|
||||||
|
return_value=None,
|
||||||
|
):
|
||||||
|
yield
|
||||||
|
|
||||||
|
|
||||||
|
@pytest.fixture
|
||||||
|
def oauth_asgi_app(monkeypatch) -> Iterator[FastAPI]:
|
||||||
|
monkeypatch.setenv("LITELLM_SALT_KEY", "integration-test-salt-key-32chars")
|
||||||
|
# Outbound token exchange must use httpx so respx can intercept (not aiohttp).
|
||||||
|
monkeypatch.setattr(litellm, "disable_aiohttp_transport", True)
|
||||||
|
|
||||||
|
from litellm.proxy._experimental.mcp_server.discoverable_endpoints import (
|
||||||
|
router as discoverable_oauth_router,
|
||||||
|
)
|
||||||
|
from litellm.proxy._experimental.mcp_server.mcp_server_manager import (
|
||||||
|
global_mcp_server_manager,
|
||||||
|
)
|
||||||
|
|
||||||
|
global_mcp_server_manager.registry.clear()
|
||||||
|
server = MCPServer(
|
||||||
|
server_id="mock-oauth-srv",
|
||||||
|
name="mock_oauth",
|
||||||
|
server_name="mock_oauth",
|
||||||
|
alias="mock_oauth",
|
||||||
|
transport=MCPTransport.http,
|
||||||
|
auth_type=MCPAuth.oauth2,
|
||||||
|
client_id="upstream-client",
|
||||||
|
client_secret="upstream-secret",
|
||||||
|
authorization_url="https://mock-idp.example/oauth/authorize",
|
||||||
|
token_url="https://mock-idp.example/oauth/token",
|
||||||
|
scopes=["openid"],
|
||||||
|
needs_user_oauth_token=False,
|
||||||
|
)
|
||||||
|
global_mcp_server_manager.registry[server.server_id] = server
|
||||||
|
|
||||||
|
app = FastAPI()
|
||||||
|
app.include_router(discoverable_oauth_router)
|
||||||
|
try:
|
||||||
|
yield app
|
||||||
|
finally:
|
||||||
|
global_mcp_server_manager.registry.clear()
|
||||||
|
|
||||||
|
|
||||||
|
@pytest.mark.asyncio
|
||||||
|
@pytest.mark.respx
|
||||||
|
async def test_authorize_redirect_uri_to_upstream_is_proxy_callback_not_client_loopback(
|
||||||
|
oauth_asgi_app: FastAPI,
|
||||||
|
) -> None:
|
||||||
|
transport = ASGITransport(app=oauth_asgi_app)
|
||||||
|
async with httpx.AsyncClient(
|
||||||
|
transport=transport, base_url="http://proxy.test", follow_redirects=False
|
||||||
|
) as client:
|
||||||
|
r = await client.get(
|
||||||
|
"/mock_oauth/authorize",
|
||||||
|
params={
|
||||||
|
"client_id": "upstream-client",
|
||||||
|
"redirect_uri": "http://127.0.0.1:60108/ui/mcp/oauth/callback",
|
||||||
|
"state": "plain-client-state",
|
||||||
|
"code_challenge": "challenge",
|
||||||
|
"code_challenge_method": "S256",
|
||||||
|
},
|
||||||
|
)
|
||||||
|
assert r.status_code in (301, 302, 303, 307, 308)
|
||||||
|
loc = r.headers["location"]
|
||||||
|
assert loc.startswith("https://mock-idp.example/oauth/authorize")
|
||||||
|
q = urllib.parse.urlparse(loc).query
|
||||||
|
parsed = urllib.parse.parse_qs(q)
|
||||||
|
upstream_redirect = parsed["redirect_uri"][0]
|
||||||
|
assert upstream_redirect == "http://proxy.test/callback"
|
||||||
|
assert "challenge" == parsed["code_challenge"][0]
|
||||||
|
encrypted_state = parsed["state"][0]
|
||||||
|
from litellm.proxy._experimental.mcp_server.discoverable_endpoints import (
|
||||||
|
decode_state_hash,
|
||||||
|
)
|
||||||
|
|
||||||
|
state_data = decode_state_hash(encrypted_state)
|
||||||
|
assert state_data["original_state"] == "plain-client-state"
|
||||||
|
assert (
|
||||||
|
state_data["client_redirect_uri"]
|
||||||
|
== "http://127.0.0.1:60108/ui/mcp/oauth/callback"
|
||||||
|
)
|
||||||
|
|
||||||
|
|
||||||
|
@pytest.mark.asyncio
|
||||||
|
async def test_authorize_rejects_non_loopback_client_redirect_uri(
|
||||||
|
oauth_asgi_app: FastAPI,
|
||||||
|
) -> None:
|
||||||
|
transport = ASGITransport(app=oauth_asgi_app)
|
||||||
|
async with httpx.AsyncClient(
|
||||||
|
transport=transport, base_url="http://proxy.test", follow_redirects=False
|
||||||
|
) as client:
|
||||||
|
r = await client.get(
|
||||||
|
"/mock_oauth/authorize",
|
||||||
|
params={
|
||||||
|
"client_id": "upstream-client",
|
||||||
|
"redirect_uri": "https://attacker.example/capture",
|
||||||
|
"state": "x",
|
||||||
|
},
|
||||||
|
)
|
||||||
|
assert r.status_code == 400
|
||||||
|
|
||||||
|
|
||||||
|
@pytest.mark.asyncio
|
||||||
|
async def test_callback_redirects_to_client_loopback_with_upstream_code(
|
||||||
|
oauth_asgi_app: FastAPI,
|
||||||
|
) -> None:
|
||||||
|
from litellm.proxy._experimental.mcp_server.discoverable_endpoints import (
|
||||||
|
encode_state_with_base_url,
|
||||||
|
)
|
||||||
|
|
||||||
|
client_cb = "http://127.0.0.1:7777/oauth/callback"
|
||||||
|
base_no_query = "http://127.0.0.1:7777/oauth/callback"
|
||||||
|
state_token = encode_state_with_base_url(
|
||||||
|
base_url=base_no_query,
|
||||||
|
original_state="csrf-token-9",
|
||||||
|
code_challenge="cc",
|
||||||
|
code_challenge_method="S256",
|
||||||
|
client_redirect_uri=client_cb,
|
||||||
|
)
|
||||||
|
transport = ASGITransport(app=oauth_asgi_app)
|
||||||
|
async with httpx.AsyncClient(
|
||||||
|
transport=transport, base_url="http://proxy.test", follow_redirects=False
|
||||||
|
) as client:
|
||||||
|
r = await client.get(
|
||||||
|
"/callback",
|
||||||
|
params={"code": "upstream-auth-code", "state": state_token},
|
||||||
|
)
|
||||||
|
assert r.status_code in (301, 302, 303, 307, 308)
|
||||||
|
loc = r.headers["location"]
|
||||||
|
assert loc.startswith("http://127.0.0.1:7777/oauth/callback")
|
||||||
|
q = urllib.parse.urlparse(loc).query
|
||||||
|
parsed = urllib.parse.parse_qs(q)
|
||||||
|
assert parsed["code"][0] == "upstream-auth-code"
|
||||||
|
assert parsed["state"][0] == "csrf-token-9"
|
||||||
|
|
||||||
|
|
||||||
|
@pytest.mark.asyncio
|
||||||
|
async def test_callback_rejects_non_loopback_in_decrypted_state(
|
||||||
|
oauth_asgi_app: FastAPI,
|
||||||
|
) -> None:
|
||||||
|
with patch(
|
||||||
|
"litellm.proxy._experimental.mcp_server.discoverable_endpoints.decode_state_hash",
|
||||||
|
return_value={
|
||||||
|
"original_state": "ok",
|
||||||
|
"client_redirect_uri": "https://evil.com/y",
|
||||||
|
"base_url": "https://evil.com/y",
|
||||||
|
},
|
||||||
|
):
|
||||||
|
transport = ASGITransport(app=oauth_asgi_app)
|
||||||
|
async with httpx.AsyncClient(
|
||||||
|
transport=transport, base_url="http://proxy.test", follow_redirects=False
|
||||||
|
) as client:
|
||||||
|
r = await client.get(
|
||||||
|
"/callback",
|
||||||
|
params={"code": "c", "state": "opaque"},
|
||||||
|
)
|
||||||
|
assert r.status_code == 400
|
||||||
|
|
||||||
|
|
||||||
|
@pytest.mark.asyncio
|
||||||
|
@pytest.mark.respx
|
||||||
|
async def test_token_exchange_posts_proxy_callback_redirect_uri_to_upstream(
|
||||||
|
oauth_asgi_app: FastAPI,
|
||||||
|
respx_mock,
|
||||||
|
) -> None:
|
||||||
|
captured: dict = {}
|
||||||
|
|
||||||
|
def on_request(request: httpx.Request) -> httpx.Response:
|
||||||
|
captured["body"] = request.content.decode()
|
||||||
|
return httpx.Response(
|
||||||
|
200,
|
||||||
|
json={
|
||||||
|
"access_token": "at-upstream",
|
||||||
|
"token_type": "Bearer",
|
||||||
|
"expires_in": 3600,
|
||||||
|
},
|
||||||
|
)
|
||||||
|
|
||||||
|
respx_mock.post("https://mock-idp.example/oauth/token").mock(side_effect=on_request)
|
||||||
|
|
||||||
|
transport = ASGITransport(app=oauth_asgi_app)
|
||||||
|
async with httpx.AsyncClient(
|
||||||
|
transport=transport, base_url="http://proxy.test", follow_redirects=False
|
||||||
|
) as client:
|
||||||
|
r = await client.post(
|
||||||
|
"/mock_oauth/token",
|
||||||
|
data={
|
||||||
|
"grant_type": "authorization_code",
|
||||||
|
"code": "code-from-upstream",
|
||||||
|
"client_id": "upstream-client",
|
||||||
|
"client_secret": "upstream-secret",
|
||||||
|
"code_verifier": "verifier",
|
||||||
|
"redirect_uri": "ignored-by-litellm-for-upstream-exchange",
|
||||||
|
},
|
||||||
|
)
|
||||||
|
assert r.status_code == 200
|
||||||
|
assert r.headers.get("cache-control") == "no-store"
|
||||||
|
assert r.headers.get("pragma") == "no-cache"
|
||||||
|
body = r.json()
|
||||||
|
assert body["access_token"] == "at-upstream"
|
||||||
|
|
||||||
|
parsed = urllib.parse.parse_qs(captured.get("body", ""))
|
||||||
|
assert parsed["grant_type"][0] == "authorization_code"
|
||||||
|
assert parsed["redirect_uri"][0] == "http://proxy.test/callback"
|
||||||
|
assert parsed["code"][0] == "code-from-upstream"
|
||||||
|
assert parsed["code_verifier"][0] == "verifier"
|
||||||
|
|
||||||
|
|
||||||
|
@pytest.mark.asyncio
|
||||||
|
@pytest.mark.respx
|
||||||
|
async def test_token_exchange_applies_token_validation_rules(
|
||||||
|
oauth_asgi_app: FastAPI,
|
||||||
|
respx_mock,
|
||||||
|
) -> None:
|
||||||
|
from litellm.proxy._experimental.mcp_server.mcp_server_manager import (
|
||||||
|
global_mcp_server_manager,
|
||||||
|
)
|
||||||
|
|
||||||
|
srv = global_mcp_server_manager.registry["mock-oauth-srv"]
|
||||||
|
prev_validation = getattr(srv, "token_validation", None)
|
||||||
|
srv.token_validation = {"org_id": "expected-org"}
|
||||||
|
try:
|
||||||
|
respx_mock.post("https://mock-idp.example/oauth/token").respond(
|
||||||
|
200,
|
||||||
|
json={
|
||||||
|
"access_token": "tok",
|
||||||
|
"token_type": "Bearer",
|
||||||
|
"expires_in": 60,
|
||||||
|
"org_id": "wrong-org",
|
||||||
|
},
|
||||||
|
)
|
||||||
|
transport = ASGITransport(app=oauth_asgi_app)
|
||||||
|
async with httpx.AsyncClient(
|
||||||
|
transport=transport, base_url="http://proxy.test", follow_redirects=False
|
||||||
|
) as client:
|
||||||
|
r = await client.post(
|
||||||
|
"/mock_oauth/token",
|
||||||
|
data={
|
||||||
|
"grant_type": "authorization_code",
|
||||||
|
"code": "c",
|
||||||
|
"client_id": "upstream-client",
|
||||||
|
"client_secret": "upstream-secret",
|
||||||
|
"code_verifier": "v",
|
||||||
|
},
|
||||||
|
)
|
||||||
|
assert r.status_code == 403
|
||||||
|
err = r.json()
|
||||||
|
assert err["detail"]["error"] == "token_validation_failed"
|
||||||
|
finally:
|
||||||
|
srv.token_validation = prev_validation
|
||||||
|
|
@ -0,0 +1,119 @@
|
||||||
|
"""Unit tests for MCP OAuth broker security helpers (discoverable / UI flow).
|
||||||
|
|
||||||
|
``_validate_token_response`` rules are covered in ``tests/mcp_tests/test_per_user_oauth_cache.py``.
|
||||||
|
"""
|
||||||
|
|
||||||
|
from __future__ import annotations
|
||||||
|
|
||||||
|
import pytest
|
||||||
|
from fastapi import HTTPException
|
||||||
|
|
||||||
|
from litellm.proxy._experimental.mcp_server.discoverable_endpoints import (
|
||||||
|
_get_validated_client_redirect_uri,
|
||||||
|
decode_state_hash,
|
||||||
|
encode_state_with_base_url,
|
||||||
|
)
|
||||||
|
from litellm.proxy._experimental.mcp_server.oauth_utils import (
|
||||||
|
validate_loopback_redirect_uri,
|
||||||
|
)
|
||||||
|
|
||||||
|
|
||||||
|
@pytest.mark.parametrize(
|
||||||
|
"uri",
|
||||||
|
[
|
||||||
|
"https://evil.com/callback",
|
||||||
|
"http://192.168.1.1/callback",
|
||||||
|
"http://10.0.0.1/callback",
|
||||||
|
"http://example.com/callback",
|
||||||
|
],
|
||||||
|
)
|
||||||
|
def test_validate_loopback_redirect_uri_rejects_non_loopback(uri: str) -> None:
|
||||||
|
with pytest.raises(HTTPException) as exc:
|
||||||
|
validate_loopback_redirect_uri(uri)
|
||||||
|
assert exc.value.status_code == 400
|
||||||
|
|
||||||
|
|
||||||
|
@pytest.mark.parametrize(
|
||||||
|
"uri",
|
||||||
|
[
|
||||||
|
"http://127.0.0.1:9/callback",
|
||||||
|
"http://127.0.0.2:4000/ui/mcp/oauth/callback",
|
||||||
|
"http://localhost:3000/cb",
|
||||||
|
"http://[::1]:8080/oauth/callback",
|
||||||
|
],
|
||||||
|
)
|
||||||
|
def test_validate_loopback_redirect_uri_accepts_loopback(uri: str) -> None:
|
||||||
|
validate_loopback_redirect_uri(uri)
|
||||||
|
|
||||||
|
|
||||||
|
def test_encode_state_with_base_url_decode_state_hash_roundtrip(monkeypatch) -> None:
|
||||||
|
"""State must survive encrypt → decrypt with a stable salt (CI-safe)."""
|
||||||
|
monkeypatch.setenv("LITELLM_SALT_KEY", "unit-test-salt-key-32chars!!!")
|
||||||
|
|
||||||
|
enc = encode_state_with_base_url(
|
||||||
|
base_url="http://127.0.0.1:60108/callback",
|
||||||
|
original_state="client-state-xyz",
|
||||||
|
code_challenge="cc",
|
||||||
|
code_challenge_method="S256",
|
||||||
|
client_redirect_uri="http://127.0.0.1:60108/callback",
|
||||||
|
)
|
||||||
|
assert enc != ""
|
||||||
|
data = decode_state_hash(enc)
|
||||||
|
assert data["base_url"] == "http://127.0.0.1:60108/callback"
|
||||||
|
assert data["original_state"] == "client-state-xyz"
|
||||||
|
assert data["code_challenge"] == "cc"
|
||||||
|
assert data["code_challenge_method"] == "S256"
|
||||||
|
assert data["client_redirect_uri"] == "http://127.0.0.1:60108/callback"
|
||||||
|
|
||||||
|
|
||||||
|
def test_get_validated_client_redirect_uri_accepts_loopback_from_state() -> None:
|
||||||
|
uri = _get_validated_client_redirect_uri(
|
||||||
|
{
|
||||||
|
"client_redirect_uri": "http://127.0.0.1:55/x",
|
||||||
|
"base_url": "ignored-when-client-set",
|
||||||
|
}
|
||||||
|
)
|
||||||
|
assert uri == "http://127.0.0.1:55/x"
|
||||||
|
|
||||||
|
|
||||||
|
def test_get_validated_client_redirect_uri_falls_back_to_base_url_loopback() -> None:
|
||||||
|
uri = _get_validated_client_redirect_uri(
|
||||||
|
{
|
||||||
|
"original_state": "s",
|
||||||
|
"base_url": "http://localhost:9/oauth",
|
||||||
|
}
|
||||||
|
)
|
||||||
|
assert uri == "http://localhost:9/oauth"
|
||||||
|
|
||||||
|
|
||||||
|
def test_get_validated_client_redirect_uri_rejects_public_client_redirect() -> None:
|
||||||
|
with pytest.raises(HTTPException) as exc:
|
||||||
|
_get_validated_client_redirect_uri(
|
||||||
|
{
|
||||||
|
"client_redirect_uri": "https://evil.com/steal",
|
||||||
|
"base_url": "http://127.0.0.1:1/x",
|
||||||
|
}
|
||||||
|
)
|
||||||
|
assert exc.value.status_code == 400
|
||||||
|
|
||||||
|
|
||||||
|
def test_get_validated_client_redirect_uri_rejects_public_base_url_fallback() -> None:
|
||||||
|
with pytest.raises(HTTPException) as exc:
|
||||||
|
_get_validated_client_redirect_uri({"base_url": "https://evil.com/noloop"})
|
||||||
|
assert exc.value.status_code == 400
|
||||||
|
|
||||||
|
|
||||||
|
def test_get_validated_client_redirect_uri_empty_client_uses_loopback_base_url() -> (
|
||||||
|
None
|
||||||
|
):
|
||||||
|
"""When client_redirect_uri is absent/empty, base_url must still be loopback-validated."""
|
||||||
|
uri = _get_validated_client_redirect_uri(
|
||||||
|
{"client_redirect_uri": "", "base_url": "http://127.0.0.1:1/x"}
|
||||||
|
)
|
||||||
|
assert uri == "http://127.0.0.1:1/x"
|
||||||
|
|
||||||
|
|
||||||
|
def test_get_validated_client_redirect_uri_rejects_missing_uri() -> None:
|
||||||
|
with pytest.raises(HTTPException) as exc:
|
||||||
|
_get_validated_client_redirect_uri({"original_state": "x"})
|
||||||
|
assert exc.value.status_code == 400
|
||||||
|
|
@ -1560,10 +1560,6 @@ class TestTemporaryMCPSessionEndpoints:
|
||||||
request = MagicMock()
|
request = MagicMock()
|
||||||
server = generate_mock_mcp_server_config_record(server_id="server-1")
|
server = generate_mock_mcp_server_config_record(server_id="server-1")
|
||||||
authorize_response = MagicMock()
|
authorize_response = MagicMock()
|
||||||
admin_auth = generate_mock_user_api_key_auth(
|
|
||||||
user_role=LitellmUserRoles.PROXY_ADMIN,
|
|
||||||
)
|
|
||||||
|
|
||||||
with (
|
with (
|
||||||
patch(
|
patch(
|
||||||
"litellm.proxy.management_endpoints.mcp_management_endpoints._get_cached_temporary_mcp_server_or_404",
|
"litellm.proxy.management_endpoints.mcp_management_endpoints._get_cached_temporary_mcp_server_or_404",
|
||||||
|
|
@ -1577,7 +1573,6 @@ class TestTemporaryMCPSessionEndpoints:
|
||||||
result = await mcp_authorize(
|
result = await mcp_authorize(
|
||||||
request=request,
|
request=request,
|
||||||
server_id="server-1",
|
server_id="server-1",
|
||||||
user_api_key_dict=admin_auth,
|
|
||||||
client_id="client-id",
|
client_id="client-id",
|
||||||
redirect_uri="https://example.com/callback",
|
redirect_uri="https://example.com/callback",
|
||||||
state="state123",
|
state="state123",
|
||||||
|
|
@ -1588,7 +1583,7 @@ class TestTemporaryMCPSessionEndpoints:
|
||||||
)
|
)
|
||||||
|
|
||||||
assert result is authorize_response
|
assert result is authorize_response
|
||||||
get_server.assert_awaited_once_with("server-1", admin_auth, request=request)
|
get_server.assert_awaited_once_with("server-1", request=request)
|
||||||
authorize_mock.assert_awaited_once_with(
|
authorize_mock.assert_awaited_once_with(
|
||||||
request=request,
|
request=request,
|
||||||
mcp_server=server,
|
mcp_server=server,
|
||||||
|
|
@ -1610,9 +1605,6 @@ class TestTemporaryMCPSessionEndpoints:
|
||||||
request = MagicMock()
|
request = MagicMock()
|
||||||
server = generate_mock_mcp_server_config_record(server_id="server-1")
|
server = generate_mock_mcp_server_config_record(server_id="server-1")
|
||||||
exchange_response = {"access_token": "token"}
|
exchange_response = {"access_token": "token"}
|
||||||
admin_auth = generate_mock_user_api_key_auth(
|
|
||||||
user_role=LitellmUserRoles.PROXY_ADMIN,
|
|
||||||
)
|
|
||||||
|
|
||||||
with (
|
with (
|
||||||
patch(
|
patch(
|
||||||
|
|
@ -1627,7 +1619,6 @@ class TestTemporaryMCPSessionEndpoints:
|
||||||
result = await mcp_token(
|
result = await mcp_token(
|
||||||
request=request,
|
request=request,
|
||||||
server_id="server-1",
|
server_id="server-1",
|
||||||
user_api_key_dict=admin_auth,
|
|
||||||
grant_type="authorization_code",
|
grant_type="authorization_code",
|
||||||
code="code-123",
|
code="code-123",
|
||||||
redirect_uri="https://example.com/callback",
|
redirect_uri="https://example.com/callback",
|
||||||
|
|
@ -1639,7 +1630,7 @@ class TestTemporaryMCPSessionEndpoints:
|
||||||
)
|
)
|
||||||
|
|
||||||
assert result is exchange_response
|
assert result is exchange_response
|
||||||
get_server.assert_awaited_once_with("server-1", admin_auth, request=request)
|
get_server.assert_awaited_once_with("server-1", request=request)
|
||||||
exchange_mock.assert_awaited_once_with(
|
exchange_mock.assert_awaited_once_with(
|
||||||
request=request,
|
request=request,
|
||||||
mcp_server=server,
|
mcp_server=server,
|
||||||
|
|
@ -1662,9 +1653,6 @@ class TestTemporaryMCPSessionEndpoints:
|
||||||
request = MagicMock()
|
request = MagicMock()
|
||||||
server = generate_mock_mcp_server_config_record(server_id="server-1")
|
server = generate_mock_mcp_server_config_record(server_id="server-1")
|
||||||
exchange_response = {"access_token": "new-token", "refresh_token": "new-rt"}
|
exchange_response = {"access_token": "new-token", "refresh_token": "new-rt"}
|
||||||
admin_auth = generate_mock_user_api_key_auth(
|
|
||||||
user_role=LitellmUserRoles.PROXY_ADMIN,
|
|
||||||
)
|
|
||||||
|
|
||||||
with (
|
with (
|
||||||
patch(
|
patch(
|
||||||
|
|
@ -1679,7 +1667,6 @@ class TestTemporaryMCPSessionEndpoints:
|
||||||
result = await mcp_token(
|
result = await mcp_token(
|
||||||
request=request,
|
request=request,
|
||||||
server_id="server-1",
|
server_id="server-1",
|
||||||
user_api_key_dict=admin_auth,
|
|
||||||
grant_type="refresh_token",
|
grant_type="refresh_token",
|
||||||
code=None,
|
code=None,
|
||||||
redirect_uri=None,
|
redirect_uri=None,
|
||||||
|
|
@ -1691,7 +1678,7 @@ class TestTemporaryMCPSessionEndpoints:
|
||||||
)
|
)
|
||||||
|
|
||||||
assert result is exchange_response
|
assert result is exchange_response
|
||||||
get_server.assert_awaited_once_with("server-1", admin_auth, request=request)
|
get_server.assert_awaited_once_with("server-1", request=request)
|
||||||
exchange_mock.assert_awaited_once_with(
|
exchange_mock.assert_awaited_once_with(
|
||||||
request=request,
|
request=request,
|
||||||
mcp_server=server,
|
mcp_server=server,
|
||||||
|
|
|
||||||
31
ui/litellm-dashboard/e2e_tests/playwright.oauth.config.ts
Normal file
31
ui/litellm-dashboard/e2e_tests/playwright.oauth.config.ts
Normal file
|
|
@ -0,0 +1,31 @@
|
||||||
|
/**
|
||||||
|
* Minimal Playwright config for the MCP OAuth E2E test.
|
||||||
|
* Runs against a pre-existing proxy on port 4000 with no globalSetup.
|
||||||
|
*/
|
||||||
|
import { defineConfig, devices } from "@playwright/test";
|
||||||
|
|
||||||
|
export default defineConfig({
|
||||||
|
testDir: ".",
|
||||||
|
testMatch: ["tests/mcp/mcp_oauth_flow.spec.ts"],
|
||||||
|
fullyParallel: false,
|
||||||
|
retries: 0,
|
||||||
|
workers: 1,
|
||||||
|
reporter: [["list"], ["html", { outputFolder: "playwright-report-oauth" }]],
|
||||||
|
use: {
|
||||||
|
baseURL: "http://localhost:4000",
|
||||||
|
trace: "on-first-retry",
|
||||||
|
actionTimeout: 20 * 1000,
|
||||||
|
navigationTimeout: 45 * 1000,
|
||||||
|
},
|
||||||
|
projects: [
|
||||||
|
{
|
||||||
|
name: "chromium",
|
||||||
|
use: { ...devices["Desktop Chrome"] },
|
||||||
|
},
|
||||||
|
],
|
||||||
|
timeout: 5 * 60 * 1000,
|
||||||
|
expect: {
|
||||||
|
timeout: 15 * 1000,
|
||||||
|
},
|
||||||
|
// No globalSetup — the test handles its own login
|
||||||
|
});
|
||||||
160
ui/litellm-dashboard/e2e_tests/tests/mcp/mcp_oauth_flow.spec.ts
Normal file
160
ui/litellm-dashboard/e2e_tests/tests/mcp/mcp_oauth_flow.spec.ts
Normal file
|
|
@ -0,0 +1,160 @@
|
||||||
|
/**
|
||||||
|
* E2E test: MCP OAuth flow (Interactive / PKCE)
|
||||||
|
*
|
||||||
|
* Two-layer regression coverage:
|
||||||
|
*
|
||||||
|
* Layer 1 — API assertion (catches the original auth bug directly)
|
||||||
|
* ---------------------------------------------------------------
|
||||||
|
* Makes a real GET to /v1/mcp/server/oauth/{id}/authorize with NO
|
||||||
|
* Authorization header. If someone re-adds `user_api_key_auth` to
|
||||||
|
* that route, the proxy returns 401 and this assertion fails immediately.
|
||||||
|
* With auth absent the proxy returns 404 (unknown server) or 307
|
||||||
|
* (valid server), never 401.
|
||||||
|
*
|
||||||
|
* Layer 2 — Full UI flow (catches UI / OAuth wiring regressions)
|
||||||
|
* ---------------------------------------------------------------
|
||||||
|
* Logs in, fills the "Add MCP Server" form with OAuth settings, clicks
|
||||||
|
* "Authorize & Fetch Token", and asserts "Token fetched." appears.
|
||||||
|
*
|
||||||
|
* Intercept strategy for Layer 2:
|
||||||
|
* A. /v1/mcp/server/oauth/{*}/authorize* — return an HTML page that
|
||||||
|
* writes the fake OAuth result to sessionStorage (same encoding as
|
||||||
|
* setSecureItem) then navigates back, so the real resumeOAuthFlow()
|
||||||
|
* hook picks it up.
|
||||||
|
* B. POST /v1/mcp/server/oauth/{*}/token — return a mock token.
|
||||||
|
*/
|
||||||
|
import { test, expect } from "@playwright/test";
|
||||||
|
|
||||||
|
const BASE_URL = "http://localhost:4000";
|
||||||
|
const MOCK_OAUTH_SERVER = "http://localhost:8080";
|
||||||
|
const FAKE_CODE = "e2e-fake-auth-code";
|
||||||
|
|
||||||
|
test.describe("MCP OAuth - Authorize & Fetch Token", () => {
|
||||||
|
// =========================================================================
|
||||||
|
// Layer 1: direct API check — catches the original "401 auth added" bug
|
||||||
|
// =========================================================================
|
||||||
|
test("authorize endpoint must be accessible without an API key", async ({ request }) => {
|
||||||
|
// Use a nonexistent server ID. Without auth the proxy returns 404 (server
|
||||||
|
// not found). With auth re-added it returns 401 before touching the DB.
|
||||||
|
const resp = await request.get(
|
||||||
|
BASE_URL + "/v1/mcp/server/oauth/regression-check/authorize" +
|
||||||
|
"?redirect_uri=http%3A%2F%2Flocalhost%3A4000%2Fui%2Fmcp%2Foauth%2Fcallback" +
|
||||||
|
"&state=regression-test" +
|
||||||
|
"&response_type=code" +
|
||||||
|
"&code_challenge=abc123" +
|
||||||
|
"&code_challenge_method=S256" +
|
||||||
|
"&client_id=regression-check",
|
||||||
|
{ failOnStatusCode: false }
|
||||||
|
);
|
||||||
|
// 401 = auth gate was added. Any other status means no auth gate.
|
||||||
|
expect(
|
||||||
|
resp.status(),
|
||||||
|
"authorize endpoint returned 401 — user_api_key_auth was added back"
|
||||||
|
).not.toBe(401);
|
||||||
|
});
|
||||||
|
|
||||||
|
// =========================================================================
|
||||||
|
// Layer 2: full UI flow — catches UI / OAuth wiring regressions
|
||||||
|
// =========================================================================
|
||||||
|
test("creates an OAuth MCP server and completes the authorize flow", async ({ page }) => {
|
||||||
|
const serverName = "e2e_mcp_oauth_" + Date.now();
|
||||||
|
|
||||||
|
// ---- login as admin --------------------------------------------------
|
||||||
|
await page.goto(BASE_URL + "/ui/login");
|
||||||
|
await page.getByPlaceholder("Enter your username").fill("admin");
|
||||||
|
await page.getByPlaceholder("Enter your password").fill(
|
||||||
|
process.env.LITELLM_MASTER_KEY || "sk-1234"
|
||||||
|
);
|
||||||
|
await page.getByRole("button", { name: "Login", exact: true }).click();
|
||||||
|
await page.waitForURL(
|
||||||
|
function(url) { return url.pathname.startsWith("/ui") && !url.pathname.includes("/login"); },
|
||||||
|
{ timeout: 30000 }
|
||||||
|
);
|
||||||
|
const dismiss = page.getByText("Don't ask me again");
|
||||||
|
if (await dismiss.isVisible({ timeout: 2000 }).catch(function() { return false; })) {
|
||||||
|
await dismiss.click();
|
||||||
|
}
|
||||||
|
|
||||||
|
// ---- intercept A: proxy's authorize endpoint -------------------------
|
||||||
|
// We respond with HTML that writes the fake OAuth result to sessionStorage
|
||||||
|
// (using the same encode() logic as setSecureItem) then navigates back to
|
||||||
|
// the MCP servers page — so resumeOAuthFlow() picks it up naturally.
|
||||||
|
//
|
||||||
|
// NOTE: this intercept runs BEFORE the proxy processes the request.
|
||||||
|
// Layer 1 (above) covers the auth-on-authorize regression separately.
|
||||||
|
await page.route("**/v1/mcp/server/oauth/*/authorize*", async function(route) {
|
||||||
|
const url = new URL(route.request().url());
|
||||||
|
const clientState = url.searchParams.get("state") || "";
|
||||||
|
|
||||||
|
const encodedPayload = Buffer.from(
|
||||||
|
encodeURIComponent(
|
||||||
|
JSON.stringify({ type: "litellm-mcp-oauth", code: FAKE_CODE, state: clientState })
|
||||||
|
).replace(/%([0-9A-F]{2})/g, function(_, p1) {
|
||||||
|
return String.fromCharCode(parseInt(p1, 16));
|
||||||
|
})
|
||||||
|
).toString("base64");
|
||||||
|
|
||||||
|
const html = "<!DOCTYPE html><html><script>" +
|
||||||
|
"window.sessionStorage.setItem('litellm-mcp-oauth-result', '" + encodedPayload + "');" +
|
||||||
|
"window.location.replace('" + BASE_URL + "/ui?page=mcp-servers');" +
|
||||||
|
"</script></html>";
|
||||||
|
|
||||||
|
await route.fulfill({ status: 200, contentType: "text/html", body: html });
|
||||||
|
});
|
||||||
|
|
||||||
|
// ---- intercept B: token exchange -------------------------------------
|
||||||
|
await page.route("**/v1/mcp/server/oauth/*/token", async function(route) {
|
||||||
|
if (route.request().method() !== "POST") {
|
||||||
|
await route.continue();
|
||||||
|
return;
|
||||||
|
}
|
||||||
|
await route.fulfill({
|
||||||
|
status: 200,
|
||||||
|
contentType: "application/json",
|
||||||
|
headers: { "Cache-Control": "no-store", "Pragma": "no-cache" },
|
||||||
|
body: JSON.stringify({
|
||||||
|
access_token: "mock-e2e-access-token",
|
||||||
|
token_type: "Bearer",
|
||||||
|
expires_in: 3600,
|
||||||
|
}),
|
||||||
|
});
|
||||||
|
});
|
||||||
|
|
||||||
|
// ---- navigate to MCP servers page ------------------------------------
|
||||||
|
await page.goto(BASE_URL + "/ui?page=mcp-servers");
|
||||||
|
await expect(page.getByText("MCP Servers").first()).toBeVisible({ timeout: 20000 });
|
||||||
|
const dismiss2 = page.getByText("Don't ask me again");
|
||||||
|
if (await dismiss2.isVisible({ timeout: 2000 }).catch(function() { return false; })) {
|
||||||
|
await dismiss2.click();
|
||||||
|
}
|
||||||
|
|
||||||
|
// ---- open discovery -> custom server -> create modal -----------------
|
||||||
|
await page.getByRole("button", { name: "+ Add New MCP Server" }).click();
|
||||||
|
await expect(page.getByText("+ Custom Server").first()).toBeVisible({ timeout: 10000 });
|
||||||
|
await page.getByText("+ Custom Server").first().click();
|
||||||
|
await expect(page.getByRole("heading", { name: "Add New MCP Server" })).toBeVisible({ timeout: 15000 });
|
||||||
|
|
||||||
|
// ---- fill the form ---------------------------------------------------
|
||||||
|
await page.getByPlaceholder("e.g., GitHub_MCP, Zapier_MCP, etc.").first().fill(serverName);
|
||||||
|
await page.locator(".ant-select", { hasText: "Select transport" }).click();
|
||||||
|
await page.locator(".ant-select-dropdown:visible").getByText("Streamable HTTP (Recommended)").click();
|
||||||
|
await expect(page.getByPlaceholder("https://your-mcp-server.com")).toBeVisible({ timeout: 5000 });
|
||||||
|
await page.getByPlaceholder("https://your-mcp-server.com").fill(MOCK_OAUTH_SERVER + "/mcp");
|
||||||
|
await page.locator(".ant-select", { hasText: "Select auth type" }).click();
|
||||||
|
await page.locator(".ant-select-dropdown:visible").getByText("OAuth").click();
|
||||||
|
await expect(page.locator(".ant-select", { hasText: "Interactive (PKCE)" })).toBeVisible({ timeout: 5000 });
|
||||||
|
await page.getByPlaceholder("https://example.com/oauth/authorize").fill(MOCK_OAUTH_SERVER + "/authorize");
|
||||||
|
await page.getByPlaceholder("https://example.com/oauth/token").fill(MOCK_OAUTH_SERVER + "/token");
|
||||||
|
|
||||||
|
// ---- click Authorize & Fetch Token -----------------------------------
|
||||||
|
const authorizeBtn = page.getByRole("button", { name: "Authorize & Fetch Token" });
|
||||||
|
await expect(authorizeBtn).toBeVisible({ timeout: 5000 });
|
||||||
|
await authorizeBtn.click();
|
||||||
|
|
||||||
|
// ---- wait for the full flow to complete ------------------------------
|
||||||
|
// Chain: /authorize [A: intercepted] -> HTML writes sessionStorage + navigates
|
||||||
|
// -> back to mcp-servers -> resumeOAuthFlow fires -> POST /token [B: intercepted]
|
||||||
|
// -> "Token fetched."
|
||||||
|
await expect(page.getByText(/Token fetched/)).toBeVisible({ timeout: 60000 });
|
||||||
|
});
|
||||||
|
});
|
||||||
Loading…
Add table
Reference in a new issue