Reset tokens live only in Redis as a paired key shape (mirrors 010's own revocation-denylist pattern) - never in Postgres, never storing the raw token. Password-strength policy is one shared validator called from both the new reset-consume endpoint and 010's existing POST /admin/users. Login rate-limiting reuses the existing checkRateLimit helper from 002's own inbound trust boundary, keyed by submitted email, checked before any credential verification. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
5.9 KiB
5.9 KiB
Research: Authentication Hardening
Decision: reset tokens live only in Redis, as a paired key shape, never in Postgres
- Decision: A random 32-byte token (
crypto.randomBytes(32).toString('hex')) is generated per request; only its SHA-256 hash is ever stored (the raw token is returned to the caller ofrequestPasswordResetfor the stub-delivery step to log, then discarded). Two Redis keys per active token, both with the same TTL (the reset lifetime):password-reset:token:<hash>→userId(resolves a presented token at consume time)password-reset:user:<userId>→hash(lets issuing a new token find and delete the prior one'stoken:key, invalidating it — FR-002)
- Rationale: Storing only the hash (never the raw token) mirrors this codebase's own
password-hashing discipline (010's
hashPassword) and 002's encrypted-credential-at-rest precedent — a Redis compromise alone shouldn't hand over usable reset tokens. The paired-key shape gets "only one active token per account" (FR-002) without a database table or a list scan; it's the same Redis-key-with-TTL pattern 010's own revocation denylist and 002's jti replay-guard already established, not a new pattern for this codebase. - Alternatives considered: A signed JWT with a
purpose: 'password-reset'claim — rejected; a JWT can't be "invalidated by issuing a new one" without also tracking issued tokens somewhere (defeating the point of using a stateless token), so it would need the same Redis bookkeeping anyway while adding JWT-parsing overhead for no benefit. A Postgres table — works, but adds a migration and a cleanup/expiry job for data Redis's own TTL already expires for free; rejected as unnecessary durability for a short-lived, non-audit-relevant credential.
Decision: the "delivery" stub is a structured log line, not a fake email object
- Decision:
requestPasswordResetlogs{ event: 'password_reset_requested', userId, resetUrl }atinfolevel via the existing Pino logger — no new "mock email" abstraction, noEmailServiceinterface to later swap out. - Rationale: Per the user's own explicit choice (stub delivery, not real email), the
simplest honest stub is exactly what a developer needs during this phase: the token, visible
in the same place every other structured log already goes. Building a fake
EmailServiceinterface now, before any real provider is chosen, would be speculative abstraction for a contract nobody has decided yet (which provider, which template). - Alternatives considered: A dedicated
EmailService/NotificationServiceinterface with a console/log implementation, swapped for a real one later — rejected as premature infrastructure for a single call site; revisit when a real provider is actually chosen (a separate, later decision per spec.md Assumptions).
Decision: one shared validatePasswordStrength, minimum length only, PASSWORD_MIN_LENGTH-configured
- Decision:
identity/auth/mapper/password-policy.tsexportsvalidatePasswordStrength(password: string): void, throwingValidationErrornaming the actual requirement (e.g. "Password must be at least N characters.") ifpassword.length < env.PASSWORD_MIN_LENGTH. Called from bothAuthService's newresetPasswordandidentity/agents's existingUsersService.create. - Rationale: FR-005 requires one policy enforced identically everywhere a password is set — a shared function is the only way to guarantee that rather than trusting two call sites to stay in sync by convention. Minimum length only (no character-class rules) matches current NIST guidance (length matters far more than forced complexity) and spec.md's own explicit scope boundary.
- Alternatives considered: A zod
.refine()embedded separately in each schema — rejected; duplicates the rule text and the minimum-length constant at two call sites, exactly the drift FR-005 exists to prevent.
Decision: login rate-limit reuses the existing checkRateLimit helper, keyed by email
- Decision:
AuthService.logincallscheckRateLimit(login:${email}, env.LOGIN_RATE_LIMIT_MAX_ATTEMPTS, env.LOGIN_RATE_LIMIT_WINDOW_SECONDS)as its very first step, beforerepo.findByEmailorverifyPassword— throwingRateLimitError(already a distinct error/status fromAuthenticationError, per the existingcommon/errors) if exceeded. - Rationale:
checkRateLimit(src/infrastructure/cache/rate-limiter.ts) already exists, already used by 002's own inbound-request rate limiting, and is exactly the fixed-window Redis-INCRshape this feature needs — reusing it is the literal instruction 010's own Assumptions gave ("beyond what 002's existing generic rate-limit infrastructure might already cover"). Keying by the submitted email (not a resolved user id) means the limiter runs identically whether or not the account exists, so it can't itself become a second account-existence oracle. - Alternatives considered:
@fastify/rate-limit's own global plugin (already registered, 1000 req/min) — insufficient on its own; that's a blunt per-IP-or-global HTTP-level limit, not a per-account brute-force defense, and 010's own Assumptions already anticipated needing something more targeted for login specifically.
Decision: POST /admin/users gets the shared validator via a one-line call-site change
- Decision:
UsersService.createcallsvalidatePasswordStrength(body.password)before hashing, right alongside its existing duplicate-email check — no schema change, no new route. - Rationale: FR-005's "identically everywhere" requirement includes this pre-existing 010 endpoint, which today accepts any non-empty string as a password. Minimal, surgical fix at the one call site that needed it.
- Alternatives considered: None — this is the only other password-setting call site in the
codebase (confirmed by searching for every
hashPassword(call).