fix(ui): navigate to /ui/login/ with trailing slash via hard navigation (#33561)

* fix(ui): navigate to /ui/login/ with trailing slash via hard navigation

Logged-out redirects targeted /ui/login without the trailing slash, so
Starlette's StaticFiles(html=True) mount answered with a 307 whose
absolute Location is built from the scheme the container sees. Behind a
TLS-terminating reverse proxy uvicorn does not trust X-Forwarded-Proto
by default, so the redirect downgraded https to http and stranded users
on an unreachable URL (#33454). The auth guard also used the Next client
router for this navigation, which first requests an RSC payload that the
static export cannot serve, producing 404s before falling back to a full
page load.

Centralize the login URL in getLoginUrl(), which always emits the
trailing slash so no server redirect fires, and use
window.location.replace for the login redirects so no RSC fetch is
attempted.

* test(ui): expect trailing slash in expired-token login redirect
This commit is contained in:
ryan-crabbe-berri 2026-07-16 12:18:55 -07:00 committed by GitHub
parent ebdf0bbfd7
commit 74ff8d0ff9
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
13 changed files with 101 additions and 33 deletions

View file

@ -1,7 +1,7 @@
/* @vitest-environment jsdom */
import React from "react";
import { renderHook, waitFor } from "@testing-library/react";
import { afterEach, describe, expect, it, vi } from "vitest";
import { afterEach, beforeEach, describe, expect, it, vi } from "vitest";
import { QueryClient, QueryClientProvider } from "@tanstack/react-query";
import useAuthorized from "./useAuthorized";
@ -26,12 +26,6 @@ const {
buildLoginUrlWithReturnMock: vi.fn((baseUrl: string) => baseUrl),
}));
vi.mock("next/navigation", () => ({
useRouter: () => ({
replace: replaceMock,
}),
}));
vi.mock("@/components/networking", async (importOriginal) => {
const actual = await importOriginal<typeof import("@/components/networking")>();
return {
@ -91,7 +85,28 @@ const clearCookie = () => {
};
describe("useAuthorized", () => {
const originalLocation = window.location;
beforeEach(() => {
Object.defineProperty(window, "location", {
value: {
href: "http://proxy.example/ui/?page=api-keys",
origin: "http://proxy.example",
hostname: "proxy.example",
pathname: "/ui/",
search: "?page=api-keys",
protocol: "http:",
replace: replaceMock,
},
writable: true,
});
});
afterEach(() => {
Object.defineProperty(window, "location", {
value: originalLocation,
writable: true,
});
replaceMock.mockReset();
clearTokenCookiesMock.mockReset();
getProxyBaseUrlMock.mockClear();
@ -164,7 +179,7 @@ describe("useAuthorized", () => {
expect(clearTokenCookiesMock).toHaveBeenCalled();
});
expect(replaceMock).toHaveBeenCalledWith("http://proxy.example/ui/login");
expect(replaceMock).toHaveBeenCalledWith("http://proxy.example/ui/login/");
expect(result.current.accessToken).toBeNull();
expect(result.current.userRole).toBe("Undefined Role");
});
@ -197,7 +212,7 @@ describe("useAuthorized", () => {
const { result } = renderHook(() => useAuthorized(), { wrapper });
await waitFor(() => {
expect(replaceMock).toHaveBeenCalledWith("http://proxy.example/ui/login");
expect(replaceMock).toHaveBeenCalledWith("http://proxy.example/ui/login/");
});
expect(result.current.accessToken).toBe("api-key-123");
@ -221,7 +236,7 @@ describe("useAuthorized", () => {
const { result } = renderHook(() => useAuthorized(), { wrapper });
await waitFor(() => {
expect(replaceMock).toHaveBeenCalledWith("http://proxy.example/ui/login");
expect(replaceMock).toHaveBeenCalledWith("http://proxy.example/ui/login/");
});
expect(clearTokenCookiesMock).not.toHaveBeenCalled();
@ -256,7 +271,7 @@ describe("useAuthorized", () => {
expect(clearTokenCookiesMock).toHaveBeenCalled();
});
expect(replaceMock).toHaveBeenCalledWith("http://proxy.example/ui/login");
expect(replaceMock).toHaveBeenCalledWith("http://proxy.example/ui/login/");
expect(checkTokenValidityMock).toHaveBeenCalledWith(token);
});
});

View file

@ -3,14 +3,12 @@
import { getProxyBaseUrl } from "@/components/networking";
import { clearTokenCookies, getCookie } from "@/utils/cookieUtils";
import { checkTokenValidity, decodeToken } from "@/utils/jwtUtils";
import { buildLoginUrlWithReturn, storeReturnUrl } from "@/utils/returnUrlUtils";
import { useRouter } from "next/navigation";
import { buildLoginUrlWithReturn, getLoginUrl, storeReturnUrl } from "@/utils/returnUrlUtils";
import { useCallback, useEffect, useMemo } from "react";
import { formatUserRole } from "@/utils/roles";
import { useUIConfig } from "./uiConfig/useUIConfig";
const useAuthorized = () => {
const router = useRouter();
const { data: uiConfig, isLoading: isUIConfigLoading } = useUIConfig();
const token = typeof document !== "undefined" ? getCookie("token") : null;
@ -23,10 +21,10 @@ const useAuthorized = () => {
// Helper function to redirect to login while preserving the current URL
const redirectToLogin = useCallback(() => {
storeReturnUrl();
const baseLoginUrl = `${getProxyBaseUrl()}/ui/login`;
const baseLoginUrl = getLoginUrl(getProxyBaseUrl());
const loginUrlWithReturn = buildLoginUrlWithReturn(baseLoginUrl);
router.replace(loginUrlWithReturn);
}, [router]);
window.location.replace(loginUrlWithReturn);
}, []);
// Single useEffect for all redirect logic
useEffect(() => {

View file

@ -7,6 +7,7 @@ import { useAuth } from "@/contexts/AuthContext";
import {
buildLoginUrlWithReturn,
consumeReturnUrl,
getLoginUrl,
isValidReturnUrl,
normalizeUrlForCompare,
storeReturnUrl,
@ -33,7 +34,7 @@ function CreateKeyPageContent() {
// Store the current URL so we can redirect back after login
storeReturnUrl();
// Build login URL with return URL parameter
const baseLoginUrl = (proxyBaseUrl || "") + "/ui/login";
const baseLoginUrl = getLoginUrl(proxyBaseUrl || "");
const dest = buildLoginUrlWithReturn(baseLoginUrl);
// Replace instead of assigning to avoid back-button loops
window.location.replace(dest);

View file

@ -6,7 +6,7 @@ import LoadingScreen from "@/components/common_components/LoadingScreen";
import { exchangeLoginCode, getProxyBaseUrl, switchToWorkerUrl } from "@/components/networking";
import { clearTokenCookies, getCookieFromDocument } from "@/utils/cookieUtils";
import { isJwtExpired } from "@/utils/jwtUtils";
import { consumeReturnUrl, getReturnUrl, isValidReturnUrl } from "@/utils/returnUrlUtils";
import { consumeReturnUrl, getLoginUrl, getReturnUrl, isValidReturnUrl } from "@/utils/returnUrlUtils";
import { InfoCircleOutlined, CloudServerOutlined } from "@ant-design/icons";
import { Alert, Button, Card, Form, Input, Popover, Select, Space, Typography } from "antd";
import { useRouter } from "next/navigation";
@ -295,7 +295,7 @@ function LoginPageContent() {
// SSO on the worker (or this instance if no worker), always
// include return_to so the callback redirects back here
const ssoBase = selectedWorker?.url ?? getProxyBaseUrl();
const returnTo = encodeURIComponent(window.location.origin + "/ui/login");
const returnTo = encodeURIComponent(getLoginUrl(window.location.origin));
router.push(`${ssoBase}/sso/key/generate?return_to=${returnTo}`);
}}
block

View file

@ -14,10 +14,10 @@ describe("OnboardingErrorView", () => {
expect(screen.getByText("The invitation link may be invalid or expired.")).toBeInTheDocument();
});
it("should render a Back to Login link pointing to /ui/login", () => {
it("should render a Back to Login link pointing to /ui/login/", () => {
render(<OnboardingErrorView />);
// antd Button with href renders as an <a> element
const link = screen.getByRole("link", { name: "Back to Login" });
expect(link).toHaveAttribute("href", "/ui/login");
expect(link).toHaveAttribute("href", "/ui/login/");
});
});

View file

@ -1,5 +1,6 @@
import React from "react";
import { Alert, Button } from "antd";
import { getLoginUrl } from "@/utils/returnUrlUtils";
export function OnboardingErrorView() {
return (
@ -11,7 +12,7 @@ export function OnboardingErrorView() {
showIcon
/>
<div className="mt-4">
<Button href="/ui/login">Back to Login</Button>
<Button href={getLoginUrl()}>Back to Login</Button>
</div>
</div>
);

View file

@ -1,5 +1,5 @@
import * as networking from "@/components/networking";
import { afterEach, describe, expect, it, vi } from "vitest";
import { afterEach, beforeEach, describe, expect, it, vi } from "vitest";
import { renderWithProviders, screen, waitFor } from "../../../tests/test-utils";
import ModelHubTable from "./ModelHubTable";
@ -7,6 +7,7 @@ const mockUseUISettings = vi.hoisted(() => vi.fn());
const mockGetCookie = vi.hoisted(() => vi.fn());
const mockCheckTokenValidity = vi.hoisted(() => vi.fn());
const mockRouterReplace = vi.hoisted(() => vi.fn());
const mockLocationReplace = vi.hoisted(() => vi.fn());
vi.mock("@/components/networking", () => ({
getUiConfig: vi.fn(),
@ -43,7 +44,28 @@ vi.mock("@/utils/jwtUtils", () => ({
}));
describe("ModelHubTable", () => {
const originalLocation = window.location;
beforeEach(() => {
Object.defineProperty(window, "location", {
value: {
href: "http://localhost:4000/ui/model_hub_table",
origin: "http://localhost:4000",
hostname: "localhost",
pathname: "/ui/model_hub_table",
search: "",
protocol: "http:",
replace: mockLocationReplace,
},
writable: true,
});
});
afterEach(() => {
Object.defineProperty(window, "location", {
value: originalLocation,
writable: true,
});
vi.clearAllMocks();
});
@ -60,6 +82,7 @@ describe("ModelHubTable", () => {
mockGetCookie.mockReturnValue(tokenValue);
mockCheckTokenValidity.mockReturnValue(isTokenValid);
mockRouterReplace.mockClear();
mockLocationReplace.mockClear();
// Setup other required mocks
vi.mocked(networking.getUiConfig).mockResolvedValue({
@ -92,9 +115,10 @@ describe("ModelHubTable", () => {
await waitFor(() => {
if (shouldRedirect) {
expect(mockRouterReplace).toHaveBeenCalledWith("http://localhost:4000/ui/login");
} else {
expect(mockLocationReplace).toHaveBeenCalledWith("http://localhost:4000/ui/login/");
expect(mockRouterReplace).not.toHaveBeenCalled();
} else {
expect(mockLocationReplace).not.toHaveBeenCalled();
}
});
});

View file

@ -33,6 +33,7 @@ import { Prism as SyntaxHighlighter } from "react-syntax-highlighter";
import { useUISettings } from "@/app/(dashboard)/hooks/uiSettings/useUISettings";
import { checkTokenValidity } from "@/utils/jwtUtils";
import { getCookie } from "@/utils/cookieUtils";
import { getLoginUrl } from "@/utils/returnUrlUtils";
interface ModelHubTableProps {
accessToken: string | null;
@ -108,12 +109,12 @@ const ModelHubTable: React.FC<ModelHubTableProps> = ({ accessToken, publicPage,
// If token is invalid, redirect to login
if (!isTokenValid) {
router.replace(`${getProxyBaseUrl()}/ui/login`);
window.location.replace(getLoginUrl(getProxyBaseUrl()));
return;
}
}
// If require_auth_for_public_ai_hub is false, allow public access (no change)
}, [isUISettingsLoading, publicPage, uiSettings, router]);
}, [isUISettingsLoading, publicPage, uiSettings]);
useEffect(() => {
const fetchData = async (accessToken: string) => {

View file

@ -18,7 +18,7 @@ import WorkerDropdown from "@/components/Navbar/WorkerDropdown/WorkerDropdown";
import { useWorker } from "@/hooks/useWorker";
import { useDisableShowPrompts } from "@/app/(dashboard)/hooks/useDisableShowPrompts";
import { clearTokenCookies } from "@/utils/cookieUtils";
import { clearStoredReturnUrl } from "@/utils/returnUrlUtils";
import { clearStoredReturnUrl, getLoginUrl } from "@/utils/returnUrlUtils";
interface DashboardHeaderProps {
page: string;
@ -37,7 +37,7 @@ export function DashboardHeader({ page }: DashboardHeaderProps) {
clearStoredReturnUrl();
localStorage.removeItem("litellm_selected_worker_id");
localStorage.removeItem("litellm_worker_url");
window.location.href = `/ui/login?worker=${encodeURIComponent(workerId)}`;
window.location.href = `${getLoginUrl()}?worker=${encodeURIComponent(workerId)}`;
};
return (

View file

@ -5,7 +5,7 @@ import { useWorker } from "@/hooks/useWorker";
import { getProxyBaseUrl } from "@/components/networking";
import { useTheme } from "@/contexts/ThemeContext";
import { clearTokenCookies } from "@/utils/cookieUtils";
import { clearStoredReturnUrl } from "@/utils/returnUrlUtils";
import { clearStoredReturnUrl, getLoginUrl } from "@/utils/returnUrlUtils";
import useProxySettings from "@/app/(dashboard)/hooks/proxySettings/useProxySettings";
import { DownOutlined, MenuFoldOutlined, MenuUnfoldOutlined } from "@ant-design/icons";
import { Tag } from "antd";
@ -56,7 +56,7 @@ const Navbar: React.FC<NavbarProps> = ({
clearStoredReturnUrl();
localStorage.removeItem("litellm_selected_worker_id");
localStorage.removeItem("litellm_worker_url");
window.location.href = `/ui/login?worker=${encodeURIComponent(workerId)}`;
window.location.href = `${getLoginUrl()}?worker=${encodeURIComponent(workerId)}`;
};
return (

View file

@ -3,6 +3,7 @@ import {
clearStoredReturnUrl,
consumeReturnUrl,
getCurrentUrl,
getLoginUrl,
getReturnUrl,
getReturnUrlFromParams,
getStoredReturnUrl,
@ -98,6 +99,29 @@ describe("returnUrlUtils", () => {
});
});
describe("getLoginUrl", () => {
it("should build a relative login URL with a trailing slash", () => {
expect(getLoginUrl()).toBe("/ui/login/");
});
it("should prepend the given base URL and keep the trailing slash", () => {
expect(getLoginUrl("http://proxy.example")).toBe("http://proxy.example/ui/login/");
});
it("should keep the trailing slash before the query when composed with buildLoginUrlWithReturn", () => {
Object.defineProperty(window, "location", {
value: {
...window.location,
href: "http://localhost:3000/ui?page=api-keys",
},
writable: true,
});
const loginUrl = buildLoginUrlWithReturn(getLoginUrl());
expect(loginUrl).toBe("/ui/login/?redirect_to=http%3A%2F%2Flocalhost%3A3000%2Fui%3Fpage%3Dapi-keys");
});
});
describe("buildLoginUrlWithReturn", () => {
it("should build login URL with return URL parameter", () => {
Object.defineProperty(window, "location", {

View file

@ -13,6 +13,10 @@
const RETURN_URL_COOKIE_NAME = "litellm_return_url";
const RETURN_URL_PARAM = "redirect_to";
export function getLoginUrl(baseUrl: string = ""): string {
return `${baseUrl}/ui/login/`;
}
/**
* Gets the current URL with all query parameters.
* Returns null if running on server-side.

View file

@ -232,7 +232,7 @@ describe("CreateKeyPage auth behavior", () => {
// Assert: we eventually redirect to SSO login with return URL (single replace, not assign/href)
await waitFor(() => {
expect(window.location.replace).toHaveBeenCalledWith(
expect.stringContaining("https://example.com/ui/login?redirect_to="),
expect.stringContaining("https://example.com/ui/login/?redirect_to="),
);
});