From d6411424c1ecefa73d79ddedc7870e5e8c6c0766 Mon Sep 17 00:00:00 2001 From: agent_coder Date: Sun, 12 Jul 2026 01:36:17 +0300 Subject: [PATCH] feat(api-key): distinguish expired from expiring-soon + cover service unwrap MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Fix 1: an already-expired key rendered the forward-looking "Expiring soon" badge/tooltip ("expires within 30 days"), which is false for a past expiry. Add a pure isExpired() helper and, in ApiKeysManager, render a red "Expired" badge ("This key has expired") for past expiries; keep the orange "Expiring soon" badge only for keys expiring in the future within 30 days (the two states are made mutually exclusive at the call site). Extend utils.test.ts with isExpired coverage (past/future/null/exact-now boundary) and add a component assertion that an expired key shows "Expired" and NOT "Expiring soon". Fix 2: the response-contract unwrap (res.data turning the server envelope {data:{token,apiKey},success,status} into {token,apiKey}) was exercised by no test — component tests fully mock the service. Add a service-layer unit test that mocks the api-client (axios instance) and asserts createApiKey unwraps to the inner {token,apiKey} payload and getApiKeys resolves to the row array. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../components/api-keys-manager.test.tsx | 12 +++ .../api-key/components/api-keys-manager.tsx | 24 +++++- .../api-key/services/api-key-service.test.ts | 78 +++++++++++++++++++ .../client/src/features/api-key/utils.test.ts | 31 ++++++++ apps/client/src/features/api-key/utils.ts | 18 ++++- 5 files changed, 160 insertions(+), 3 deletions(-) create mode 100644 apps/client/src/features/api-key/services/api-key-service.test.ts diff --git a/apps/client/src/features/api-key/components/api-keys-manager.test.tsx b/apps/client/src/features/api-key/components/api-keys-manager.test.tsx index e6bbc4cc..dec6fea0 100644 --- a/apps/client/src/features/api-key/components/api-keys-manager.test.tsx +++ b/apps/client/src/features/api-key/components/api-keys-manager.test.tsx @@ -33,6 +33,7 @@ import ApiKeysManager from "./api-keys-manager"; const ISO_SOON = new Date(Date.now() + 10 * 864e5).toISOString(); const ISO_FAR = new Date(Date.now() + 200 * 864e5).toISOString(); +const ISO_EXPIRED = new Date(Date.now() - 3 * 864e5).toISOString(); // Dump the storage stub via the Web Storage API — its data lives in a closure // (see vitest.setup.ts), so JSON.stringify(localStorage) would be vacuous. @@ -102,6 +103,17 @@ describe("ApiKeysManager — list rendering", () => { expect(screen.getAllByText("Expiring soon")).toHaveLength(1); }); + it('shows "Expired" (not "Expiring soon") for an already-expired key', async () => { + vi.mocked(getApiKeys).mockResolvedValue([ + makeKey({ id: "k-dead", name: "Dead key", expiresAt: ISO_EXPIRED }), + ]); + renderManager(UserRole.MEMBER); + await screen.findByText("Dead key"); + // A past expiry is labelled "Expired", never the forward-looking badge. + expect(screen.getByText("Expired")).toBeDefined(); + expect(screen.queryByText("Expiring soon")).toBeNull(); + }); + it('shows "Never" for an unlimited key and no highlight', async () => { vi.mocked(getApiKeys).mockResolvedValue([ makeKey({ id: "k-forever", name: "Forever", expiresAt: null }), diff --git a/apps/client/src/features/api-key/components/api-keys-manager.tsx b/apps/client/src/features/api-key/components/api-keys-manager.tsx index fdf45831..ba134166 100644 --- a/apps/client/src/features/api-key/components/api-keys-manager.tsx +++ b/apps/client/src/features/api-key/components/api-keys-manager.tsx @@ -27,7 +27,11 @@ import { IApiKey, ICreateApiKeyResponse, } from "@/features/api-key/types/api-key.types"; -import { isExpiringSoon, lastUsedBucket } from "@/features/api-key/utils"; +import { + isExpired, + isExpiringSoon, + lastUsedBucket, +} from "@/features/api-key/utils"; import { CreateApiKeyModal } from "./create-api-key-modal"; import { ShowTokenModal } from "./show-token-modal"; @@ -120,7 +124,11 @@ export default function ApiKeysManager() { } const rows = (keys ?? []).map((key) => { - const soon = isExpiringSoon(key.expiresAt); + // Mutually exclusive: an already-expired key is labelled "Expired" (a past + // expiry) rather than the forward-looking "Expiring soon". isExpiringSoon + // also matches past expiries, so gate "soon" on !expired. + const expired = isExpired(key.expiresAt); + const soon = !expired && isExpiringSoon(key.expiresAt); return ( @@ -131,6 +139,18 @@ export default function ApiKeysManager() { {key.expiresAt ? ( {formatDate(key.expiresAt)} + {expired && ( + + } + > + {t("Expired")} + + + )} {soon && ( ({ post: vi.fn(), get: vi.fn() })); +vi.mock("@/lib/api-client", () => ({ + default: { post, get }, +})); + +import { + createApiKey, + getApiKeys, +} from "@/features/api-key/services/api-key-service"; + +beforeEach(() => { + vi.clearAllMocks(); +}); + +describe("api-key-service response-contract unwrap", () => { + it("createApiKey unwraps the envelope to { token, apiKey }", async () => { + const payload = { + token: "gm_secret-abc", + apiKey: { + id: "key-1", + name: "CI token", + expiresAt: "2027-07-11T12:00:00.000Z", + createdAt: "2026-07-11T12:00:00.000Z", + }, + }; + // The server envelope as the interceptor hands it to the service. + post.mockResolvedValue({ data: payload, success: true, status: 200 }); + + const result = await createApiKey({ + name: "CI token", + expiresAt: "2027-07-11T12:00:00.000Z", + }); + + // The inner payload only — not the { data, success, status } wrapper. + expect(result).toEqual(payload); + expect(result.token).toBe("gm_secret-abc"); + expect(result.apiKey).toEqual(payload.apiKey); + expect(post).toHaveBeenCalledWith("/api-keys/create", { + name: "CI token", + expiresAt: "2027-07-11T12:00:00.000Z", + }); + }); + + it("getApiKeys unwraps the envelope to the array of rows", async () => { + const rows = [ + { + id: "key-1", + name: "CI token", + expiresAt: null, + lastUsedAt: null, + createdAt: "2026-01-01T00:00:00.000Z", + }, + { + id: "key-2", + name: "Deploy token", + expiresAt: "2027-01-01T00:00:00.000Z", + lastUsedAt: null, + createdAt: "2026-02-01T00:00:00.000Z", + }, + ]; + post.mockResolvedValue({ data: rows, success: true, status: 200 }); + + const result = await getApiKeys(); + + expect(result).toEqual(rows); + expect(result).toHaveLength(2); + expect(post).toHaveBeenCalledWith("/api-keys/list"); + }); +}); diff --git a/apps/client/src/features/api-key/utils.test.ts b/apps/client/src/features/api-key/utils.test.ts index 8f697067..32360286 100644 --- a/apps/client/src/features/api-key/utils.test.ts +++ b/apps/client/src/features/api-key/utils.test.ts @@ -2,6 +2,7 @@ import { describe, it, expect } from "vitest"; import { DEFAULT_LIFETIME, EXPIRY_WARNING_DAYS, + isExpired, isExpiringSoon, lastUsedBucket, lifetimeToExpiresAt, @@ -50,6 +51,36 @@ describe("isExpiringSoon (acceptance #3 highlight)", () => { }); }); +describe("isExpired (past vs future expiry)", () => { + it("an unlimited key is never expired", () => { + expect(isExpired(null, NOW)).toBe(false); + }); + + it("a past expiry is expired", () => { + expect(isExpired(daysFromNow(-3), NOW)).toBe(true); + expect(isExpired(daysFromNow(-1), NOW)).toBe(true); + }); + + it("a future expiry is not expired (even within the warning window)", () => { + expect(isExpired(daysFromNow(1), NOW)).toBe(false); + expect(isExpired(daysFromNow(EXPIRY_WARNING_DAYS - 1), NOW)).toBe(false); + expect(isExpired(daysFromNow(200), NOW)).toBe(false); + }); + + it("an expiry exactly at 'now' counts as expired (boundary is inclusive)", () => { + expect(isExpired(NOW.toISOString(), NOW)).toBe(true); + }); + + it("is mutually distinguishable from isExpiringSoon: expired vs soon-but-future", () => { + // A key 3 days in the past: expired, and (by design) also matches + // isExpiringSoon — the UI resolves this by checking isExpired first. + expect(isExpired(daysFromNow(-3), NOW)).toBe(true); + // A key 10 days in the future: NOT expired, but expiring soon. + expect(isExpired(daysFromNow(10), NOW)).toBe(false); + expect(isExpiringSoon(daysFromNow(10), NOW)).toBe(true); + }); +}); + describe("lastUsedBucket (within-the-last-hour semantics)", () => { const minutesAgo = (n: number) => new Date(NOW.getTime() - n * 60 * 1000).toISOString(); diff --git a/apps/client/src/features/api-key/utils.ts b/apps/client/src/features/api-key/utils.ts index 6582ff12..f43ffbdb 100644 --- a/apps/client/src/features/api-key/utils.ts +++ b/apps/client/src/features/api-key/utils.ts @@ -32,8 +32,24 @@ export function lifetimeToExpiresAt( } } +// True when a bounded key's expiry is already in the past (or exactly now). An +// unlimited key (null) is never expired. This is distinct from "expiring soon": +// the two states are mutually exclusive at the call site (see api-keys-manager), +// so an already-expired key is labelled "Expired", not "Expiring soon". +export function isExpired( + expiresAt: string | null, + now: Date = new Date(), +): boolean { + if (!expiresAt) return false; + const expiry = new Date(expiresAt).getTime(); + if (Number.isNaN(expiry)) return false; + return expiry <= now.getTime(); +} + // True when a bounded key expires within the warning window (or is already -// expired). An unlimited key (null) is never "expiring soon". +// expired). An unlimited key (null) is never "expiring soon". Callers that need +// to distinguish an already-past expiry should check isExpired() first, as this +// predicate deliberately also covers the already-expired case. export function isExpiringSoon( expiresAt: string | null, now: Date = new Date(),