saqib mirandClaude Sonnet 5
273e73be07
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>
86 lines
6.3 KiB
Markdown
86 lines
6.3 KiB
Markdown
# 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`, `Secure` in production, `SameSite=Lax`, `Max-Age`
|
|
matching the token's own 4-hour lifetime) holding the raw JWT. `lib/api`'s axios instance reads
|
|
it via a request interceptor and attaches `Authorization: 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 the `role` claim 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 the `Authorization` header, 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). `localStorage` alone 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.ts` runs on every `(support)`/`(admin)` request: no `sh_session`
|
|
cookie → redirect to `/sign-in`; cookie present → base64-decode the JWT payload (no signature
|
|
check) to read `role`, and redirect a non-`ADMIN` role 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 own `fastify.authenticate`/`requireRole` reject
|
|
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-compatible `jose`) 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_SECRET` into 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.ts` exports 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 a `401`
|
|
(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 own
|
|
`specs/*/contracts/*.md` shapes. `features/*` modules only ever call these functions through
|
|
TanStack Query's `useQuery`/`useMutation`, never `axios`/`fetch` directly.
|
|
- **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.ts` exists but isn't a
|
|
build-time contract source consumed here); hand-written typed functions against the `contracts/
|
|
*.md` files 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 `ApiError` carrying the backend's own `error.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`,
|
|
and `data.length === 0` for list queries), and every list/detail view switches on that single
|
|
value rather than independently checking `isLoading`/`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.
|