From 23f7083e08add1c736889e1007a76cc2a8e8ea10 Mon Sep 17 00:00:00 2001 From: Marcel Wege Date: Thu, 30 Jul 2026 10:16:22 +0200 Subject: [PATCH 1/2] feat(web-ui): admin UI for public API keys (issues #438/#439) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adds /admin/api-keys — create/list/revoke for the server-to-server bearer credentials exposed by @omadia/channel-api's admin router (/api/public/v1/admin/keys). Modeled on admin/webhooks (the closest existing 'secret shown once' pattern) and admin/mcp for list/create/ loading/empty/error state conventions. - app/_lib/api.ts: ApiKeyPublicView type + listApiKeys/createApiKey/ revokeApiKey client functions. - app/admin/api-keys/: page.tsx + ApiKeysPanel.tsx + shared.tsx (status badge, chip, card/input classnames — feature-scoped, same pattern as admin/webhooks/_components/shared.tsx). - Reveal-once token UI: shown only in create-response state, dismissed explicitly (never auto-hidden), never re-fetched — the list endpoint never returns a token field. - scopes:[] footgun avoided: the only real scope (chat:write) is a checkbox that must stay checked to submit; unchecking blocks the create button with an explanation rather than silently sending [] or omitting the field against the operator's intent. - Revoke requires an explicit two-step confirm (danger-variant Button, text/edge only, no filled red) before calling the API. - admin/page.tsx: unconditional static card under the Access group, matching the convention already used for webhooks/mcp (plugin-backed but not gated behind requiresNavFrom/registerNav). - messages/{en,de}.json: adminApiKeys namespace + admin.index.cards.apiKeys. - 11 new vitest cases covering empty/loaded/revoked states, the create scope-checkbox footgun (asserts payload.scopes is never []), the reveal-once flow, and the two-step revoke confirmation. --- web-ui/app/_lib/api.ts | 54 +++ .../api-keys/_components/ApiKeysPanel.tsx | 327 ++++++++++++++++++ .../__tests__/ApiKeysPanel.test.tsx | 189 ++++++++++ .../app/admin/api-keys/_components/shared.tsx | 47 +++ web-ui/app/admin/api-keys/page.tsx | 32 ++ web-ui/app/admin/page.tsx | 1 + web-ui/messages/de.json | 46 +++ web-ui/messages/en.json | 46 +++ 8 files changed, 742 insertions(+) create mode 100644 web-ui/app/admin/api-keys/_components/ApiKeysPanel.tsx create mode 100644 web-ui/app/admin/api-keys/_components/__tests__/ApiKeysPanel.test.tsx create mode 100644 web-ui/app/admin/api-keys/_components/shared.tsx create mode 100644 web-ui/app/admin/api-keys/page.tsx diff --git a/web-ui/app/_lib/api.ts b/web-ui/app/_lib/api.ts index 96e4bd794..d588dc89a 100644 --- a/web-ui/app/_lib/api.ts +++ b/web-ui/app/_lib/api.ts @@ -4525,3 +4525,57 @@ export async function listWebhookSubscriptionDeliveries( ): Promise<{ deliveries: ConductorWebhookOutboundDelivery[] }> { return getJson(`${WEBHOOKS_BASE}/subscriptions/${encodeURIComponent(id)}/deliveries`); } + +// ----------------------------------------------------------------------------- +// Public API keys (issues #438/#439) — /api/public/v1/admin/keys. +// +// This router lives in @omadia/channel-api, not under /v1/operator/* like the +// rest of this file's admin surfaces — it is mounted at API_PREFIX +// `/api/public/v1`, gated by the same operator-session cookie via its own +// `operatorAuth` middleware (see adminKeysRouter.ts). getJson/postJson still +// apply here unchanged: same cookie, same 401-bounces-to-/login behavior. +// ----------------------------------------------------------------------------- + +const API_KEYS_BASE = '/public/v1/admin/keys'; + +export interface ApiKeyPublicView { + id: string; + label?: string; + rateLimitPerMinute: number; + scopes: string[]; + /** Epoch ms. */ + createdAt: number; + /** Epoch ms. Present iff the key has been revoked. */ + revokedAt?: number; +} + +export interface CreateApiKeyInput { + label?: string; + rateLimitPerMinute?: number; + /** + * Omit this field entirely to accept the backend's legacy default + * (`['chat:write']`). An explicitly empty array is REJECTED by the + * backend with 400 — it reads `[]` as a deliberate "grant nothing" + * request, never as "use the default". Callers must never pass `[]`. + */ + scopes?: string[]; +} + +export interface CreateApiKeyResult { + key: ApiKeyPublicView; + /** Plaintext — present only in this one response. Never returned again by + * any other endpoint; do not persist it beyond the reveal-once UI. */ + token: string; +} + +export async function listApiKeys(): Promise<{ keys: ApiKeyPublicView[] }> { + return getJson(API_KEYS_BASE); +} + +export async function createApiKey(input: CreateApiKeyInput): Promise { + return postJson(API_KEYS_BASE, input); +} + +export async function revokeApiKey(id: string): Promise<{ key: ApiKeyPublicView }> { + return postJson(`${API_KEYS_BASE}/${encodeURIComponent(id)}/revoke`, {}); +} diff --git a/web-ui/app/admin/api-keys/_components/ApiKeysPanel.tsx b/web-ui/app/admin/api-keys/_components/ApiKeysPanel.tsx new file mode 100644 index 000000000..cad861dc4 --- /dev/null +++ b/web-ui/app/admin/api-keys/_components/ApiKeysPanel.tsx @@ -0,0 +1,327 @@ +'use client'; + +import { useCallback, useEffect, useMemo, useState } from 'react'; + +import { useTranslations } from 'next-intl'; + +import { Button } from '@/app/_components/ui/Button'; +import { + createApiKey, + listApiKeys, + revokeApiKey, + type ApiKeyPublicView, +} from '@/app/_lib/api'; + +import { KeyStatusBadge, card, chipCls, inputCls, toFriendlyError } from './shared'; + +type State = + | { kind: 'loading' } + | { kind: 'ready'; keys: ApiKeyPublicView[] } + | { kind: 'error'; message: string }; + +/** + * The only real scope today (issue #439). Kept as a single hardcoded + * checkbox rather than a generic scope picker — there is nothing else valid + * to pick from yet. Not read from a backend catalog endpoint because none + * exists; if a second scope ships, this is the one place to widen (ideally + * to a list driven by a real catalog at that point, not a second hardcode). + */ +const CHAT_WRITE_SCOPE = 'chat:write'; + +const MIN_RATE_LIMIT = 1; +const MAX_RATE_LIMIT = 6000; +const DEFAULT_RATE_LIMIT_HINT = 60; + +function parseRateLimitInput(raw: string): number | undefined { + const trimmed = raw.trim(); + if (trimmed === '') return undefined; + const n = Number(trimmed); + if (!Number.isFinite(n)) return undefined; + return Math.trunc(n); +} + +export function ApiKeysPanel(): React.ReactElement { + const t = useTranslations('adminApiKeys'); + + const [state, setState] = useState({ kind: 'loading' }); + + // Create form. + const [label, setLabel] = useState(''); + const [rateLimitInput, setRateLimitInput] = useState(''); + const [grantChatWrite, setGrantChatWrite] = useState(true); + const [creating, setCreating] = useState(false); + const [createError, setCreateError] = useState(null); + + // Reveal-once token, set only by a successful create. Cleared by an + // explicit dismiss — never auto-hidden, never re-derived from a fetch (the + // list endpoint never returns a token field, so there is nothing to leak + // even if this state were repopulated from a reload). + const [revealed, setRevealed] = useState<{ id: string; token: string } | null>(null); + const [copyState, setCopyState] = useState<'idle' | 'copied' | 'failed'>('idle'); + + // Revoke flow. + const [confirmingId, setConfirmingId] = useState(null); + const [pendingId, setPendingId] = useState(null); + const [actionError, setActionError] = useState(null); + + const reload = useCallback(async (): Promise => { + try { + const res = await listApiKeys(); + setState({ kind: 'ready', keys: res.keys }); + } catch (err) { + setState({ kind: 'error', message: toFriendlyError(err) }); + } + }, []); + + useEffect(() => { + // eslint-disable-next-line react-hooks/set-state-in-effect + void reload(); + }, [reload]); + + const rateLimitValue = useMemo(() => parseRateLimitInput(rateLimitInput), [rateLimitInput]); + const rateLimitInvalid = + rateLimitInput.trim() !== '' && + (rateLimitValue === undefined || rateLimitValue < MIN_RATE_LIMIT || rateLimitValue > MAX_RATE_LIMIT); + + // A key with zero scopes is a credential that authenticates and can do + // nothing — not a useful thing to mint, and the backend rejects an + // explicit `scopes: []` outright (see CreateApiKeyInput doc comment). + // Blocking submission here keeps that a clear, explained dead end instead + // of a 400 the operator has to decode. + const canSubmit = grantChatWrite && !rateLimitInvalid && !creating; + + const onCreate = useCallback(async (): Promise => { + if (!canSubmit) return; + setCreateError(null); + setCreating(true); + try { + const created = await createApiKey({ + ...(label.trim() ? { label: label.trim() } : {}), + ...(rateLimitValue !== undefined ? { rateLimitPerMinute: rateLimitValue } : {}), + // Sent explicitly (never omitted) so the request always reflects + // exactly what the checkbox shows — omitting would coincidentally + // resolve to the same legacy default today, but that's an + // implementation detail of the backend this UI shouldn't lean on. + scopes: [CHAT_WRITE_SCOPE], + }); + setLabel(''); + setRateLimitInput(''); + setCopyState('idle'); + setRevealed({ id: created.key.id, token: created.token }); + await reload(); + } catch (err) { + setCreateError(toFriendlyError(err)); + } finally { + setCreating(false); + } + }, [canSubmit, label, rateLimitValue, reload]); + + const onCopyToken = useCallback(async (): Promise => { + if (!revealed) return; + try { + await navigator.clipboard.writeText(revealed.token); + setCopyState('copied'); + } catch { + // Clipboard API can fail (permissions, insecure context). The token + // stays visible and selectable — this is a soft failure, not an error + // banner: the operator can still copy it manually. + setCopyState('failed'); + } + }, [revealed]); + + const onDismissRevealed = useCallback((): void => { + setRevealed(null); + setCopyState('idle'); + }, []); + + const onConfirmRevoke = useCallback( + async (id: string): Promise => { + setActionError(null); + setPendingId(id); + try { + const { key } = await revokeApiKey(id); + setState((prev) => + prev.kind === 'ready' + ? { kind: 'ready', keys: prev.keys.map((k) => (k.id === id ? key : k)) } + : prev, + ); + } catch (err) { + setActionError(toFriendlyError(err)); + // The key may have been revoked/removed by someone else already — + // resync the list rather than leaving a stale row on screen. + await reload(); + } finally { + setPendingId(null); + setConfirmingId(null); + } + }, + [reload], + ); + + return ( + <> +
+

+ {t('create.heading')} +

+
+ + + + +
+ {rateLimitInvalid && ( +

+ {t('create.rateLimitInvalid', { min: MIN_RATE_LIMIT, max: MAX_RATE_LIMIT })} +

+ )} + {!grantChatWrite && ( +

{t('create.scopesRequired')}

+ )} + {createError &&

{createError}

} +
+ + {revealed && ( +
+

+ {t('reveal.heading')} +

+

{t('reveal.warning')}

+ + {revealed.token} + +
+ + + {copyState === 'failed' && ( + {t('reveal.copyFailed')} + )} +
+
+ )} + +
+

+ {t('list.heading')} +

+ + {state.kind === 'loading' ? ( +

{t('list.loading')}

+ ) : state.kind === 'error' ? ( +

{state.message}

+ ) : state.keys.length === 0 ? ( +

{t('list.empty')}

+ ) : ( +
    + {state.keys.map((key) => { + const isActive = key.revokedAt === undefined; + const isPending = pendingId === key.id; + const isConfirming = confirmingId === key.id; + return ( +
  • +
    +
    +
    + + {key.label || t('list.unlabeled')} + + +
    +
    + {key.scopes.map((scope) => ( + + {scope} + + ))} +
    + + {t('list.rateLimitValue', { value: key.rateLimitPerMinute })} + {' · '} + {t('list.createdAt', { date: new Date(key.createdAt).toLocaleString() })} + +
    + {isActive && !isConfirming && ( + + )} +
    + + {isActive && isConfirming && ( +
    +

    + {t('list.confirmRevoke.message')} +

    +
    + + +
    +
    + )} +
    • + ); + })} +
+ )} + + {actionError &&

{actionError}

} +
+ + ); +} diff --git a/web-ui/app/admin/api-keys/_components/__tests__/ApiKeysPanel.test.tsx b/web-ui/app/admin/api-keys/_components/__tests__/ApiKeysPanel.test.tsx new file mode 100644 index 000000000..9389adb29 --- /dev/null +++ b/web-ui/app/admin/api-keys/_components/__tests__/ApiKeysPanel.test.tsx @@ -0,0 +1,189 @@ +import { fireEvent, screen, waitFor } from '@testing-library/react'; +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; + +import { renderWithIntl } from '../../../../_lib/test-utils'; +import { ApiKeysPanel } from '../ApiKeysPanel'; +import type { ApiKeyPublicView } from '../../../../_lib/api'; + +const { mockListApiKeys, mockCreateApiKey, mockRevokeApiKey } = vi.hoisted(() => ({ + mockListApiKeys: vi.fn(), + mockCreateApiKey: vi.fn(), + mockRevokeApiKey: vi.fn(), +})); + +vi.mock('../../../../_lib/api', () => ({ + listApiKeys: mockListApiKeys, + createApiKey: mockCreateApiKey, + revokeApiKey: mockRevokeApiKey, + ApiError: class ApiError extends Error { + constructor( + public status: number, + message: string, + public body?: string, + ) { + super(message); + } + }, +})); + +function key(over: Partial = {}): ApiKeyPublicView { + return { + id: 'key-1', + label: 'CI bot', + rateLimitPerMinute: 60, + scopes: ['chat:write'], + createdAt: Date.parse('2026-07-01T00:00:00Z'), + ...over, + }; +} + +describe('', () => { + beforeEach(() => { + vi.clearAllMocks(); + Object.assign(navigator, { clipboard: { writeText: vi.fn().mockResolvedValue(undefined) } }); + // Sane defaults so tests that only assert *whether* the API was called + // (not its resolved shape) don't hit a destructuring TypeError on an + // unmocked resolved value. + mockRevokeApiKey.mockResolvedValue({ key: key() }); + }); + afterEach(() => { + vi.clearAllMocks(); + }); + + it('shows the empty state when there are no keys', async () => { + mockListApiKeys.mockResolvedValue({ keys: [] }); + renderWithIntl(); + + expect(await screen.findByText(/No API keys yet/i)).toBeTruthy(); + }); + + it('renders an existing key with its label, scope chip, and Active status', async () => { + mockListApiKeys.mockResolvedValue({ keys: [key()] }); + renderWithIntl(); + + expect(await screen.findByText('CI bot')).toBeTruthy(); + // The scope also appears as the (checked) create-form checkbox label, so + // there are two "chat:write" nodes on the page — the chip is one of them. + expect(screen.getAllByText('chat:write').length).toBeGreaterThanOrEqual(2); + expect(screen.getByText('Active')).toBeTruthy(); + }); + + it('falls back to "Unlabeled" when a key has no label', async () => { + mockListApiKeys.mockResolvedValue({ keys: [key({ label: undefined })] }); + renderWithIntl(); + + expect(await screen.findByText('Unlabeled')).toBeTruthy(); + }); + + it('shows a revoked key with Revoked status and no revoke button', async () => { + mockListApiKeys.mockResolvedValue({ + keys: [key({ revokedAt: Date.parse('2026-07-02T00:00:00Z') })], + }); + renderWithIntl(); + + expect(await screen.findByText('Revoked')).toBeTruthy(); + expect(screen.queryByText('Revoke')).toBeNull(); + }); + + it('creates a key with the checked default scope and never sends an empty scopes array', async () => { + mockListApiKeys.mockResolvedValue({ keys: [] }); + mockCreateApiKey.mockResolvedValue({ + key: key(), + token: 'sk-live-plaintext-token-value', + }); + renderWithIntl(); + + await screen.findByText(/No API keys yet/i); + fireEvent.click(screen.getByText('Create key')); + + await waitFor(() => expect(mockCreateApiKey).toHaveBeenCalledTimes(1)); + const call = mockCreateApiKey.mock.calls[0]; + expect(call).toBeDefined(); + const payload = call?.[0]; + expect(payload.scopes).toEqual(['chat:write']); + expect(payload.scopes.length).toBeGreaterThan(0); // regression guard: never [] + }); + + it('reveals the plaintext token exactly once after creation, with a dismiss-to-confirm reveal', async () => { + mockListApiKeys.mockResolvedValue({ keys: [] }); + mockCreateApiKey.mockResolvedValue({ + key: key(), + token: 'sk-live-plaintext-token-value', + }); + renderWithIntl(); + + await screen.findByText(/No API keys yet/i); + fireEvent.click(screen.getByText('Create key')); + + expect(await screen.findByText('sk-live-plaintext-token-value')).toBeTruthy(); + expect(screen.getByText(/only time this token is shown/i)).toBeTruthy(); + + fireEvent.click(screen.getByText(/dismiss/i)); + expect(screen.queryByText('sk-live-plaintext-token-value')).toBeNull(); + }); + + it('blocks submission and explains why when the only scope checkbox is unchecked', async () => { + mockListApiKeys.mockResolvedValue({ keys: [] }); + renderWithIntl(); + + await screen.findByText(/No API keys yet/i); + fireEvent.click(screen.getByRole('checkbox')); + + expect(screen.getByText('Create key')).toHaveProperty('disabled', true); + expect(screen.getByText(/at least one scope/i)).toBeTruthy(); + expect(mockCreateApiKey).not.toHaveBeenCalled(); + }); + + it('requires a confirmation step before revoking — the first click only arms it', async () => { + mockListApiKeys.mockResolvedValue({ keys: [key()] }); + renderWithIntl(); + + await screen.findByText('CI bot'); + fireEvent.click(screen.getByText('Revoke')); + + expect(mockRevokeApiKey).not.toHaveBeenCalled(); + expect(screen.getByText(/can't be undone/i)).toBeTruthy(); + + fireEvent.click(screen.getByText('Confirm revoke')); + await waitFor(() => expect(mockRevokeApiKey).toHaveBeenCalledWith('key-1')); + }); + + it('cancelling the revoke confirmation does not call the API', async () => { + mockListApiKeys.mockResolvedValue({ keys: [key()] }); + renderWithIntl(); + + await screen.findByText('CI bot'); + fireEvent.click(screen.getByText('Revoke')); + fireEvent.click(screen.getByText('Cancel')); + + expect(mockRevokeApiKey).not.toHaveBeenCalled(); + expect(screen.getByText('Revoke')).toBeTruthy(); + }); + + it('updates the row to Revoked in place after a confirmed revoke', async () => { + mockListApiKeys.mockResolvedValue({ keys: [key()] }); + mockRevokeApiKey.mockResolvedValue({ + key: key({ revokedAt: Date.parse('2026-07-03T00:00:00Z') }), + }); + renderWithIntl(); + + await screen.findByText('CI bot'); + fireEvent.click(screen.getByText('Revoke')); + fireEvent.click(screen.getByText('Confirm revoke')); + + expect(await screen.findByText('Revoked')).toBeTruthy(); + expect(screen.queryByText('Confirm revoke')).toBeNull(); + }); + + it('shows a validation error and disables submit for an out-of-range rate limit', async () => { + mockListApiKeys.mockResolvedValue({ keys: [] }); + renderWithIntl(); + + await screen.findByText(/No API keys yet/i); + const rateLimitInput = screen.getByPlaceholderText('60'); + fireEvent.change(rateLimitInput, { target: { value: '99999' } }); + + expect(screen.getByText('Create key')).toHaveProperty('disabled', true); + expect(mockCreateApiKey).not.toHaveBeenCalled(); + }); +}); diff --git a/web-ui/app/admin/api-keys/_components/shared.tsx b/web-ui/app/admin/api-keys/_components/shared.tsx new file mode 100644 index 000000000..11e8c0847 --- /dev/null +++ b/web-ui/app/admin/api-keys/_components/shared.tsx @@ -0,0 +1,47 @@ +'use client'; + +import { useTranslations } from 'next-intl'; + +import { ApiError } from '@/app/_lib/api'; + +/** Shared styling + error-formatting for the API-keys admin panel — mirrors + * the same tiny per-feature `shared.tsx` pattern already used by + * `admin/webhooks/_components/shared.tsx` rather than reaching for a + * cross-feature util module. */ + +export const inputCls = + 'w-full rounded-md border border-[color:var(--border)] bg-transparent px-3 py-2 text-sm outline-none focus:border-[color:var(--accent)]'; + +export const card = 'rounded-lg border border-[color:var(--border)] bg-[color:var(--card)]/40 p-4'; + +/** Scope / data chip — Geist Mono register per the Lume type-scale (§2.7, + * `.type-mono-data`), since a scope string is data, not UI chrome. */ +export const chipCls = + 'type-mono-data inline-flex items-center rounded-full border border-[color:var(--border)] px-2 py-0.5 text-[11px] lowercase text-[color:var(--fg-muted)]'; + +export function toFriendlyError(err: unknown): string { + if (err instanceof ApiError) return err.body || err.message; + return err instanceof Error ? err.message : String(err); +} + +/** + * Active/revoked status — text + a 1px edge + at most a light tint, never a + * solid-filled pill (Lume state-color rule, spec §2.6). Same recipe as + * `admin/webhooks/_components/shared.tsx`'s `StatusBadge`. + */ +export function KeyStatusBadge({ revokedAt }: { revokedAt?: number }): React.ReactElement { + const t = useTranslations('adminApiKeys.list.status'); + const revoked = revokedAt !== undefined; + return ( + + {revoked ? t('revoked') : t('active')} + + ); +} diff --git a/web-ui/app/admin/api-keys/page.tsx b/web-ui/app/admin/api-keys/page.tsx new file mode 100644 index 000000000..876b64ea7 --- /dev/null +++ b/web-ui/app/admin/api-keys/page.tsx @@ -0,0 +1,32 @@ +'use client'; + +import { useTranslations } from 'next-intl'; + +import { ApiKeysPanel } from './_components/ApiKeysPanel'; + +/** + * Issues #438/#439 admin surface — server-to-server bearer credentials for + * the public chat API (`POST /api/public/v1/chat`). Create/list/revoke only; + * there is no update or rotate — a key that needs different scopes or a + * different rate limit is revoked and replaced by a new one, keeping the + * "plaintext shown exactly once, at creation" invariant simple to reason + * about (no second code path that could leak a token after the fact). + */ +export default function AdminApiKeysPage(): React.ReactElement { + const t = useTranslations('adminApiKeys'); + + return ( +
+
+

+ {t('title')} +

+

+ {t('intro')} +

+
+ + +
+ ); +} diff --git a/web-ui/app/admin/page.tsx b/web-ui/app/admin/page.tsx index ebae6b198..32aba7ef5 100644 --- a/web-ui/app/admin/page.tsx +++ b/web-ui/app/admin/page.tsx @@ -84,6 +84,7 @@ const GROUPS: readonly GroupDef[] = [ cards: [ { href: '/admin/auth', key: 'auth' }, { href: '/admin/users', key: 'users' }, + { href: '/admin/api-keys', key: 'apiKeys' }, ], }, { diff --git a/web-ui/messages/de.json b/web-ui/messages/de.json index 351c76236..749c5dea9 100644 --- a/web-ui/messages/de.json +++ b/web-ui/messages/de.json @@ -240,6 +240,10 @@ "dangerZone": { "title": "Memory-Purge", "description": "Memory unwiderruflich entlang einer Achse löschen (Alles, Agent, User, Team, Channel); Vorschau-gated mit Confirm-Phrase, kein Undo." + }, + "apiKeys": { + "title": "Öffentliche API-Schlüssel", + "description": "Bearer-Zugangsdaten für die öffentliche Chat-API erstellen, auflisten und widerrufen (Issues #438/#439)." } } }, @@ -4138,5 +4142,47 @@ "keySourceDevFile": "dev-file (nicht für Produktion)", "keySourceDevFileCreated": "dev-file (neu generiert — DEV ONLY)" } + }, + "adminApiKeys": { + "title": "Öffentliche API-Schlüssel", + "intro": "Server-zu-Server-Bearer-Zugangsdaten für die öffentliche Chat-API. Das Klartext-Token eines Schlüssels wird genau einmal angezeigt, direkt nach dem Erstellen — sofort kopieren, es kann danach nicht erneut abgerufen werden, auch nicht von einem Operator.", + "create": { + "heading": "Neuen Schlüssel erstellen", + "fields": { + "label": "Label (optional)", + "rateLimit": "Rate-Limit (Anfragen/Minute, optional)" + }, + "rateLimitInvalid": "Rate-Limit muss zwischen {min} und {max} Anfragen/Minute liegen.", + "scopesRequired": "Mindestens einen Scope auswählen — ein Schlüssel ohne Scope kann nichts tun.", + "submit": "Schlüssel erstellen", + "creating": "Wird erstellt" + }, + "reveal": { + "heading": "Neuer API-Schlüssel erstellt", + "warning": "Dieses Token wird nur dieses eine Mal angezeigt. Jetzt kopieren — es kann danach nicht erneut abgerufen werden, auch nicht von einem Operator.", + "copy": "Kopieren", + "copied": "Kopiert", + "copyFailed": "Automatisches Kopieren fehlgeschlagen — Token oben markieren und manuell kopieren.", + "dismiss": "Kopiert, ausblenden" + }, + "list": { + "heading": "Vorhandene Schlüssel", + "loading": "Lädt …", + "empty": "Noch keine API-Schlüssel. Oben einen erstellen, um loszulegen.", + "unlabeled": "Ohne Label", + "createdAt": "Erstellt {date}", + "rateLimitValue": "{value}/Min", + "status": { + "active": "Aktiv", + "revoked": "Widerrufen" + }, + "revoke": "Widerrufen", + "revoking": "Wird widerrufen", + "confirmRevoke": { + "message": "Diesen Schlüssel widerrufen? Alles, was ihn verwendet, hört sofort auf zu funktionieren — das kann nicht rückgängig gemacht werden.", + "confirm": "Widerruf bestätigen", + "cancel": "Abbrechen" + } + } } } diff --git a/web-ui/messages/en.json b/web-ui/messages/en.json index 121b21096..de7147312 100644 --- a/web-ui/messages/en.json +++ b/web-ui/messages/en.json @@ -240,6 +240,10 @@ "dangerZone": { "title": "Memory Purge", "description": "Irreversibly delete memory along one axis (all, agent, user, team, channel); preview-gated with a confirm phrase, no undo." + }, + "apiKeys": { + "title": "Public API Keys", + "description": "Create, list, and revoke bearer credentials for the public chat API (issues #438/#439)." } } }, @@ -4138,5 +4142,47 @@ "keySourceDevFile": "dev file (not for production)", "keySourceDevFileCreated": "dev file (newly generated — DEV ONLY)" } + }, + "adminApiKeys": { + "title": "Public API Keys", + "intro": "Server-to-server bearer credentials for the public chat API. A key’s plaintext token is shown exactly once, right after creation — copy it immediately, it cannot be retrieved again, even by an operator.", + "create": { + "heading": "Create a new key", + "fields": { + "label": "Label (optional)", + "rateLimit": "Rate limit (requests/minute, optional)" + }, + "rateLimitInvalid": "Rate limit must be between {min} and {max} requests/minute.", + "scopesRequired": "Select at least one scope — a key with none cannot do anything.", + "submit": "Create key", + "creating": "Creating" + }, + "reveal": { + "heading": "New API key created", + "warning": "This is the only time this token is shown. Copy it now — it cannot be retrieved again, even by an operator.", + "copy": "Copy", + "copied": "Copied", + "copyFailed": "Couldn't copy automatically — select the token above and copy it manually.", + "dismiss": "I've copied it, dismiss" + }, + "list": { + "heading": "Existing keys", + "loading": "Loading …", + "empty": "No API keys yet. Create one above to get started.", + "unlabeled": "Unlabeled", + "createdAt": "Created {date}", + "rateLimitValue": "{value}/min", + "status": { + "active": "Active", + "revoked": "Revoked" + }, + "revoke": "Revoke", + "revoking": "Revoking", + "confirmRevoke": { + "message": "Revoke this key? Anything using it stops working immediately — this can't be undone.", + "confirm": "Confirm revoke", + "cancel": "Cancel" + } + } } } From 100cecd1c898b40b784431163855fffb6e8ab019 Mon Sep 17 00:00:00 2001 From: Marcel Wege Date: Thu, 30 Jul 2026 10:59:04 +0200 Subject: [PATCH 2/2] fix(web-ui): address codex review findings on API-keys admin UI Fixes 5 blocking findings + 1 lower-severity note from a codex adversarial review of the /admin/api-keys page (issues #438/#439): 1. Token-overwrite bug: creating a second key while a first key's one-time token reveal was still showing silently overwrote it in React state. `canSubmit` now requires `!revealed`, and the create form (button + all fields) stays disabled until the reveal is explicitly dismissed. 2. Race conditions: a single global `pendingId`/`confirmingId` let concurrent per-row revokes clobber each other's busy/confirm UI state, and unsequenced list reloads could resolve out of order. Both are now `ReadonlySet` keyed per key id, and `reload()` uses a monotonic sequence ref so only the most recently issued reload's result applies. 3. Lume state-color violation: several error texts rendered in danger color without the mandatory 1px edge. Added `errorTextCls`/ `errorInlineCls` (border + tint + text) to shared.tsx and applied them at all 6 flagged spots. 4. Hard i18n rule violation: the error helper returned raw backend response bodies, and the created-date display used `toLocaleString()`. `toFriendlyError(err, t)` now maps known backend error shapes to translated `adminApiKeys.errors.*` catalog keys (mirrors `admin/registries/page.tsx`), never rendering the raw body. Dates now use `useFormatter().dateTime(...)`. 5. Missing documentation: added a CHANGELOG bullet and a middleware-agent-handoff.md section describing the new admin UI page. Lower severity: decimal rate-limit input ("60.7") is now rejected via Number.isInteger instead of being silently truncated. Two new interaction-level tests exercise findings 1 and 2 directly via React Testing Library. Re-reviewed by codex (prReady: true). --- docs/CHANGELOG.md | 14 +++ docs/middleware-agent-handoff.md | 26 +++++ .../api-keys/_components/ApiKeysPanel.tsx | 108 +++++++++++++----- .../__tests__/ApiKeysPanel.test.tsx | 90 ++++++++++++++- .../app/admin/api-keys/_components/shared.tsx | 37 +++++- web-ui/messages/de.json | 8 ++ web-ui/messages/en.json | 8 ++ 7 files changed, 256 insertions(+), 35 deletions(-) diff --git a/docs/CHANGELOG.md b/docs/CHANGELOG.md index 4d81791ae..f40510b42 100644 --- a/docs/CHANGELOG.md +++ b/docs/CHANGELOG.md @@ -95,6 +95,20 @@ entry. See `CONTRIBUTING.md` § Releases & changelog. primitives, and `middleware/src` imports no channel plugin), because "where does this code live" is a property no runtime assertion can express and the cheapest one to regress. +- **Admin UI** (`web-ui/app/admin/api-keys/`): create/list/revoke against + `/api/public/v1/admin/keys`. A created key's plaintext token is shown + exactly once, right after creation, and creation is blocked while that + one-time reveal is still on screen — the create button and every form + field stay disabled until the operator explicitly dismisses it, so a + second key can never silently overwrite the first one's only-ever-shown + token in React state before it's copied. Revoking is a two-step + confirm-then-revoke per row, with independent busy/confirm state per key + (concurrent revokes on different rows don't clobber each other), and the + list reload is guarded against out-of-order responses (a slower in-flight + fetch can't stomp a newer one's result). Errors map known backend codes + (`not_found`, `operator_auth.unavailable`, `auth.missing`/`auth.invalid`, + `invalid_request`) to translated messages rather than surfacing the raw + response body. ### Added — public API channel: chat over HTTP with per-key auth (#438) diff --git a/docs/middleware-agent-handoff.md b/docs/middleware-agent-handoff.md index 21b743c69..5c67fa157 100644 --- a/docs/middleware-agent-handoff.md +++ b/docs/middleware-agent-handoff.md @@ -966,6 +966,14 @@ vom eigenen Server aus aufruft, ohne menschliche Session. - **`publicPaths.ts` bleibt unverändert eng:** weiterhin nur `/api/public/v1/chat`. Wer `requireApiKey` auf eine neue Route mountet, braucht dort einen eigenen, möglichst engen Eintrag. +- **Admin-UI (`web-ui/app/admin/api-keys/`)** — separate, auf diesem Branch + gestackte Web-UI-PR. `ApiKeysPanel.tsx` deckt create/list/revoke gegen + genau die drei Routen oben ab, keine neuen Backend-Endpunkte. `scopes: []` + wird nie gesendet — die Checkbox für `chat:write` muss angehakt bleiben, + sonst bleibt der Create-Button deaktiviert, statt den Footgun aus dem + `CreateKeyRequestSchema`-Kommentar oben zu reproduzieren. Details zum + Reveal-/Revoke-/Fehler-Verhalten stehen direkt nach der Testliste unten, + um Doppelung zu vermeiden. Tests: `test/auth/requireApiKey.test.ts` (Auth/Scope/Rate-Limit/Audit der Middleware), `test/auth/apiKeyScopes.test.ts` (Scope-Modell inkl. @@ -975,6 +983,24 @@ Kernel kein Channel-Plugin importiert). Die bestehenden `test/channelApi/`- Suites laufen inhaltlich unverändert weiter, nur die Importpfade der verschobenen Module zeigen jetzt auf `packages/harness-api-key-auth/`. +**Admin-UI** (`web-ui/app/admin/api-keys/`, Issue #438/#439): Create/List/ +Revoke gegen `/api/public/v1/admin/keys` (`ApiKeysPanel.tsx`). Das +Klartext-Token wird — wie beim Webhook-Secret-Reveal — genau einmal direkt +nach dem Create angezeigt, nie erneut aus einem Reload rekonstruiert (die +Listen-Response enthält nie ein Token-Feld). Solange dieser Reveal auf dem +Schirm ist, ist das Erstellen eines weiteren Keys blockiert (Formular +disabled) — sonst würde ein zweiter Create das erste, noch nicht kopierte +Token in React-State kommentarlos überschreiben. Revoke ist zweistufig +(Arm → Confirm) mit Busy-/Confirm-State **pro Key-Id** (Set statt einem +einzelnen globalen Id-String), damit ein gleichzeitiges Revoke auf einer +anderen Zeile den Confirm-/Busy-Zustand dieser Zeile nicht zurücksetzt; der +Listen-Reload trägt eine Sequenznummer, damit ein langsamer, überholter +Fetch nicht das Ergebnis eines neueren überschreibt. Fehler werden über +bekannte Backend-Codes (`not_found`, `operator_auth.unavailable`, +`auth.missing`/`auth.invalid`, `invalid_request`) auf übersetzte +Catalog-Strings gemappt statt den rohen Response-Body anzuzeigen (web-ui +i18n Hard Rule, `web-ui/CLAUDE.md`). + --- ## 4. Migration Managed Agents → Lokal diff --git a/web-ui/app/admin/api-keys/_components/ApiKeysPanel.tsx b/web-ui/app/admin/api-keys/_components/ApiKeysPanel.tsx index cad861dc4..f76fa9a99 100644 --- a/web-ui/app/admin/api-keys/_components/ApiKeysPanel.tsx +++ b/web-ui/app/admin/api-keys/_components/ApiKeysPanel.tsx @@ -1,8 +1,8 @@ 'use client'; -import { useCallback, useEffect, useMemo, useState } from 'react'; +import { useCallback, useEffect, useMemo, useRef, useState } from 'react'; -import { useTranslations } from 'next-intl'; +import { useFormatter, useTranslations } from 'next-intl'; import { Button } from '@/app/_components/ui/Button'; import { @@ -12,7 +12,7 @@ import { type ApiKeyPublicView, } from '@/app/_lib/api'; -import { KeyStatusBadge, card, chipCls, inputCls, toFriendlyError } from './shared'; +import { KeyStatusBadge, card, chipCls, errorInlineCls, errorTextCls, inputCls, toFriendlyError } from './shared'; type State = | { kind: 'loading' } @@ -37,13 +37,34 @@ function parseRateLimitInput(raw: string): number | undefined { if (trimmed === '') return undefined; const n = Number(trimmed); if (!Number.isFinite(n)) return undefined; - return Math.trunc(n); + return n; +} + +/** Immutable add/remove for the per-key id sets below (`pendingIds`, + * `confirmingIds`) — every row's transient UI state is tracked by its own + * key id rather than a single shared value, so two rows can be mid-action + * at once without one clobbering the other's state (codex review finding). */ +function withId(set: ReadonlySet, id: string): ReadonlySet { + if (set.has(id)) return set; + return new Set(set).add(id); +} +function withoutId(set: ReadonlySet, id: string): ReadonlySet { + if (!set.has(id)) return set; + const next = new Set(set); + next.delete(id); + return next; } export function ApiKeysPanel(): React.ReactElement { const t = useTranslations('adminApiKeys'); + const format = useFormatter(); const [state, setState] = useState({ kind: 'loading' }); + // Guards against out-of-order responses: only the most recently ISSUED + // reload's result is ever applied to `state`. Without this, a slow initial + // mount fetch that resolves AFTER a post-create reload (which already + // reflects the new key) could stomp the newer state with stale data. + const reloadSeqRef = useRef(0); // Create form. const [label, setLabel] = useState(''); @@ -55,23 +76,31 @@ export function ApiKeysPanel(): React.ReactElement { // Reveal-once token, set only by a successful create. Cleared by an // explicit dismiss — never auto-hidden, never re-derived from a fetch (the // list endpoint never returns a token field, so there is nothing to leak - // even if this state were repopulated from a reload). + // even if this state were repopulated from a reload). Creation is BLOCKED + // while a reveal is on screen (see `canSubmit`) — otherwise a second + // create would silently overwrite the first key's only-ever-shown token + // before the operator had a chance to copy it. const [revealed, setRevealed] = useState<{ id: string; token: string } | null>(null); const [copyState, setCopyState] = useState<'idle' | 'copied' | 'failed'>('idle'); - // Revoke flow. - const [confirmingId, setConfirmingId] = useState(null); - const [pendingId, setPendingId] = useState(null); + // Revoke flow. Both sets are keyed by key id (not a single shared value) + // so two different rows can be armed/revoked concurrently without one's + // confirm/busy state clobbering the other's. + const [confirmingIds, setConfirmingIds] = useState>(new Set()); + const [pendingIds, setPendingIds] = useState>(new Set()); const [actionError, setActionError] = useState(null); const reload = useCallback(async (): Promise => { + const seq = ++reloadSeqRef.current; try { const res = await listApiKeys(); + if (seq !== reloadSeqRef.current) return; // superseded by a newer reload setState({ kind: 'ready', keys: res.keys }); } catch (err) { - setState({ kind: 'error', message: toFriendlyError(err) }); + if (seq !== reloadSeqRef.current) return; + setState({ kind: 'error', message: toFriendlyError(err, t) }); } - }, []); + }, [t]); useEffect(() => { // eslint-disable-next-line react-hooks/set-state-in-effect @@ -81,14 +110,18 @@ export function ApiKeysPanel(): React.ReactElement { const rateLimitValue = useMemo(() => parseRateLimitInput(rateLimitInput), [rateLimitInput]); const rateLimitInvalid = rateLimitInput.trim() !== '' && - (rateLimitValue === undefined || rateLimitValue < MIN_RATE_LIMIT || rateLimitValue > MAX_RATE_LIMIT); + (rateLimitValue === undefined || + !Number.isInteger(rateLimitValue) || + rateLimitValue < MIN_RATE_LIMIT || + rateLimitValue > MAX_RATE_LIMIT); // A key with zero scopes is a credential that authenticates and can do // nothing — not a useful thing to mint, and the backend rejects an // explicit `scopes: []` outright (see CreateApiKeyInput doc comment). // Blocking submission here keeps that a clear, explained dead end instead - // of a 400 the operator has to decode. - const canSubmit = grantChatWrite && !rateLimitInvalid && !creating; + // of a 400 the operator has to decode. `!revealed` blocks a second create + // while the previous key's one-time token is still on screen unconfirmed. + const canSubmit = grantChatWrite && !rateLimitInvalid && !creating && !revealed; const onCreate = useCallback(async (): Promise => { if (!canSubmit) return; @@ -110,11 +143,11 @@ export function ApiKeysPanel(): React.ReactElement { setRevealed({ id: created.key.id, token: created.token }); await reload(); } catch (err) { - setCreateError(toFriendlyError(err)); + setCreateError(toFriendlyError(err, t)); } finally { setCreating(false); } - }, [canSubmit, label, rateLimitValue, reload]); + }, [canSubmit, label, rateLimitValue, reload, t]); const onCopyToken = useCallback(async (): Promise => { if (!revealed) return; @@ -137,7 +170,7 @@ export function ApiKeysPanel(): React.ReactElement { const onConfirmRevoke = useCallback( async (id: string): Promise => { setActionError(null); - setPendingId(id); + setPendingIds((prev) => withId(prev, id)); try { const { key } = await revokeApiKey(id); setState((prev) => @@ -146,16 +179,16 @@ export function ApiKeysPanel(): React.ReactElement { : prev, ); } catch (err) { - setActionError(toFriendlyError(err)); + setActionError(toFriendlyError(err, t)); // The key may have been revoked/removed by someone else already — // resync the list rather than leaving a stale row on screen. await reload(); } finally { - setPendingId(null); - setConfirmingId(null); + setPendingIds((prev) => withoutId(prev, id)); + setConfirmingIds((prev) => withoutId(prev, id)); } }, - [reload], + [reload, t], ); return ( @@ -173,6 +206,7 @@ export function ApiKeysPanel(): React.ReactElement { className={inputCls} value={label} maxLength={120} + disabled={!!revealed} onChange={(e) => setLabel(e.target.value)} /> @@ -187,6 +221,7 @@ export function ApiKeysPanel(): React.ReactElement { max={MAX_RATE_LIMIT} placeholder={String(DEFAULT_RATE_LIMIT_HINT)} value={rateLimitInput} + disabled={!!revealed} onChange={(e) => setRateLimitInput(e.target.value)} /> @@ -194,6 +229,7 @@ export function ApiKeysPanel(): React.ReactElement { setGrantChatWrite(e.target.checked)} /> @@ -205,14 +241,15 @@ export function ApiKeysPanel(): React.ReactElement { {rateLimitInvalid && ( -

+

{t('create.rateLimitInvalid', { min: MIN_RATE_LIMIT, max: MAX_RATE_LIMIT })}

)} {!grantChatWrite && ( -

{t('create.scopesRequired')}

+

{t('create.scopesRequired')}

)} - {createError &&

{createError}

} + {revealed &&

{t('create.blockedByReveal')}

} + {createError &&

{createError}

} {revealed && ( @@ -235,7 +272,7 @@ export function ApiKeysPanel(): React.ReactElement { {t('reveal.dismiss')} {copyState === 'failed' && ( - {t('reveal.copyFailed')} + {t('reveal.copyFailed')} )} @@ -249,15 +286,15 @@ export function ApiKeysPanel(): React.ReactElement { {state.kind === 'loading' ? (

{t('list.loading')}

) : state.kind === 'error' ? ( -

{state.message}

+

{state.message}

) : state.keys.length === 0 ? (

{t('list.empty')}

) : (
    {state.keys.map((key) => { const isActive = key.revokedAt === undefined; - const isPending = pendingId === key.id; - const isConfirming = confirmingId === key.id; + const isPending = pendingIds.has(key.id); + const isConfirming = confirmingIds.has(key.id); return (
  • @@ -278,11 +315,20 @@ export function ApiKeysPanel(): React.ReactElement { {t('list.rateLimitValue', { value: key.rateLimitPerMinute })} {' · '} - {t('list.createdAt', { date: new Date(key.createdAt).toLocaleString() })} + {t('list.createdAt', { + date: format.dateTime(new Date(key.createdAt), { + dateStyle: 'medium', + timeStyle: 'short', + }), + })}
    {isActive && !isConfirming && ( - )} @@ -306,7 +352,7 @@ export function ApiKeysPanel(): React.ReactElement {
)} - {actionError &&

{actionError}

} + {actionError &&

{actionError}

} ); diff --git a/web-ui/app/admin/api-keys/_components/__tests__/ApiKeysPanel.test.tsx b/web-ui/app/admin/api-keys/_components/__tests__/ApiKeysPanel.test.tsx index 9389adb29..d8a8ebec3 100644 --- a/web-ui/app/admin/api-keys/_components/__tests__/ApiKeysPanel.test.tsx +++ b/web-ui/app/admin/api-keys/_components/__tests__/ApiKeysPanel.test.tsx @@ -1,4 +1,4 @@ -import { fireEvent, screen, waitFor } from '@testing-library/react'; +import { fireEvent, screen, waitFor, within } from '@testing-library/react'; import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; import { renderWithIntl } from '../../../../_lib/test-utils'; @@ -118,7 +118,7 @@ describe('', () => { expect(await screen.findByText('sk-live-plaintext-token-value')).toBeTruthy(); expect(screen.getByText(/only time this token is shown/i)).toBeTruthy(); - fireEvent.click(screen.getByText(/dismiss/i)); + fireEvent.click(screen.getByRole('button', { name: /dismiss/i })); expect(screen.queryByText('sk-live-plaintext-token-value')).toBeNull(); }); @@ -186,4 +186,90 @@ describe('', () => { expect(screen.getByText('Create key')).toHaveProperty('disabled', true); expect(mockCreateApiKey).not.toHaveBeenCalled(); }); + + it('rejects a non-integer rate limit instead of silently truncating it', async () => { + mockListApiKeys.mockResolvedValue({ keys: [] }); + renderWithIntl(); + + await screen.findByText(/No API keys yet/i); + fireEvent.change(screen.getByPlaceholderText('60'), { target: { value: '60.7' } }); + + expect(screen.getByText('Create key')).toHaveProperty('disabled', true); + expect(mockCreateApiKey).not.toHaveBeenCalled(); + }); + + // Regression guard for a codex review finding: creating a second key while + // the first one's plaintext token is still displayed would silently + // overwrite it in React state before the operator could copy it — the + // create form must stay blocked until the reveal is explicitly dismissed. + it('blocks creating a second key while a token reveal is still showing', async () => { + mockListApiKeys.mockResolvedValue({ keys: [] }); + mockCreateApiKey.mockResolvedValue({ + key: key(), + token: 'sk-live-plaintext-token-value', + }); + renderWithIntl(); + + await screen.findByText(/No API keys yet/i); + fireEvent.click(screen.getByText('Create key')); + + expect(await screen.findByText('sk-live-plaintext-token-value')).toBeTruthy(); + expect(screen.getByText('Create key')).toHaveProperty('disabled', true); + expect(screen.getByText(/Dismiss the token above/i)).toBeTruthy(); + expect(mockCreateApiKey).toHaveBeenCalledTimes(1); + + // Dismissing re-enables it. + fireEvent.click(screen.getByRole('button', { name: /dismiss/i })); + expect(screen.getByText('Create key')).toHaveProperty('disabled', false); + }); + + // Regression guard for a codex review finding: a single global `pendingId` + // meant a second row's revoke could clear the first row's busy/confirm + // state (and vice versa) when either promise settled. + it('revokes two different keys concurrently without one clobbering the other\'s confirm state', async () => { + mockListApiKeys.mockResolvedValue({ keys: [key({ id: 'key-1' }), key({ id: 'key-2', label: 'Second bot' })] }); + let resolveFirst: ((v: { key: ApiKeyPublicView }) => void) | undefined; + mockRevokeApiKey.mockImplementation((id: string) => { + if (id === 'key-1') { + return new Promise((resolve) => { + resolveFirst = resolve; + }); + } + return Promise.resolve({ key: key({ id: 'key-2', label: 'Second bot', revokedAt: Date.now() }) }); + }); + renderWithIntl(); + + await screen.findByText('CI bot'); + await screen.findByText('Second bot'); + + // Arm + confirm the first row — its revoke call hangs (not yet resolved). + const firstRevokeButton = screen.getAllByText('Revoke')[0]; + expect(firstRevokeButton).toBeDefined(); + fireEvent.click(firstRevokeButton as HTMLElement); + fireEvent.click(screen.getByText('Confirm revoke')); + await waitFor(() => expect(mockRevokeApiKey).toHaveBeenCalledWith('key-1')); + + // Arm + confirm the second row while the first is still in flight. Only + // one "Revoke" button remains (row 1 is now showing its confirm UI). + const secondRevokeButton = screen.getAllByText('Revoke')[0]; + expect(secondRevokeButton).toBeDefined(); + fireEvent.click(secondRevokeButton as HTMLElement); + fireEvent.click(screen.getByText('Confirm revoke')); + await waitFor(() => expect(mockRevokeApiKey).toHaveBeenCalledWith('key-2')); + + // Row 2 finishing must not resurrect row 1's plain "Revoke" button or + // otherwise clear row 1's still-pending confirm state. Row 1's confirm + // button itself now reads "Revoking" (it's genuinely busy), so assert on + // the stable confirm-panel copy rather than the label that flips to busy + // text — and assert the plain (non-armed) "Revoke" button is gone from + // that row. + await screen.findByText('Revoked'); + const row1 = screen.queryByText('CI bot')?.closest('li') as HTMLElement; + expect(within(row1).getByText(/can't be undone/i)).toBeTruthy(); + expect(within(row1).queryByText('Revoke')).toBeNull(); + + // Now let row 1 resolve too. + resolveFirst?.({ key: key({ id: 'key-1', revokedAt: Date.now() }) }); + await waitFor(() => expect(screen.getAllByText('Revoked').length).toBe(2)); + }); }); diff --git a/web-ui/app/admin/api-keys/_components/shared.tsx b/web-ui/app/admin/api-keys/_components/shared.tsx index 11e8c0847..f24dbadcd 100644 --- a/web-ui/app/admin/api-keys/_components/shared.tsx +++ b/web-ui/app/admin/api-keys/_components/shared.tsx @@ -19,8 +19,41 @@ export const card = 'rounded-lg border border-[color:var(--border)] bg-[color:va export const chipCls = 'type-mono-data inline-flex items-center rounded-full border border-[color:var(--border)] px-2 py-0.5 text-[11px] lowercase text-[color:var(--fg-muted)]'; -export function toFriendlyError(err: unknown): string { - if (err instanceof ApiError) return err.body || err.message; +/** + * Block-level error text — text + a 1px edge + a light tint, never + * text-only and never a solid fill (Lume state-color rule, spec §2.6). Same + * recipe used repo-wide for inline error banners, e.g. + * `admin/domains/page.tsx`'s `loadError` paragraph. + */ +export const errorTextCls = + 'rounded-md border border-[color:var(--danger-edge)]/40 bg-[color:var(--danger)]/10 px-3 py-2 text-[13px] text-[color:var(--danger)]'; + +/** Compact inline variant of `errorTextCls` for a short note sitting next to + * buttons rather than a full-width paragraph (e.g. a copy-to-clipboard + * fallback notice) — same border+tint recipe, smaller footprint. */ +export const errorInlineCls = + 'inline-flex items-center gap-1 rounded-md border border-[color:var(--danger-edge)]/50 bg-[color:var(--danger)]/8 px-2 py-0.5 text-[12px] text-[color:var(--danger)]'; + +type TFn = (key: string, values?: Record) => string; + +/** + * web-ui/CLAUDE.md hard rule #3: never render a raw ApiError/exception + * message as the primary UI text. Maps the backend's known `{error|code}` + * response shapes (see `adminKeysRouter.ts`) to translated catalog messages + * and falls back to a generic "request failed (status N)" message — never + * the raw response body, which for a 400 can be a zod `issues` array dumped + * as JSON. Mirrors `admin/registries/page.tsx`'s `toFriendlyError`. + */ +export function toFriendlyError(err: unknown, t: TFn): string { + if (err instanceof ApiError) { + if (err.body.includes('not_found')) return t('errors.notFound'); + if (err.body.includes('operator_auth.unavailable')) return t('errors.authUnavailable'); + if (err.body.includes('auth.missing') || err.body.includes('auth.invalid')) { + return t('errors.sessionExpired'); + } + if (err.body.includes('invalid_request')) return t('errors.invalidRequest'); + return t('errors.generic', { status: err.status }); + } return err instanceof Error ? err.message : String(err); } diff --git a/web-ui/messages/de.json b/web-ui/messages/de.json index 749c5dea9..5e5e4c264 100644 --- a/web-ui/messages/de.json +++ b/web-ui/messages/de.json @@ -4154,6 +4154,7 @@ }, "rateLimitInvalid": "Rate-Limit muss zwischen {min} und {max} Anfragen/Minute liegen.", "scopesRequired": "Mindestens einen Scope auswählen — ein Schlüssel ohne Scope kann nichts tun.", + "blockedByReveal": "Das Token oben zuerst ausblenden, bevor ein weiterer Schlüssel erstellt wird.", "submit": "Schlüssel erstellen", "creating": "Wird erstellt" }, @@ -4183,6 +4184,13 @@ "confirm": "Widerruf bestätigen", "cancel": "Abbrechen" } + }, + "errors": { + "notFound": "Dieser Schlüssel existiert nicht mehr — die Liste wurde aktualisiert.", + "authUnavailable": "Operator-Authentifizierung ist gerade nicht verfügbar. Bitte in Kürze erneut versuchen.", + "sessionExpired": "Die Sitzung ist abgelaufen. Weiterleitung zur Anmeldung …", + "invalidRequest": "Die Anfrage war ungültig — bitte die Werte oben prüfen und erneut versuchen.", + "generic": "Etwas ist schiefgelaufen (Fehler {status})." } } } diff --git a/web-ui/messages/en.json b/web-ui/messages/en.json index de7147312..85edfb8dd 100644 --- a/web-ui/messages/en.json +++ b/web-ui/messages/en.json @@ -4154,6 +4154,7 @@ }, "rateLimitInvalid": "Rate limit must be between {min} and {max} requests/minute.", "scopesRequired": "Select at least one scope — a key with none cannot do anything.", + "blockedByReveal": "Dismiss the token above before creating another key.", "submit": "Create key", "creating": "Creating" }, @@ -4183,6 +4184,13 @@ "confirm": "Confirm revoke", "cancel": "Cancel" } + }, + "errors": { + "notFound": "That key no longer exists — the list has been refreshed.", + "authUnavailable": "Operator authentication is unavailable right now. Try again shortly.", + "sessionExpired": "Your session has expired. Redirecting to sign in…", + "invalidRequest": "That request was invalid — check the values above and try again.", + "generic": "Something went wrong (error {status})." } } }