-
-
Notifications
You must be signed in to change notification settings - Fork 11.6k
fix(ui): source api-keys identity from useAuthorized to stop "User ID is not set" #30903
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,63 @@ | ||
| import { render } from "@testing-library/react"; | ||
| import { describe, it, expect, vi } from "vitest"; | ||
|
|
||
| const { userDashboardSpy } = vi.hoisted(() => ({ | ||
| userDashboardSpy: vi.fn((_props: Record<string, unknown>) => null), | ||
| })); | ||
|
|
||
| vi.mock("@/components/user_dashboard", () => ({ | ||
| default: (props: Record<string, unknown>) => userDashboardSpy(props), | ||
| })); | ||
|
|
||
| // AuthContext is still hydrating: userID has not been populated yet (the regression). | ||
| vi.mock("@/contexts/AuthContext", () => ({ | ||
| useAuth: () => ({ | ||
| userID: null, | ||
| userRole: "", | ||
| userEmail: null, | ||
| accessToken: null, | ||
| premiumUser: false, | ||
| setUserRole: vi.fn(), | ||
| setUserEmail: vi.fn(), | ||
| }), | ||
| })); | ||
|
|
||
| // useAuthorized decodes the cookie synchronously, so identity is already available. | ||
| vi.mock("@/app/(dashboard)/hooks/useAuthorized", () => ({ | ||
| default: () => ({ | ||
| isLoading: false, | ||
| isAuthorized: true, | ||
| token: "jwt", | ||
| accessToken: "sk-access", | ||
| userId: "u-123", | ||
| userEmail: "admin@example.com", | ||
| userRole: "Admin", | ||
| premiumUser: false, | ||
| disabledPersonalKeyCreation: false, | ||
| showSSOBanner: false, | ||
| }), | ||
| })); | ||
|
|
||
| vi.mock("@/app/(dashboard)/hooks/teams/useTeams", () => ({ | ||
| teamListCall: vi.fn(() => new Promise(() => {})), | ||
| })); | ||
|
|
||
| vi.mock("@/components/organizations", () => ({ | ||
| fetchOrganizations: vi.fn(), | ||
| })); | ||
|
|
||
| vi.mock("next/navigation", () => ({ | ||
| useSearchParams: () => new URLSearchParams(""), | ||
| })); | ||
|
|
||
| import ApiKeysDashboard from "./ApiKeysDashboard"; | ||
|
|
||
| describe("ApiKeysDashboard identity source", () => { | ||
| it("passes the useAuthorized userID through even while AuthContext.userID is still null", () => { | ||
| render(<ApiKeysDashboard />); | ||
|
|
||
| expect(userDashboardSpy).toHaveBeenCalled(); | ||
| const props = userDashboardSpy.mock.calls[0][0]; | ||
| expect(props.userID).toBe("u-123"); | ||
| }); | ||
| }); |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,6 +1,7 @@ | ||
| "use client"; | ||
|
|
||
| import { teamListCall as v2TeamListCall } from "@/app/(dashboard)/hooks/teams/useTeams"; | ||
| import useAuthorized from "@/app/(dashboard)/hooks/useAuthorized"; | ||
| import { KeyResponse, Team } from "@/components/key_team_helpers/key_list"; | ||
| import { Organization } from "@/components/networking"; | ||
| import { CreateKeyPrefillData } from "@/components/organisms/create_key_button"; | ||
|
|
@@ -11,7 +12,10 @@ import { useSearchParams } from "next/navigation"; | |
| import { useEffect, useMemo, useState } from "react"; | ||
|
|
||
| export default function ApiKeysDashboard() { | ||
| const { userID, userRole, userEmail, accessToken, premiumUser, setUserRole, setUserEmail } = useAuth(); | ||
| // 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(); | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||
| const searchParams = useSearchParams()!; | ||
|
|
||
| const [teams, setTeams] = useState<Team[] | null>(null); | ||
|
|
@@ -82,7 +86,7 @@ export default function ApiKeysDashboard() { | |
| <UserDashboard | ||
| userID={userID} | ||
| userRole={userRole} | ||
| premiumUser={premiumUser} | ||
| premiumUser={premiumUser ?? false} | ||
| teams={teams} | ||
| keys={keys} | ||
| setUserRole={setUserRole} | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
useAuthorized()call in the same render pathpage.tsx'sApiKeysPageContentalready callsuseAuthorized()and only rendersApiKeysDashboardonce the hook confirms authorization.ApiKeysDashboardnow calls the hook a second time in the same tree, resulting in a duplicate cookie read, twouseMemoevaluations fordecodeToken, and two subscriptions touseUIConfig(). SinceApiKeysPageContentalready holds the full return value ofuseAuthorized(), the cleanest alternative is to accept the identity values as props from the parent rather than re-invoking the hook internally.