From fe309ea0b0633929cfd62f8e6ca32b56529d38ce Mon Sep 17 00:00:00 2001 From: Sameer Kankute Date: Mon, 4 May 2026 12:51:23 +0530 Subject: [PATCH] fix(mcp): remove auth gate from OAuth broker authorize and token endpoints MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 --- .github/workflows/test-mcp.yml | 7 + .../mcp_management_endpoints.py | 17 +- .../test_mcp_oauth_flow_http_respx.py | 277 ++++++++++++++++++ .../test_mcp_oauth_security_unit.py | 119 ++++++++ .../test_mcp_management_endpoints.py | 19 +- .../e2e_tests/playwright.oauth.config.ts | 31 ++ .../tests/mcp/mcp_oauth_flow.spec.ts | 160 ++++++++++ 7 files changed, 605 insertions(+), 25 deletions(-) create mode 100644 tests/test_litellm/proxy/_experimental/mcp_server/test_mcp_oauth_flow_http_respx.py create mode 100644 tests/test_litellm/proxy/_experimental/mcp_server/test_mcp_oauth_security_unit.py create mode 100644 ui/litellm-dashboard/e2e_tests/playwright.oauth.config.ts create mode 100644 ui/litellm-dashboard/e2e_tests/tests/mcp/mcp_oauth_flow.spec.ts diff --git a/.github/workflows/test-mcp.yml b/.github/workflows/test-mcp.yml index 313043e12fe..f47c8e998d4 100644 --- a/.github/workflows/test-mcp.yml +++ b/.github/workflows/test-mcp.yml @@ -44,3 +44,10 @@ jobs: - name: Run MCP tests run: | 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 diff --git a/litellm/proxy/management_endpoints/mcp_management_endpoints.py b/litellm/proxy/management_endpoints/mcp_management_endpoints.py index fca08e591fd..9da14cc968c 100644 --- a/litellm/proxy/management_endpoints/mcp_management_endpoints.py +++ b/litellm/proxy/management_endpoints/mcp_management_endpoints.py @@ -1449,7 +1449,7 @@ if MCP_AVAILABLE: async def _get_cached_temporary_mcp_server_or_404( server_id: str, - user_api_key_dict: UserAPIKeyAuth, + user_api_key_dict: Optional[UserAPIKeyAuth] = None, request: Optional[Request] = None, ) -> MCPServer: 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 # allowed-servers set. Temporary cached servers come from the # admin-only `/server/oauth/session` setup flow and are not exposed - # to non-admins. - if not _user_has_admin_view(user_api_key_dict): + # to non-admins. Unauthenticated OAuth browser flows omit the key and + # 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: raise HTTPException( status_code=status.HTTP_403_FORBIDDEN, @@ -1497,12 +1500,10 @@ if MCP_AVAILABLE: @router.get( "/server/oauth/{server_id}/authorize", include_in_schema=False, - dependencies=[Depends(user_api_key_auth)], ) async def mcp_authorize( request: Request, server_id: str, - user_api_key_dict: UserAPIKeyAuth = Depends(user_api_key_auth), client_id: Optional[str] = None, redirect_uri: str = Query(...), state: str = "", @@ -1512,7 +1513,7 @@ if MCP_AVAILABLE: scope: Optional[str] = None, ): 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 resolved_client_id = mcp_server.client_id or client_id or "" @@ -1542,12 +1543,10 @@ if MCP_AVAILABLE: @router.post( "/server/oauth/{server_id}/token", include_in_schema=False, - dependencies=[Depends(user_api_key_auth)], ) async def mcp_token( request: Request, server_id: str, - user_api_key_dict: UserAPIKeyAuth = Depends(user_api_key_auth), grant_type: str = Form(...), code: Optional[str] = Form(None), redirect_uri: Optional[str] = Form(None), @@ -1558,7 +1557,7 @@ if MCP_AVAILABLE: scope: Optional[str] = Form(None), ): 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 "" if not resolved_client_id: diff --git a/tests/test_litellm/proxy/_experimental/mcp_server/test_mcp_oauth_flow_http_respx.py b/tests/test_litellm/proxy/_experimental/mcp_server/test_mcp_oauth_flow_http_respx.py new file mode 100644 index 00000000000..e92f221b362 --- /dev/null +++ b/tests/test_litellm/proxy/_experimental/mcp_server/test_mcp_oauth_flow_http_respx.py @@ -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 diff --git a/tests/test_litellm/proxy/_experimental/mcp_server/test_mcp_oauth_security_unit.py b/tests/test_litellm/proxy/_experimental/mcp_server/test_mcp_oauth_security_unit.py new file mode 100644 index 00000000000..455cdaaa126 --- /dev/null +++ b/tests/test_litellm/proxy/_experimental/mcp_server/test_mcp_oauth_security_unit.py @@ -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 diff --git a/tests/test_litellm/proxy/management_endpoints/test_mcp_management_endpoints.py b/tests/test_litellm/proxy/management_endpoints/test_mcp_management_endpoints.py index 821e2002906..e3437cfd8f5 100644 --- a/tests/test_litellm/proxy/management_endpoints/test_mcp_management_endpoints.py +++ b/tests/test_litellm/proxy/management_endpoints/test_mcp_management_endpoints.py @@ -1560,10 +1560,6 @@ class TestTemporaryMCPSessionEndpoints: request = MagicMock() server = generate_mock_mcp_server_config_record(server_id="server-1") authorize_response = MagicMock() - admin_auth = generate_mock_user_api_key_auth( - user_role=LitellmUserRoles.PROXY_ADMIN, - ) - with ( patch( "litellm.proxy.management_endpoints.mcp_management_endpoints._get_cached_temporary_mcp_server_or_404", @@ -1577,7 +1573,6 @@ class TestTemporaryMCPSessionEndpoints: result = await mcp_authorize( request=request, server_id="server-1", - user_api_key_dict=admin_auth, client_id="client-id", redirect_uri="https://example.com/callback", state="state123", @@ -1588,7 +1583,7 @@ class TestTemporaryMCPSessionEndpoints: ) 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( request=request, mcp_server=server, @@ -1610,9 +1605,6 @@ class TestTemporaryMCPSessionEndpoints: request = MagicMock() server = generate_mock_mcp_server_config_record(server_id="server-1") exchange_response = {"access_token": "token"} - admin_auth = generate_mock_user_api_key_auth( - user_role=LitellmUserRoles.PROXY_ADMIN, - ) with ( patch( @@ -1627,7 +1619,6 @@ class TestTemporaryMCPSessionEndpoints: result = await mcp_token( request=request, server_id="server-1", - user_api_key_dict=admin_auth, grant_type="authorization_code", code="code-123", redirect_uri="https://example.com/callback", @@ -1639,7 +1630,7 @@ class TestTemporaryMCPSessionEndpoints: ) 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( request=request, mcp_server=server, @@ -1662,9 +1653,6 @@ class TestTemporaryMCPSessionEndpoints: request = MagicMock() server = generate_mock_mcp_server_config_record(server_id="server-1") exchange_response = {"access_token": "new-token", "refresh_token": "new-rt"} - admin_auth = generate_mock_user_api_key_auth( - user_role=LitellmUserRoles.PROXY_ADMIN, - ) with ( patch( @@ -1679,7 +1667,6 @@ class TestTemporaryMCPSessionEndpoints: result = await mcp_token( request=request, server_id="server-1", - user_api_key_dict=admin_auth, grant_type="refresh_token", code=None, redirect_uri=None, @@ -1691,7 +1678,7 @@ class TestTemporaryMCPSessionEndpoints: ) 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( request=request, mcp_server=server, diff --git a/ui/litellm-dashboard/e2e_tests/playwright.oauth.config.ts b/ui/litellm-dashboard/e2e_tests/playwright.oauth.config.ts new file mode 100644 index 00000000000..f6ab86a73d2 --- /dev/null +++ b/ui/litellm-dashboard/e2e_tests/playwright.oauth.config.ts @@ -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 +}); diff --git a/ui/litellm-dashboard/e2e_tests/tests/mcp/mcp_oauth_flow.spec.ts b/ui/litellm-dashboard/e2e_tests/tests/mcp/mcp_oauth_flow.spec.ts new file mode 100644 index 00000000000..84de4e1dc0e --- /dev/null +++ b/ui/litellm-dashboard/e2e_tests/tests/mcp/mcp_oauth_flow.spec.ts @@ -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 = ""; + + 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 }); + }); +});