Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
3 changes: 2 additions & 1 deletion CLAUDE.md
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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.
Expand Down
42 changes: 42 additions & 0 deletions src/DataProcessor/StateProcessor/UserPasswordStateProcessor.php
Original file line number Diff line number Diff line change
@@ -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);
}
}
3 changes: 1 addition & 2 deletions src/Entity/User/AbstractUser.php
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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();
}

Expand Down
1 change: 1 addition & 0 deletions src/EventListener/Form/EntityPersistFormListener.php
Original file line number Diff line number Diff line change
Expand Up @@ -102,6 +102,7 @@ public function __invoke(FormSuccessEvent $event): void
}

$this->userDataProcessor->processChanges($data, $oldUser);
$this->userDataProcessor->hashPassword($data);
$this->userWriteNotifier->notify($data, $oldUser);
}

Expand Down
5 changes: 2 additions & 3 deletions src/Helper/User/UserDataProcessor.php
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down Expand Up @@ -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();
}
}
13 changes: 13 additions & 0 deletions src/Resources/config/services.php
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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)
Expand Down
Original file line number Diff line number Diff line change
@@ -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;
}
}
7 changes: 7 additions & 0 deletions tests/DependencyInjection/StateDecoratorServicesTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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,
Expand Down
28 changes: 26 additions & 2 deletions tests/EventListener/Form/EntityPersistFormListenerTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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;
Expand Down Expand Up @@ -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 {
Expand All @@ -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);
Expand All @@ -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;
Expand Down
Loading
Loading