From 560e96570e9ebb6e517154c2e6a360a60285f503 Mon Sep 17 00:00:00 2001 From: Jack Venberg Date: Thu, 2 Oct 2025 15:02:07 -0700 Subject: [PATCH] Fix: Session Token Cookie Infinite Logout Loop --- .../src/components/networking.test.ts | 44 +++++++++ .../src/components/networking.tsx | 96 +++++++++---------- .../src/utils/cookieUtils.test.ts | 75 +++++++++++++++ 3 files changed, 167 insertions(+), 48 deletions(-) create mode 100644 ui/litellm-dashboard/src/components/networking.test.ts create mode 100644 ui/litellm-dashboard/src/utils/cookieUtils.test.ts diff --git a/ui/litellm-dashboard/src/components/networking.test.ts b/ui/litellm-dashboard/src/components/networking.test.ts new file mode 100644 index 00000000000..4d74eda28ef --- /dev/null +++ b/ui/litellm-dashboard/src/components/networking.test.ts @@ -0,0 +1,44 @@ +import { describe, it, expect, beforeEach, vi } from 'vitest'; +import { clearTokenCookies } from '@/utils/cookieUtils'; + +vi.mock('@/utils/cookieUtils', () => ({ + clearTokenCookies: vi.fn(), + getCookie: vi.fn(), +})); + +vi.mock('./molecules/notifications_manager', () => ({ + default: { + info: vi.fn(), + success: vi.fn(), + error: vi.fn(), + fromBackend: vi.fn(), + }, +})); + +describe('networking - expired session handling', () => { + beforeEach(() => { + vi.clearAllMocks(); + }); + + it('should call clearTokenCookies on expired session', async () => { + const errorData = "Authentication Error - Expired Key"; + const { default: NotificationsManager } = await import('./molecules/notifications_manager'); + + if (errorData.includes("Authentication Error - Expired Key")) { + NotificationsManager.info("UI Session Expired. Logging out."); + clearTokenCookies(); + } + + expect(clearTokenCookies).toHaveBeenCalledOnce(); + }); + + it('should not clear cookies for non-authentication errors', () => { + const errorData = "Some other error"; + + if (errorData.includes("Authentication Error - Expired Key")) { + clearTokenCookies(); + } + + expect(clearTokenCookies).not.toHaveBeenCalled(); + }); +}); diff --git a/ui/litellm-dashboard/src/components/networking.tsx b/ui/litellm-dashboard/src/components/networking.tsx index b16543b8f7a..6b8c7671a10 100644 --- a/ui/litellm-dashboard/src/components/networking.tsx +++ b/ui/litellm-dashboard/src/components/networking.tsx @@ -10,6 +10,7 @@ export const formatDate = (date: Date) => { */ import { all_admin_roles } from "@/utils/roles"; import { message } from "antd"; +import { clearTokenCookies } from "@/utils/cookieUtils"; import { TagNewRequest, TagUpdateRequest, @@ -169,8 +170,7 @@ const handleError = async (errorData: string) => { if (errorData.includes("Authentication Error - Expired Key")) { NotificationsManager.info("UI Session Expired. Logging out."); lastErrorTime = currentTime; - document.cookie = - "token=; expires=Thu, 01 Jan 1970 00:00:00 UTC; path=/;"; + clearTokenCookies(); window.location.href = window.location.pathname; } lastErrorTime = currentTime; @@ -335,14 +335,14 @@ export const getModelCostMapReloadStatus = async (accessToken: string) => { "Content-Type": "application/json", }, }); - + if (!response.ok) { console.error(`Status request failed with status: ${response.status}`); const errorText = await response.text(); console.error("Error response:", errorText); throw new Error(`HTTP ${response.status}: ${errorText}`); } - + const jsonData = await response.json(); console.log(`Model cost map reload status:`, jsonData); return jsonData; @@ -4090,7 +4090,7 @@ export const teamMemberUpdateCall = async ( const url = proxyBaseUrl ? `${proxyBaseUrl}/team/member_update` : `/team/member_update`; - + const requestBody: any = { team_id: teamId, role: formValues.role, @@ -4373,9 +4373,9 @@ export const userBulkUpdateUserCall = async ( const url = proxyBaseUrl ? `${proxyBaseUrl}/user/bulk_update` : `/user/bulk_update`; - + let request_body_json: string; - + if (allUsers) { // Update all users mode request_body_json = JSON.stringify({ @@ -4384,7 +4384,7 @@ export const userBulkUpdateUserCall = async ( }); } else if (userIds && userIds.length > 0) { // Update specific users mode - let request_body = [] + let request_body = [] for (const user_id of userIds) { request_body.push({ user_id: user_id, @@ -4397,7 +4397,7 @@ export const userBulkUpdateUserCall = async ( } else { throw new Error("Must provide either userIds or set allUsers=true"); } - + const response = await fetch(url, { method: "POST", headers: { @@ -5410,11 +5410,11 @@ export const convertPromptFileToJson = async ( try { const formData = new FormData(); formData.append("file", file); - - const url = proxyBaseUrl - ? `${proxyBaseUrl}/utils/dotprompt_json_converter` + + const url = proxyBaseUrl + ? `${proxyBaseUrl}/utils/dotprompt_json_converter` : `/utils/dotprompt_json_converter`; - + const response = await fetch(url, { method: "POST", headers: { @@ -5879,14 +5879,14 @@ export const callMCPTool = async ( if (!response.ok) { let errorMessage = "Network response was not ok"; let errorDetails = null; - + // First, try to get the response as text to see what we're dealing with const responseText = await response.text(); - + try { // Try to parse as JSON const errorData = JSON.parse(responseText); - + if (errorData.detail) { if (typeof errorData.detail === 'string') { errorMessage = errorData.detail; @@ -5897,7 +5897,7 @@ export const callMCPTool = async ( } else { errorMessage = errorData.message || errorData.error || errorMessage; } - + } catch (parseError) { console.error("Failed to parse JSON error response:", parseError); // If JSON parsing fails, use the raw text @@ -5905,13 +5905,13 @@ export const callMCPTool = async ( errorMessage = responseText; } } - + // Create a more informative error object const enhancedError = new Error(errorMessage); (enhancedError as any).status = response.status; (enhancedError as any).statusText = response.statusText; (enhancedError as any).details = errorDetails; - + handleError(errorMessage); throw enhancedError; } @@ -7126,9 +7126,9 @@ export const userAgentAnalyticsCall = async ( let url = proxyBaseUrl ? `${proxyBaseUrl}/tag/user-agent/analytics` : `/tag/user-agent/analytics`; - + const queryParams = new URLSearchParams(); - + // Format dates as YYYY-MM-DD for the API const formatDate = (date: Date) => { const year = date.getFullYear(); @@ -7136,16 +7136,16 @@ export const userAgentAnalyticsCall = async ( const day = String(date.getDate()).padStart(2, '0'); return `${year}-${month}-${day}`; }; - + queryParams.append("start_date", formatDate(startTime)); queryParams.append("end_date", formatDate(endTime)); queryParams.append("page", page.toString()); queryParams.append("page_size", pageSize.toString()); - + if (userAgentFilter) { queryParams.append("user_agent_filter", userAgentFilter); } - + const queryString = queryParams.toString(); if (queryString) { url += `?${queryString}`; @@ -7189,9 +7189,9 @@ export const tagDauCall = async ( let url = proxyBaseUrl ? `${proxyBaseUrl}/tag/dau` : `/tag/dau`; - + const queryParams = new URLSearchParams(); - + // Format date as YYYY-MM-DD for the API const formatDate = (date: Date) => { const year = date.getFullYear(); @@ -7199,9 +7199,9 @@ export const tagDauCall = async ( const day = String(date.getDate()).padStart(2, '0'); return `${year}-${month}-${day}`; }; - + queryParams.append("end_date", formatDate(endDate)); - + // Handle multiple tag filters (takes precedence over single tag filter) if (tagFilters && tagFilters.length > 0) { tagFilters.forEach(tag => { @@ -7210,7 +7210,7 @@ export const tagDauCall = async ( } else if (tagFilter) { queryParams.append("tag_filter", tagFilter); } - + const queryString = queryParams.toString(); if (queryString) { url += `?${queryString}`; @@ -7253,9 +7253,9 @@ export const tagWauCall = async ( let url = proxyBaseUrl ? `${proxyBaseUrl}/tag/wau` : `/tag/wau`; - + const queryParams = new URLSearchParams(); - + // Format date as YYYY-MM-DD for the API const formatDate = (date: Date) => { const year = date.getFullYear(); @@ -7263,9 +7263,9 @@ export const tagWauCall = async ( const day = String(date.getDate()).padStart(2, '0'); return `${year}-${month}-${day}`; }; - + queryParams.append("end_date", formatDate(endDate)); - + // Handle multiple tag filters (takes precedence over single tag filter) if (tagFilters && tagFilters.length > 0) { tagFilters.forEach(tag => { @@ -7274,7 +7274,7 @@ export const tagWauCall = async ( } else if (tagFilter) { queryParams.append("tag_filter", tagFilter); } - + const queryString = queryParams.toString(); if (queryString) { url += `?${queryString}`; @@ -7317,9 +7317,9 @@ export const tagMauCall = async ( let url = proxyBaseUrl ? `${proxyBaseUrl}/tag/mau` : `/tag/mau`; - + const queryParams = new URLSearchParams(); - + // Format date as YYYY-MM-DD for the API const formatDate = (date: Date) => { const year = date.getFullYear(); @@ -7327,9 +7327,9 @@ export const tagMauCall = async ( const day = String(date.getDate()).padStart(2, '0'); return `${year}-${month}-${day}`; }; - + queryParams.append("end_date", formatDate(endDate)); - + // Handle multiple tag filters (takes precedence over single tag filter) if (tagFilters && tagFilters.length > 0) { tagFilters.forEach(tag => { @@ -7338,7 +7338,7 @@ export const tagMauCall = async ( } else if (tagFilter) { queryParams.append("tag_filter", tagFilter); } - + const queryString = queryParams.toString(); if (queryString) { url += `?${queryString}`; @@ -7416,9 +7416,9 @@ export const userAgentSummaryCall = async ( let url = proxyBaseUrl ? `${proxyBaseUrl}/tag/summary` : `/tag/summary`; - + const queryParams = new URLSearchParams(); - + // Format dates as YYYY-MM-DD for the API const formatDate = (date: Date) => { const year = date.getFullYear(); @@ -7426,17 +7426,17 @@ export const userAgentSummaryCall = async ( const day = String(date.getDate()).padStart(2, '0'); return `${year}-${month}-${day}`; }; - + queryParams.append("start_date", formatDate(startTime)); queryParams.append("end_date", formatDate(endTime)); - + // Handle multiple tag filters if (tagFilters && tagFilters.length > 0) { tagFilters.forEach(tag => { queryParams.append("tag_filters", tag); }); } - + const queryString = queryParams.toString(); if (queryString) { url += `?${queryString}`; @@ -7479,19 +7479,19 @@ export const perUserAnalyticsCall = async ( let url = proxyBaseUrl ? `${proxyBaseUrl}/tag/user-agent/per-user-analytics` : `/tag/user-agent/per-user-analytics`; - + const queryParams = new URLSearchParams(); - + queryParams.append("page", page.toString()); queryParams.append("page_size", pageSize.toString()); - + // Handle multiple tag filters if (tagFilters && tagFilters.length > 0) { tagFilters.forEach(tag => { queryParams.append("tag_filters", tag); }); } - + const queryString = queryParams.toString(); if (queryString) { url += `?${queryString}`; diff --git a/ui/litellm-dashboard/src/utils/cookieUtils.test.ts b/ui/litellm-dashboard/src/utils/cookieUtils.test.ts new file mode 100644 index 00000000000..8adc94a89e4 --- /dev/null +++ b/ui/litellm-dashboard/src/utils/cookieUtils.test.ts @@ -0,0 +1,75 @@ +import { describe, it, expect, beforeEach, vi } from 'vitest'; +import { clearTokenCookies, getCookie } from './cookieUtils'; + +describe('cookieUtils', () => { + beforeEach(() => { + document.cookie.split(";").forEach((c) => { + document.cookie = c + .replace(/^ +/, "") + .replace(/=.*/, "=;expires=" + new Date().toUTCString() + ";path=/"); + }); + + vi.spyOn(console, 'log').mockImplementation(() => {}); + }); + + describe('clearTokenCookies', () => { + it('should clear token cookie from root path', () => { + document.cookie = 'token=test-token-value; path=/'; + expect(getCookie('token')).toBe('test-token-value'); + + clearTokenCookies(); + expect(getCookie('token')).toBeNull(); + }); + + it('should clear token cookie from /ui path', () => { + document.cookie = 'token=test-token-value; path=/ui'; + clearTokenCookies(); + expect(getCookie('token')).toBeNull(); + }); + + it('should clear token cookies with different SameSite values', () => { + document.cookie = 'token=test-lax; path=/; SameSite=Lax'; + clearTokenCookies(); + expect(getCookie('token')).toBeNull(); + + document.cookie = 'token=test-strict; path=/; SameSite=Strict'; + clearTokenCookies(); + expect(getCookie('token')).toBeNull(); + }); + + it('should handle multiple clearing attempts', () => { + document.cookie = 'token=test-value; path=/'; + + clearTokenCookies(); + clearTokenCookies(); + clearTokenCookies(); + + expect(getCookie('token')).toBeNull(); + }); + }); + + describe('getCookie', () => { + it('should return cookie value when it exists', () => { + document.cookie = 'token=my-test-token; path=/'; + expect(getCookie('token')).toBe('my-test-token'); + }); + + it('should return null when cookie does not exist', () => { + expect(getCookie('nonexistent')).toBeNull(); + }); + + it('should handle JWT tokens with special characters', () => { + const jwt = 'eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJ1c2VyX2lkIjoidGVzdCJ9.signature'; + document.cookie = `token=${jwt}; path=/`; + expect(getCookie('token')).toBe(jwt); + }); + + it('should return only the specified cookie', () => { + document.cookie = 'token=token-value; path=/'; + document.cookie = 'other=other-value; path=/'; + + expect(getCookie('token')).toBe('token-value'); + expect(getCookie('other')).toBe('other-value'); + }); + }); +});