fix(ui): route 401/403 by backend error.type, heuristic only as fallback - #30
Merged
Merged
Conversation
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.
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
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/.
1 task
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/.
songkuan-zheng
added a commit
that referenced
this pull request
Jun 5, 2026
…working.tsx (#68) Wave 7 of the v1.87.0 bump originally added structured auth error handling to ui/litellm-dashboard/src/components/networking.tsx via three commits: - be8a895 fix(ui): route 401/403 by backend error.type, heuristic only as fallback (#30) - f73ae47 fix(ui): redirect to login on session-expired 401, not on all 401s - 0383afb fix(ui): redirect to login on 401 token_not_found_in_db (#38) PR #61 dropped them with a careless `git checkout --theirs` during the v1.87.0 cherry-pick. PRs #62 and #63 reset networking.tsx + networking.test.ts to upstream verbatim to unblock the Docker build. This PR re-layers that work on top of the upstream-clean baseline. The matching backend change (`_classify_auth_failure` in litellm/proxy/auth/auth_exception_handler.py, Wave 6a) is already in production. It emits `error.type` with one of: - auth_session_expired → REDIRECT_LOGIN - auth_invalid_credentials → REDIRECT_LOGIN - token_not_found_in_db → REDIRECT_LOGIN (legacy specific type) - expired_key → REDIRECT_LOGIN (legacy specific type) - auth_permission_denied → TOAST (the original bug) - *_model_access_denied → TOAST - team_member_permission_error → TOAST - budget_exceeded → TOAST - auth_error → HEURISTIC (legacy marker fallback) Three layers added to networking.tsx: 1. AUTH_ERROR_TYPE_TO_ACTION dispatch table + extractErrorType helper. 2. Status-aware handleErrorResponse(response, errorData) — preferred when caller has the Response object. Three-tier decision: structured type → status+cookie heuristic → delegate to legacy. 3. Legacy handleError(errorData) now also reads error.type FIRST. This was the live-e2e bug fix (commit 2 of #30): the ~30 existing fetch sites call handleError directly, not handleErrorResponse, so making the legacy entry point smart auto-propagates the D1 taxonomy contract without touching call sites. Callsite migration (commit 3 of #38, Patch 1): 163 fetch sites in networking.tsx were calling handleError(errorMessage) — pre-stringifying the body before handleError could read error.type. Bulk-replaced with handleError(errorData) so the structured-type path actually fires in production. The MCP tool call path has a special errorData scope (declared inside `try`), so hoist `let errorData: any = null` outside the try and pass `errorData ?? responseText` to handleError. Tests: 38 new test cases across three describe blocks: - handleErrorResponse - status-aware auth handling (5 tests) - handleErrorResponse - type-based auth routing (D1 contract) (14 tests) - handleError (legacy) - now also reads error.type (8 tests) - existing networking - expired session handling (3 tests, unchanged) Total: 45 tests pass (was 7). Verification: - npm run build → ✓ Compiled successfully in 26.2s, 37 static pages - npx vitest run src/components/networking.test.ts → 45/45 pass Tier: C (universal bug fix — auth error UX correctness) Tried upstream first? No — this is companion code to our backend _classify_auth_failure (Wave 6a) which is itself an internal Tier D mechanism. The structured-type contract is the carry; upstream may adopt a similar taxonomy independently. Conflict resolutions: N/A (clean re-application on upstream-clean baseline, no `git cherry-pick` was used — manual port of the three source commits' intent onto upstream's current networking.tsx structure).
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
Companion to #29 (backend D1) which gives every wrapped auth failure a structured
error.type. This PR makes the dashboard'shandleErrorResponseread that field first and dispatch via a static lookup table. The previous cookie + marker heuristic stays as a graceful-degradation fallback.Three-tier decision
errorData.error.typeorerrorData.typematches a known auth typeAUTH_ERROR_TYPE_TO_ACTIONtableauth_error)handleErrorAction table
typeauth_session_expiredauth_invalid_credentialsexpired_key(legacy, predates D1)token_not_found_in_db(legacy)auth_permission_deniedkey_model_access_denied/team_model_access_denied/user_model_access_denied/org_model_access_denied/project_model_access_denied/key_vector_store_access_deniedteam_member_permission_errorbudget_exceededauth_error(backend couldn't classify)Why a static table
The action is a pure function of the type. Adding a new auth type on the backend (e.g. for a new auth mode) is one line here — no logic change, no test rewriting beyond a new parametrized case.
Test plan
vitest run src/components/networking.test.ts→ 35 passed (14 new in thetype-based auth routing (D1 contract)describe block)auth_session_expired(even when cookie still present — the type wins),auth_invalid_credentials,expired_key,token_not_found_in_dbauth_permission_denied(the original bug),key_model_access_denied,team_member_permission_error,budget_exceededtype=auth_error+ cookie present (no redirect, safe default),type=auth_error+ cookie absent (redirect), unknown typetype(admin-endpoint shape), string body (legacy), missingtypefieldGraceful degradation
Order of D1 + D2 deployment doesn't matter:
type=auth_error(current backend), routes to heuristic — keeps workingCompanion to