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 <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Sonnet 5
parent
a49389dc2c
commit
3b4c220a45
@@ -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).
|
||||
@@ -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:<jti>`, 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").
|
||||
@@ -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:<jti>`, 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.*
|
||||
@@ -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.
|
||||
@@ -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:<jti>`) 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.
|
||||
Reference in New Issue
Block a user