diff --git a/src/components/shared/SecretsManagement/SelectSecretDialog.tsx b/src/components/shared/SecretsManagement/SelectSecretDialog.tsx index 6650e82bbe..389856e01a 100644 --- a/src/components/shared/SecretsManagement/SelectSecretDialog.tsx +++ b/src/components/shared/SecretsManagement/SelectSecretDialog.tsx @@ -22,8 +22,8 @@ import { useTourMockBackend } from "@/providers/TourProvider/tourMockBackend"; import { AddSecretForm } from "./components/AddSecretForm"; import { SecretsBackendUnavailable } from "./components/SecretsBackendUnavailable"; -import { fetchSecretsList } from "./secretsStorage"; -import { type Secret, SecretsQueryKeys } from "./types"; +import { secretsListQuery } from "./secretsStorage"; +import type { Secret } from "./types"; type DialogMode = "select" | "add"; @@ -107,10 +107,7 @@ interface SelectSecretDialogContentProps { function SelectSecretDialogContentInternal({ onSelect, }: SelectSecretDialogContentProps) { - const { data: secrets } = useSuspenseQuery({ - queryKey: SecretsQueryKeys.All(), - queryFn: fetchSecretsList, - }); + const { data: secrets } = useSuspenseQuery(secretsListQuery()); const [mode, setMode] = useState("select"); diff --git a/src/components/shared/SecretsManagement/components/ReplaceSecretView.tsx b/src/components/shared/SecretsManagement/components/ReplaceSecretView.tsx index 70b87d8b58..4c48db4474 100644 --- a/src/components/shared/SecretsManagement/components/ReplaceSecretView.tsx +++ b/src/components/shared/SecretsManagement/components/ReplaceSecretView.tsx @@ -8,8 +8,7 @@ import { Paragraph } from "@/components/ui/typography"; import useToastNotification from "@/hooks/useToastNotification"; import { useAnalytics } from "@/providers/AnalyticsProvider"; -import { fetchSecretsList } from "../secretsStorage"; -import { SecretsQueryKeys } from "../types"; +import { secretsListQuery } from "../secretsStorage"; import { AddSecretForm } from "./AddSecretForm"; import { SecretsBreadcrumbs } from "./SecretsBreadcrumbs"; @@ -19,10 +18,7 @@ export function ReplaceSecretView() { const navigate = useNavigate(); const { track } = useAnalytics(); - const { data: secrets } = useSuspenseQuery({ - queryKey: SecretsQueryKeys.All(), - queryFn: fetchSecretsList, - }); + const { data: secrets } = useSuspenseQuery(secretsListQuery()); const secret = secrets.find((s) => s.id === secretId); diff --git a/src/components/shared/SecretsManagement/components/SecretsList.tsx b/src/components/shared/SecretsManagement/components/SecretsList.tsx index 1c867c5be9..305ab17759 100644 --- a/src/components/shared/SecretsManagement/components/SecretsList.tsx +++ b/src/components/shared/SecretsManagement/components/SecretsList.tsx @@ -12,8 +12,8 @@ import { useTourMockBackend } from "@/providers/TourProvider/tourMockBackend"; import { formatRelativeTime } from "@/utils/date"; import { withSuspenseWrapper } from "../../SuspenseWrapper"; -import { fetchSecretsList } from "../secretsStorage"; -import { type Secret, SecretsQueryKeys } from "../types"; +import { secretsListQuery } from "../secretsStorage"; +import type { Secret } from "../types"; import { RemoveSecretButton } from "./RemoveSecretButton"; import { SecretsBackendUnavailable } from "./SecretsBackendUnavailable"; @@ -26,10 +26,7 @@ function SecretsListInternal({ onRemoveSuccess, onEditSecret, }: SecretsListProps) { - const { data: secrets } = useSuspenseQuery({ - queryKey: SecretsQueryKeys.All(), - queryFn: fetchSecretsList, - }); + const { data: secrets } = useSuspenseQuery(secretsListQuery()); if (secrets.length === 0) { return ( diff --git a/src/components/shared/SecretsManagement/secretsStorage.test.ts b/src/components/shared/SecretsManagement/secretsStorage.test.ts new file mode 100644 index 0000000000..617466cfee --- /dev/null +++ b/src/components/shared/SecretsManagement/secretsStorage.test.ts @@ -0,0 +1,236 @@ +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; + +vi.mock("@/api/sdk.gen", () => ({ + createSecretApiSecretsPost: vi.fn(), + deleteSecretApiSecretsSecretNameDelete: vi.fn(), + listSecretsApiSecretsGet: vi.fn(), + updateSecretApiSecretsSecretNamePut: vi.fn(), +})); + +vi.mock("@/providers/TourProvider/tourMockBackend", () => ({ + isTourMockActive: () => false, + mockAddSecret: vi.fn(), + mockListSecrets: vi.fn(), + mockRemoveSecret: vi.fn(), + mockUpdateSecret: vi.fn(), +})); + +import { + createSecretApiSecretsPost, + deleteSecretApiSecretsSecretNameDelete, + listSecretsApiSecretsGet, + updateSecretApiSecretsSecretNamePut, +} from "@/api/sdk.gen"; +import { TeamImpersonationError } from "@/services/teams/errors"; + +import { + addSecret, + fetchSecretsList, + removeSecret, + SecretsApiError, + secretsListQuery, + updateSecret, +} from "./secretsStorage"; + +const TEAM_ID = "ml-team@project.iam.gserviceaccount.com"; +const HEADER_NAME = "x-tangle-impersonate-team-id"; +const TEAM_HEADERS = { [HEADER_NAME]: TEAM_ID }; + +const sdkResult = (status: number, body: { data?: unknown; error?: unknown }) => + ({ + data: body.data, + error: body.error, + request: new Request("http://localhost/api/secrets/"), + response: new Response(null, { status }), + }) as never; + +describe("secretsStorage", () => { + beforeEach(() => { + vi.clearAllMocks(); + window.__TANGLE_TEAM_IMPERSONATION__ = { headerName: HEADER_NAME }; + }); + + afterEach(() => { + delete window.__TANGLE_TEAM_IMPERSONATION__; + }); + + describe("fetchSecretsList", () => { + beforeEach(() => { + vi.mocked(listSecretsApiSecretsGet).mockResolvedValue( + sdkResult(200, { + data: { + secrets: [ + { + secret_name: "API_KEY", + created_at: "2026-10-01T10:00:00", + updated_at: "2026-10-02T10:00:00Z", + expires_at: "2026-12-31T23:59:59Z", + description: "For the model provider", + }, + ], + }, + }), + ); + }); + + it("lists the user's own secrets without the team header", async () => { + await expect(fetchSecretsList()).resolves.toEqual([ + { + id: "API_KEY", + name: "API_KEY", + createdAt: new Date("2026-10-01T10:00:00Z"), + updatedAt: new Date("2026-10-02T10:00:00Z"), + expiresAt: new Date("2026-12-31T23:59:59Z"), + description: "For the model provider", + }, + ]); + expect(listSecretsApiSecretsGet).toHaveBeenCalledWith({ headers: {} }); + }); + + it("lists a team's secrets while acting as the team", async () => { + await fetchSecretsList(TEAM_ID); + + expect(listSecretsApiSecretsGet).toHaveBeenCalledWith({ + headers: TEAM_HEADERS, + }); + }); + + it("throws a TeamImpersonationError when the backend refuses the team", async () => { + const refusal = `Impersonation error: Unknown team ${TEAM_ID}`; + vi.mocked(listSecretsApiSecretsGet).mockResolvedValue( + sdkResult(401, { error: { detail: refusal } }), + ); + + const failure = fetchSecretsList(TEAM_ID); + + await expect(failure).rejects.toBeInstanceOf(TeamImpersonationError); + await expect(failure).rejects.toMatchObject({ + message: refusal, + status: 401, + }); + }); + }); + + describe("secretsListQuery", () => { + it("keeps a team's secrets apart from the user's in the cache", () => { + expect(secretsListQuery().queryKey).toEqual(["secrets", "user"]); + expect(secretsListQuery(TEAM_ID).queryKey).toEqual([ + "secrets", + "team", + TEAM_ID, + ]); + }); + + it("fetches the secrets of the team its key names", async () => { + vi.mocked(listSecretsApiSecretsGet).mockResolvedValue( + sdkResult(200, { data: { secrets: [] } }), + ); + + await secretsListQuery(TEAM_ID).queryFn(); + + expect(listSecretsApiSecretsGet).toHaveBeenCalledWith({ + headers: TEAM_HEADERS, + }); + }); + }); + + describe("addSecret", () => { + it("sends the description and expiry with the team header", async () => { + vi.mocked(createSecretApiSecretsPost).mockResolvedValue( + sdkResult(200, { data: {} }), + ); + + await addSecret( + { + name: "API_KEY", + value: "s3cret", + description: "For the model provider", + expiresAt: new Date("2026-12-31T23:59:59.999Z"), + }, + TEAM_ID, + ); + + expect(createSecretApiSecretsPost).toHaveBeenCalledWith({ + query: { + secret_name: "API_KEY", + description: "For the model provider", + expires_at: "2026-12-31T23:59:59.999Z", + }, + body: { secret_value: "s3cret" }, + headers: TEAM_HEADERS, + }); + }); + + it("carries the backend's reason when it refuses", async () => { + vi.mocked(createSecretApiSecretsPost).mockResolvedValue( + sdkResult(409, { + error: { detail: "Secret API_KEY already exists" }, + }), + ); + + const failure = addSecret({ name: "API_KEY", value: "s3cret" }); + + await expect(failure).rejects.toBeInstanceOf(SecretsApiError); + await expect(failure).rejects.toMatchObject({ + message: "Failed to add secret: Secret API_KEY already exists", + status: 409, + }); + }); + }); + + describe("updateSecret", () => { + it("replaces the value, description and expiry with the team header", async () => { + vi.mocked(updateSecretApiSecretsSecretNamePut).mockResolvedValue( + sdkResult(200, { data: {} }), + ); + + await updateSecret( + "API_KEY", + { + value: "n3w", + description: "Rotated", + expiresAt: new Date("2027-01-31T23:59:59.999Z"), + }, + TEAM_ID, + ); + + expect(updateSecretApiSecretsSecretNamePut).toHaveBeenCalledWith({ + path: { secret_name: "API_KEY" }, + query: { + description: "Rotated", + expires_at: "2027-01-31T23:59:59.999Z", + }, + body: { secret_value: "n3w" }, + headers: TEAM_HEADERS, + }); + }); + }); + + describe("removeSecret", () => { + it("removes a team's secret with the team header", async () => { + vi.mocked(deleteSecretApiSecretsSecretNameDelete).mockResolvedValue( + sdkResult(200, { data: {} }), + ); + + await removeSecret("API_KEY", TEAM_ID); + + expect(deleteSecretApiSecretsSecretNameDelete).toHaveBeenCalledWith({ + path: { secret_name: "API_KEY" }, + headers: TEAM_HEADERS, + }); + }); + + it("removes the user's own secret without it", async () => { + vi.mocked(deleteSecretApiSecretsSecretNameDelete).mockResolvedValue( + sdkResult(200, { data: {} }), + ); + + await removeSecret("API_KEY"); + + expect(deleteSecretApiSecretsSecretNameDelete).toHaveBeenCalledWith({ + path: { secret_name: "API_KEY" }, + headers: {}, + }); + }); + }); +}); diff --git a/src/components/shared/SecretsManagement/secretsStorage.ts b/src/components/shared/SecretsManagement/secretsStorage.ts index 4842838745..30c0d58333 100644 --- a/src/components/shared/SecretsManagement/secretsStorage.ts +++ b/src/components/shared/SecretsManagement/secretsStorage.ts @@ -11,8 +11,15 @@ import { mockRemoveSecret, mockUpdateSecret, } from "@/providers/TourProvider/tourMockBackend"; +import { ApiError } from "@/services/projects/errors"; +import { describeErrorBody } from "@/services/requestFailureMessage"; +import { + readImpersonationRefusal, + TeamImpersonationError, +} from "@/services/teams/errors"; +import { teamRequestHeaders } from "@/services/teams/teamRequestHeaders"; -import type { Secret } from "./types"; +import { type Secret, SecretsQueryKeys } from "./types"; /** * Parses a date string, assuming UTC if no timezone is specified. @@ -39,15 +46,37 @@ function parseAsUtc(dateString: string): Date { return new Date(dateString + "Z"); } -export async function fetchSecretsList() { +export class SecretsApiError extends ApiError { + constructor(message: string, status: number) { + super("SecretsApiError", message, status); + } +} + +function secretsFailure( + action: string, + result: { error: unknown; response: Response }, +): ApiError { + const status = result.response.status; + const refusal = readImpersonationRefusal(result.error); + if (refusal) { + return new TeamImpersonationError(refusal, status); + } + + const reason = describeErrorBody(result.error); + return new SecretsApiError(reason ? `${action}: ${reason}` : action, status); +} + +export async function fetchSecretsList(teamId?: string) { if (isTourMockActive()) { return mockListSecrets(); } - const response = await listSecretsApiSecretsGet(); + const response = await listSecretsApiSecretsGet({ + headers: teamRequestHeaders(teamId), + }); if (response.response.status !== 200) { - throw new Error(`Failed to fetch secrets: ${response.response.body}`); + throw secretsFailure("Failed to fetch secrets", response); } return ( @@ -67,9 +96,17 @@ export async function fetchSecretsList() { ); } +export function secretsListQuery(teamId?: string) { + return { + queryKey: SecretsQueryKeys.All(teamId), + queryFn: () => fetchSecretsList(teamId), + }; +} + export async function updateSecret( secretId: string, secret: Partial & Pick, + teamId?: string, ) { if (isTourMockActive()) { mockUpdateSecret(secretId, secret.value); @@ -80,13 +117,18 @@ export async function updateSecret( path: { secret_name: secretId, }, + query: { + description: secret.description, + expires_at: secret.expiresAt?.toISOString(), + }, body: { secret_value: secret.value ?? "", }, + headers: teamRequestHeaders(teamId), }); if (response.response.status !== 200) { - throw new Error(`Failed to update secret: ${response.response.body}`); + throw secretsFailure("Failed to update secret", response); } return true; @@ -94,6 +136,7 @@ export async function updateSecret( export async function addSecret( secret: Partial & Pick, + teamId?: string, ) { if (isTourMockActive()) { mockAddSecret(secret.name ?? "", secret.value); @@ -103,20 +146,23 @@ export async function addSecret( const response = await createSecretApiSecretsPost({ query: { secret_name: secret.name ?? "", + description: secret.description, + expires_at: secret.expiresAt?.toISOString(), }, body: { secret_value: secret.value ?? "", }, + headers: teamRequestHeaders(teamId), }); if (response.response.status !== 200) { - throw new Error(`Failed to add secret: ${response.response.body}`); + throw secretsFailure("Failed to add secret", response); } return true; } -export async function removeSecret(secretId: string) { +export async function removeSecret(secretId: string, teamId?: string) { if (isTourMockActive()) { mockRemoveSecret(secretId); return true; @@ -126,10 +172,11 @@ export async function removeSecret(secretId: string) { path: { secret_name: secretId, }, + headers: teamRequestHeaders(teamId), }); if (response.response.status !== 200) { - throw new Error(`Failed to remove secret: ${response.response.body}`); + throw secretsFailure("Failed to remove secret", response); } return true; diff --git a/src/components/shared/SecretsManagement/types.ts b/src/components/shared/SecretsManagement/types.ts index 8e708cedd7..287bcea52a 100644 --- a/src/components/shared/SecretsManagement/types.ts +++ b/src/components/shared/SecretsManagement/types.ts @@ -38,10 +38,9 @@ export function extractSecretName(arg: DynamicDataArgument): string | null { return null; } -/** - * Query keys for React Query. - */ export const SecretsQueryKeys = { - All: () => ["secrets"] as const, - Id: (id: string) => ["secrets", id] as const, + All: (teamId?: string) => + teamId + ? (["secrets", "team", teamId] as const) + : (["secrets", "user"] as const), } as const;