saqib mirandClaude Sonnet 5
79bc2ef25b
Closes the two gaps 010-identity-auth explicitly deferred (password reset, login rate-limiting), plus a shared password-strength validator both the reset-consume endpoint and admin account creation now depend on. - Password reset: single-use, paired-Redis-key tokens (never in Postgres), identical response regardless of account existence, stubbed delivery via a structured log line (no email infrastructure exists yet). - Password strength: one validatePasswordStrength() call site, wired into both POST /admin/users and the reset-consume flow. - Login rate-limiting: checkRateLimit keyed by submitted email, checked before any credential verification. Also fixes tests/helpers/auth.ts's shared loginAs() helper, which reused two fixed accounts across the whole integration suite via upsert — now rate-limited per email, that collided across ~30 files sharing one budget. Each call now gets a unique email; no call sites needed to change. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
3.8 KiB
3.8 KiB
Specification Quality Checklist: Authentication Hardening
Purpose: Validate specification completeness and quality before proceeding to planning Created: 2026-09-07 Feature: spec.md
Content Quality
- No implementation details (languages, frameworks, APIs)
- Focused on user value and business needs
- Written for non-technical stakeholders
- All mandatory sections completed
Requirement Completeness
- No [NEEDS CLARIFICATION] markers remain
- Requirements are testable and unambiguous
- Success criteria are measurable
- Success criteria are technology-agnostic (no implementation details)
- All acceptance scenarios are defined
- Edge cases are identified
- Scope is clearly bounded
- Dependencies and assumptions identified
Feature Readiness
- All functional requirements have clear acceptance criteria
- User scenarios cover primary flows
- Feature meets measurable outcomes defined in Success Criteria
- No implementation details leak into specification
Notes
- This is
docs/10-implementation-roadmap.md's own Phase 11 ("security hardening pass"), first slice, per explicit user direction — the two concrete gaps 010-identity-auth's own Assumptions named as deliberately out of its scope: password-reset and login rate-limiting. MFA, the third item 010 named, is intentionally excluded here as its own larger follow-up. - Password-reset's email-delivery step is explicitly stubbed (server-side log, not a real send) per explicit user decision — this codebase has no email-sending infrastructure at all today (no library, no configured provider), discovered while scoping this feature, and introducing one is a separate decision the user chose to defer rather than bundle into this pass.
- Password-strength policy (User Story 2) was added beyond the two named gaps because it's a direct, unavoidable dependency of User Story 1 — a password-reset flow that accepts any password would be hardening one gap while leaving the other wide open at the same door.
- All items pass; no revision iterations were needed.
Implementation Notes (post-build)
tests/helpers/auth.ts's sharedloginAs()helper previously reused two fixed accounts (test-admin@supporthub.test/test-agent@supporthub.test) across every integration test file viaupsert. Once login became rate-limited per email (User Story 3), the ~30 files that each call it once in their ownbeforeAllcollectively exceeded the attempt budget for those two shared addresses well before most files' own tests ran, turning their legitimate logins into429s. Fixed by giving eachloginAs()call its own unique, randomly-suffixed email — nothing in the suite depended on the literal fixed addresses, so no call sites needed to change, only the helper itself.- While re-running the full suite for regression,
tests/integration/orchestration-strategies.test.ts's "SKILL_BASED prefers the eligible agent with the higher proficiency level" test was found failing (picks the lower-proficiency agent). Verified viagit stashthat this reproduces identically on the clean pre-013HEADwith none of this feature's changes present — it is a pre-existing bug in 007-orchestration-assignment'sSKILL_BASEDstrategy, unrelated to and out of scope for this feature. Left unfixed here; worth its own follow-up. tests/integration/ticket-attachments.test.ts's 2 known MinIO-dependent failures (accepted baseline, this project doesn't run MinIO) remain unchanged by this feature.- All other integration and unit tests pass, including 010-identity-auth's own login/admin-account
tests, confirming no regression from
AuthService.login's new rate-limit check or the sharedvalidatePasswordStrengthcall added toUsersService.create.