diff --git a/litellm/ui/litellm-dashboard/src/app/(dashboard)/page.test.tsx b/litellm/ui/litellm-dashboard/src/app/(dashboard)/page.test.tsx index d1d9d1b43d0..42b278c2a20 100644 --- a/litellm/ui/litellm-dashboard/src/app/(dashboard)/page.test.tsx +++ b/litellm/ui/litellm-dashboard/src/app/(dashboard)/page.test.tsx @@ -43,18 +43,22 @@ vi.mock("@/app/(dashboard)/home/_components/HomePage", () => ({ vi.mock("@/components/common_components/LoadingScreen", () => ({ default: () =>
, })); -vi.mock("@/components/networking", () => ({ - getRuntimeBackendBaseUrl: () => null, - getUiPageUrl: (url: string) => url, - proxyBaseUrl: "", -})); +vi.mock("@/components/networking", async () => { + const { normalizeBackendPageUrlToUiOrigin } = + await vi.importActual("@/utils/returnUrlUtils"); + return { + getRuntimeBackendBaseUrl: () => "https://backend.example", + getUiPageUrl: (url: string) => + normalizeBackendPageUrlToUiOrigin(url, "https://backend.example", "http://localhost:3000"), + proxyBaseUrl: "", + }; +}); vi.mock("@/utils/uiHref", () => ({ uiHref: mockUiHref })); -vi.mock("@/utils/returnUrlUtils", () => ({ +vi.mock("@/utils/returnUrlUtils", async (importOriginal) => ({ + ...(await importOriginal()), buildLoginUrlWithReturn: (u: string) => u, consumeReturnUrl: () => state.returnUrl, getLoginUrl: () => "/login", - isValidReturnUrl: () => true, - normalizeUrlForCompare: (u: string) => u, storeReturnUrl: () => undefined, })); @@ -128,4 +132,14 @@ describe("dashboard landing", () => { render(); expect(mockLocationReplace).toHaveBeenCalledWith("http://localhost:3000/ui/models-and-endpoints"); }); + + it.each([ + "/ui/?login=success", + "http://localhost:3000/ui?login=success", + "https://backend.example/ui/?login=success", + ])("does not reload when the return URL %s already matches the current page", (returnUrl) => { + state.returnUrl = returnUrl; + render(); + expect(mockLocationReplace).not.toHaveBeenCalled(); + }); }); diff --git a/litellm/ui/litellm-dashboard/src/app/(dashboard)/page.tsx b/litellm/ui/litellm-dashboard/src/app/(dashboard)/page.tsx index 86821470303..10041063813 100644 --- a/litellm/ui/litellm-dashboard/src/app/(dashboard)/page.tsx +++ b/litellm/ui/litellm-dashboard/src/app/(dashboard)/page.tsx @@ -8,10 +8,10 @@ import { buildLoginUrlWithReturn, consumeReturnUrl, getLoginUrl, - isValidReturnUrl, normalizeUrlForCompare, storeReturnUrl, } from "@/utils/returnUrlUtils"; +import { navigateToUiPage } from "@/utils/uiNavigation"; import { legacyPageRedirectHref } from "@/app/(dashboard)/legacyPageRoutes"; import { useRouter, useSearchParams } from "next/navigation"; import { Suspense, useEffect, useRef } from "react"; @@ -60,19 +60,14 @@ function CreateKeyPageContent() { // Check for a stored return URL const returnUrl = consumeReturnUrl(getRuntimeBackendBaseUrl()); - if (returnUrl && isValidReturnUrl(returnUrl, getRuntimeBackendBaseUrl())) { - // Inline origin check: only redirect to same-origin URLs to prevent open redirect. - const safeUrl = new URL(getUiPageUrl(returnUrl, true), window.location.origin); - if (safeUrl.origin !== window.location.origin) { - return; - } + if (returnUrl) { const currentUrl = window.location.href; - const normalizedReturnUrl = normalizeUrlForCompare(returnUrl); + const normalizedReturnUrl = normalizeUrlForCompare(getUiPageUrl(returnUrl)); const normalizedCurrentUrl = normalizeUrlForCompare(currentUrl); // Only redirect if the return URL is different from the current URL // This prevents infinite redirect loops if (normalizedReturnUrl !== normalizedCurrentUrl) { - window.location.replace(safeUrl.href); + navigateToUiPage(returnUrl, true); } } }, [authLoading, token]); diff --git a/litellm/ui/litellm-dashboard/src/app/login/LoginPage.test.tsx b/litellm/ui/litellm-dashboard/src/app/login/LoginPage.test.tsx index 6a61f5ed85f..f51a86c3102 100644 --- a/litellm/ui/litellm-dashboard/src/app/login/LoginPage.test.tsx +++ b/litellm/ui/litellm-dashboard/src/app/login/LoginPage.test.tsx @@ -1,10 +1,13 @@ -import { describe, it, expect, vi, beforeEach } from "vitest"; -import { render, screen, waitFor } from "@testing-library/react"; +import { describe, it, expect, vi, beforeEach, afterEach } from "vitest"; +import { fireEvent, render, screen, waitFor } from "@testing-library/react"; import { QueryClient, QueryClientProvider } from "@tanstack/react-query"; import LoginPage from "./LoginPage"; const mockPush = vi.fn(); const mockReplace = vi.fn(); +const mockAssignDocument = vi.fn(); +const mockReplaceDocument = vi.fn(); +const mockMutate = vi.fn(); vi.mock("next/navigation", () => ({ useRouter: vi.fn(() => ({ @@ -31,12 +34,13 @@ vi.mock("@/components/networking", async (importOriginal) => { return { ...actual, getProxyBaseUrl: vi.fn().mockReturnValue("http://localhost:4000"), + exchangeLoginCode: vi.fn(), }; }); vi.mock("@/app/(dashboard)/hooks/login/useLogin", () => ({ useLogin: vi.fn(() => ({ - mutate: vi.fn(), + mutate: mockMutate, isPending: false, error: null, })), @@ -56,6 +60,7 @@ vi.mock("@/hooks/useWorker", () => ({ import { useUIConfig } from "@/app/(dashboard)/hooks/uiConfig/useUIConfig"; import { getCookieFromDocument } from "@/utils/cookieUtils"; import { isJwtExpired } from "@/utils/jwtUtils"; +import { exchangeLoginCode } from "@/components/networking"; const createQueryClient = () => new QueryClient({ @@ -68,10 +73,23 @@ const createQueryClient = () => }); describe("LoginPage", () => { + const originalLocation = window.location; + beforeEach(() => { vi.clearAllMocks(); mockPush.mockClear(); mockReplace.mockClear(); + mockMutate.mockReset(); + Object.defineProperty(window, "location", { + value: { ...originalLocation, assign: mockAssignDocument, replace: mockReplaceDocument }, + writable: true, + }); + document.cookie = "litellm_return_url=; Max-Age=0; Path=/"; + }); + + afterEach(() => { + Object.defineProperty(window, "location", { value: originalLocation, writable: true }); + document.cookie = "litellm_return_url=; Max-Age=0; Path=/"; }); it("should render", async () => { @@ -98,7 +116,7 @@ describe("LoginPage", () => { }); }); - it("should call router.replace to dashboard when jwt is valid", async () => { + it("loads a fresh dashboard document when jwt is valid", async () => { const validToken = "valid-token"; (useUIConfig as ReturnType).mockReturnValue({ data: { @@ -120,7 +138,7 @@ describe("LoginPage", () => { ); await waitFor(() => { - expect(mockReplace).toHaveBeenCalledWith("/ui"); + expect(mockReplaceDocument).toHaveBeenCalledWith(new URL("/ui", window.location.origin).href); }); }); @@ -201,7 +219,7 @@ describe("LoginPage", () => { ); await waitFor(() => { - expect(mockReplace).toHaveBeenCalledWith("/ui"); + expect(mockReplaceDocument).toHaveBeenCalledWith(new URL("/ui", window.location.origin).href); }); expect(mockPush).not.toHaveBeenCalled(); @@ -291,13 +309,104 @@ describe("LoginPage", () => { expect(ssoButton).toBeDisabled(); }); + describe("post-login document navigation", () => { + beforeEach(() => { + vi.mocked(useUIConfig).mockReturnValue({ + data: { auto_redirect_to_sso: false, server_root_path: "/", proxy_base_url: null, sso_configured: false }, + isLoading: false, + } as ReturnType); + vi.mocked(getCookieFromDocument).mockReturnValue(null); + vi.mocked(isJwtExpired).mockReturnValue(true); + }); + + function renderLogin() { + return render( + + + , + ); + } + + it.each([ + [null, "/ui/?login=success"], + ["/ui/teams/?tab=all", "/ui/teams/?tab=all"], + ["https://untrusted.example/ui/", "/ui/?login=success"], + ["/\\attacker.example/ui/", "/ui/?login=success"], + ["/\t/attacker.example/ui/", "/ui/?login=success"], + ["javascript://localhost/%0Aalert(1)", "/ui/?login=success"], + ])("loads the dashboard document after password login with return URL %s", async (returnUrl, expectedUrl) => { + if (returnUrl) document.cookie = `litellm_return_url=${encodeURIComponent(returnUrl)}; Path=/`; + mockMutate.mockImplementation((_input, options) => options.onSuccess({ redirect_url: "/ui/?login=success" })); + renderLogin(); + await screen.findByRole("heading", { name: "Login" }); + fireEvent.change(screen.getByLabelText("Username"), { target: { value: "admin" } }); + fireEvent.change(screen.getByLabelText("Password"), { target: { value: "test-password" } }); + fireEvent.click(screen.getByRole("button", { name: "Login", exact: true })); + + await waitFor(() => + expect(mockAssignDocument).toHaveBeenCalledWith(new URL(expectedUrl, window.location.origin).href), + ); + expect(mockPush).not.toHaveBeenCalled(); + }); + + it("replaces the login document after SSO code exchange succeeds", async () => { + window.location.search = "?code=single-use-code"; + vi.mocked(exchangeLoginCode).mockResolvedValue("session-jwt"); + renderLogin(); + + await waitFor(() => expect(mockReplaceDocument).toHaveBeenCalledWith("/ui/?login=success")); + expect(exchangeLoginCode).toHaveBeenCalledWith("single-use-code", null); + expect(mockReplace).not.toHaveBeenCalled(); + }); + + it.each(["https://untrusted.example/ui/", "javascript://localhost/%0Aalert(1)"])( + "rejects an unsafe server-provided login redirect %j", + async (redirectUrl) => { + mockMutate.mockImplementation((_input, options) => options.onSuccess({ redirect_url: redirectUrl })); + renderLogin(); + await screen.findByRole("heading", { name: "Login" }); + fireEvent.change(screen.getByLabelText("Username"), { target: { value: "admin" } }); + fireEvent.change(screen.getByLabelText("Password"), { target: { value: "test-password" } }); + fireEvent.click(screen.getByRole("button", { name: "Login", exact: true })); + + await waitFor(() => expect(mockAssignDocument).toHaveBeenCalledWith("/ui")); + }, + ); + + it("replaces the login document with a valid return URL for an existing session", async () => { + document.cookie = `litellm_return_url=${encodeURIComponent("/ui/teams/")}; Path=/`; + vi.mocked(getCookieFromDocument).mockReturnValue("valid-session"); + vi.mocked(isJwtExpired).mockReturnValue(false); + renderLogin(); + + await waitFor(() => + expect(mockReplaceDocument).toHaveBeenCalledWith(new URL("/ui/teams/", window.location.origin).href), + ); + expect(mockReplace).not.toHaveBeenCalled(); + }); + + it.each(["/\\attacker.example/ui/", "javascript://localhost/%0Aalert(1)"])( + "keeps an existing session on the dashboard for unsafe return URL %j", + async (returnUrl) => { + window.location.search = `?redirect_to=${encodeURIComponent(returnUrl)}`; + vi.mocked(getCookieFromDocument).mockReturnValue("valid-session"); + vi.mocked(isJwtExpired).mockReturnValue(false); + renderLogin(); + + await waitFor(() => + expect(mockReplaceDocument).toHaveBeenCalledWith(new URL("/ui", window.location.origin).href), + ); + }, + ); + }); + describe("URL ?token= legacy path is rejected (security regression test)", () => { const originalLocation = window.location; beforeEach(() => { Object.defineProperty(window, "location", { value: { - ...originalLocation, + ...window.location, href: "http://localhost:3000/ui/login?token=attacker.jwt.value", pathname: "/ui/login", search: "?token=attacker.jwt.value", @@ -339,7 +448,7 @@ describe("LoginPage", () => { }); expect(document.cookie).not.toContain("token=attacker.jwt.value"); - expect(mockReplace).not.toHaveBeenCalledWith("/ui/?login=success"); + expect(mockReplaceDocument).not.toHaveBeenCalledWith("/ui/?login=success"); }); it("must not overwrite an existing valid session cookie when ?token= is in the URL", async () => { @@ -363,11 +472,11 @@ describe("LoginPage", () => { ); await waitFor(() => { - expect(mockReplace).toHaveBeenCalledWith("/ui"); + expect(mockReplaceDocument).toHaveBeenCalledWith(new URL("/ui", window.location.origin).href); }); expect(document.cookie).not.toContain("token=attacker.jwt.value"); - expect(mockReplace).not.toHaveBeenCalledWith("/ui/?login=success"); + expect(mockReplaceDocument).not.toHaveBeenCalledWith("/ui/?login=success"); }); }); }); diff --git a/litellm/ui/litellm-dashboard/src/app/login/LoginPage.tsx b/litellm/ui/litellm-dashboard/src/app/login/LoginPage.tsx index 35b847b5334..ec0594fafb8 100644 --- a/litellm/ui/litellm-dashboard/src/app/login/LoginPage.tsx +++ b/litellm/ui/litellm-dashboard/src/app/login/LoginPage.tsx @@ -7,7 +7,6 @@ import { exchangeLoginCode, getProxyBaseUrl, getRuntimeBackendBaseUrl, - getUiPageUrl, switchToWorkerUrl, } from "@/components/networking"; import { Alert, AlertAction, AlertDescription, AlertTitle } from "@/components/shared/Alert"; @@ -24,6 +23,7 @@ import { useZodForm } from "@/lib/forms/useZodForm"; import { clearTokenCookies, getCookieFromDocument } from "@/utils/cookieUtils"; import { isJwtExpired } from "@/utils/jwtUtils"; import { consumeReturnUrl, getLoginUrl, getReturnUrl, isValidReturnUrl } from "@/utils/returnUrlUtils"; +import { navigateToUiPage } from "@/utils/uiNavigation"; import { CircleAlert, Info, TriangleAlert, X } from "lucide-react"; import { useRouter } from "next/navigation"; import { useEffect, useId, useState } from "react"; @@ -122,7 +122,7 @@ function LoginPageContent() { params.delete("code"); const cleanSearch = params.toString(); window.history.replaceState(null, "", window.location.pathname + (cleanSearch ? `?${cleanSearch}` : "")); - router.replace("/ui/?login=success"); + window.location.replace("/ui/?login=success"); }); return; } @@ -138,12 +138,7 @@ function LoginPageContent() { const rawToken = getCookieFromDocument("token"); if (rawToken && !isJwtExpired(rawToken)) { // User already logged in - redirect to return URL or default - const returnUrl = consumeReturnUrl(getRuntimeBackendBaseUrl()); - if (returnUrl) { - router.replace(getUiPageUrl(returnUrl, true)); - } else { - router.replace("/ui"); - } + navigateToUiPage(consumeReturnUrl(getRuntimeBackendBaseUrl()) ?? "/ui", true); return; } @@ -176,15 +171,10 @@ function LoginPageContent() { if (selectedWorker) { selectWorker(selectedWorker.worker_id); // Stay on the CP's UI — proxyBaseUrl already points at the worker - router.push("/ui/?login=success"); + navigateToUiPage("/ui/?login=success"); } else { // Normal (non-control-plane) login — follow the server's redirect - const returnUrl = consumeReturnUrl(getRuntimeBackendBaseUrl()); - if (returnUrl) { - router.push(getUiPageUrl(returnUrl, true)); - } else { - router.push(getUiPageUrl(data.redirect_url, true)); - } + navigateToUiPage(consumeReturnUrl(getRuntimeBackendBaseUrl()) ?? data.redirect_url); } }, onError: () => { diff --git a/litellm/ui/litellm-dashboard/src/utils/returnUrlUtils.test.ts b/litellm/ui/litellm-dashboard/src/utils/returnUrlUtils.test.ts index 4432a39a7b8..04d7cffe7ae 100644 --- a/litellm/ui/litellm-dashboard/src/utils/returnUrlUtils.test.ts +++ b/litellm/ui/litellm-dashboard/src/utils/returnUrlUtils.test.ts @@ -303,6 +303,18 @@ describe("returnUrlUtils", () => { }); describe("isValidReturnUrl", () => { + it.each([ + "/\\attacker.example/ui/", + "/\t/attacker.example/ui/", + "/\n/attacker.example/ui/", + "/\r/attacker.example/ui/", + "javascript://localhost/%0Aalert(1)", + "data://localhost/text/html,test", + "ftp://localhost/ui/", + ])("rejects unsafe navigation target %j", (url) => { + expect(isValidReturnUrl(url)).toBe(false); + }); + it("should validate relative URLs starting with /", () => { expect(isValidReturnUrl("/ui?page=api-keys")).toBe(true); expect(isValidReturnUrl("/ui/teams")).toBe(true); @@ -398,6 +410,18 @@ describe("returnUrlUtils", () => { }); describe("consumeReturnUrl", () => { + it.each(["params", "cookie"])("rejects a backslash redirect from %s", (source) => { + const unsafeUrl = "/\\attacker.example/ui/"; + if (source === "params") { + window.location.search = `?redirect_to=${encodeURIComponent(unsafeUrl)}`; + } else { + window.location.search = ""; + document.cookie = `litellm_return_url=${encodeURIComponent(unsafeUrl)}; Path=/`; + } + + expect(consumeReturnUrl()).toBeNull(); + }); + it("should return and clear the stored return URL", () => { Object.defineProperty(window, "location", { value: { diff --git a/litellm/ui/litellm-dashboard/src/utils/returnUrlUtils.ts b/litellm/ui/litellm-dashboard/src/utils/returnUrlUtils.ts index 0259aa8c2ec..6533393c36f 100644 --- a/litellm/ui/litellm-dashboard/src/utils/returnUrlUtils.ts +++ b/litellm/ui/litellm-dashboard/src/utils/returnUrlUtils.ts @@ -227,7 +227,7 @@ function isDevEnvironment(): boolean { * @returns true if the URL is safe to redirect to */ export function isValidReturnUrl(url: string, runtimeBackendUrl: string | null = null): boolean { - if (!url) { + if (!url || /[\\\r\n\t]/.test(url)) { return false; } @@ -243,6 +243,9 @@ export function isValidReturnUrl(url: string, runtimeBackendUrl: string | null = try { const returnUrlObj = new URL(url); + if (returnUrlObj.protocol !== "http:" && returnUrlObj.protocol !== "https:") { + return false; + } const currentHostname = window.location.hostname; if (runtimeBackendUrl && returnUrlObj.origin === new URL(runtimeBackendUrl).origin) { diff --git a/litellm/ui/litellm-dashboard/src/utils/uiNavigation.test.ts b/litellm/ui/litellm-dashboard/src/utils/uiNavigation.test.ts new file mode 100644 index 00000000000..9383e82a445 --- /dev/null +++ b/litellm/ui/litellm-dashboard/src/utils/uiNavigation.test.ts @@ -0,0 +1,46 @@ +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; +import { normalizeBackendPageUrlToUiOrigin } from "./returnUrlUtils"; +import { navigateToUiPage } from "./uiNavigation"; + +vi.mock("@/components/networking", () => ({ + getUiPageUrl: (url: string) => + normalizeBackendPageUrlToUiOrigin(url, "https://backend.example", "https://ui.example"), +})); + +const assign = vi.fn(); +const replace = vi.fn(); + +describe("navigateToUiPage", () => { + beforeEach(() => { + vi.clearAllMocks(); + vi.stubGlobal("window", { location: { origin: "https://ui.example", assign, replace } }); + }); + + afterEach(() => vi.unstubAllGlobals()); + + it.each([false, true])("uses document navigation with replace=%s", (replaceHistory) => { + navigateToUiPage("/ui/teams?tab=all#members", replaceHistory); + + expect(replaceHistory ? replace : assign).toHaveBeenCalledWith("https://ui.example/ui/teams?tab=all#members"); + expect(replaceHistory ? assign : replace).not.toHaveBeenCalled(); + }); + + it.each([ + ["https://backend.example/ui/teams?tab=all#members", "https://ui.example/ui/teams?tab=all#members"], + ["https://backend.example//attacker.example/login", "https://ui.example//attacker.example/login"], + ["https://backend.example/\\attacker.example/login", "https://ui.example//attacker.example/login"], + ["https://ui.example/ui/teams", "https://ui.example/ui/teams"], + ["https://attacker.example/ui", "/ui"], + ["//attacker.example/ui", "/ui"], + ["/\\attacker.example/ui", "/ui"], + ["/\t/attacker.example/ui", "/ui"], + ["javascript://ui.example/%0Aalert(1)", "/ui"], + ["ftp://ui.example/ui", "/ui"], + ["https://ui.example:444/ui", "/ui"], + ["http://[invalid", "/ui"], + ])("navigates safely for %j", (url, expected) => { + navigateToUiPage(url); + + expect(assign).toHaveBeenCalledWith(expected); + }); +}); diff --git a/litellm/ui/litellm-dashboard/src/utils/uiNavigation.ts b/litellm/ui/litellm-dashboard/src/utils/uiNavigation.ts new file mode 100644 index 00000000000..fe971a1bf3c --- /dev/null +++ b/litellm/ui/litellm-dashboard/src/utils/uiNavigation.ts @@ -0,0 +1,20 @@ +import { getUiPageUrl } from "@/components/networking"; + +export function navigateToUiPage(url: string, replace = false): void { + const destination = (() => { + try { + const target = new URL(getUiPageUrl(url), window.location.origin); + const isHttp = target.protocol === "http:" || target.protocol === "https:"; + + return isHttp && target.origin === window.location.origin ? target.href : "/ui"; + } catch { + return "/ui"; + } + })(); + + if (replace) { + window.location.replace(destination); + } else { + window.location.assign(destination); + } +} diff --git a/litellm/ui/nginx.conf b/litellm/ui/nginx.conf index e695686c670..4df724895a1 100644 --- a/litellm/ui/nginx.conf +++ b/litellm/ui/nginx.conf @@ -92,6 +92,9 @@ http { # /ui/.txt would otherwise be rewritten to HTML and break RSC # for nested routes. A genuinely missing payload must 404 (the # router degrades to a hard navigation); never fall back to HTML. + location ~ ^/ui/(.+\.txt)$ { + try_files /$1 =404; + } location ~ \.txt$ { try_files $uri =404; } diff --git a/tests/integration/database/image/test_ui_image_serves_offline.py b/tests/integration/database/image/test_ui_image_serves_offline.py index eecfcdc15f6..6da71c6377a 100644 --- a/tests/integration/database/image/test_ui_image_serves_offline.py +++ b/tests/integration/database/image/test_ui_image_serves_offline.py @@ -23,6 +23,7 @@ import subprocess import time import uuid from collections.abc import Iterator +from typing import Final import pytest @@ -130,3 +131,21 @@ def test_ui_serves_as_arbitrary_uid_read_only(ui_container: tuple[str, str]) -> f"GET {path} returned {page.stdout.strip()!r} as uid {ARBITRARY_UID}.\n" f"{_container_logs(container)}" ) + + for payload in ("/__next._tree.txt", "/login/index.txt", "/login/__next._tree.txt", "/projects/__next._full.txt"): + exported: Final = _docker("exec", container, "cat", f"/var/lib/litellm/ui{payload}").stdout + assert exported, f"empty exported payload at {payload}" + for prefix in ("", "/ui"): + response: Final = _docker( + "run", "--rm", "--network", network, CURL_IMAGE, + "--silent", "--show-error", "--max-time", "10", + "--write-out", "\n%{http_code} %{content_type}", + f"http://{container}:{UI_PORT}{prefix}{payload}?_rsc=regression", + ) + assert response.stdout == f"{exported}\n200 text/plain", ( + f"GET {prefix}{payload} did not serve the exported RSC payload.\n{response.stdout}" + ) + + for path in ("/ui/login/__next.missing.txt", "/ui/missing.txt", "/missing.txt"): + missing: Final = _probe(network, container, path) + assert missing.stdout.strip() == "404", f"GET {path} must not fall back to HTML, got {missing.stdout!r}"