From 1b6cee9b123872101b695699ca0f7e0d9a5ca2a7 Mon Sep 17 00:00:00 2001 From: ryan-crabbe-berri Date: Fri, 24 Jul 2026 16:08:51 -0700 Subject: [PATCH] fix(ui): logout no longer reloads the current page before redirecting to login When PROXY_LOGOUT_URL is unset (the default), handleLogout navigated to an empty string, which reloads whatever page the user is on with the token already cleared. The per-route auth guard then waited for the async UI config fetch before redirecting, so users saw the old page (with 401ing data calls) for a few seconds after clicking logout. Logout now goes straight to the login page via location.replace and clears the stored return URL so a later login cannot bounce back to the pre-logout page. useAuthorized also redirects immediately when the token is missing or invalid instead of waiting on the UI config fetch, which covers expired sessions hard-loading any dashboard route. --- .../(dashboard)/hooks/useAuthorized.test.ts | 33 +++++++++++++++ .../app/(dashboard)/hooks/useAuthorized.ts | 13 +++--- .../src/components/navbar.test.tsx | 42 +++++++++++++++++-- .../src/components/navbar.tsx | 3 +- 4 files changed, 80 insertions(+), 11 deletions(-) diff --git a/ui/litellm-dashboard/src/app/(dashboard)/hooks/useAuthorized.test.ts b/ui/litellm-dashboard/src/app/(dashboard)/hooks/useAuthorized.test.ts index 7076e69edc2..afcc3bc3947 100644 --- a/ui/litellm-dashboard/src/app/(dashboard)/hooks/useAuthorized.test.ts +++ b/ui/litellm-dashboard/src/app/(dashboard)/hooks/useAuthorized.test.ts @@ -243,6 +243,39 @@ describe("useAuthorized", () => { expect(result.current.token).toBeNull(); }); + it("should redirect immediately when token is missing, without waiting for the UI config fetch", async () => { + getUiConfigMock.mockReturnValue(new Promise(() => {})); + + decodeTokenMock.mockReturnValue(null); + checkTokenValidityMock.mockReturnValue(false); + + const { result } = renderHook(() => useAuthorized(), { wrapper }); + + await waitFor(() => { + expect(replaceMock).toHaveBeenCalledWith("http://proxy.example/ui/login/"); + }); + + expect(clearTokenCookiesMock).not.toHaveBeenCalled(); + expect(result.current.token).toBeNull(); + }); + + it("should clear cookies and redirect immediately when token is expired, without waiting for the UI config fetch", async () => { + getUiConfigMock.mockReturnValue(new Promise(() => {})); + + decodeTokenMock.mockReturnValue({ key: "api-key-123", user_id: "user-1" }); + checkTokenValidityMock.mockReturnValue(false); + + document.cookie = "token=expired-token; path=/;"; + + renderHook(() => useAuthorized(), { wrapper }); + + await waitFor(() => { + expect(clearTokenCookiesMock).toHaveBeenCalled(); + }); + + expect(replaceMock).toHaveBeenCalledWith("http://proxy.example/ui/login/"); + }); + it("should clear cookies and redirect when token is expired", async () => { getUiConfigMock.mockResolvedValue({ server_root_path: "/", diff --git a/ui/litellm-dashboard/src/app/(dashboard)/hooks/useAuthorized.ts b/ui/litellm-dashboard/src/app/(dashboard)/hooks/useAuthorized.ts index bb22ebf5edc..fe95113c1b1 100644 --- a/ui/litellm-dashboard/src/app/(dashboard)/hooks/useAuthorized.ts +++ b/ui/litellm-dashboard/src/app/(dashboard)/hooks/useAuthorized.ts @@ -28,15 +28,14 @@ const useAuthorized = () => { // Single useEffect for all redirect logic useEffect(() => { - if (isLoading) return; + if (isTokenValid && isLoading) return; + if (isAuthorized) return; - if (!isAuthorized) { - if (token) { - clearTokenCookies(); - } - redirectToLogin(); + if (token) { + clearTokenCookies(); } - }, [isLoading, isAuthorized, token, redirectToLogin]); + redirectToLogin(); + }, [isLoading, isTokenValid, isAuthorized, token, redirectToLogin]); return { isLoading, diff --git a/ui/litellm-dashboard/src/components/navbar.test.tsx b/ui/litellm-dashboard/src/components/navbar.test.tsx index ba0b4bf55f5..bbf78aa9bc7 100644 --- a/ui/litellm-dashboard/src/components/navbar.test.tsx +++ b/ui/litellm-dashboard/src/components/navbar.test.tsx @@ -132,9 +132,17 @@ vi.mock("@/utils/cookieUtils", () => ({ clearTokenCookies: vi.fn(), })); -// Mock window.location.href for logout testing +vi.mock("@/utils/returnUrlUtils", async (importOriginal) => { + const actual = await importOriginal(); + return { + ...actual, + clearStoredReturnUrl: vi.fn(), + }; +}); + +// Mock window.location for logout testing Object.defineProperty(window, "location", { - value: { href: "" }, + value: { href: "", replace: vi.fn() }, writable: true, }); @@ -285,6 +293,7 @@ describe("Navbar", () => { it("should handle logout functionality", async () => { const user = userEvent.setup(); + vi.mocked(window.location.replace).mockClear(); renderWithProviders(); @@ -298,9 +307,36 @@ describe("Navbar", () => { await user.click(screen.getByText("Logout")); const cookieUtils = vi.mocked(await import("@/utils/cookieUtils")); + const returnUrlUtils = vi.mocked(await import("@/utils/returnUrlUtils")); expect(cookieUtils.clearTokenCookies).toHaveBeenCalled(); + expect(returnUrlUtils.clearStoredReturnUrl).toHaveBeenCalled(); await waitFor(() => { - expect(window.location.href).toBe("https://example.com/logout"); + expect(window.location.replace).toHaveBeenCalledWith("https://example.com/logout"); + }); + }); + + it("should redirect to the login page on logout when PROXY_LOGOUT_URL is not configured", async () => { + const user = userEvent.setup(); + vi.mocked(window.location.replace).mockClear(); + + const proxyUtils = vi.mocked(await import("@/utils/proxyUtils")); + proxyUtils.fetchProxySettings.mockResolvedValueOnce({ + PROXY_BASE_URL: "", + PROXY_LOGOUT_URL: "", + }); + + renderWithProviders(); + + await user.click(screen.getByRole("button", { name: /open account menu/i })); + + await waitFor(() => { + expect(screen.getByText("test-user")).toBeInTheDocument(); + }); + + await user.click(screen.getByText("Logout")); + + await waitFor(() => { + expect(window.location.replace).toHaveBeenCalledWith("http://localhost:4000/ui/login/"); }); }); diff --git a/ui/litellm-dashboard/src/components/navbar.tsx b/ui/litellm-dashboard/src/components/navbar.tsx index 40638e7b8ba..5376d1490f7 100644 --- a/ui/litellm-dashboard/src/components/navbar.tsx +++ b/ui/litellm-dashboard/src/components/navbar.tsx @@ -46,9 +46,10 @@ const Navbar: React.FC = ({ const handleLogout = () => { clearTokenCookies(); + clearStoredReturnUrl(); localStorage.removeItem("litellm_selected_worker_id"); localStorage.removeItem("litellm_worker_url"); - window.location.href = proxySettings.PROXY_LOGOUT_URL || ""; + window.location.replace(proxySettings.PROXY_LOGOUT_URL || getLoginUrl(baseUrl)); }; const handleWorkerSwitch = (workerId: string) => {