fix(ui): redirect to login only on session-expired 401, not on all 401s - #27
Merged
Merged
Conversation
The dashboard's handleError() only matched the exact string
"Authentication Error - Expired Key" to detect session expiry. Three
problems with that:
1. String-match is fragile — backend reworded the message (or returned
an empty message, after the auth log cleanup) and the redirect
stopped firing. Users saw 401 toasts and nothing happened.
2. The single marker missed every other session-loss variant: no auth
header, key revoked, invalid token, master-key-required, ...
3. No HTTP status awareness — both "session is gone" (401 with no
cookie) and "permission denied for this endpoint" (401 because
LiteLLM uses 401 for role mismatch too) hit the same path. The
"permission denied" case must NOT bounce the user to login because
they're already logged in fine; only this specific call fails.
Fix:
- Add `handleErrorResponse(response, errorData)` — status-aware,
preferred whenever the caller has the Response object:
* 401 + no auth cookie OR session-expired marker -> redirect to login
* 401 + cookie still present + no expiry marker -> toast, no redirect
(this is the role-mismatch case)
* 403 -> toast only, never redirect (permission denied)
* other status -> delegate to handleError (rate-limited path)
- Expand handleError's marker list (SESSION_EXPIRED_SIGNALS) to catch
more wording variants. handleError keeps its existing signature for
backward compat with the ~30 existing callers; new code should use
handleErrorResponse instead.
- Extract triggerSessionExpiredRedirect() so the "clear cookies +
redirect" mechanics live in one place across both code paths.
Test plan:
- vitest run src/components/networking.test.ts -> 21 passed
- New tests (5): handleErrorResponse with 401-no-cookie, 401-with-
expiry-marker, 401-still-valid-cookie, 403, 500 — verifies redirect
fires only when the session is actually gone.
Companion to backend PR that downgrades these auth failures from ERROR
to WARN — together they make 401 both quiet in logs and correctly
handled in UI.
3 tasks
songkuan-zheng
added a commit
that referenced
this pull request
May 29, 2026
…31) PROBLEM The upstream BerriAI/litellm flow commits the Next.js dashboard bundle to git at litellm/proxy/_experimental/out/ and the Dockerfile just COPYs it. docker/build_admin_ui.sh exists but is a no-op unless the enterprise_colors.json customization marker is present. For this fork that means: any UI source change in ui/litellm-dashboard/ src/ does NOT reach the runtime image — the image keeps serving whatever was last committed to _experimental/out/ (currently April 2026 upstream version). PR #25 (credential form reset), #27 (401 status-aware redirect v1), and #30 (auth error.type routing v2) all shipped UI source but their deployed effect on docker-built images is zero. The defect was only visible via e2e ("why doesn't the live bundle have my code?"). FIX Add an explicit UI build step in the Dockerfile builder stage that runs `npm ci && npm run build` from current source, then overwrites /app/litellm/proxy/_experimental/out/ with the fresh `out/` directory. node_modules and the build cache are pruned in the same RUN so the builder layer stays slim. The existing build_admin_ui.sh hook is kept as the enterprise customization path (no-op by default). Trade-offs vs the upstream commit-the-bundle approach: + UI source changes ALWAYS flow into the image (no "did I run npm run build?" foot-gun). + PR diffs stay clean — no more 100+ file _experimental/out/ churn per UI PR (the recent UI PRs would have been 685 fewer files). + Git history is honest about what's source vs build artifact. - docker build is 2-3 minutes slower (npm install + next build). - Diverges from upstream's pattern, so future upstream rebases that touch _experimental/out/ will need conflict resolution. Manageable because we're now ignoring that path and rebuilding ourselves. GITIGNORE litellm/proxy/_experimental/out/ is added to .gitignore along with the sibling next.js outputs (.next/, out/, node_modules under ui/litellm-dashboard/). 683 previously-tracked artifact files are removed via `git rm --cached` so the working tree stays usable for local non-docker dev (the files remain on disk) but they no longer add diff noise. LOCAL DEV NOTE Developers running the proxy outside docker need `cd ui/litellm-dashboard && npm run build` once to populate _experimental/out/. CLAUDE.md will be updated in a follow-up to reflect this; for now the comment in .gitignore documents the path. VERIFICATION - `git status` after this commit: clean except untracked _experimental/out/ on disk. - `e2e/tools/proxy build` (which invokes docker buildx against this Dockerfile) successfully produces an image. Tested in a follow-up step by stacking PR #30 onto this branch and verifying the served JS chunks contain the auth_session_expired / auth_permission_denied strings — proving the UI build step actually picks up source changes that were never committed to _experimental/out/.
songkuan-zheng
added a commit
that referenced
this pull request
Jun 4, 2026
…e (Wave 7 — final) (#61) * fix(ui): route 401/403 by backend error.type, heuristic only as fallback (#30) * fix(ui): route 401/403 by backend error.type, heuristic only as fallback Companion to fix/proxy-auth-error-type-taxonomy (PR D1) which gives the backend a structured `error.type` for every wrapped auth failure. This PR makes handleErrorResponse read that field FIRST and dispatch by table lookup, with the existing cookie + marker heuristic preserved as a fallback for: - older backend builds that don't emit the new types yet - `type=auth_error` (generic, backend couldn't classify) - unknown / typo'd / future type strings (graceful degradation) Three-tier decision: 1. errorData.error.type or errorData.type: auth_session_expired / auth_invalid_credentials → REDIRECT_LOGIN expired_key / token_not_found_in_db (legacy) → REDIRECT_LOGIN auth_permission_denied → TOAST key/team/user/org/project/vector_store *_access_denied → TOAST team_member_permission_error → TOAST budget_exceeded → TOAST auth_error or unknown → HEURISTIC (step 2) 2. status-based heuristic (legacy / unknown-type fallback): 401 + no cookie OR has marker → REDIRECT 401 + cookie + no marker → TOAST (safe default for 401-as-403) 403 → TOAST 3. non-auth errors → delegate to legacy handleError Why a static table instead of more code: the action is a pure function of the type. Adding a new auth_* type on the backend (e.g. for a new auth mode) requires one line here, no logic change. Test plan: - vitest run src/components/networking.test.ts → 35 passed - 14 new tests in "handleErrorResponse - type-based auth routing (D1 contract)" describe block: - REDIRECT_LOGIN: auth_session_expired (even with cookie present), auth_invalid_credentials, expired_key (legacy), token_not_found_in_db (legacy) - TOAST: auth_permission_denied (the original bug), key_model_access_denied, team_member_permission_error, budget_exceeded - HEURISTIC fallback: auth_error + cookie present (no redirect), auth_error + cookie absent (redirect), unknown type (heuristic) - Body-shape robustness: top-level type field, string body, no type field Graceful degradation: if D1 ships AFTER this PR, the legacy heuristic keeps the UI working. If D2 ships AFTER D1, the new types just route through the heuristic until D2 lands — no breakage either way. * fix(ui): make legacy handleError also read error.type Live e2e revealed handleErrorResponse was dead code — ~30 existing callers use handleError(errorData) directly, not handleErrorResponse. The new type-aware dispatch table only fired for tests, never for real fetch sites. Make the legacy handleError itself read errorData.error.type first and dispatch via AUTH_ERROR_TYPE_TO_ACTION: REDIRECT_LOGIN (auth_session_expired / auth_invalid_credentials / expired_key / token_not_found_in_db) -> triggerSessionExpiredRedirect() TOAST (auth_permission_denied / *_access_denied / *_permission_error / budget_exceeded) -> do nothing here (preserves legacy "silent for non-session" behavior — caller decides whether to toast) HEURISTIC / missing / unknown -> existing message-marker scan + redirect-on-marker This way every existing caller automatically benefits from D1's backend taxonomy with zero call-site migration. Live e2e (rebuilt docker bundle + actual served chunks): 9/9 cases pass — for each backend-emitted type, the live JS chunk maps it to the expected REDIRECT_LOGIN / TOAST action: C1 no-auth-header -> auth_invalid_credentials -> REDIRECT_LOGIN C2 invalid-bearer -> auth_invalid_credentials -> REDIRECT_LOGIN C3 admin-only -> auth_permission_denied -> TOAST (the bug) C5 empty-bearer -> auth_invalid_credentials -> REDIRECT_LOGIN C6 admin+nokey -> auth_invalid_credentials -> REDIRECT_LOGIN C8 unknown-sk-key -> token_not_found_in_db -> REDIRECT_LOGIN C10 unknown-sk-chat -> token_not_found_in_db -> REDIRECT_LOGIN C11 deleted-key -> token_not_found_in_db -> REDIRECT_LOGIN C12 lowpriv-userlist -> (no type, 403) -> TOAST Test plan: - vitest run src/components/networking.test.ts -> 43 passed (5 new "handleError (legacy) - now also reads error.type" cases covering session_expired / invalid_credentials / token_not_found_in_db -> REDIRECT, permission_denied / budget_exceeded -> no redirect, marker fallback when no type, type=auth_error fallback) - Live e2e: built UI -> copied to litellm/proxy/_experimental/out/ -> e2e/tools/proxy build -> 9/9 contract tests pass against live container * fix(ui): redirect to login on session-expired 401, not on all 401s The dashboard's handleError() only matched the exact string "Authentication Error - Expired Key" to detect session expiry. Three problems with that: 1. String-match is fragile — backend reworded the message (or returned an empty message, after the auth log cleanup) and the redirect stopped firing. Users saw 401 toasts and nothing happened. 2. The single marker missed every other session-loss variant: no auth header, key revoked, invalid token, master-key-required, ... 3. No HTTP status awareness — both "session is gone" (401 with no cookie) and "permission denied for this endpoint" (401 because LiteLLM uses 401 for role mismatch too) hit the same path. The "permission denied" case must NOT bounce the user to login because they're already logged in fine; only this specific call fails. Fix: - Add `handleErrorResponse(response, errorData)` — status-aware, preferred whenever the caller has the Response object: * 401 + no auth cookie OR session-expired marker -> redirect to login * 401 + cookie still present + no expiry marker -> toast, no redirect (this is the role-mismatch case) * 403 -> toast only, never redirect (permission denied) * other status -> delegate to handleError (rate-limited path) - Expand handleError's marker list (SESSION_EXPIRED_SIGNALS) to catch more wording variants. handleError keeps its existing signature for backward compat with the ~30 existing callers; new code should use handleErrorResponse instead. - Extract triggerSessionExpiredRedirect() so the "clear cookies + redirect" mechanics live in one place across both code paths. Test plan: - vitest run src/components/networking.test.ts -> 21 passed - New tests (5): handleErrorResponse with 401-no-cookie, 401-with- expiry-marker, 401-still-valid-cookie, 403, 500 — verifies redirect fires only when the session is actually gone. Companion to backend PR that downgrades these auth failures from ERROR to WARN — together they make 401 both quiet in logs and correctly handled in UI. * fix(ui): redirect to login on 401 token_not_found_in_db (#38) * fix(ui): redirect to login on 401 token_not_found_in_db (and siblings) Two-part fix for the bug where the dashboard never redirects to the login page on 401 errors with `error.type=token_not_found_in_db` (and other AUTH_ERROR_TYPE_TO_ACTION REDIRECT_LOGIN types). Patch 1 — root cause: fetch callsites were calling `handleError(deriveErrorMessage(errorData))`, pre-stringifying the error before `handleError` could read `error.type`. `extractErrorType` explicitly rejects strings, so the type-aware fast path never fired. Now callsites pass the raw `errorData` object; `handleError`'s already-implemented D1 taxonomy dispatch finally takes effect. Affects 178 callsites across `networking.tsx` (163) and `src/app/(dashboard)/hooks/` (15 files). Patch 2 — defense in depth: the string-based fallback `SESSION_EXPIRED_SIGNALS` listed `"Authentication Error - Invalid"` (dash) but the backend's token-not-found path emits `"Authentication Error, Invalid"` (comma). Added the comma variant so the heuristic path also catches it when the structured `type` field is absent (older backends). The existing test `handleError redirects when error.type= token_not_found_in_db` (networking.test.ts:394) already asserted the intended behavior; this PR makes production match the test. Updated `useAccessGroups.test.ts` to reflect the new contract (callers pass the object, not the derived message string). * fix(ui): handle MCP tool call errorData scope after Patch 1 sed sweep The previous commit (Patch 1) bulk-replaced `handleError(errorMessage)` with `handleError(errorData)` across 178 callsites. One callsite — the MCP tool call error path — declared `errorData` inside a `try` block, so the replacement referenced an out-of-scope variable and broke the Next.js TypeScript compile (verified via `next build` in the litellm-e2e Docker image rebuild). Fix: hoist `let errorData: any = null` outside the try; assign inside on successful JSON parse. Pass `errorData ?? responseText` to handleError so the structured-type path still fires when JSON is valid, falling back to the raw text otherwise (which `handleError` already handles via its string-marker heuristic). Verified end-to-end against the e2e Docker image: login → inject fake JWT (valid format, key field points to non-existent virtual key) → navigate to /ui/?page=teams → React fires /team/list and friends → backend returns 401 with `error.type=auth_invalid_credentials` → handleError reads the type → triggerSessionExpiredRedirect → window.location.href to /ui/ → server-side guard sends to /ui/login. Critically, the error message "Authentication Error, LiteLLM Virtual Key expected..." does NOT match any SESSION_EXPIRED_SIGNALS marker, so this redirect path is reachable ONLY via the structured-type channel (Patch 1), confirming the fix's contract. * build(docker): rebuild dashboard UI from source on every image build (#31) PROBLEM The upstream BerriAI/litellm flow commits the Next.js dashboard bundle to git at litellm/proxy/_experimental/out/ and the Dockerfile just COPYs it. docker/build_admin_ui.sh exists but is a no-op unless the enterprise_colors.json customization marker is present. For this fork that means: any UI source change in ui/litellm-dashboard/ src/ does NOT reach the runtime image — the image keeps serving whatever was last committed to _experimental/out/ (currently April 2026 upstream version). PR #25 (credential form reset), #27 (401 status-aware redirect v1), and #30 (auth error.type routing v2) all shipped UI source but their deployed effect on docker-built images is zero. The defect was only visible via e2e ("why doesn't the live bundle have my code?"). FIX Add an explicit UI build step in the Dockerfile builder stage that runs `npm ci && npm run build` from current source, then overwrites /app/litellm/proxy/_experimental/out/ with the fresh `out/` directory. node_modules and the build cache are pruned in the same RUN so the builder layer stays slim. The existing build_admin_ui.sh hook is kept as the enterprise customization path (no-op by default). Trade-offs vs the upstream commit-the-bundle approach: + UI source changes ALWAYS flow into the image (no "did I run npm run build?" foot-gun). + PR diffs stay clean — no more 100+ file _experimental/out/ churn per UI PR (the recent UI PRs would have been 685 fewer files). + Git history is honest about what's source vs build artifact. - docker build is 2-3 minutes slower (npm install + next build). - Diverges from upstream's pattern, so future upstream rebases that touch _experimental/out/ will need conflict resolution. Manageable because we're now ignoring that path and rebuilding ourselves. GITIGNORE litellm/proxy/_experimental/out/ is added to .gitignore along with the sibling next.js outputs (.next/, out/, node_modules under ui/litellm-dashboard/). 683 previously-tracked artifact files are removed via `git rm --cached` so the working tree stays usable for local non-docker dev (the files remain on disk) but they no longer add diff noise. LOCAL DEV NOTE Developers running the proxy outside docker need `cd ui/litellm-dashboard && npm run build` once to populate _experimental/out/. CLAUDE.md will be updated in a follow-up to reflect this; for now the comment in .gitignore documents the path. VERIFICATION - `git status` after this commit: clean except untracked _experimental/out/ on disk. - `e2e/tools/proxy build` (which invokes docker buildx against this Dockerfile) successfully produces an image. Tested in a follow-up step by stacking PR #30 onto this branch and verifying the served JS chunks contain the auth_session_expired / auth_permission_denied strings — proving the UI build step actually picks up source changes that were never committed to _experimental/out/.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
The dashboard's
handleError()redirected to login by string-matching the literal"Authentication Error - Expired Key". Three failure modes:Change
ui/litellm-dashboard/src/components/networking.tsx:handleErrorResponse(response, errorData)— status-aware, preferred whenever the caller has theResponseobject:handleError(rate-limited fallback path)SESSION_EXPIRED_SIGNALSlist — expanded from one marker to seven so legacyhandleErrorcallers also catch more variants when the body still looks string-shaped.triggerSessionExpiredRedirect()— extracted so the "clear cookies + redirect" mechanics live in one place across both paths.handleErrorsignature unchanged — backward-compatible with the ~30 existing callers. New code should usehandleErrorResponsefor status-aware behavior.Test plan
vitest run src/components/networking.test.ts→ 21 passedhandleErrorResponse - status-aware auth handlingdescribe:redirects on 401 when the auth cookie is gone (session expired)redirects on 401 when the body carries a session-expired markerdoes NOT redirect on 401 when the cookie is still valid and body says no session-expired marker(role-mismatch case)does NOT redirect on 403 (permission denied)falls through to handleError for non-401/403 errorsCompanion PR
This pairs with #26 (backend) — that one demoted auth-failure ERROR+traceback to WARN, which is what made the body messages go empty on some 401s (the issue this PR addresses). Together they make 401 both quiet in logs and correctly handled in UI.
Note on rollout
Existing callers continue using the legacy
handleError(errorData)API — the broaderSESSION_EXPIRED_SIGNALSlist immediately benefits them. Migrating individual fetch sites tohandleErrorResponse(response, errorData)for the full status-aware behavior can happen incrementally in follow-up PRs.