Merge pull request #577 from components-web-app/fix/570-password-validation

Validate a password set through the API before hashing it; a password change no longer verifies the email (#570)

Author: silverbackdan

CHANGELOG.md

@@ -9,6 +9,7 @@ Every change that reaches `main` adds a line under **Unreleased**. When a versio
 
 ### Fixed
 - A user referenced by IRI from another resource's write is no longer processed as a new registration, which reset their `emailAddressVerified` ([#575](https://github.com/components-web-app/api-components-bundle/pull/575))
+- A `plainPassword` set through the API is validated (6 to 4096 characters, 422 on `plainPassword`) before it is hashed, and setting a password no longer marks the email address verified; only a password reset does ([#577](https://github.com/components-web-app/api-components-bundle/pull/577))
 
 ## [2.0.0-alpha.39](https://github.com/components-web-app/api-components-bundle/compare/2.0.0-alpha.38...2.0.0-alpha.39) - 2026-10-30
 

CLAUDE.md

@@ -296,6 +296,7 @@ Resources are fetched and cached **individually**. Never embed related data outs
 
 - **A `PUT` on a user merges, never replaces** (#426). API Platform's `standard_put` defaults to `true` on 4.x and 5.x: the body is denormalized into a new object and every property is copied onto the stored one, including those nobody can write (password hash, tokens), so they took the constructor's defaults. `UserResourceMetadataCollectionFactory` sets `standard_put: false` on every `Put` of an `AbstractUser`, over any application default. `User\UserPutTest` pins it.
 - **`UserNormalizer` handles only a user payload (an array)** (#572). A user referenced by IRI from another resource's write reaches the serializer as a string; running `processChanges()` on it treated the stored user as a new registration and reset `emailAddressVerified`. `User\UserReferenceTest` pins it.
+- **A plain password is hashed only after validation, and never verifies the email address** (#570). `UserDataProcessor::processChanges()` does not hash: the API hashes in `UserPasswordStateProcessor` (on `state_processor.locator`, before the persist) and the form listener after `processChanges()`, so the `plainPassword` `Length` (in `Default` as well as `User:password:create`) sees the value on API writes; hashed during denormalization, it was already erased. Only a password reset, which proves the inbox, sets `emailAddressVerified`. `User\UserPasswordTest` pins it.
 - **User email links** (#316): `RefererUrlResolver` uses `Origin`, else `Referer`, reduced to `scheme://host[:port]`, only if it matches `user.email_links.allowed_origins` (anchored by the bundle as `{\A(?:p)\z}i`), else `default_origin`, else the link is refused. The request host is not implicitly trusted. Request input may only be a `RelativeUrlPath`. **Never build an outbound link from a request header without an allow-list.**
 - **A refused email link depends on which kind of email it is** (#326). Where the email is the job (password reset, both resends), the request is a 400 that changes nothing: build and send the email before flushing the token it carries. After a completed write (every email from `UserWriteNotifier::notify()`), the write keeps its status and `UserMailer::afterWrite()` logs the refusal at error; the verify and change-email emails serve both kinds, so the notifier must call their `…AfterWrite` variants. Only `UnparseableRequestHeaderException` is caught.
 - **A throttled email request is a 429 with `Retry-After`** (#331), never a 200 that sends nothing. `UserDataProcessor` throws `RequestLimitReachedException` carrying the seconds remaining; the action returns it with nothing sent or changed. Each flow has its own `repeat_ttl_seconds`: `password_reset` (86400), `new_email_confirmation` (300) and `email_verification` (300). Never share one throttle between flows.
@@ -468,7 +469,7 @@ The bundle does not set `use_symfony_listeners`. API Platform's default (`false`
 | `api_platform.state_provider.read`, negative priority, so it wraps the `security` access checkers (0, 10) | `POST_READ`, `PRE_DESERIALIZE` | deny access (-20), publishable merge (-30), component usage (-40), upload and download (-50), resource version (-60) |
 | `api_platform.state_provider.deserialize` | `POST_DESERIALIZE` | publication date guard (-10) |
 | `api_platform.state_provider.validate` | `POST_VALIDATE` | route generation (-10) |
-| `api_platform.state_processor.locator`, the persist/remove call | `PRE_WRITE` before the inner call, `POST_WRITE` after it | version conflict (5), publishable (10), user emails (15), uploadable (20), delete cascade (30), position removal (40), route redirects (50), position placement (60) |
+| `api_platform.state_processor.locator`, the persist/remove call | `PRE_WRITE` before the inner call, `POST_WRITE` after it | version conflict (5), publishable (10), user emails (15), user password hash (16), uploadable (20), delete cascade (30), position removal (40), route redirects (50), position placement (60) |
 | `api_platform.state_processor.serialize` | `PRE_SERIALIZE` | form (10), collection (20) |
 
 - **Never decorate `state_processor.write` or `state_*.main`.** In controller mode, `write`'s inner call is serialize and respond, so its "after" runs after serialization. The `main` services do not exist in listener mode.

src/DataProcessor/StateProcessor/UserPasswordStateProcessor.php

@@ -0,0 +1,42 @@
+<?php
+
+/*
+ * This file is part of the Silverback API Components Bundle Project
+ *
+ * (c) Daniel West <daniel@silverback.is>
+ *
+ * For the full copyright and license information, please view the LICENSE
+ * file that was distributed with this source code.
+ */
+
+namespace Silverback\ApiComponentsBundle\DataProcessor\StateProcessor;
+
+use ApiPlatform\Metadata\HttpOperation;
+use ApiPlatform\Metadata\Operation;
+use ApiPlatform\State\ProcessorInterface;
+use Silverback\ApiComponentsBundle\Entity\User\AbstractUser;
+use Silverback\ApiComponentsBundle\Helper\User\UserDataProcessor;
+
+/**
+ * @implements ProcessorInterface<mixed, mixed>
+ */
+final readonly class UserPasswordStateProcessor implements ProcessorInterface
+{
+    /**
+     * @param ProcessorInterface<mixed, mixed> $decorated
+     */
+    public function __construct(
+        private ProcessorInterface $decorated,
+        private UserDataProcessor $userDataProcessor,
+    ) {
+    }
+
+    public function process(mixed $data, Operation $operation, array $uriVariables = [], array $context = []): mixed
+    {
+        if ($data instanceof AbstractUser && $operation instanceof HttpOperation) {
+            $this->userDataProcessor->hashPassword($data);
+        }
+
+        return $this->decorated->process($data, $operation, $uriVariables, $context);
+    }
+}

src/Entity/User/AbstractUser.php

@@ -67,7 +67,7 @@ abstract class AbstractUser implements SymfonyUserInterface, PasswordAuthenticat
     protected string $password;
 
     #[Assert\NotBlank(message: 'Please enter your desired password.', groups: ['User:password:create'])]
-    #[Assert\Length(min: 6, max: 4096, minMessage: 'Your password must be more than 6 characters long.', maxMessage: 'Your password cannot be over 4096 characters', groups: ['User:password:create'])]
+    #[Assert\Length(min: 6, max: 4096, minMessage: 'Your password must be more than 6 characters long.', maxMessage: 'Your password cannot be over 4096 characters', groups: ['Default', 'User:password:create'])]
     #[ApiProperty(readable: false)]
     #[Groups(['User:input'])]
     protected ?string $plainPassword = null;
@@ -225,7 +225,6 @@ public function setPlainPassword(?string $plainPassword): self
     {
         $this->plainPassword = $plainPassword;
         if ($plainPassword) {
-            // Needs to update mapped field to trigger update event which will encode the plain password
             $this->passwordUpdatedAt = new \DateTime();
         }
 

src/EventListener/Form/EntityPersistFormListener.php

@@ -102,6 +102,7 @@ public function __invoke(FormSuccessEvent $event): void
             }
 
             $this->userDataProcessor->processChanges($data, $oldUser);
+            $this->userDataProcessor->hashPassword($data);
             $this->userWriteNotifier->notify($data, $oldUser);
         }
 

src/Helper/User/UserDataProcessor.php

@@ -67,14 +67,14 @@ public function passwordReset(string $username, string $token, string $newPasswo
         $user->setPlainPassword($newPassword);
         $user->setNewPasswordConfirmationToken(null);
         $user->setPasswordRequestedAt(null);
+        $user->setEmailAddressVerified(true);
         $this->hashPassword($user);
 
         return $user;
     }
 
     public function processChanges(AbstractUser $user, ?AbstractUser $previousUser): void
     {
-        $this->hashPassword($user);
         if (!$previousUser) {
             $user->setEmailAddressVerified($this->initialEmailVerifiedState);
             if (!$this->initialEmailVerifiedState && $this->verifyEmailOnRegister) {
@@ -151,14 +151,13 @@ private function setEmailAddressVerifyToken(AbstractUser $user): void
         $user->plainEmailAddressVerifyToken = $token;
     }
 
-    private function hashPassword(AbstractUser $entity): void
+    public function hashPassword(AbstractUser $entity): void
     {
         if (!$entity->getPlainPassword()) {
             return;
         }
         $encoded = $this->passwordHasher->hashPassword($entity, $entity->getPlainPassword());
         $entity->setPassword($encoded);
-        $entity->setEmailAddressVerified(true);
         $entity->eraseCredentials();
     }
 }

src/Resources/config/services.php

@@ -66,6 +66,7 @@
 use Silverback\ApiComponentsBundle\DataProcessor\StateProcessor\RouteRedirectStateProcessor;
 use Silverback\ApiComponentsBundle\DataProcessor\StateProcessor\UploadableWriteStateProcessor;
 use Silverback\ApiComponentsBundle\DataProcessor\StateProcessor\UserNotificationStateProcessor;
+use Silverback\ApiComponentsBundle\DataProcessor\StateProcessor\UserPasswordStateProcessor;
 use Silverback\ApiComponentsBundle\DataProvider\PageDataProvider;
 use Silverback\ApiComponentsBundle\DataProvider\StateProvider\ComponentGroupOrderStateProvider;
 use Silverback\ApiComponentsBundle\DataProvider\StateProvider\ComponentGroupStateProvider;
@@ -1915,6 +1916,18 @@
         );
     $services->alias(UserNotificationStateProcessor::class, 'silverback.api_components.api_platform.state_processor.user_notification');
 
+    $services
+        ->set('silverback.api_components.api_platform.state_processor.user_password')
+        ->class(UserPasswordStateProcessor::class)
+        ->decorate('api_platform.state_processor.locator', null, 16)
+        ->args(
+            [
+                new Reference('silverback.api_components.api_platform.state_processor.user_password.inner'),
+                new Reference(UserDataProcessor::class),
+            ]
+        );
+    $services->alias(UserPasswordStateProcessor::class, 'silverback.api_components.api_platform.state_processor.user_password');
+
     $services
         ->set('silverback.api_components.factory.user')
         ->class(UserFactory::class)

tests/DataProcessor/StateProcessor/UserPasswordStateProcessorTest.php

@@ -0,0 +1,81 @@
+<?php
+
+/*
+ * This file is part of the Silverback API Components Bundle Project
+ *
+ * (c) Daniel West <daniel@silverback.is>
+ *
+ * For the full copyright and license information, please view the LICENSE
+ * file that was distributed with this source code.
+ */
+
+namespace Silverback\ApiComponentsBundle\Tests\DataProcessor\StateProcessor;
+
+use ApiPlatform\Metadata\GraphQl\Mutation;
+use ApiPlatform\Metadata\Patch;
+use ApiPlatform\Metadata\Post;
+use ApiPlatform\State\ProcessorInterface;
+use PHPUnit\Framework\TestCase;
+use Silverback\ApiComponentsBundle\DataProcessor\StateProcessor\UserPasswordStateProcessor;
+use Silverback\ApiComponentsBundle\Helper\User\UserDataProcessor;
+use Silverback\ApiComponentsBundle\Repository\User\UserRepositoryInterface;
+use Silverback\ApiComponentsBundle\Tests\Functional\TestBundle\Entity\User;
+use Symfony\Component\PasswordHasher\Hasher\PasswordHasherFactoryInterface;
+use Symfony\Component\PasswordHasher\Hasher\UserPasswordHasherInterface;
+
+final class UserPasswordStateProcessorTest extends TestCase
+{
+    public function test_a_users_plain_password_is_hashed_before_the_write(): void
+    {
+        $user = $this->userWithNewPassword();
+        $operation = new Patch();
+        $inner = $this->createMock(ProcessorInterface::class);
+        $inner->expects(self::once())->method('process')
+            ->with(self::callback(static fn (User $written): bool => 'hashed' === $written->getPassword() && null === $written->getPlainPassword()), $operation, ['id' => 1], ['previous_data' => null])
+            ->willReturn('written');
+
+        self::assertSame('written', $this->processor($inner)->process($user, $operation, ['id' => 1], ['previous_data' => null]));
+    }
+
+    public function test_a_user_written_through_an_operation_that_is_not_http_is_not_hashed(): void
+    {
+        $user = $this->userWithNewPassword();
+
+        self::assertSame('written', $this->processor($this->inner())->process($user, new Mutation()));
+        self::assertSame('old', $user->getPassword());
+        self::assertSame('new password', $user->getPlainPassword());
+    }
+
+    public function test_data_other_than_a_user_is_passed_to_the_write(): void
+    {
+        $data = new \stdClass();
+        $inner = $this->createMock(ProcessorInterface::class);
+        $inner->expects(self::once())->method('process')->with($data)->willReturn('written');
+
+        self::assertSame('written', $this->processor($inner)->process($data, new Post()));
+    }
+
+    private function userWithNewPassword(): User
+    {
+        $user = new User('daniel', 'daniel@example.com');
+        $user->setPassword('old')->setPlainPassword('new password');
+
+        return $user;
+    }
+
+    private function processor(ProcessorInterface $inner): UserPasswordStateProcessor
+    {
+        $hasher = $this->createStub(UserPasswordHasherInterface::class);
+        $hasher->method('hashPassword')->willReturn('hashed');
+
+        return new UserPasswordStateProcessor($inner, new UserDataProcessor($hasher, $this->createStub(UserRepositoryInterface::class), $this->createStub(PasswordHasherFactoryInterface::class), false, false, false));
+    }
+
+    private function inner(): ProcessorInterface
+    {
+        $inner = $this->createStub(ProcessorInterface::class);
+        $inner->method('process')->willReturn('written');
+
+        return $inner;
+    }
+}

tests/DependencyInjection/StateDecoratorServicesTest.php

@@ -23,6 +23,7 @@
 use Silverback\ApiComponentsBundle\DataProcessor\StateProcessor\RouteRedirectStateProcessor;
 use Silverback\ApiComponentsBundle\DataProcessor\StateProcessor\UploadableWriteStateProcessor;
 use Silverback\ApiComponentsBundle\DataProcessor\StateProcessor\UserNotificationStateProcessor;
+use Silverback\ApiComponentsBundle\DataProcessor\StateProcessor\UserPasswordStateProcessor;
 use Silverback\ApiComponentsBundle\DataProvider\StateProvider\ComponentUsageStateProvider;
 use Silverback\ApiComponentsBundle\DataProvider\StateProvider\DenyAccessStateProvider;
 use Silverback\ApiComponentsBundle\DataProvider\StateProvider\DownloadStateProvider;
@@ -64,6 +65,12 @@ public static function decorators(): iterable
             'api_platform.state_processor.locator',
             15,
         ];
+        yield 'user password hashed before the persist, after validation, so the notification sees the new hash' => [
+            'silverback.api_components.api_platform.state_processor.user_password',
+            UserPasswordStateProcessor::class,
+            'api_platform.state_processor.locator',
+            16,
+        ];
         yield 'route generation after validation and its security check' => [
             'silverback.api_components.api_platform.state_provider.route_generate',
             RouteGenerateStateProvider::class,

tests/EventListener/Form/EntityPersistFormListenerTest.php

@@ -11,6 +11,8 @@
 
 namespace Silverback\ApiComponentsBundle\Tests\EventListener\Form;
 
+use Doctrine\ORM\EntityManagerInterface;
+use Doctrine\ORM\UnitOfWork;
 use Doctrine\Persistence\ManagerRegistry;
 use Doctrine\Persistence\ObjectManager;
 use PHPUnit\Framework\TestCase;
@@ -24,9 +26,12 @@
 use Silverback\ApiComponentsBundle\Helper\User\UserMailer;
 use Silverback\ApiComponentsBundle\Helper\User\UserWriteNotifier;
 use Silverback\ApiComponentsBundle\Model\Form\FormView;
+use Silverback\ApiComponentsBundle\Repository\User\UserRepositoryInterface;
 use Silverback\ApiComponentsBundle\Tests\Functional\TestBundle\Entity\User;
 use Symfony\Component\Form\Extension\Core\Type\FormType;
 use Symfony\Component\Form\Forms;
+use Symfony\Component\PasswordHasher\Hasher\PasswordHasherFactoryInterface;
+use Symfony\Component\PasswordHasher\Hasher\UserPasswordHasherInterface;
 use Symfony\Component\Serializer\Encoder\JsonEncoder;
 use Symfony\Component\Serializer\Normalizer\NormalizerInterface;
 use Symfony\Component\Serializer\Serializer;
@@ -57,6 +62,25 @@ public function test_data_other_than_a_user_is_persisted_by_any_object_manager()
         self::assertSame($data, $event->result);
     }
 
+    public function test_a_users_new_password_is_hashed_before_the_user_is_persisted(): void
+    {
+        $user = new User('bob', 'bob@example.com');
+        $user->setPassword('old')->setPlainPassword('new password');
+        $unitOfWork = $this->createStub(UnitOfWork::class);
+        $unitOfWork->method('getOriginalEntityData')->willReturn([]);
+        $manager = $this->createMock(EntityManagerInterface::class);
+        $manager->method('getUnitOfWork')->willReturn($unitOfWork);
+        $manager->expects(self::once())->method('persist')->with(self::callback(static fn (User $persisted): bool => 'hashed' === $persisted->getPassword() && null === $persisted->getPlainPassword()));
+        $manager->expects(self::once())->method('flush');
+        $hasher = $this->createStub(UserPasswordHasherInterface::class);
+        $hasher->method('hashPassword')->willReturn('hashed');
+        $processor = new UserDataProcessor($hasher, $this->createStub(UserRepositoryInterface::class), $this->createStub(PasswordHasherFactoryInterface::class), true, false, false);
+
+        $this->createListener(User::class, $manager, $processor)($this->createEvent($user));
+
+        self::assertSame('hashed', $user->getPassword());
+    }
+
     public function test_the_normalizer_must_also_denormalize(): void
     {
         $listener = new class('form_type', User::class) extends EntityPersistFormListener {
@@ -73,7 +97,7 @@ public function test_the_normalizer_must_also_denormalize(): void
         );
     }
 
-    private function createListener(string $dataClass, ObjectManager $manager): EntityPersistFormListener
+    private function createListener(string $dataClass, ObjectManager $manager, ?UserDataProcessor $userDataProcessor = null): EntityPersistFormListener
     {
         $registry = $this->createStub(ManagerRegistry::class);
         $registry->method('getManagerForClass')->willReturn($manager);
@@ -86,7 +110,7 @@ private function createListener(string $dataClass, ObjectManager $manager): Enti
             $this->createStub(TimestampedDataPersister::class),
             new UserWriteNotifier($this->createStub(UserMailer::class)),
             new Serializer([], [new JsonEncoder()]),
-            $this->createStub(UserDataProcessor::class),
+            $userDataProcessor ?? $this->createStub(UserDataProcessor::class),
         );
 
         return $listener;