fix(ui): serve navigation payloads under the ui prefix (#50758)

* test(ui): cover prefixed static navigation payloads

* fix(ui): strip ui prefix from static navigation payloads

* fix(ui): refresh auth state after login soft navigation

Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>

* Use a shared dashboard login token store

Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>

* Revert "Use a shared dashboard login token store"

This reverts commit 7fd0dcb8eb.

* Revert "fix(ui): refresh auth state after login soft navigation"

This reverts commit 5b6ebe8918.

* Reload the dashboard after login

Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>

* fix(ui): validate login return URLs and read the image export root

* fix(ui): constrain post-login navigation to the UI origin

* refactor(ui): centralize guarded document navigation

* refactor(ui): compute navigation destination once

---------

Co-authored-by: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
This commit is contained in:
mrinal-berri 2026-11-12 14:10:10 -08:00 • committed by GitHub
parent 98dbe80ada
commit 40debbb9c0
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
10 changed files with 266 additions and 43 deletions

View file

@ -43,18 +43,22 @@ vi.mock("@/app/(dashboard)/home/_components/HomePage", () => ({
vi.mock("@/components/common_components/LoadingScreen", () => ({
default: () => <div data-testid="loading-screen" />,
}));
vi.mock("@/components/networking", () => ({
getRuntimeBackendBaseUrl: () => null,
getUiPageUrl: (url: string) => url,
proxyBaseUrl: "",
}));
vi.mock("@/components/networking", async () => {
const { normalizeBackendPageUrlToUiOrigin } =
await vi.importActual<typeof import("@/utils/returnUrlUtils")>("@/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<typeof import("@/utils/returnUrlUtils")>()),
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(<CreateKeyPage />);
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(<CreateKeyPage />);
expect(mockLocationReplace).not.toHaveBeenCalled();
});
});

View file

@ -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]);

View file

@ -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<typeof vi.fn>).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<typeof useUIConfig>);
vi.mocked(getCookieFromDocument).mockReturnValue(null);
vi.mocked(isJwtExpired).mockReturnValue(true);
});
function renderLogin() {
return render(
<QueryClientProvider client={createQueryClient()}>
<LoginPage />
</QueryClientProvider>,
);
}
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");
});
});
});

View file

@ -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: () => {

View file

@ -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: {

View file

@ -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) {

View file

@ -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);
});
});

View file

@ -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);
}
}

View file

@ -92,6 +92,9 @@ http {
# /ui/<page>.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;
}

View file

@ -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}"