From 3b4c220a45c45e6cd5efc1708e19691397d68652 Mon Sep 17 00:00:00 2001 From: saqib mir Date: Mon, 7 Sep 2026 11:14:42 +0530 Subject: [PATCH] plan: design for identity and authentication feature (010) Phase 0 research resolves the library choices (jsonwebtoken + bcryptjs, chosen partly to avoid native-build friction on Windows dev environments), the Redis-backed revocation-denylist shape (reusing 002's own jti-replay-protection pattern exactly), a 4-hour token lifetime, and why fastify.authenticate populating the already-shared reqContext.actorId/actorType retroactively makes every audit trail since 007 accurate for real agent/admin actions instead of always 'unknown'. Also surfaces and scopes a real gap found along the way: User (login identity) and Agent (routing/skills profile) have never been linked. Adds Agent.userId as a nullable FK now (cheap, additive) without building the actual linking workflow, which belongs in 006's own identity/agents admin screens as a later, separate piece of work. Phase 1 adds data-model.md, the login/self-identity/account-creation/ logout contract, and five quickstart scenarios including a specific requirement to re-verify at least one already-shipped admin route per module (002-009), not just this feature's own new endpoints. Co-Authored-By: Claude Sonnet 5 --- .../contracts/identity-auth-contract.md | 48 +++++++ specs/010-identity-auth/data-model.md | 60 ++++++++ specs/010-identity-auth/plan.md | 136 ++++++++++++++++++ specs/010-identity-auth/quickstart.md | 52 +++++++ specs/010-identity-auth/research.md | 136 ++++++++++++++++++ 5 files changed, 432 insertions(+) create mode 100644 specs/010-identity-auth/contracts/identity-auth-contract.md create mode 100644 specs/010-identity-auth/data-model.md create mode 100644 specs/010-identity-auth/plan.md create mode 100644 specs/010-identity-auth/quickstart.md create mode 100644 specs/010-identity-auth/research.md diff --git a/specs/010-identity-auth/contracts/identity-auth-contract.md b/specs/010-identity-auth/contracts/identity-auth-contract.md new file mode 100644 index 0000000..5f65575 --- /dev/null +++ b/specs/010-identity-auth/contracts/identity-auth-contract.md @@ -0,0 +1,48 @@ +# Contract: Identity and Authentication + +## Login + +- `POST /auth/login` — body `{ email, password }`. `401` on any failure (wrong password, no + such account, or a deactivated account) with an identical response body/status in every case + (FR-002/SC-003) — never a distinguishable "no such user" vs "wrong password." `200` with + `{ token, user: { id, email, name, role } }` on success. + +## Self-identity + +- `GET /auth/me` — gated by `fastify.authenticate`. `401` if the token is missing/invalid/ + expired/revoked. `401` if the account behind a structurally-valid token no longer exists or + is deactivated (re-validated against current state, not the token's own claims alone). `200` + with `{ id, email, name, role }` on success. + +## Account creation (admin-only) + +- `POST /admin/users` — gated by `fastify.authenticate` + `requireRole('ADMIN')`. Body + `{ email, name, role, password }` (`role` one of `ADMIN`/`AGENT`). `403` for a valid non-admin + session. `409` if `email` is already in use. `201` with the created `{ id, email, name, role + }` (never the password or its hash) on success. + +## Logout + +- `POST /auth/logout` — gated by `fastify.authenticate`. Revokes the calling token's own `jti` + (Redis denylist, TTL = remaining lifetime) so it's rejected on any further use even before its + natural expiry. `200` on success. + +## Guarantees (callable contract) + +1. **Every route already gated by `fastify.authenticate` across 002-009 continues to accept a + valid session and now genuinely rejects a missing/invalid/expired/revoked one** — the gate + itself changes from a no-op to a real check; which routes carry the gate is unchanged + (FR-006, SC-001). +2. **A route additionally gated by `requireRole('ADMIN')` rejects a structurally valid session + whose role isn't `ADMIN`, with a response distinguishable from "no valid session at all"** + (403 vs 401) (FR-005, SC-002). +3. **A login failure never reveals whether the submitted email corresponds to an existing + account** — verified by comparing the exact response for a wrong password against a wholly + nonexistent email (FR-002, SC-003). +4. **No password is ever stored, logged, or returned anywhere in plaintext** — only + `passwordHash` is persisted, and no response body (login, self-identity, account creation) + ever includes it (FR-003, SC-004). +5. **An admin-created account can log in immediately with the password it was created with, no + manual step in between** (FR-008, SC-005). +6. **A token revoked via logout is rejected on any further use, even before its natural expiry** + (FR-009). diff --git a/specs/010-identity-auth/data-model.md b/specs/010-identity-auth/data-model.md new file mode 100644 index 0000000..a658d4c --- /dev/null +++ b/specs/010-identity-auth/data-model.md @@ -0,0 +1,60 @@ +# Data Model: Identity and Authentication + +## User (modified — two additive columns) + +| Field | Type | Notes | +|---|---|---| +| `id` | `String @id @default(uuid())` | unchanged | +| `email` | `String @unique` | unchanged | +| `name` | `String` | unchanged | +| `role` | `UserRole @default(CUSTOMER)` | unchanged enum (`ADMIN \| AGENT \| CUSTOMER`) — this feature never assigns `CUSTOMER` (research.md/spec.md Assumptions); every row this feature creates or updates is `ADMIN` or `AGENT` | +| **`passwordHash`** | **`String`** | **new** — bcryptjs hash, never the plaintext password; `NOT NULL` since every account this feature manages must be able to log in (FR-010 requires both seeded demo accounts to get a real one) | +| **`active`** | **`Boolean @default(true)`** | **new** — mirrors `Agent.active`'s existing convention exactly; a deactivated account's session is rejected on re-validation (Edge Cases/User Story 3), without a hard delete | +| `createdAt` / `updatedAt` | `DateTime` | unchanged | + +## Agent (modified — one additive, nullable column) + +| Field | Type | Notes | +|---|---|---| +| **`userId`** | **`String? @unique`** | **new** — nullable FK to `User.id`, the schema capability to identify which login identity a routing/skills profile belongs to (research.md). No endpoint in this feature sets it; a follow-up in `identity/agents` (006) is expected to. | + +No new Prisma model for "Session" — a session is a signed JWT the server never persists +(research.md's short-lived-JWT-plus-revocation-denylist decision); the denylist itself lives in +Redis (`auth:revoked:`, TTL = remaining token lifetime), not Postgres. + +## JwtPayload (existing type, `src/common/types/auth.types.ts` — one additive field) + +| Field | Type | Notes | +|---|---|---| +| `sub` | `string` | the `User.id` | +| `email` | `string` | unchanged | +| `role` | `string` | unchanged — `User.role` at issuance time | +| `actorType` | `ActorType` | unchanged — always `ActorType.USER` for these sessions (research.md) | +| **`jti`** | **`string`** | **new** — random UUID per issued token, the revocation-denylist key | +| `iat` / `exp` | `number` | unchanged, standard JWT claims | + +## AuthUser (existing type, unchanged) + +`{ id, email, role, actorType }` — what `fastify.authenticate` sets on `request.user` after +verifying the token; the same fields returned by login (FR-001) and the self-identity endpoint +(FR-007), minus `jti`/`iat`/`exp` (those are token bookkeeping, not identity). + +## Validation rules + +- Login: `email` a valid email string, `password` non-empty. The response for "no such user" + and "wrong password" MUST be byte-for-byte identical (FR-002/SC-003) — achieved by always + running the bcrypt comparison against either the found user's hash or a fixed dummy hash when + no user is found, so the response timing and shape never differ by branch. +- Account creation (User Story 4): `email` valid and not already in use, `name` non-empty, + `role` one of `ADMIN`/`AGENT` (never `CUSTOMER`, research.md), `password` non-empty (hashed + before storage, never persisted or logged in plaintext). + +## State / lifecycle + +- `User.active` (new column, above) is the deactivation flag. The self-identity endpoint (User + Story 3) re-fetches the `User` row by `sub` on every call and rejects if it no longer exists + or `active: false` — this is the feature's only server-side re-validation path; `fastify + .authenticate` itself does not re-fetch on every request (that would defeat the point of a + stateless JWT check), so a deactivated account's *other* already-issued-token requests remain + valid until that token's natural expiry or an explicit logout, exactly as spec.md's Edge Cases + already scopes it ("a short token lifetime bounds the rest"). diff --git a/specs/010-identity-auth/plan.md b/specs/010-identity-auth/plan.md new file mode 100644 index 0000000..0c4b75a --- /dev/null +++ b/specs/010-identity-auth/plan.md @@ -0,0 +1,136 @@ +# Implementation Plan: Identity and Authentication + +**Branch**: `010-identity-auth` | **Date**: 2026-09-07 | **Spec**: [spec.md](./spec.md) + +**Input**: Feature specification from `specs/010-identity-auth/spec.md` + +## Summary + +Finishes the original, never-wired scaffold: `identity/auth`'s email-only login stub becomes a +real bcryptjs-verified, JWT-issuing login; `fastify.authenticate` (currently a complete no-op) +becomes a real signature/expiry/revocation check populating `request.user` and the already- +shared `request.reqContext.actorId`/`actorType` fields every module since 007 already reads; a +new `requireRole(...roles)` preHandler factory adds role-based gating on top. `User` gains +`passwordHash` and `active` columns. Logout revokes a token's `jti` via the same Redis-denylist +shape 002's replay protection already established. + +## Technical Context + +**Language/Version**: TypeScript 5.4 / Node.js 20+. + +**Primary Dependencies**: `jsonwebtoken` (new — JWT sign/verify), `bcryptjs` (new — password +hashing, pure JS to avoid native-build friction on Windows dev environments). Reuses existing +`ioredis` for the revocation denylist. + +**Storage**: PostgreSQL via Prisma (`User.passwordHash`, `User.active`). Redis for the +revocation denylist (`auth:revoked:`, TTL = remaining token lifetime) — same shape as +002's `hasSeenJti`/`markJtiSeen`. + +**Testing**: Vitest — unit tests for password verification's identical-response-on-failure +behavior and the `requireRole` preHandler's role-matching logic; integration tests against real +Postgres/Redis for the full login → gated-route → logout flow, and specifically re-verifying at +least one already-shipped admin route per module (002-009) now genuinely rejects an invalid +session. + +**Target Platform**: Same Fastify modular monolith. Modifies `src/plugins/auth.plugin.ts`, +populates `src/modules/identity/auth/`, adds `POST /admin/users` (a new small surface, placed +alongside `identity/agents`'s own admin routes since account management is an identity concern, +not `identity/auth`'s own — `identity/auth` owns login/logout/self-identity, not account CRUD). + +**Project Type**: Backend service — single project. + +**Performance Goals**: Token verification (signature + expiry + Redis denylist check) must stay +a single Redis round trip, not a Postgres query, on every gated request — only the self-identity +endpoint (User Story 3) re-fetches from Postgres, by design (research.md). + +**Constraints**: MUST NOT reveal account existence via login's failure response (FR-002); MUST +NOT ever store or return a plaintext password (FR-003); MUST NOT change which existing routes +are gated, only make the gate real (FR-006); MUST re-validate against current account state on +the self-identity endpoint specifically, not on every request (data-model.md). + +**Scale/Scope**: One modified plugin (`auth.plugin.ts`), one populated module +(`identity/auth`), one new small admin-account-creation surface, two new dependencies, two new +`User` columns, one seed-script update. Explicitly excludes: password reset, MFA, login-specific +rate-limiting, and retroactively adding `requireRole('ADMIN')` to every existing admin route +beyond a representative sample (research.md — tracked as this feature's own Polish-phase +mechanical task, not a redesign of any other feature's access model). + +## Constitution Check + +*GATE: Must pass before Phase 0 research. Re-check after Phase 1 design.* + +| Principle / Section | Check | Result | +|---|---|---| +| I. SaaS Is the Sole Identity & Access Authority | This feature authenticates SupportHub's own staff (`User`/`Agent`), explicitly never `CUSTOMER`-role accounts (research.md/spec.md Assumptions) — customer identity remains exclusively SaaS-delegated via 002's own trust boundary, untouched by this feature. Matches the constitution's own carve-out: "SupportHub is the sole authority only for its own domain: ... support org structure." | PASS | +| II. Configuration Over Hardcoding | Token lifetime and any future role list are read from a config value (research.md's 4-hour default), never a magic number duplicated at each call site. | PASS | +| III. Layered Architecture With Enforced Module Boundaries | `identity/auth` keeps its standard shape; `requireRole` is exported from `identity/auth`'s own public `index.ts` for other modules' routes to compose with, the same way `fastify.authenticate` itself is already a cross-cutting plugin-level primitive, not a module import. | PASS | +| IV. AI Recommends, Deterministic Policy Decides | Not applicable — no AI involvement in this feature. | PASS — N/A | +| V. Evidence-Based Verification | Not applicable — no resolution/verification concept in this feature. | PASS — N/A | +| VI. Durable Audit & History | This feature is what finally makes 007-009's own audit fields (`AssignmentHistory.actor`, `EscalationEvent.triggeredBy`, etc.) accurate for real agent/admin actions instead of always falling back to `'unknown'` (research.md) — directly strengthens, not just satisfies, this principle. | PASS | +| VII. Concurrency-Safe, Durable Job Handling | Token verification and revocation are stateless/Redis-TTL-based, not an in-memory timer; two concurrent login attempts for the same account are independently evaluated with no shared mutable state (spec.md Edge Cases). | PASS | +| VIII. Problem and Ticket Are Separate, Related Entities | Not applicable — this feature doesn't touch tickets or problems. | PASS — N/A | +| Technology & Platform Constraints | Two new, narrowly-scoped dependencies (`jsonwebtoken`, `bcryptjs`), both justified in research.md; reuses existing Redis infrastructure, no new infrastructure category introduced. | PASS | + +No violations requiring Complexity Tracking justification. + +## Post-Design Constitution Re-check + +All gates above remain PASS after Phase 1 design. Principle VI is worth restating post-design: +this feature has no user-facing "audit" screen of its own, but its real effect is retroactively +correcting the audit trail of every feature since 007 that could only ever record `'unknown'` +as the acting agent/admin — a materially more accurate audit history the moment this ships. + +## Project Structure + +### Documentation (this feature) + +```text +specs/010-identity-auth/ +├── plan.md # This file +├── research.md # Phase 0 output +├── data-model.md # Phase 1 output +├── quickstart.md # Phase 1 output +├── contracts/ # Phase 1 output +└── tasks.md # Phase 2 output (/speckit-tasks — not created here) +``` + +### Source Code (repository root) + +```text +supporthub-api/ +├── prisma/ +│ ├── schema.prisma # MODIFIED — User.passwordHash, User.active +│ └── seed/roles.seed.ts # MODIFIED — seeded accounts get real password hashes +├── src/ +│ ├── plugins/ +│ │ └── auth.plugin.ts # REPLACED stub — real JWT verify + revocation +│ │ check, populates request.user + reqContext +│ ├── infrastructure/ +│ │ └── cache/ # MODIFIED — revocation denylist helpers +│ │ alongside the existing jti-replay helpers +│ └── modules/ +│ └── identity/ +│ ├── auth/ # REPLACED stub — full real login/logout/ +│ │ ├── controller/ routes/ schema/ self-identity, requireRole exported from +│ │ │ repository/ service/ types/ its own public index.ts +│ │ │ mapper/ constants/ index.ts +│ │ └── (no engine/ — no real decision logic beyond password/token checks) +│ └── agents/ # MODIFIED — new POST /admin/users route +│ └── (existing module, account-creation surface added alongside its own +│ existing agent-roster admin routes) +└── tests/ + ├── unit/identity/ # password-failure-response-parity, + │ requireRole matching logic + └── integration/ # full login/gating/logout flow, spot-checks + across 002-009's own existing admin routes +``` + +**Structure Decision**: Single project. `POST /admin/users` (account creation) lives under +`identity/agents` rather than `identity/auth`, since `identity/auth` owns authentication +mechanics (login/logout/self-identity) while account/roster management is already that +module's own established concern — mirrors 006's own precedent of `identity/agents` owning +agent-roster CRUD. + +## Complexity Tracking + +*No constitution violations — table intentionally omitted.* diff --git a/specs/010-identity-auth/quickstart.md b/specs/010-identity-auth/quickstart.md new file mode 100644 index 0000000..ef6f078 --- /dev/null +++ b/specs/010-identity-auth/quickstart.md @@ -0,0 +1,52 @@ +# Quickstart: Validating Identity and Authentication + +Prerequisites: migrations applied; `npm run prisma:seed` run so the two demo accounts exist +with their new real passwords (documented in the seed script itself, local/dev use only). + +## Scenario 1 — login (User Story 1) + +1. `POST /auth/login` with the seeded admin's correct email/password. **Expected**: `200`, a + token, and `{id, email, name, role: 'ADMIN'}`. +2. Repeat with the correct email but a wrong password. **Expected**: `401`. +3. Repeat with an email that doesn't exist at all. **Expected**: the exact same `401` body/ + status as step 2 — diff the two responses to confirm they're indistinguishable. + +## Scenario 2 — route gating and role enforcement (User Story 2) + +1. Call an existing admin route (e.g. `POST /admin/teams`) with no `Authorization` header. + **Expected**: `401`. +2. Repeat with a malformed token (`Bearer not-a-real-token`). **Expected**: `401`. +3. Log in as the seeded agent (role `AGENT`); call an admin-only route gated by + `requireRole('ADMIN')`. **Expected**: `403`. +4. Log in as the seeded admin; repeat step 3's call. **Expected**: `200`/`201` (whatever that + route normally returns on success). + +## Scenario 3 — self-identity (User Story 3) + +1. Log in; call `GET /auth/me` with the resulting token. **Expected**: `200`, identity matches + the login response exactly. +2. Directly deactivate that account (`active: false`) via a direct DB update (simulating an + admin action no UI exists for yet); repeat the same `GET /auth/me` call with the same, + still-unexpired token. **Expected**: `401` — re-validated against current account state, not + the token's own claims. + +## Scenario 4 — admin creates an account (User Story 4) + +1. Log in as admin; `POST /admin/users` with a new email/name/role `AGENT`/password. + **Expected**: `201`, response never includes the password or its hash. +2. Log in as a non-admin (the seeded agent); repeat step 1. **Expected**: `403`. +3. Immediately log in as the newly-created account with the password from step 1. **Expected**: + `200` — no manual step needed in between. +4. Repeat step 1 with an email already in use. **Expected**: `409`. + +## Scenario 5 — logout (User Story 5) + +1. Log in; call `POST /auth/logout` with the resulting token. **Expected**: `200`. +2. Immediately reuse that same token on any gated route. **Expected**: `401` — rejected even + though it hasn't naturally expired. + +## What "done" looks like + +All five scenarios pass, and Scenario 2 is additionally verified against at least one +already-shipped admin route from each of 002-009 (not just a route this feature itself adds), +proving the real gate actually protects what the no-op stub never did. diff --git a/specs/010-identity-auth/research.md b/specs/010-identity-auth/research.md new file mode 100644 index 0000000..1fa0701 --- /dev/null +++ b/specs/010-identity-auth/research.md @@ -0,0 +1,136 @@ +# Phase 0 Research: Identity and Authentication + +## Decision: Finish the existing scaffold's own intended design, not a new one + +- **Decision**: `User`/`UserRole`, the two seeded-but-passwordless demo accounts, and + `AuthUser`/`JwtPayload` in `src/common/types` are the real target — this feature adds a + `passwordHash` column, replaces `identity/auth`'s email-only stub with real password + verification and JWT issuance, and makes `fastify.authenticate` actually verify that JWT. +- **Rationale**: Every shape needed (the JWT payload's exact fields, the user/role model, even + the bootstrap accounts) was already scaffolded before this session's spec-driven rebuild + began — this is the same "give an existing, unwired scaffold its first real implementation" + pattern every other phase in this codebase has followed, not a new design decision. +- **Alternatives considered**: A separate, purpose-built `Session`/`Credential` model instead of + extending `User` — rejected; `User` already has exactly the fields a staff account needs + (email, name, role), and doc 06 never defined a competing entity for this. + +## Decision: `jsonwebtoken` for signing/verifying, `bcryptjs` for password hashing + +- **Decision**: Add `jsonwebtoken` (plain library, no Fastify plugin registration — kept + consistent with `auth.plugin.ts`'s existing manual-decorator style rather than introducing the + `@fastify/jwt` plugin ecosystem) and `bcryptjs` (pure JavaScript, no native compilation step — + `bcrypt`/`argon2`'s native bindings are a real source of friction on this team's Windows dev + environment, confirmed earlier this session when Docker/Prisma tooling already needed + workarounds for the same class of platform friction). +- **Rationale**: Both are the standard, widely-used choice for their job; `bcryptjs` + specifically avoids re-litigating the native-module build problems this session has already + hit more than once on Windows. +- **Alternatives considered**: `@fastify/jwt` — rejected only for consistency with this + codebase's existing hand-rolled decorator style, not a correctness concern. `argon2` — + rejected for the same native-build-friction reason as `bcrypt`; `bcryptjs` is a well- + established, secure-enough choice for this scale (JWT `Vitest`-verified via existing + precedent, not a cryptographic novelty). + +## Decision: Redis-backed revocation denylist, reusing 002's own jti-tracking mechanism + +- **Decision**: `JwtPayload` gains a `jti` (JWT ID, a random UUID per issued token). Logout adds + that `jti` to a Redis key (`auth:revoked:`) with a TTL equal to the token's own remaining + lifetime. `fastify.authenticate` checks this key (in addition to verifying the signature and + expiry) before accepting a token. +- **Rationale**: This is the exact same shape as 002's `hasSeenJti`/`markJtiSeen` replay- + protection mechanism (`src/infrastructure/cache`) — reused directly rather than inventing a + second Redis-backed token-tracking pattern. A TTL equal to remaining lifetime means the + denylist entry is automatically cleaned up and never grows unbounded. +- **Alternatives considered**: A full server-side session table (every issued token recorded in + Postgres, checked on every request) — rejected as unnecessary weight; spec.md's own + Assumptions explicitly chose "short-lived JWT + revocation-on-logout-only" over full session + tracking, and Redis is already the right tool for this exact shape of check (fast, TTL-native). + +## Decision: A 4-hour token lifetime + +- **Decision**: Issued JWTs expire 4 hours after issuance (`exp` claim). +- **Rationale**: Long enough that a working agent isn't repeatedly forced to re-authenticate + mid-shift, short enough that a leaked/forgotten token's exposure window is bounded in hours, + not days — a reasonable default for an internal staff tool with no remember-me/refresh-token + flow in this feature's scope (Assumptions: no MFA/hardening pass yet either). +- **Alternatives considered**: A refresh-token pair (short-lived access token + long-lived + refresh token) — rejected as more mechanism than this feature's scope calls for; nothing in + spec.md's user stories requires silent re-authentication, and it can be added later without + breaking the token shape this feature establishes. + +## Decision: `fastify.authenticate` also populates the existing, already-shared `reqContext.actorId`/`actorType` + +- **Decision**: On a valid token, `fastify.authenticate` sets `request.user` (the full + `AuthUser`) AND `request.reqContext.actorId = user.id`, `request.reqContext.actorType = + ActorType.USER` — the same two `RequestContext` fields `authenticateProductIntegration` + already populates for customer-originated requests (002). +- **Rationale**: Every module from 007 onward already reads `request.reqContext?.actorId ?? + 'unknown'` in its controllers (`actorFrom(request)` helpers in assignments, tickets, + escalation, resolutions) expecting exactly this to eventually be populated by a real staff + auth mechanism — this was a forward-compatible convention already in place, not something + this feature needs to change call sites for. Every one of those audit trails ( + `AssignmentHistory.actor`, `EscalationEvent.triggeredBy`, etc.) becomes accurate for real + agent/admin actions the moment this feature ships, with no changes to 007-009's own code. +- **Alternatives considered**: A separate `request.user`-only convention, leaving `reqContext + .actorId` customer-only — rejected; would require touching every existing `actorFrom` call + site across four already-shipped features for no benefit, when the field was clearly designed + to be auth-mechanism-agnostic from the start. + +## Decision: Role-gating via a `requireRole(...roles)` preHandler factory, not a fixed decorator + +- **Decision**: A new exported function, `requireRole(...allowedRoles: string[])`, returns a + Fastify preHandler that checks `request.user?.role` against the given list, throwing + `AuthorizationError` (403) if it doesn't match — used as + `{ preHandler: [fastify.authenticate, requireRole('ADMIN')] }`. Not a fixed + `fastify.requireAdmin` decorator, even though `ADMIN` is the only role checked today. +- **Rationale**: A factory function generalizes to any future role/permission check (e.g. a + hypothetical `SENIOR_AGENT`) without a new decorator per role; `fastify.authenticate` and + `requireRole` compose as two separate preHandlers, matching this codebase's existing + `[fastify.authenticateProductIntegration, fastify.checkIntegrationRateLimit]` two-step + preHandler-array convention exactly. +- **Alternatives considered**: A single combined `fastify.authenticateAdmin` decorator — + rejected; would duplicate `fastify.authenticate`'s own token-verification logic for every new + role instead of composing with it. + +## Decision: `Agent.userId` is added as a nullable link, but linking is not this feature's own workflow + +- **Decision**: `Agent` gains `userId String? @unique`, a nullable FK to `User.id` — the schema + capability to say "this login identity's routing/skills profile is that `Agent` row" — but + this feature does not add an endpoint or admin screen to set it. No seed data links the + demo agent account to an `Agent` row either (none is seeded for it today). +- **Rationale**: While investigating account creation (User Story 4), a real gap surfaced: a + `User` (the thing that logs in) and an `Agent` (the thing 006/007 route tickets to) have never + been connected — an `AGENT`-role `User` today has no way to be identified as a *specific* + `Agent` for "tickets assigned to me"-style queries supporthub-web's own agent dashboard will + need. Adding the column now is cheap and unblocks that later without a schema change at that + point; building the actual linking workflow (which almost certainly belongs in 006's + `identity/agents` admin screens, alongside team/skill assignment, not this identity/auth + feature) is a real, separate piece of scope this feature doesn't need to solve today. +- **Alternatives considered**: Building the full link-an-account-to-an-agent workflow as part of + this feature — rejected as scope creep; this feature's own job is proving a `User` *can* + authenticate and be authorized, not completing every downstream consumer of that identity. + Making `Agent.userId` required — rejected; an `Agent` created via 006's existing screens has + no `User` account requirement today and shouldn't suddenly need one just because this feature + exists. + +## Decision: Which existing routes get gated is unchanged — only the gate itself becomes real + +- **Decision**: This feature does not add `fastify.authenticate` to any route that doesn't + already have it, and does not add `requireRole('ADMIN')` to every existing admin route as + part of this feature's own implementation — SC-002 is satisfied by demonstrating the + mechanism works on a representative sample (one action per module), with the mechanical work + of adding `requireRole('ADMIN')` to every remaining `/admin/*` route across 002-009 tracked as + this feature's own Polish-phase task, not a scope expansion into re-designing any other + feature's authorization model. +- **Rationale**: FR-006 is explicit: "this feature does not change which routes are gated, only + makes the gate real." Deciding which of the many already-shipped admin routes should be + admin-only vs. any-authenticated-agent is a real per-route judgment call (e.g., should an + agent be able to create a hierarchy node? almost certainly not; should an agent read one? + probably yes) — this feature makes that judgment call possible to enforce, and applies it + everywhere in its own Polish phase, but doesn't silently redesign any other feature's own + intended access model beyond what's obviously admin-only (write/config endpoints) vs. + read/agent-usable. +- **Alternatives considered**: Leaving every existing route exactly as `fastify.authenticate`- + only (no `requireRole`) and treating role-based gating as entirely out of scope — rejected; + spec.md's own User Story 2/FR-005 explicitly requires admin-only enforcement to exist + somewhere concrete, not just as an available-but-unused mechanism.