Files
saqib mirandClaude Sonnet 5 52f1fa3db0 docs(013-auth-hardening): plan, research, data model, contract, quickstart
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>
2026-09-07 17:26:49 +05:30

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 of requestPasswordReset for 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's token: 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: requestPasswordReset logs { event: 'password_reset_requested', userId, resetUrl } at info level via the existing Pino logger — no new "mock email" abstraction, no EmailService interface 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 EmailService interface 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/NotificationService interface 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.ts exports validatePasswordStrength(password: string): void, throwing ValidationError naming the actual requirement (e.g. "Password must be at least N characters.") if password.length < env.PASSWORD_MIN_LENGTH. Called from both AuthService's new resetPassword and identity/agents's existing UsersService.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.login calls checkRateLimit(login:${email}, env.LOGIN_RATE_LIMIT_MAX_ATTEMPTS, env.LOGIN_RATE_LIMIT_WINDOW_SECONDS) as its very first step, before repo.findByEmail or verifyPassword — throwing RateLimitError (already a distinct error/status from AuthenticationError, per the existing common/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-INCR shape 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.create calls validatePasswordStrength(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).