Replaces the no-op fastify.authenticate stub and the never-implemented
identity/auth login with real bcrypt password verification, JWT session
issuance/verification (reusing the existing JWT_SECRET), and a Redis-backed
revocation denylist for logout. Adds requireRole('ADMIN') to admin-only
configuration writes across 002-009 that previously relied on a decorator
that never actually checked anything. Adds self-identity (GET /auth/me,
re-validated against live account state) and admin-provisioned accounts
(POST /admin/users).
Making the auth check genuinely reject invalid/missing tokens exposed that
~18 pre-existing integration test files called already-gated routes with no
Authorization header (safe against the old no-op stub, broken against a real
one) — fixed via a shared tests/helpers/auth.ts (loginAs/authHeader) and a
file-by-file pass, plus two related SLA-run cleanup races exposed once admin
setup calls in those files' own beforeAll blocks started actually succeeding.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
11 KiB
11 KiB
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, andAuthUser/JwtPayloadinsrc/common/typesare the real target — this feature adds apasswordHashcolumn, replacesidentity/auth's email-only stub with real password verification and JWT issuance, and makesfastify.authenticateactually 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/Credentialmodel instead of extendingUser— rejected;Useralready has exactly the fields a staff account needs (email, name, role), and doc 06 never defined a competing entity for this.
Decision: reuse the existing, already-required JWT_SECRET env var — don't invent a new one
- Decision: Token signing/verification uses
env.JWT_SECRET— az.string().min(16), no-default, required environment variable already defined insrc/config/env.tsand already set in.env.test/.env.example/vitest.config.tssince before this session's spec-driven rebuild began. This feature adds no new secret env var, onlyAUTH_TOKEN_LIFETIME_HOURS(a non-secret, defaultable number). - Rationale: Same "finish the scaffold's own intended design" pattern as
User/JwtPayloadthemselves —JWT_SECRETwas clearly provisioned for exactly this feature and has simply never been read by any code until now. - Alternatives considered: A feature-specific
AUTH_JWT_SECRET— considered and rejected onceJWT_SECRETwas found; would create two secrets doing the identical job.
Decision: jsonwebtoken for signing/verifying, bcryptjs for password hashing
- Decision: Add
jsonwebtoken(plain library, no Fastify plugin registration — kept consistent withauth.plugin.ts's existing manual-decorator style rather than introducing the@fastify/jwtplugin ecosystem) andbcryptjs(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;
bcryptjsspecifically 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 asbcrypt;bcryptjsis a well- established, secure-enough choice for this scale (JWTVitest-verified via existing precedent, not a cryptographic novelty).
Decision: Redis-backed revocation denylist, reusing 002's own jti-tracking mechanism
- Decision:
JwtPayloadgains ajti(JWT ID, a random UUID per issued token). Logout adds thatjtito a Redis key (auth:revoked:<jti>) with a TTL equal to the token's own remaining lifetime.fastify.authenticatechecks 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/markJtiSeenreplay- 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 (
expclaim). - 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.authenticatesetsrequest.user(the fullAuthUser) ANDrequest.reqContext.actorId = user.id,request.reqContext.actorType = ActorType.USER— the same twoRequestContextfieldsauthenticateProductIntegrationalready 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, leavingreqContext .actorIdcustomer-only — rejected; would require touching every existingactorFromcall 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 checksrequest.user?.roleagainst the given list, throwingAuthorizationError(403) if it doesn't match — used as{ preHandler: [fastify.authenticate, requireRole('ADMIN')] }. Not a fixedfastify.requireAdmindecorator, even thoughADMINis 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.authenticateandrequireRolecompose 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.authenticateAdmindecorator — rejected; would duplicatefastify.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:
AgentgainsuserId String? @unique, a nullable FK toUser.id— the schema capability to say "this login identity's routing/skills profile is thatAgentrow" — but this feature does not add an endpoint or admin screen to set it. No seed data links the demo agent account to anAgentrow 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 anAgent(the thing 006/007 route tickets to) have never been connected — anAGENT-roleUsertoday has no way to be identified as a specificAgentfor "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'sidentity/agentsadmin 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
Usercan authenticate and be authorized, not completing every downstream consumer of that identity. MakingAgent.userIdrequired — rejected; anAgentcreated via 006's existing screens has noUseraccount 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.authenticateto any route that doesn't already have it, and does not addrequireRole('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 addingrequireRole('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 (norequireRole) 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.