Scopes this plan's immediate implementation target to Setup + User Stories 0-3 (sign-in, agent dashboard, ticket workbench, support-org admin) - the P1 MVP - given the feature's overall size (8 user stories against a fully empty lib/api, lib/auth, lib/query, and empty test configs). Key decisions: session token in a plain cookie (readable by both Next.js middleware and client-side axios, since supporthub-api only reads a Bearer header, never a cookie), middleware does a cheap unverified presence/role check for UX redirects only - the real gate stays supporthub-api's own token verification - and lib/api is the one typed client layer per Constitution Principle IV. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
6.3 KiB
6.3 KiB
Research: Agent and Admin UI
Decision: session token lives in a plain (non-httpOnly) cookie, not localStorage
- Decision: After a successful
POST /auth/login(supporthub-api, 010-identity-auth), the frontend itself sets a cookie (sh_session,Securein production,SameSite=Lax,Max-Agematching the token's own 4-hour lifetime) holding the raw JWT.lib/api's axios instance reads it via a request interceptor and attachesAuthorization: Bearer <token>. Next.js middleware (FR-000) reads the same cookie server-side to decide whether to redirect to sign-in, and decodes (not verifies) its payload to read theroleclaim for portal gating. - Rationale: A cookie is the only session-storage mechanism readable from both Next.js
middleware (which runs before any page renders, server-side, and cannot read
localStorage) and client-side code (which needs the raw token to build theAuthorizationheader, since supporthub-api only ever reads that header — never a cookie itself, and this feature does not modify supporthub-api's auth plugin to accept one).localStoragealone would leave middleware blind, defeating FR-000's redirect-before-render requirement. - Alternatives considered: An httpOnly cookie plus a Next.js Route Handler (BFF) proxy for every supporthub-api call, so the browser never touches the raw token — more secure against XSS, but adds a full proxy layer duplicating every backend route as a Next.js route handler, for a feature whose own constitution (Principle IV) already mandates a single typed client layer as the XSS mitigation surface (sanitizing/escaping is still React's own default behavior). Rejected as disproportionate infrastructure for an internal agent/admin tool; revisit if a future security review calls for it.
Decision: Next.js middleware does a cheap, unverified role/presence check; the API is still the real gate
- Decision:
middleware.tsruns on every(support)/(admin)request: nosh_sessioncookie → redirect to/sign-in; cookie present → base64-decode the JWT payload (no signature check) to readrole, and redirect a non-ADMINrole away from(admin)routes. The actual security boundary remains supporthub-api itself (Constitution Principle I) — every real data fetch still carries the same token, and 010's ownfastify.authenticate/requireRolereject an invalid, expired, or revoked token regardless of what the middleware decided. - Rationale: Matches spec.md's Edge Cases: middleware gating is a UX/navigation concern (not
rendering a portal that will just fail its own data fetches), not a re-implementation of
authorization (Principle I forbids that). A full signature verification in middleware would
need
jsonwebtoken(or the Edge-compatiblejose) plus the same secret duplicated into the frontend's own environment — an unnecessary second copy of a decision supporthub-api already makes correctly on every request. - Alternatives considered: Full JWT verification in middleware — rejected; duplicates
JWT_SECRETinto a second codebase for no additional real security (an unverified decode still redirects instantly on outright tampering the moment the first API call 401s), and contradicts Principle I's "never re-derive" language more directly than a presence-only check.
Decision: axios instance + interceptors, wrapped by TanStack Query hooks (Principle IV)
- Decision:
lib/api/client.tsexports one configured axios instance (baseURL: env.NEXT_PUBLIC_API_URL), a request interceptor attaching the session cookie's token, and a response interceptor that clears the session and redirects to sign-in on a401(FR-000 Scenario 4). Every domain concern (lib/api/tickets.ts,lib/api/agents.ts, etc.) exports plain async functions calling that instance, each typed against the backend's ownspecs/*/contracts/*.mdshapes.features/*modules only ever call these functions through TanStack Query'suseQuery/useMutation, neveraxios/fetchdirectly. - Rationale: This is Principle IV's own literal requirement — one client layer is what keeps a backend contract change from becoming a scattered runtime break.
- Alternatives considered: A codegen'd client from an OpenAPI spec — supporthub-api doesn't
currently publish one as part of its own CI (
scripts/generate-openapi.tsexists but isn't a build-time contract source consumed here); hand-written typed functions against thecontracts/ *.mdfiles were preferred to avoid introducing a new cross-repo build dependency for this feature specifically.
Decision: business-rule rejections (409/400) surface the backend's own message verbatim
- Decision: The axios response interceptor does NOT swallow or rewrite 4xx error bodies —
it re-throws an
ApiErrorcarrying the backend's ownerror.message/error.code({success:false,error:{code,message}}, the shape every supporthub-api response already uses). UI components render that message directly for a failed mutation (FR-012/SC-002), never a generic "Something went wrong." - Rationale: SC-002 requires the backend's own reason be shown 100% of the time — this is only possible if the client layer preserves it rather than mapping every error to one generic state.
- Alternatives considered: A generic per-mutation error message with the detail only in a console log — rejected outright by SC-002's own wording.
Decision: loading/empty/error as an explicit three-state discriminated union, not three booleans
- Decision: A small shared hook/helper (
lib/query/query-state.ts) derives one of'loading' | 'empty' | 'error' | 'ready'from a TanStack Query result (isLoading,isError, anddata.length === 0for list queries), and every list/detail view switches on that single value rather than independently checkingisLoading/isError/data. - Rationale: FR-010/SC-003 require never conflating loading, empty, and error — a discriminated union makes "forgot to handle the error case" a TypeScript exhaustiveness error at the call site instead of a runtime bug found by manual testing.
- Alternatives considered: Ad hoc
if (isLoading) ... else if (isError) ...per component — workable but leaves FR-010 compliance unenforced by the type system; rejected once the shared helper's cost was seen to be trivial.