fix(ui): source api-keys identity from useAuthorized to stop "User ID is not set" - #30903
Conversation
… is not set" The migrated /ui/api-keys route gates rendering on useAuthorized() but read userID from the AuthContext (useAuth), which hydrates asynchronously. On a hard refresh or deep link the route could render UserDashboard before AuthContext had populated userID, so UserDashboard hit its `userID == null` guard and showed "User ID is not set". The legacy index page avoided this by gating on AuthContext's own authLoading; the migration switched the gate to useAuthorized without aligning the identity source. Read identity from useAuthorized (a synchronous cookie decode) so userID is populated whenever the route is authorized. useAuth is kept only for the backfill setters UserDashboard still expects, until the planned AuthContext consolidation removes them. Refs LIT-3687
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
Greptile SummaryThis PR fixes a race condition on the
Confidence Score: 4/5Safe to merge — the change is narrowly scoped to ApiKeysDashboard's identity sourcing and is backed by a regression test that directly reproduces the reported failure condition. The fix correctly targets the async hydration race and the regression test properly pins it. Two observations worth noting: page.tsx already calls useAuthorized() and passes control to ApiKeysDashboard only once authorized, so ApiKeysDashboard invoking the hook a second time duplicates the cookie decode and useUIConfig subscription in the same render path. Separately, UserDashboard internally calls setUserRole/setUserEmail to backfill AuthContext, but since ApiKeysDashboard no longer reads those values from AuthContext, those writes no longer influence this component's rendering — the values remain consistent in practice because both sources decode the same JWT, and the PR explicitly acknowledges this as a temporary state. No files require special attention; both changed files are straightforward and well-scoped.
|
| Filename | Overview |
|---|---|
| ui/litellm-dashboard/src/app/(dashboard)/api-keys/ApiKeysDashboard.tsx | Redirects identity sourcing from AuthContext (async) to useAuthorized (sync cookie decode); adds premiumUser ?? false to satisfy the boolean prop type on UserDashboard. Creates a second invocation of useAuthorized in the same render path as page.tsx. |
| ui/litellm-dashboard/src/app/(dashboard)/api-keys/ApiKeysDashboard.test.tsx | New regression test that mocks AuthContext.userID as null while useAuthorized returns a populated userId, and asserts UserDashboard receives the correct id — correctly pins the reported failure condition. |
Reviews (1): Last reviewed commit: "fix(ui): source api-keys identity from u..." | Re-trigger Greptile
| const { userId: userID, userRole, userEmail, accessToken, premiumUser } = useAuthorized(); | ||
| const { setUserRole, setUserEmail } = useAuth(); |
There was a problem hiding this comment.
Redundant
useAuthorized() call in the same render path
page.tsx's ApiKeysPageContent already calls useAuthorized() and only renders ApiKeysDashboard once the hook confirms authorization. ApiKeysDashboard now calls the hook a second time in the same tree, resulting in a duplicate cookie read, two useMemo evaluations for decodeToken, and two subscriptions to useUIConfig(). Since ApiKeysPageContent already holds the full return value of useAuthorized(), the cleanest alternative is to accept the identity values as props from the parent rather than re-invoking the hook internally.
| // Identity comes from useAuthorized (synchronous cookie decode) so userID is set whenever the | ||
| // route is authorized; useAuth only supplies the backfill setters UserDashboard still expects. | ||
| const { userId: userID, userRole, userEmail, accessToken, premiumUser } = useAuthorized(); | ||
| const { setUserRole, setUserEmail } = useAuth(); |
There was a problem hiding this comment.
setUserRole/setUserEmail updates now orphaned from ApiKeysDashboard's render
UserDashboard decodes the JWT in its own useEffect and calls setUserRole/setUserEmail (lines 153 and 159 in user_dashboard.tsx) to backfill AuthContext. With the old code those AuthContext writes triggered a re-render of ApiKeysDashboard because it read userRole/userEmail from useAuth(). After this change ApiKeysDashboard reads those values from useAuthorized() (cookie-derived, immutable between renders), so the AuthContext writes from UserDashboard no longer affect this component. In practice the values are identical since both sources decode the same JWT, so behavior is unchanged. The PR comment acknowledges this is a temporary state pending AuthContext consolidation.
8125ddd
into
litellm_internal_staging
… is not set" (BerriAI#30903) The migrated /ui/api-keys route gates rendering on useAuthorized() but read userID from the AuthContext (useAuth), which hydrates asynchronously. On a hard refresh or deep link the route could render UserDashboard before AuthContext had populated userID, so UserDashboard hit its `userID == null` guard and showed "User ID is not set". The legacy index page avoided this by gating on AuthContext's own authLoading; the migration switched the gate to useAuthorized without aligning the identity source. Read identity from useAuthorized (a synchronous cookie decode) so userID is populated whenever the route is authorized. useAuth is kept only for the backfill setters UserDashboard still expects, until the planned AuthContext consolidation removes them. Refs LIT-3687
Relevant issues
Linear ticket
Refs LIT-3687
Pre-Submission checklist
Please complete all items before asking a LiteLLM maintainer to review your PR
make test-unit@greptileaiand received a Confidence Score of at least 4/5 before requesting a maintainer reviewCI (LiteLLM team)
Branch creation CI run
Link:
CI run for the last commit
Link:
Merge / cherry-pick CI run
Links:
Screenshots / Proof of Fix
Testers on the new App Router build saw "User ID is not set" on the Virtual Keys page when landing on it directly (deep link or hard refresh). Reproduce and verify against a live proxy:
http://localhost:4000/ui/api-keys/(or press the browser refresh while on Virtual Keys). The content area shows "User ID is not set" while the sidebar renders normallyWhy it happened: the migrated
/ui/api-keysroute gates rendering onuseAuthorized()(a synchronous cookie decode), but the dashboard readuserIDfrom the ReactAuthContext, which hydrates through async effects. The route could renderUserDashboardbeforeAuthContexthad setuserID, trippingUserDashboard'suserID == nullguard. The fix reads identity fromuseAuthorized, the same source the route already uses to authorize, so the two can no longer disagree.Type
🐛 Bug Fix
Changes
ApiKeysDashboardnow sourcesuserID,userRole,userEmail,accessToken, andpremiumUserfromuseAuthorized()instead ofuseAuth().useAuthorizeddecodes the auth cookie synchronously, so identity is available on the first render whenever the route is authorized, which removes the race againstAuthContext's asynchronous hydration.useAuthis retained only for thesetUserRole/setUserEmailbackfill setters thatUserDashboardstill expects; those go away with the plannedAuthContextconsolidation.The regression test mocks the exact failure condition (
useAuth().userIDstillnullwhileuseAuthorized().userIdis populated) and asserts thatUserDashboardreceives the populated id. It fails when identity is read fromuseAuthand passes when read fromuseAuthorized, so it pins the regression.