From 140a34b81e7879ca40ae762504f7f44d65315cc0 Mon Sep 17 00:00:00 2001 From: ux-frontend Date: Sat, 23 May 2026 11:14:32 +0200 Subject: [PATCH] fix(frontend): align types + UI to backend contract (docs/api.md @ dd5c508) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Backend pushed the authoritative contract in docs/api.md and tightened the error envelope via a global HTTPException handler (dd5c508). This commit folds the frontend onto that contract — every drift flagged by the code-reviewer MAJOR is closed. Types (src/types/api.ts) - User: `id` → `user_id`; `display_name` is `string | null`; add `permissions: string[]` and `groups: string[]`; drop `engagement_id` and `engagement_name` (not part of CurrentUser). - Engagement: drop `name`, `client_name` is non-null `string`; status enum aligned to `draft | active | closed | archived`; `c2_type` is non-null `C2Type`; drop `created_at` (not in EngagementRead v1). - EngagementCreate body: `client_name` required, plus optional `description`, `c2_type`, `start_date`, `end_date`. No `name`. - Replace ApiError + ApiValidationError with a single uniform envelope: `{ error: string, message: string, details?: PydanticErrorItem[] }`, matching the new HTTPException handler. PydanticErrorItem is the per-field shape on 422 (`{ loc, msg, type }`). Fetch client (src/lib/api.ts) - `bodyAsApiError` now recognizes the uniform envelope by shape (error+message strings). Anything else returns null so callers fall back to a generic message — keeps us robust if the backend ever emits a non-JSON response. Engagements API (src/screens/engagements/engagementsApi.ts) - Drop the `{ items: [] }` envelope tolerance — backend serves a bare `Engagement[]`. - Hit `/engagements/` with trailing slash explicitly; backend now sets `strict_slashes=False` but staying consistent with docs/api.md. EngagementsPage - Status tone map switched to the new enum (`draft → pending`, `closed → soc`). - Drop "Name" column. `client_name` is the primary identifier; the description column replaces the now-meaningless name field. - `c2_type` is non-null, so no nullable rendering path. EngagementCreateDialog - Drop `name` field. New required field is `client_name`; add a `c2_type` select (default `mythic`); brief textarea stays optional. - `mapValidationErrors` now reads `body.details[*].loc` (last segment matches the form field) — direct alignment with the backend's new shape after dd5c508. - 401 still surfaces "Session expirée"; 403 gains a dedicated message; other errors fall back to a capitalized backend `message` when available, then to a generic French string. Sidebar - Display fallback: `user.display_name ?? user.username` (now nullable). - Drop the `ENG · {engagement_name}` line; show `user.username` (the email) as the secondary identity instead. LoginPage - Field label "Username" → "Email or username" so RT users with email accounts find the field semantically obvious (per docs/api.md note on the username/email mapping). Tests (Vitest, 14 cases, all green) - Refreshed fixtures to the new shapes (no more `name`, no `created_at`, status `draft`, envelopes carry `error`+`message`). - New 422 test exercises the `details[*].loc` mapping shape. - New 401 test on the dialog covers the top-of-form alert path. --- frontend/src/components/shell/Sidebar.tsx | 13 +- frontend/src/lib/api.ts | 36 ++-- .../EngagementCreateDialog.test.tsx | 46 +++-- .../engagements/EngagementCreateDialog.tsx | 172 ++++++++++++------ .../engagements/EngagementsPage.test.tsx | 11 +- .../screens/engagements/EngagementsPage.tsx | 27 ++- .../src/screens/engagements/engagementsApi.ts | 16 +- frontend/src/screens/login/LoginPage.tsx | 4 +- frontend/src/types/api.ts | 82 +++++---- 9 files changed, 256 insertions(+), 151 deletions(-) diff --git a/frontend/src/components/shell/Sidebar.tsx b/frontend/src/components/shell/Sidebar.tsx index 643ec34..e955228 100644 --- a/frontend/src/components/shell/Sidebar.tsx +++ b/frontend/src/components/shell/Sidebar.tsx @@ -108,7 +108,7 @@ export function Sidebar() {
- {user.display_name} + {user.display_name ?? user.username}
- {user.engagement_name && ( -
- ENG · {user.engagement_name} -
- )} +
+ {user.username} +
); diff --git a/frontend/src/lib/api.ts b/frontend/src/lib/api.ts index 1cbdd26..5cb04b6 100644 --- a/frontend/src/lib/api.ts +++ b/frontend/src/lib/api.ts @@ -3,27 +3,27 @@ * * Responsibilities: * - Inject `credentials: 'include'` so the session cookie travels with - * every call (the cookie itself is HttpOnly + Secure, set by the backend). - * - Set JSON Content-Type on bodied requests and parse JSON responses. - * - Normalize 4xx/5xx into a typed `ApiClientError` so callers can branch - * on `error.status` and `error.detail` without duplicating parsing. + * every call (cookie is HttpOnly + Secure, set by the backend). + * - Send JSON on bodied requests and parse JSON responses. + * - Normalize 4xx/5xx into a typed `ApiClientError` so callers can + * branch on `error.status` and `error.body` without re-parsing. * * Deliberate non-features: * - No retry loop. TanStack Query owns that policy. - * - No CSRF token: same-origin in prod (Caddy), same-origin via Vite proxy - * in dev. SameSite=Lax cookie is enough for our threat model (no - * cross-site form posts in scope). + * - No CSRF token: same-origin in prod (Caddy), same-origin via the + * Vite proxy in dev. SameSite=Lax cookie is enough — no cross-site + * form posts in scope. */ -import type { ApiError, ApiValidationError } from '@/types/api'; +import type { ApiError } from '@/types/api'; const DEFAULT_BASE = '/api/v1'; export class ApiClientError extends Error { status: number; - body: ApiError | ApiValidationError | null; + body: ApiError | null; - constructor(status: number, message: string, body: ApiError | ApiValidationError | null) { + constructor(status: number, message: string, body: ApiError | null) { super(message); this.name = 'ApiClientError'; this.status = status; @@ -48,13 +48,17 @@ async function parseBody(response: Response): Promise { } } -function bodyAsApiError(body: unknown): ApiError | ApiValidationError | null { +/** + * Try to coerce the response body into the `{error, message, details?}` + * envelope documented in api.md. Backends that haven't routed every error + * through that handler yet (raw `flask.abort(...)` HTML output, plain text, + * other shapes) return `null` so callers can fall back to a generic message. + */ +function bodyAsApiError(body: unknown): ApiError | null { if (typeof body !== 'object' || body === null) return null; - if (Array.isArray((body as { detail?: unknown }).detail)) { - return body as ApiValidationError; - } - if (typeof (body as { detail?: unknown }).detail === 'string') { - return body as ApiError; + const obj = body as Record; + if (typeof obj.error === 'string' && typeof obj.message === 'string') { + return obj as unknown as ApiError; } return null; } diff --git a/frontend/src/screens/engagements/EngagementCreateDialog.test.tsx b/frontend/src/screens/engagements/EngagementCreateDialog.test.tsx index 330034f..be6ec18 100644 --- a/frontend/src/screens/engagements/EngagementCreateDialog.test.tsx +++ b/frontend/src/screens/engagements/EngagementCreateDialog.test.tsx @@ -10,23 +10,25 @@ describe('EngagementCreateDialog', () => { fetchMock?.restore(); }); - it('rejects empty name client-side without calling the backend', () => { + it('rejects empty client name client-side without calling the backend', () => { fetchMock = installFetchMock([]); renderWithProviders(); fireEvent.click(screen.getByRole('button', { name: /arm engagement/i })); - expect(screen.getByText(/nom requis/i)).toBeInTheDocument(); + expect(screen.getByText(/client requis/i)).toBeInTheDocument(); expect(fetchMock.calls).toHaveLength(0); }); - it('maps 422 Pydantic errors to per-field messages', async () => { + it('maps 422 backend errors (details[].loc) to per-field messages', async () => { fetchMock = installFetchMock([ { status: 422, body: { - detail: [ + error: 'validation_error', + message: 'request failed', + details: [ { - loc: ['body', 'name'], - msg: 'String should have at least 3 characters', + loc: ['client_name'], + msg: 'String should have at least 1 character', type: 'string_too_short', }, ], @@ -35,11 +37,11 @@ describe('EngagementCreateDialog', () => { ]); renderWithProviders(); - fireEvent.change(screen.getByLabelText(/engagement name/i), { target: { value: 'AB' } }); + fireEvent.change(screen.getByLabelText(/client name/i), { target: { value: 'x' } }); fireEvent.click(screen.getByRole('button', { name: /arm engagement/i })); await waitFor(() => { - expect(screen.getByText(/at least 3 characters/i)).toBeInTheDocument(); + expect(screen.getByText(/at least 1 character/i)).toBeInTheDocument(); }); }); @@ -50,20 +52,18 @@ describe('EngagementCreateDialog', () => { status: 201, body: { id: 'eng_new', - name: 'OPERATION ZETA', - client_name: null, + client_name: 'OPERATION ZETA', description: null, - status: 'planning', - c2_type: null, + status: 'draft', + c2_type: 'mythic', start_date: null, end_date: null, - created_at: '2026-05-23T08:00:00Z', }, }, ]); renderWithProviders(); - fireEvent.change(screen.getByLabelText(/engagement name/i), { + fireEvent.change(screen.getByLabelText(/client name/i), { target: { value: 'OPERATION ZETA' }, }); fireEvent.click(screen.getByRole('button', { name: /arm engagement/i })); @@ -71,7 +71,23 @@ describe('EngagementCreateDialog', () => { await waitFor(() => { expect(onClose).toHaveBeenCalledTimes(1); }); - expect(fetchMock.calls[0]?.url).toBe('/api/v1/engagements'); + expect(fetchMock.calls[0]?.url).toBe('/api/v1/engagements/'); expect(fetchMock.calls[0]?.init?.method).toBe('POST'); }); + + it('surfaces a generic top-of-form error on 401', async () => { + fetchMock = installFetchMock([ + { + status: 401, + body: { error: 'not_authenticated', message: 'no active session' }, + }, + ]); + renderWithProviders(); + fireEvent.change(screen.getByLabelText(/client name/i), { + target: { value: 'Anyone' }, + }); + fireEvent.click(screen.getByRole('button', { name: /arm engagement/i })); + const alert = await screen.findByRole('alert'); + expect(alert.textContent).toMatch(/session expirée/i); + }); }); diff --git a/frontend/src/screens/engagements/EngagementCreateDialog.tsx b/frontend/src/screens/engagements/EngagementCreateDialog.tsx index 3a0581b..8497bb4 100644 --- a/frontend/src/screens/engagements/EngagementCreateDialog.tsx +++ b/frontend/src/screens/engagements/EngagementCreateDialog.tsx @@ -12,14 +12,20 @@ import { import { useMutation, useQueryClient } from '@tanstack/react-query'; import { Button } from '@/components/ui/Button'; import { ApiClientError } from '@/lib/api'; -import type { ApiValidationError } from '@/types/api'; +import type { ApiError, C2Type, PydanticErrorItem } from '@/types/api'; import { createEngagement, ENGAGEMENTS_QUERY_KEY } from './engagementsApi'; interface EngagementCreateDialogProps { onClose: () => void; } -type FieldErrors = Partial>; +type FieldKey = 'client_name' | 'description' | 'c2_type'; +type FieldErrors = Partial>; + +const C2_OPTIONS: ReadonlyArray<{ value: C2Type; label: string }> = [ + { value: 'mythic', label: 'Mythic' }, + { value: 'home', label: 'Home (RT-internal)' }, +]; /** * "Arm engagement" dialog. @@ -35,19 +41,21 @@ type FieldErrors = Partial(null); const firstFieldRef = useRef(null); - const [name, setName] = useState(''); const [clientName, setClientName] = useState(''); const [description, setDescription] = useState(''); + const [c2Type, setC2Type] = useState('mythic'); const [fieldErrors, setFieldErrors] = useState({}); const [topError, setTopError] = useState(null); @@ -60,14 +68,24 @@ export function EngagementCreateDialog({ onClose }: EngagementCreateDialogProps) onClose(); }, onError: (err) => { - if (err instanceof ApiClientError && err.status === 422 && err.body) { - setFieldErrors(mapValidationErrors(err.body as ApiValidationError)); - setTopError(null); - return; - } - if (err instanceof ApiClientError && err.status === 401) { - setTopError('Session expirée. Reconnectez-vous.'); - return; + if (err instanceof ApiClientError) { + if (err.status === 422 && err.body?.details) { + setFieldErrors(mapValidationErrors(err.body.details)); + setTopError(null); + return; + } + if (err.status === 401) { + setTopError('Session expirée. Reconnectez-vous.'); + return; + } + if (err.status === 403) { + setTopError('Action interdite pour ce rôle.'); + return; + } + if (err.body?.message) { + setTopError(genericMessage(err.body)); + return; + } } setTopError('Création impossible. Réessayez dans un instant.'); }, @@ -75,7 +93,6 @@ export function EngagementCreateDialog({ onClose }: EngagementCreateDialogProps) const isPending = mutation.isPending; - // Focus the first field on open. ESC closes unless a request is in flight. useEffect(() => { firstFieldRef.current?.focus(); const onKeyDown = (e: globalThis.KeyboardEvent) => { @@ -88,11 +105,10 @@ export function EngagementCreateDialog({ onClose }: EngagementCreateDialogProps) return () => window.removeEventListener('keydown', onKeyDown); }, [isPending, onClose]); - // Rudimentary focus trap: cycle Tab/Shift+Tab within the dialog. const handleSurfaceKeyDown = (e: KeyboardEvent) => { if (e.key !== 'Tab' || !surfaceRef.current) return; const focusables = surfaceRef.current.querySelectorAll( - 'input, textarea, button, [tabindex]:not([tabindex="-1"])', + 'input, textarea, select, button, [tabindex]:not([tabindex="-1"])', ); if (focusables.length === 0) return; const first = focusables[0]; @@ -112,14 +128,14 @@ export function EngagementCreateDialog({ onClose }: EngagementCreateDialogProps) e.preventDefault(); setFieldErrors({}); setTopError(null); - if (!name.trim()) { - setFieldErrors({ name: 'Nom requis.' }); + if (!clientName.trim()) { + setFieldErrors({ client_name: 'Client requis.' }); return; } mutation.mutate({ - name: name.trim(), - client_name: clientName.trim() || undefined, - description: description.trim() || undefined, + client_name: clientName.trim(), + description: description.trim() || null, + c2_type: c2Type, }); }; @@ -140,7 +156,6 @@ export function EngagementCreateDialog({ onClose }: EngagementCreateDialogProps) backgroundColor: 'oklch(5.8% 0.012 247 / 0.78)', }} > - {/* Faint scanline texture overlay — reads as "instrument feed paused" */}