fix(security): hash password-reset tokens at rest (#313)

config/services_test.yaml

@@ -38,6 +38,10 @@ services:
       $innerLogger: '@App\Tests\Behat\Support\RecordingLogger.inner'
     public: true
 
+  App\Tests\Behat\Support\PasswordResetTokenRecorder:
+    public: true
+    tags: ['app.event_subscriber']
+
   App\Tests\Behat\:
     resource: '../tests/Behat/*'
     exclude: '../tests/Behat/Support/*'

specs/security-313-reset-confirm-tokens-plaintext/prd.md

@@ -0,0 +1,75 @@
+# PRD — Security #313: Password-reset tokens stored in plaintext at rest
+
+## Problem
+
+Password-reset tokens were persisted verbatim in the `password_reset_tokens`
+MongoDB collection. `PasswordResetTokenFactory::create()` generated a
+high-entropy raw token (`bin2hex(random_bytes($tokenLength))`) and that raw value
+was both:
+
+1. **Stored** as the document identifier (`<id field-name="tokenValue"
+strategy="NONE">`), and
+2. **Emailed** to the user as the reset link credential.
+
+`MongoDBPasswordResetTokenRepository::findByToken()` looked the token up by exact
+equality on the raw value (`findOneBy(['tokenValue' => $token])`). Because the
+stored value equalled the emailed bearer credential, any read access to the
+collection (DB backup, replica snapshot, query log, read-only breach, or a
+NoSQL-injection sink) yielded directly usable, unexpired reset tokens for every
+account with a pending reset, enabling account takeover without further
+interaction (CWE-256, HIGH).
+
+The refresh-token subsystem already demonstrates the correct pattern
+(`AuthRefreshToken` stores `hash('sha256', $plainToken)` and looks up by hash);
+the password-reset path did not follow it.
+
+## Functional Requirements
+
+- **FR-1** — `PasswordResetToken` MUST persist only a SHA-256 hash of the token
+  (`hash('sha256', $plainToken)`) as its stored value / document identifier. The
+  plaintext token MUST NOT be persisted.
+- **FR-2** — `PasswordResetToken` MUST expose the plaintext token transiently for
+  e-mail delivery (`getPlainToken()`), populated at creation and re-attachable
+  for delivery, never written to storage.
+- **FR-3** — `MongoDBPasswordResetTokenRepository::findByToken()` MUST hash the
+  incoming candidate (`PasswordResetToken::hashToken()`) before querying, so the
+  lookup is by stored hash, never by raw plaintext.
+- **FR-4** — `PasswordResetToken::matchesToken()` MUST compare a candidate
+  plaintext against the stored hash using a constant-time comparison
+  (`hash_equals`).
+- **FR-5** — The password-reset request → e-mail flow MUST continue to deliver the
+  usable plaintext token in the reset link: the request command handler carries
+  the plaintext in the domain event, and the request subscriber re-attaches it to
+  the reconstituted entity before the e-mail send event is built.
+
+## Non-Functional Requirements
+
+- **NFR-Security** — A read-only exposure of `password_reset_tokens` MUST NOT
+  yield usable reset credentials. Submitting a stored hash to the confirm
+  endpoint MUST fail (the hash of a hash does not match). Token generation
+  continues to use `random_bytes` (256-bit entropy at the default length 32) and
+  single-use / short expiry semantics are unchanged.
+- **NFR-Compatibility** — The public reset/confirm HTTP and GraphQL contracts are
+  unchanged. The Doctrine document identifier remains `tokenValue` (now a 64-char
+  SHA-256 hex string); no migration of mapping strategy is required. Existing
+  seeders, fixtures, and E2E flows continue to function by replaying the
+  plaintext captured at creation time.
+- **NFR-Maintainability** — The fix mirrors the established `AuthRefreshToken`
+  hash-at-rest precedent, stays within hexagonal/DDD boundaries (hashing lives in
+  the framework-free Domain entity, lookup hashing in Infrastructure), introduces
+  no new directories or `*Service` suffixes, and keeps PHPInsights complexity and
+  quality/style thresholds intact (Deptrac 0, Psalm clean, CS-Fixer clean).
+
+## Out of Scope
+
+- **Email-confirmation tokens** (`ConfirmationToken` via `RedisTokenRepository`).
+  These are stored in an ephemeral Redis cache (24h TTL), keyed by both raw token
+  value and user id, with the serialized document carrying the raw value. The
+  brief grades this path "low"; hashing it would change the cache key/serialization
+  contract and the user-id reverse lookup, a materially different and broader
+  change than the HIGH MongoDB at-rest finding. Tracked separately to keep this
+  remediation minimal and focused.
+- Re-keying / re-hashing of tokens already persisted before deployment (short
+  1-hour expiry drains them quickly).
+- Adding a server-side pepper/HMAC. The token has 256 bits of entropy, so SHA-256
+  is sufficient and consistent with the existing refresh-token precedent.

specs/security-313-reset-confirm-tokens-plaintext/stories.md

@@ -0,0 +1,86 @@
+# Stories — Security #313: Hash password-reset tokens at rest
+
+Verification commands (one-off containers; do not start the shared stack):
+
+```sh
+# Unit tests (security-critical + regression)
+docker run --rm -v /home/kravtsov/Projects/secfix-313:/app \
+  -v /home/kravtsov/Projects/user-service/vendor:/app/vendor:ro -w /app \
+  -e APP_ENV=test --entrypoint sh secfix-312-php:latest -lc \
+  'php -d memory_limit=-1 vendor/bin/phpunit --testsuite=Unit \
+   --filter "PasswordResetToken|ConfirmationToken|Repository|SeedSchemathesisData" --no-coverage'
+
+# Architecture boundaries
+docker run --rm ... -lc 'vendor/bin/deptrac analyse --config-file=deptrac.yaml --no-progress'
+# Static analysis (changed files)
+docker run --rm ... -lc 'vendor/bin/psalm --no-cache --no-progress <changed .php files>'
+# Style (changed files)
+docker run --rm ... -lc 'PHP_CS_FIXER_IGNORE_ENV=1 vendor/bin/php-cs-fixer fix --dry-run \
+   --allow-risky=yes --config=.php-cs-fixer.dist.php --path-mode=intersection <changed .php files>'
+```
+
+---
+
+## Story 1 — Persist only a SHA-256 hash of the reset token (FR-1, FR-4, NFR-Security)
+
+Change `PasswordResetToken` so the constructor receives the plaintext, stores
+`hash('sha256', $plain)` as `tokenValue` (the document id), keeps a transient
+`plainToken`, and adds `hashToken()` / `matchesToken()` (constant-time).
+
+Tests: `tests/Unit/User/Domain/Entity/PasswordResetTokenHashingTest.php`
+
+- **Positive** — `testStoredValueIsHashNotPlaintext`, `testMatchesTokenAcceptsCorrectPlaintext`:
+  stored value equals the SHA-256 of the plaintext and the correct plaintext matches.
+- **Negative** — `testMatchesTokenRejectsWrongPlaintext`,
+  `testMatchesTokenRejectsStoredHashSubmittedAsToken`: a wrong plaintext and the
+  stored hash itself (what a DB-read attacker sees) both fail to match.
+- **Edge** — `testHashTokenIsDeterministicAndSha256` (64-char deterministic hex),
+  `testAttachPlainTokenRestoresDeliveryValue` (transient re-attachment).
+
+## Story 2 — Look up reset tokens by hash, never by raw plaintext (FR-3, NFR-Security)
+
+Change `MongoDBPasswordResetTokenRepository::findByToken()` to query
+`['tokenValue' => PasswordResetToken::hashToken($token)]`.
+
+Tests: `tests/Unit/User/Infrastructure/Repository/MongoDBPasswordResetTokenRepositoryTest.php`
+
+- **Positive** — `testFindByTokenLooksUpByHashNotPlaintext`: `findOneBy` is called
+  with the hashed criterion and returns the token.
+- **Negative/Edge** — `testFindByTokenDoesNotQueryWithRawPlaintext`: asserts the
+  query criterion is NOT the raw value and IS its SHA-256 hash.
+
+## Story 3 — Deliver the usable plaintext token through the request → e-mail flow (FR-2, FR-5, NFR-Compatibility)
+
+`RequestPasswordResetCommandHandler` emits `getPlainToken()` in the event;
+`PasswordResetRequestedEventSubscriber` re-attaches `event->token` to the
+reloaded entity; `PasswordResetEmailSendEventFactory` emits the plaintext.
+
+Tests:
+
+- `tests/Unit/User/Application/CommandHandler/RequestPasswordResetCommandHandlerTest.php`
+  — **Positive**: event factory receives the plaintext token; **Negative**:
+  unknown user publishes nothing.
+- `tests/Unit/User/Application/EventSubscriber/PasswordResetRequestedEventSubscriberTest.php`
+  — **Positive**: `attachPlainToken($event->token)` is invoked then the e-mail is
+  dispatched; **Negative**: missing token short-circuits.
+- `tests/Unit/User/Domain/Factory/Event/PasswordResetEmailSendEventFactoryTest.php`
+  — **Positive/Edge**: emitted `tokenValue` equals the plaintext and differs from
+  the stored hash.
+- `tests/Unit/User/Domain/Factory/PasswordResetTokenFactoryTest.php`
+  — **Edge**: plaintext length stays `tokenLength * 2`; stored value is 64 hex
+  chars and matches its plaintext.
+
+## Story 4 — Keep seeders, fixtures, and E2E flows working with hashed storage (NFR-Compatibility)
+
+`InMemoryPasswordResetTokenRepository::findByToken()` hashes the candidate; the
+Schemathesis/seeder tests assert by hashed key; E2E/Memory helpers replay the
+plaintext captured at creation (`getPlainToken()` / `getLastPasswordResetToken()`).
+
+Tests:
+
+- `tests/Unit/DataFixtures/Seeder/PasswordResetTokenSeederTest.php` — **Positive**:
+  seeded tokens are keyed by hash and resolvable via `findByToken(plain)`;
+  **Edge**: existing-token refresh/extend/reset-usage paths still match by hash.
+- `tests/Unit/DataFixtures/Command/SeedSchemathesisDataCommandTest.php` —
+  **Positive/Edge**: stored tokens keyed by hashed fixture constants; existing
+  token removal still counted.

src/User/Application/CommandHandler/RequestPasswordResetCommandHandler.php

@@ -13,6 +13,7 @@
 use App\User\Domain\Factory\PasswordResetTokenFactoryInterface;
 use App\User\Domain\Repository\PasswordResetTokenRepositoryInterface;
 use App\User\Domain\Repository\UserRepositoryInterface;
+use LogicException;
 use Symfony\Component\Uid\Factory\UuidFactory;
 
 final readonly class RequestPasswordResetCommandHandler implements
@@ -45,7 +46,9 @@ public function __invoke(
         $this->eventBus->publish(
             $this->eventFactory->create(
                 $user,
-                $token->getTokenValue(),
+                $token->getPlainToken() ?? throw new LogicException(
+                    'Password reset plain token is missing.'
+                ),
                 (string) $this->uuidFactory->create()
             )
         );

src/User/Application/EventSubscriber/PasswordResetRequestedEventSubscriber.php

@@ -34,6 +34,8 @@ public function __invoke(PasswordResetRequestedEvent $event): void
             return;
         }
 
+        $token->attachPlainToken($event->token);
+
         $this->commandBus->dispatch(
             $this->cmdFactory->create(
                 $this->emailFactory->create($token, $user)

src/User/Application/Factory/Event/PasswordResetEmailSendEventFactory.php

@@ -8,6 +8,7 @@
 use App\User\Domain\Entity\UserInterface;
 use App\User\Domain\Event\PasswordResetEmailSentEvent;
 use App\User\Domain\Factory\Event\PasswordResetEmailSendEventFactoryInterface;
+use LogicException;
 
 final class PasswordResetEmailSendEventFactory implements
     PasswordResetEmailSendEventFactoryInterface
@@ -19,7 +20,9 @@ public function create(
         string $eventID,
     ): PasswordResetEmailSentEvent {
         return new PasswordResetEmailSentEvent(
-            $token->getTokenValue(),
+            $token->getPlainToken() ?? throw new LogicException(
+                'Plain password reset token must be attached before sending email.'
+            ),
             $token->getUserID(),
             $user->getEmail(),
             $eventID,

src/User/Domain/Entity/PasswordResetToken.php

@@ -10,21 +10,63 @@ class PasswordResetToken implements PasswordResetTokenInterface
 {
     private bool $isUsed;
 
+    private string $tokenValue;
+
+    /**
+     * Transient plaintext token. Only populated when the token is created or
+     * re-attached for delivery; it is never persisted (only the hash is).
+     * Defaults to null so reads stay safe after Doctrine hydration, which
+     * bypasses the constructor and never assigns this transient field.
+     */
+    private ?string $plainToken = null;
+
     public function __construct(
-        private string $tokenValue,
+        string $plainToken,
         private string $userID,
         private DateTimeImmutable $expiresAt,
         private DateTimeImmutable $createdAt
     ) {
+        $this->plainToken = $plainToken;
+        $this->tokenValue = self::hashToken($plainToken);
         $this->isUsed = false;
     }
 
+    public static function hashToken(string $plainToken): string
+    {
+        return hash('sha256', $plainToken);
+    }
+
+    /**
+     * Stored hash of the token (used as the document identifier).
+     */
     #[\Override]
     public function getTokenValue(): string
     {
         return $this->tokenValue;
     }
 
+    /**
+     * Plaintext token to embed in the reset link; null once reconstituted
+     * from storage and not re-attached.
+     */
+    #[\Override]
+    public function getPlainToken(): ?string
+    {
+        return $this->plainToken;
+    }
+
+    #[\Override]
+    public function attachPlainToken(string $plainToken): void
+    {
+        $this->plainToken = $plainToken;
+    }
+
+    #[\Override]
+    public function matchesToken(string $plainToken): bool
+    {
+        return hash_equals($this->tokenValue, self::hashToken($plainToken));
+    }
+
     #[\Override]
     public function getUserID(): string
     {

src/User/Domain/Entity/PasswordResetTokenInterface.php

@@ -8,6 +8,12 @@ interface PasswordResetTokenInterface
 {
     public function getTokenValue(): string;
 
+    public function getPlainToken(): ?string;
+
+    public function attachPlainToken(string $plainToken): void;
+
+    public function matchesToken(string $plainToken): bool;
+
     public function getUserID(): string;
 
     public function getExpiresAt(): \DateTimeImmutable;

src/User/Infrastructure/Repository/MongoDBPasswordResetTokenRepository.php

@@ -39,7 +39,13 @@ public function save(
     #[\Override]
     public function findByToken(string $token): ?PasswordResetTokenInterface
     {
-        return $this->findOneBy(['tokenValue' => $token]);
+        // Hash mirrors PasswordResetToken::hashToken(); kept inline because the
+        // project's PHPMD cleancode ruleset forbids static access here. The
+        // MongoDBPasswordResetTokenRepositoryTest pins both sides to the same
+        // algorithm, so any future drift is caught by the test suite.
+        return $this->findOneBy([
+            'tokenValue' => hash('sha256', $token),
+        ]);
     }
 
     public function findByUserID(