fix: MCP OAuth callback routing and URL handling (#17789)

* fix: MCP OAuth callback routing and URL handling

* test: add test for proxy_server
This commit is contained in:
YutaSaito 2025-12-12 01:22:59 +09:00 committed by GitHub
parent 6a3e6465ea
commit e9571ddbc4
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
4 changed files with 88 additions and 21 deletions

View file

@ -1022,21 +1022,33 @@ try:
app.mount("/ui", StaticFiles(directory=ui_path, html=True), name="ui")
def _restructure_ui_html_files(ui_root: str) -> None:
"""Ensure each exported HTML route is available as <route>/index.html."""
for current_root, _, files in os.walk(ui_root):
rel_root = os.path.relpath(current_root, ui_root)
first_segment = "" if rel_root == "." else rel_root.split(os.sep)[0]
# Ignore Next.js asset directories
if first_segment in {"_next", "litellm-asset-prefix"}:
continue
for filename in files:
if not filename.endswith(".html") or filename == "index.html":
continue
file_path = os.path.join(current_root, filename)
target_dir = os.path.splitext(file_path)[0]
target_path = os.path.join(target_dir, "index.html")
os.makedirs(target_dir, exist_ok=True)
os.replace(file_path, target_path)
# Handle HTML file restructuring
# Skip this for non-root Docker since it's done at build time
# Support both "true" and "True" for case-insensitive comparison
if os.getenv("LITELLM_NON_ROOT", "").lower() != "true":
for filename in os.listdir(ui_path):
if filename.endswith(".html") and filename != "index.html":
# Create a folder with the same name as the HTML file
folder_name = os.path.splitext(filename)[0]
folder_path = os.path.join(ui_path, folder_name)
os.makedirs(folder_path, exist_ok=True)
# Move the HTML file into the folder and rename it to 'index.html'
src = os.path.join(ui_path, filename)
dst = os.path.join(folder_path, "index.html")
os.rename(src, dst)
_restructure_ui_html_files(ui_path)
else:
verbose_proxy_logger.info(
"Skipping runtime HTML restructuring for non-root Docker (already done at build time)"

View file

@ -5,6 +5,7 @@ import os
import socket
import subprocess
import sys
from pathlib import Path
from datetime import datetime
from unittest import mock
from unittest.mock import AsyncMock, MagicMock, mock_open, patch
@ -162,6 +163,39 @@ def test_sso_key_generate_shows_deprecation_banner(client_no_auth, monkeypatch):
assert "Deprecated:" in html
def test_restructure_ui_html_files_handles_nested_routes(tmp_path):
from litellm.proxy import proxy_server
ui_root = tmp_path / "ui"
ui_root.mkdir()
def write_file(path: Path, content: str) -> None:
path.parent.mkdir(parents=True, exist_ok=True)
path.write_text(content)
write_file(ui_root / "home.html", "home")
write_file(ui_root / "mcp" / "oauth" / "callback.html", "callback")
write_file(ui_root / "existing" / "index.html", "keep")
write_file(ui_root / "_next" / "ignore.html", "asset")
write_file(ui_root / "litellm-asset-prefix" / "ignore.html", "asset")
proxy_server._restructure_ui_html_files(str(ui_root))
assert not (ui_root / "home.html").exists()
assert (ui_root / "home" / "index.html").read_text() == "home"
assert not (ui_root / "mcp" / "oauth" / "callback.html").exists()
assert (
(ui_root / "mcp" / "oauth" / "callback" / "index.html").read_text()
== "callback"
)
assert (ui_root / "existing" / "index.html").read_text() == "keep"
assert (ui_root / "_next" / "ignore.html").read_text() == "asset"
assert (
(ui_root / "litellm-asset-prefix" / "ignore.html").read_text()
== "asset"
)
@pytest.mark.asyncio
async def test_initialize_scheduled_jobs_credentials(monkeypatch):
"""
@ -2791,4 +2825,3 @@ def test_get_image_root_case_uses_current_dir(monkeypatch):
# Verify FileResponse was called
assert mock_file_response.called, "FileResponse should be called"

View file

@ -6,6 +6,21 @@ import { useSearchParams } from "next/navigation";
const RESULT_STORAGE_KEY = "litellm-mcp-oauth-result";
const RETURN_URL_STORAGE_KEY = "litellm-mcp-oauth-return-url";
const resolveDefaultRedirect = () => {
if (typeof window === "undefined") {
return "/ui";
}
const path = window.location.pathname || "";
const uiIndex = path.indexOf("/ui");
if (uiIndex >= 0) {
const prefix = path.slice(0, uiIndex + 3);
return prefix.endsWith("/") ? prefix : `${prefix}`;
}
return "/";
};
const McpOAuthCallbackPage = () => {
const searchParams = useSearchParams();
@ -33,11 +48,8 @@ const McpOAuthCallbackPage = () => {
const returnUrl = window.sessionStorage.getItem(RETURN_URL_STORAGE_KEY);
console.info("[MCP OAuth callback] returnUrl", returnUrl);
if (returnUrl) {
window.location.replace(returnUrl);
} else {
window.location.replace("/");
}
const destination = returnUrl || resolveDefaultRedirect();
window.location.replace(destination);
}, [payload]);
return (

View file

@ -8,6 +8,7 @@ import {
exchangeMcpOAuthToken,
getProxyBaseUrl,
registerMcpOAuthClient,
serverRootPath,
} from "@/components/networking";
export type McpOAuthStatus = "idle" | "authorizing" | "exchanging" | "success" | "error";
@ -87,13 +88,22 @@ export const useMcpOAuthFlow = ({
}
};
const callbackUrl = () => {
if (typeof window === "undefined") {
return `${getProxyBaseUrl()}/v1/mcp/oauth/callback`;
const buildCallbackUrl = () => {
if (typeof window !== "undefined") {
const path = window.location.pathname || "";
const uiIndex = path.indexOf("/ui");
const uiPrefix = uiIndex >= 0 ? path.slice(0, uiIndex + 3) : "";
const normalizedPrefix = uiPrefix.replace(/\/+$/, "");
return `${window.location.origin}${normalizedPrefix}/mcp/oauth/callback`;
}
return `${window.location.origin}/mcp/oauth/callback`;
const base = (getProxyBaseUrl() || "").replace(/\/+$/, "");
const rootPrefix = serverRootPath && serverRootPath !== "/" ? serverRootPath : "";
return `${base}${rootPrefix}/ui/mcp/oauth/callback`;
};
const callbackUrl = () => buildCallbackUrl();
const startOAuthFlow = useCallback(async () => {
const credentials = getCredentials() || {};