diff --git a/CHANGELOG.md b/CHANGELOG.md index a20d4ec6..40a58b1d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,9 +6,11 @@ Every change that reaches `main` adds a line under **Unreleased**. When a versio ### Breaking - Writing CWA content (pages, layouts, routes, component groups and their order, component positions, every component including `/move`, and every page data class, project classes included) requires `publishable.permission` (or the class's `#[Publishable(isGranted:)]`), ANDed with any `security` the operation already has; opt an operation or resource out with `extraProperties: ['publishable_permission' => false]`. Form `/submit` stays public ([#579](https://github.com/components-web-app/api-components-bundle/pull/579)) +- An email change's confirmation link goes to the new address instead of the current one, and setting `newEmailAddress` needs the signed-in user's current password as `oldPassword` on API writes and in `NewEmailAddressType` (a super admin enters their own; a cancel needs none) ([#580](https://github.com/components-web-app/api-components-bundle/pull/580)) ### Added - `emails.logo` (a Twig path, default the bundle's logo) and `emails.logo_width` (pixels, optional) set the logo at the top of every email the bundle sends, whose alt text is now the `website_name` instead of `Logo` ([#576](https://github.com/components-web-app/api-components-bundle/pull/576)) +- A notice to the current address when an email change is requested, with the new address masked and no link; `user.emails.change_email_notice` sets its subject or disables it ([#580](https://github.com/components-web-app/api-components-bundle/pull/580)) ### 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)) diff --git a/CLAUDE.md b/CLAUDE.md index 8c1a3f4b..42b703ce 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -304,6 +304,9 @@ Resources are fetched and cached **individually**. Never embed related data outs - **Every bundle email extends `emails/_template.html.twig`, so every sender's context carries `website_name`, `email_logo` and `email_logo_width`** (`SilverbackApiComponentsExtension::emailContext()`, #573). `emails.logo` is a Twig path, because `email.image()` loads it through the Twig loader; a filesystem path fails. **Find an embedded image from the ``, never by attachment name**: twig-bridge 7.3 (the lowest-dependency legs) names the part with the whole Twig path and writes `cid:`, 7.4+ uses the basename and a generated content id, and the lowest CSS inliner puts the element's own `style` before the inlined rules. `Email\EmailLogoTest` follows the `src` to the part whose content id or filename it names. - **A failed send returns false whether or not a logger is wired** (#340): the logger is optional in `UserMailer`'s service locator, so check `has('logger')`; `get()` on a missing id throws. - **The `…RequestedAt` timestamp starts the throttle, so `UserMailer` sets it only after `send()` succeeds**, and flushes it with the token then. A failed send (503) saves nothing: the action refreshes the user, so the token is not replaced by one nobody received. +- **An email change is confirmed from the new address and announced to the current one** (#571). `ChangeEmailConfirmationEmailFactory` overrides `AbstractUserEmailFactory::getRecipientAddress()` to send the link to `newEmailAddress`, which proves the address; confirming swaps `emailAddress` and marks it verified. `UserWriteNotifier` sends `ChangeEmailNoticeEmailFactory`'s notice to the current address on the same trigger (a new confirmation token, so not on a resend, a cancel or the confirmation). The notice carries no link, only `masked_new_email_address` (`Utility\MaskedEmailAddress`), so it is sent even when the confirmation's link is refused; `user.emails.change_email_notice.enabled` turns it off. +- **Setting `newEmailAddress` needs the signed-in user's current password** (#571), because the link no longer goes to the old address. `#[NewEmailAddressRequiresPassword]` on `AbstractUser` validates `oldPassword` with `UserPassword`, on the API and in `NewEmailAddressType` alike, only when `newEmailAddress` is non-empty and differs from the stored value (the unit of work's original data), so a cancel or a write that leaves a pending change alone needs none. It is checked against the token user, so a super admin editing another user enters their own password. A constraint validated through `inContext()` takes the caller's group, so the nested `UserPassword` carries the outer constraint's groups; with its default group the form's `User:emailAddress` validation skipped it. +- **`/resend-verify-new-email/{username}` is unauthenticated and now mails the pending new address**, so anyone who knows a username can send a confirmation to the address its owner claimed, at most once per `new_email_confirmation.repeat_ttl_seconds`. That throttle is what bounds it; keep it. - Actions calling `UserDataProcessor::findUserByUsername()` must catch its `InvalidArgumentException` and return 404; it throws rather than returning null. - **Code typed against `UserRepositoryInterface` may call only what it declares** (#266, #336). The `@method` tags are not a contract, and PHPStan trusts them, so it will not catch a call to `find()` or `findOneBy()`. `loadUserByIdentifier()` matches username *or* email, trimmed and lower-cased like every `UserRepository` lookup (#574: a mobile keyboard's trailing space), so check the returned username, compared the same way and treat `NonUniqueResultException` as not found; `Repository\User\FindUserByUsernameTrait` does both. Test against `tests/Command/InMemoryUserRepository.php`, which implements only the interface. - `user:create` throws `ValidationFailedException` rather than writing an invalid or duplicate user (#254). There is no DB unique constraint on username or email. diff --git a/src/DependencyInjection/Configuration.php b/src/DependencyInjection/Configuration.php index 090d1ddf..92aeaa95 100644 --- a/src/DependencyInjection/Configuration.php +++ b/src/DependencyInjection/Configuration.php @@ -609,6 +609,13 @@ private function addUserNode(ArrayNodeDefinition $rootNode): void ->scalarNode('subject')->cannotBeEmpty()->defaultValue('Your password has been changed')->end() ->end() ->end() + ->arrayNode('change_email_notice') + ->canBeDisabled() + ->addDefaultsIfNotSet() + ->children() + ->scalarNode('subject')->cannotBeEmpty()->defaultValue('Your email address is being changed')->end() + ->end() + ->end() ->end() ->end() ->end() diff --git a/src/DependencyInjection/SilverbackApiComponentsExtension.php b/src/DependencyInjection/SilverbackApiComponentsExtension.php index 383a06cd..aab5fb11 100644 --- a/src/DependencyInjection/SilverbackApiComponentsExtension.php +++ b/src/DependencyInjection/SilverbackApiComponentsExtension.php @@ -34,6 +34,7 @@ use Silverback\ApiComponentsBundle\Factory\OrphanedResource\OrphanedResourcesChangedEmailFactory; use Silverback\ApiComponentsBundle\Factory\Uploadable\MediaObjectFactory; use Silverback\ApiComponentsBundle\Factory\User\Mailer\ChangeEmailConfirmationEmailFactory; +use Silverback\ApiComponentsBundle\Factory\User\Mailer\ChangeEmailNoticeEmailFactory; use Silverback\ApiComponentsBundle\Factory\User\Mailer\PasswordChangedEmailFactory; use Silverback\ApiComponentsBundle\Factory\User\Mailer\PasswordResetEmailFactory; use Silverback\ApiComponentsBundle\Factory\User\Mailer\UserEnabledEmailFactory; @@ -339,6 +340,7 @@ private function setMailerServiceArguments(ContainerBuilder $container, array $c $mapping = [ PasswordChangedEmailFactory::class => 'password_changed', + ChangeEmailNoticeEmailFactory::class => 'change_email_notice', UserEnabledEmailFactory::class => 'user_enabled', UsernameChangedEmailFactory::class => 'username_changed', WelcomeEmailFactory::class => 'welcome', diff --git a/src/Entity/User/AbstractUser.php b/src/Entity/User/AbstractUser.php index c6ea6a75..206f4764 100644 --- a/src/Entity/User/AbstractUser.php +++ b/src/Entity/User/AbstractUser.php @@ -35,6 +35,7 @@ #[UniqueEntity(fields: ['username'], message: 'Sorry, that user already exists in the database.', errorPath: 'username')] #[UniqueEntity(fields: ['emailAddress'], message: 'Sorry, that email address already exists in the database.', errorPath: 'emailAddress')] #[AcbAssert\NewEmailAddress(groups: ['User:emailAddress', 'Default'])] +#[AcbAssert\NewEmailAddressRequiresPassword(groups: ['User:emailAddress', 'Default'])] abstract class AbstractUser implements SymfonyUserInterface, PasswordAuthenticatedUserInterface, JWTUserInterface { use IdTrait; diff --git a/src/Factory/User/Mailer/AbstractUserEmailFactory.php b/src/Factory/User/Mailer/AbstractUserEmailFactory.php index c353c2d2..58957ba5 100644 --- a/src/Factory/User/Mailer/AbstractUserEmailFactory.php +++ b/src/Factory/User/Mailer/AbstractUserEmailFactory.php @@ -94,7 +94,7 @@ protected function createEmailMessage(array $context = []): ?TemplatedEmail } try { - $toEmailAddress = Address::create((string) $this->user->getEmailAddress()); + $toEmailAddress = Address::create((string) $this->getRecipientAddress($this->user)); } catch (SymfonyRfcComplianceException $exception) { $exception = new RfcComplianceException($exception->getMessage()); throw $exception; @@ -127,6 +127,11 @@ protected function createEmailMessage(array $context = []): ?TemplatedEmail return $event->getEmail(); } + protected function getRecipientAddress(AbstractUser $user): ?string + { + return $user->getEmailAddress(); + } + protected function getTokenUrl(string $token, string $username, ?string $newEmail = null): string { if (null === $this->defaultRedirectPath && null === $this->redirectPathQueryKey) { diff --git a/src/Factory/User/Mailer/ChangeEmailConfirmationEmailFactory.php b/src/Factory/User/Mailer/ChangeEmailConfirmationEmailFactory.php index 3e5c9782..7f8a1f44 100644 --- a/src/Factory/User/Mailer/ChangeEmailConfirmationEmailFactory.php +++ b/src/Factory/User/Mailer/ChangeEmailConfirmationEmailFactory.php @@ -41,6 +41,11 @@ public function create(AbstractUser $user, array $context = []): ?RawMessage return $this->createEmailMessage($context); } + protected function getRecipientAddress(AbstractUser $user): ?string + { + return $user->getNewEmailAddress(); + } + protected static function getContextKeys(): ?array { return array_merge( diff --git a/src/Factory/User/Mailer/ChangeEmailNoticeEmailFactory.php b/src/Factory/User/Mailer/ChangeEmailNoticeEmailFactory.php new file mode 100644 index 00000000..e1231589 --- /dev/null +++ b/src/Factory/User/Mailer/ChangeEmailNoticeEmailFactory.php @@ -0,0 +1,55 @@ + + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ + +namespace Silverback\ApiComponentsBundle\Factory\User\Mailer; + +use Silverback\ApiComponentsBundle\Entity\User\AbstractUser; +use Silverback\ApiComponentsBundle\Exception\InvalidArgumentException; +use Silverback\ApiComponentsBundle\Utility\MaskedEmailAddress; +use Symfony\Component\Mime\RawMessage; + +final class ChangeEmailNoticeEmailFactory extends AbstractUserEmailFactory +{ + public const MESSAGE_ID_PREFIX = 'cen'; + + public function create(AbstractUser $user, array $context = []): ?RawMessage + { + if (!$this->enabled) { + return null; + } + + $this->initUser($user); + + $newEmailAddress = $user->getNewEmailAddress(); + if (null === $newEmailAddress || '' === $newEmailAddress) { + throw new InvalidArgumentException('A `new email address` must be set to send the change email notice'); + } + + $context['masked_new_email_address'] = MaskedEmailAddress::of($newEmailAddress); + + return $this->createEmailMessage($context); + } + + protected static function getContextKeys(): array + { + return array_merge( + parent::getContextKeys(), + [ + 'masked_new_email_address', + ] + ); + } + + protected function getTemplate(): string + { + return 'user_change_email_notice.html.twig'; + } +} diff --git a/src/Form/Type/User/NewEmailAddressType.php b/src/Form/Type/User/NewEmailAddressType.php index 5e87dbc0..e0f385a7 100644 --- a/src/Form/Type/User/NewEmailAddressType.php +++ b/src/Form/Type/User/NewEmailAddressType.php @@ -18,6 +18,7 @@ use Silverback\ApiComponentsBundle\Repository\User\UserRepositoryInterface; use Symfony\Bundle\SecurityBundle\Security; use Symfony\Component\Form\Extension\Core\Type\EmailType; +use Symfony\Component\Form\Extension\Core\Type\PasswordType; use Symfony\Component\Form\FormBuilderInterface; use Symfony\Component\OptionsResolver\OptionsResolver; @@ -50,7 +51,7 @@ public function buildForm(FormBuilderInterface $builder, array $options): void } $help = null; if ($data instanceof AbstractUser && $data->getNewEmailAddress()) { - $help = \sprintf('You have requested to change your email to `%s`. Please check your inbox to validate this email address.', $data->getNewEmailAddress()); + $help = \sprintf('You have requested to change your email to `%s`. Please check the inbox of the new address to confirm it.', $data->getNewEmailAddress()); } $builder ->add( @@ -62,6 +63,14 @@ public function buildForm(FormBuilderInterface $builder, array $options): void 'data' => $data ? $data->getEmailAddress() : '', 'help' => $help, ] + ) + ->add( + 'oldPassword', + PasswordType::class, + [ + 'label' => 'Current password', + 'attr' => ['autocomplete' => 'current-password'], + ] ); } diff --git a/src/Helper/User/UserMailer.php b/src/Helper/User/UserMailer.php index afa4b9e1..32146f32 100644 --- a/src/Helper/User/UserMailer.php +++ b/src/Helper/User/UserMailer.php @@ -16,6 +16,7 @@ use Silverback\ApiComponentsBundle\Exception\MailerTransportException; use Silverback\ApiComponentsBundle\Exception\UnparseableRequestHeaderException; use Silverback\ApiComponentsBundle\Factory\User\Mailer\ChangeEmailConfirmationEmailFactory; +use Silverback\ApiComponentsBundle\Factory\User\Mailer\ChangeEmailNoticeEmailFactory; use Silverback\ApiComponentsBundle\Factory\User\Mailer\PasswordChangedEmailFactory; use Silverback\ApiComponentsBundle\Factory\User\Mailer\PasswordResetEmailFactory; use Silverback\ApiComponentsBundle\Factory\User\Mailer\UserEnabledEmailFactory; @@ -79,6 +80,11 @@ public function sendChangeEmailConfirmationEmailAfterWrite(AbstractUser $user): return $this->afterWrite($user, fn (): bool => $this->sendChangeEmailConfirmationEmail($user)); } + public function sendChangeEmailNoticeEmail(AbstractUser $user): bool + { + return $this->afterWrite($user, fn (): bool => $this->send($this->container->get(ChangeEmailNoticeEmailFactory::class)->create($user, $this->context))); + } + public function sendWelcomeEmail(AbstractUser $user): bool { return $this->afterWrite($user, fn (): bool => $this->send($this->container->get(WelcomeEmailFactory::class)->create($user, $this->context))); diff --git a/src/Helper/User/UserWriteNotifier.php b/src/Helper/User/UserWriteNotifier.php index 04745a53..b2a29211 100644 --- a/src/Helper/User/UserWriteNotifier.php +++ b/src/Helper/User/UserWriteNotifier.php @@ -49,6 +49,7 @@ public function notify(AbstractUser $user, ?AbstractUser $previousUser): void if (($token = $user->getNewEmailConfirmationToken()) && $token !== $previousUser->getNewEmailConfirmationToken()) { $this->userMailer->sendChangeEmailConfirmationEmailAfterWrite($user); + $this->userMailer->sendChangeEmailNoticeEmail($user); } } } diff --git a/src/Resources/config/services.php b/src/Resources/config/services.php index 865ad6c2..90648035 100644 --- a/src/Resources/config/services.php +++ b/src/Resources/config/services.php @@ -128,6 +128,7 @@ use Silverback\ApiComponentsBundle\Factory\Uploadable\UploadableUrlGeneratorInterface; use Silverback\ApiComponentsBundle\Factory\User\Mailer\AbstractUserEmailFactory; use Silverback\ApiComponentsBundle\Factory\User\Mailer\ChangeEmailConfirmationEmailFactory; +use Silverback\ApiComponentsBundle\Factory\User\Mailer\ChangeEmailNoticeEmailFactory; use Silverback\ApiComponentsBundle\Factory\User\Mailer\PasswordChangedEmailFactory; use Silverback\ApiComponentsBundle\Factory\User\Mailer\PasswordResetEmailFactory; use Silverback\ApiComponentsBundle\Factory\User\Mailer\UserEnabledEmailFactory; @@ -228,6 +229,7 @@ use Silverback\ApiComponentsBundle\Validator\Constraints\AddedComponentPositionsValidator; use Silverback\ApiComponentsBundle\Validator\Constraints\ComponentPositionValidator; use Silverback\ApiComponentsBundle\Validator\Constraints\FormTypeClassValidator; +use Silverback\ApiComponentsBundle\Validator\Constraints\NewEmailAddressRequiresPasswordValidator; use Silverback\ApiComponentsBundle\Validator\Constraints\NewEmailAddressValidator; use Silverback\ApiComponentsBundle\Validator\Constraints\RequiresUploadedFileValidator; use Silverback\ApiComponentsBundle\Validator\Constraints\ResourceIriValidator; @@ -323,6 +325,13 @@ ->tag('kernel.reset', ['method' => 'reset']); $services->alias(ChangeEmailConfirmationEmailFactory::class, 'silverback.api_components.factory.user.mailer.change_email_confirmation_email'); + $services + ->set('silverback.api_components.factory.user.mailer.change_email_notice_email') + ->class(ChangeEmailNoticeEmailFactory::class) + ->parent(AbstractUserEmailFactory::class) + ->tag('kernel.reset', ['method' => 'reset']); + $services->alias(ChangeEmailNoticeEmailFactory::class, 'silverback.api_components.factory.user.mailer.change_email_notice_email'); + $services ->set('silverback.api_components.form.change_password_type') ->class(ChangePasswordType::class) @@ -882,6 +891,17 @@ ->tag('validator.constraint_validator'); $services->alias(NewEmailAddressValidator::class, 'silverback.api_components.validator.new_email_address'); + $services + ->set('silverback.api_components.validator.new_email_address_requires_password') + ->class(NewEmailAddressRequiresPasswordValidator::class) + ->args( + [ + new Reference(ManagerRegistry::class), + ] + ) + ->tag('validator.constraint_validator'); + $services->alias(NewEmailAddressRequiresPasswordValidator::class, 'silverback.api_components.validator.new_email_address_requires_password'); + $services ->set('silverback.api_components.data_provider.page_data') ->class(PageDataProvider::class) @@ -1960,6 +1980,7 @@ service_locator([ PasswordResetEmailFactory::class => new Reference(PasswordResetEmailFactory::class), ChangeEmailConfirmationEmailFactory::class => new Reference(ChangeEmailConfirmationEmailFactory::class), + ChangeEmailNoticeEmailFactory::class => new Reference(ChangeEmailNoticeEmailFactory::class), WelcomeEmailFactory::class => new Reference(WelcomeEmailFactory::class), UserEnabledEmailFactory::class => new Reference(UserEnabledEmailFactory::class), UsernameChangedEmailFactory::class => new Reference(UsernameChangedEmailFactory::class), diff --git a/src/Resources/views/emails/user_change_email_confirmation.html.twig b/src/Resources/views/emails/user_change_email_confirmation.html.twig index 1774c096..90595eed 100644 --- a/src/Resources/views/emails/user_change_email_confirmation.html.twig +++ b/src/Resources/views/emails/user_change_email_confirmation.html.twig @@ -4,8 +4,8 @@

Confirm Your New Email Address

Hello {{ user.username }},

-

It looks like you have requested to change your email address to {{ user.newEmailAddress }}.

-

Click the link below to complete the request and change your email address.

+

You have requested to change the email address of your {{ website_name }} account to this address.

+

Click the link below to confirm that this address is yours and complete the change.

{% include "@SilverbackApiComponents/emails/_signature.html.twig" %} {% endblock %} diff --git a/src/Resources/views/emails/user_change_email_notice.html.twig b/src/Resources/views/emails/user_change_email_notice.html.twig new file mode 100644 index 00000000..2a42947f --- /dev/null +++ b/src/Resources/views/emails/user_change_email_notice.html.twig @@ -0,0 +1,10 @@ +{% extends "@SilverbackApiComponents/emails/_template.html.twig" %} + +{% block content %} +

Your Email Address Is Being Changed

+ +

Hello {{ user.username }},

+

A request has been made to change the email address of your {{ website_name }} account to {{ masked_new_email_address }}. A link to confirm the change has been sent to that address.

+

If this was you no further action is required. If this was not you please use the forgot password feature to reset your password.

+ {% include "@SilverbackApiComponents/emails/_signature.html.twig" %} +{% endblock %} diff --git a/src/Utility/MaskedEmailAddress.php b/src/Utility/MaskedEmailAddress.php new file mode 100644 index 00000000..6d7a7897 --- /dev/null +++ b/src/Utility/MaskedEmailAddress.php @@ -0,0 +1,25 @@ + + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ + +namespace Silverback\ApiComponentsBundle\Utility; + +final class MaskedEmailAddress +{ + public static function of(string $emailAddress): string + { + $at = strrpos($emailAddress, '@'); + if (false === $at) { + return mb_substr($emailAddress, 0, 1) . '***'; + } + + return mb_substr($emailAddress, 0, 1) . '***' . substr($emailAddress, $at); + } +} diff --git a/src/Validator/Constraints/NewEmailAddressRequiresPassword.php b/src/Validator/Constraints/NewEmailAddressRequiresPassword.php new file mode 100644 index 00000000..a0aec20f --- /dev/null +++ b/src/Validator/Constraints/NewEmailAddressRequiresPassword.php @@ -0,0 +1,25 @@ + + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ + +namespace Silverback\ApiComponentsBundle\Validator\Constraints; + +use Symfony\Component\Validator\Constraint; + +#[\Attribute(\Attribute::TARGET_CLASS)] +class NewEmailAddressRequiresPassword extends Constraint +{ + public string $message = 'You have not entered your current password correctly. Please try again.'; + + public function getTargets(): string|array + { + return self::CLASS_CONSTRAINT; + } +} diff --git a/src/Validator/Constraints/NewEmailAddressRequiresPasswordValidator.php b/src/Validator/Constraints/NewEmailAddressRequiresPasswordValidator.php new file mode 100644 index 00000000..a4d33b9e --- /dev/null +++ b/src/Validator/Constraints/NewEmailAddressRequiresPasswordValidator.php @@ -0,0 +1,60 @@ + + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ + +namespace Silverback\ApiComponentsBundle\Validator\Constraints; + +use Doctrine\ORM\EntityManagerInterface; +use Doctrine\Persistence\ManagerRegistry; +use Silverback\ApiComponentsBundle\Entity\User\AbstractUser; +use Symfony\Component\Security\Core\Validator\Constraints\UserPassword; +use Symfony\Component\Validator\Constraint; +use Symfony\Component\Validator\ConstraintValidator; +use Symfony\Component\Validator\Exception\UnexpectedTypeException; + +class NewEmailAddressRequiresPasswordValidator extends ConstraintValidator +{ + public function __construct(private readonly ManagerRegistry $registry) + { + } + + public function validate(mixed $user, Constraint $constraint): void + { + if (!$constraint instanceof NewEmailAddressRequiresPassword) { + throw new UnexpectedTypeException($constraint, NewEmailAddressRequiresPassword::class); + } + + if (!$user instanceof AbstractUser) { + throw new UnexpectedTypeException($user, AbstractUser::class); + } + + $newEmailAddress = $user->getNewEmailAddress(); + if (null === $newEmailAddress || '' === $newEmailAddress || $newEmailAddress === $this->storedNewEmailAddress($user)) { + return; + } + + $this->context->getValidator() + ->inContext($this->context) + ->atPath('oldPassword') + ->validate($user->getOldPassword(), new UserPassword(message: $constraint->message, groups: $constraint->groups)); + } + + private function storedNewEmailAddress(AbstractUser $user): ?string + { + $manager = $this->registry->getManagerForClass($user::class); + if (!$manager instanceof EntityManagerInterface) { + return null; + } + + $original = $manager->getUnitOfWork()->getOriginalEntityData($user); + + return \array_key_exists('newEmailAddress', $original) && \is_string($original['newEmailAddress']) ? $original['newEmailAddress'] : null; + } +} diff --git a/tests/DependencyInjection/SilverbackApiComponentsExtensionTest.php b/tests/DependencyInjection/SilverbackApiComponentsExtensionTest.php index bfa7c9f6..8e783e64 100644 --- a/tests/DependencyInjection/SilverbackApiComponentsExtensionTest.php +++ b/tests/DependencyInjection/SilverbackApiComponentsExtensionTest.php @@ -42,6 +42,7 @@ use Silverback\ApiComponentsBundle\Factory\OrphanedResource\OrphanedResourcesChangedEmailFactory; use Silverback\ApiComponentsBundle\Factory\Uploadable\MediaObjectFactory; use Silverback\ApiComponentsBundle\Factory\User\Mailer\ChangeEmailConfirmationEmailFactory; +use Silverback\ApiComponentsBundle\Factory\User\Mailer\ChangeEmailNoticeEmailFactory; use Silverback\ApiComponentsBundle\Factory\User\Mailer\PasswordResetEmailFactory; use Silverback\ApiComponentsBundle\Factory\User\Mailer\VerifyEmailFactory; use Silverback\ApiComponentsBundle\Factory\User\Mailer\WelcomeEmailFactory; @@ -606,6 +607,19 @@ public function test_the_mailer_subjects_default_and_can_be_overridden(): void self::assertTrue($this->argument($container, PasswordResetEmailFactory::class, '$enabled')); self::assertTrue($this->argument($container, ChangeEmailConfirmationEmailFactory::class, '$enabled')); + self::assertSame('Your email address is being changed', $this->argument($container, ChangeEmailNoticeEmailFactory::class, '$subject')); + self::assertTrue($this->argument($container, ChangeEmailNoticeEmailFactory::class, '$enabled')); + } + + public function test_a_disabled_change_email_notice_is_wired_as_disabled(): void + { + $config = self::minimalConfig(); + $config['user']['emails'] = ['change_email_notice' => ['enabled' => false, 'subject' => 'Changing']]; + + [$container] = $this->load($config); + + self::assertFalse($this->argument($container, ChangeEmailNoticeEmailFactory::class, '$enabled')); + self::assertSame('Changing', $this->argument($container, ChangeEmailNoticeEmailFactory::class, '$subject')); } public function test_a_disabled_welcome_email_is_wired_as_disabled(): void diff --git a/tests/Factory/User/Mailer/ChangeEmailNoticeEmailFactoryTest.php b/tests/Factory/User/Mailer/ChangeEmailNoticeEmailFactoryTest.php new file mode 100644 index 00000000..9c28892f --- /dev/null +++ b/tests/Factory/User/Mailer/ChangeEmailNoticeEmailFactoryTest.php @@ -0,0 +1,87 @@ + + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ + +namespace Silverback\ApiComponentsBundle\Tests\Factory\User\Mailer; + +use PHPUnit\Framework\Attributes\AllowMockObjectsWithoutExpectations; +use PHPUnit\Framework\Attributes\DataProvider; +use Silverback\ApiComponentsBundle\Entity\User\AbstractUser; +use Silverback\ApiComponentsBundle\Exception\InvalidArgumentException; +use Silverback\ApiComponentsBundle\Factory\User\Mailer\ChangeEmailNoticeEmailFactory; +use Symfony\Bridge\Twig\Mime\TemplatedEmail; +use Symfony\Component\Mime\Address; + +#[AllowMockObjectsWithoutExpectations] +class ChangeEmailNoticeEmailFactoryTest extends AbstractFinalEmailFactoryTestCase +{ + public function test_skip_user_validation_if_disabled(): void + { + $factory = new ChangeEmailNoticeEmailFactory($this->containerInterfaceMock, $this->eventDispatcherMock, 'subject', false); + $this->assertNull( + $factory->create( + new class extends AbstractUser { + } + ) + ); + } + + #[DataProvider('noNewEmailAddress')] + public function test_exception_thrown_if_no_new_email_address(?string $newEmailAddress): void + { + $user = new class extends AbstractUser { + }; + $user + ->setUsername('username') + ->setEmailAddress('email@address.com') + ->setNewEmailAddress($newEmailAddress); + + $factory = new ChangeEmailNoticeEmailFactory($this->containerInterfaceMock, $this->eventDispatcherMock, 'subject'); + + $this->expectException(InvalidArgumentException::class); + $this->expectExceptionMessage('A `new email address` must be set to send the change email notice'); + + $factory->create($user); + } + + public static function noNewEmailAddress(): iterable + { + yield 'null' => [null]; + yield 'a blank string' => ['']; + } + + public function test_the_notice_goes_to_the_current_address_with_the_new_one_masked_and_no_link(): void + { + $user = new class extends AbstractUser { + }; + $user + ->setUsername('username') + ->setEmailAddress('email@address.com') + ->setNewEmailAddress('new@address.com'); + + $factory = new ChangeEmailNoticeEmailFactory($this->containerInterfaceMock, $this->eventDispatcherMock, 'subject'); + + $this->assertCommonMockMethodsCalled(); + + $email = (new TemplatedEmail()) + ->to(Address::create('email@address.com')) + ->subject('subject') + ->htmlTemplate('@SilverbackApiComponents/emails/user_change_email_notice.html.twig') + ->context( + [ + 'website_name' => 'my website', + 'user' => $user, + 'masked_new_email_address' => 'n***@address.com', + ] + ); + + $this->assertEmailEquals($email, $factory->create($user, ['website_name' => 'my website']), ChangeEmailNoticeEmailFactory::MESSAGE_ID_PREFIX); + } +} diff --git a/tests/Factory/User/Mailer/ChangeEmailVerificationEmailFactoryTest.php b/tests/Factory/User/Mailer/ChangeEmailVerificationEmailFactoryTest.php index 7cc3fd2b..22528d5f 100644 --- a/tests/Factory/User/Mailer/ChangeEmailVerificationEmailFactoryTest.php +++ b/tests/Factory/User/Mailer/ChangeEmailVerificationEmailFactoryTest.php @@ -54,7 +54,7 @@ public function test_exception_thrown_if_no_website_name(): void { $user = new class extends AbstractUser { }; - $user->setUsername('username')->setEmailAddress('email@address.com'); + $user->setUsername('username')->setEmailAddress('email@address.com')->setNewEmailAddress('new@address.com'); $user->plainNewEmailConfirmationToken = 'token'; $factory = new ChangeEmailConfirmationEmailFactory($this->containerInterfaceMock, $this->eventDispatcherMock, 'subject', true, '/default-path'); @@ -89,14 +89,15 @@ public function test_redirect_url_context_added_and_html_template_passed(): void $user ->setUsername('username') - ->setEmailAddress('email@address.com'); + ->setEmailAddress('email@address.com') + ->setNewEmailAddress('new@address.com'); $user->plainNewEmailConfirmationToken = 'token'; $factory = new ChangeEmailConfirmationEmailFactory($this->containerInterfaceMock, $this->eventDispatcherMock, 'subject', true, '/default-path'); $this->assertCommonMockMethodsCalled(true); $email = (new TemplatedEmail()) - ->to(Address::create('email@address.com')) + ->to(Address::create('new@address.com')) ->subject('subject') ->htmlTemplate('@SilverbackApiComponents/emails/user_change_email_confirmation.html.twig') ->context( diff --git a/tests/Functional/Trait/EmailTrait.php b/tests/Functional/Trait/EmailTrait.php index 73672df2..709f3d54 100644 --- a/tests/Functional/Trait/EmailTrait.php +++ b/tests/Functional/Trait/EmailTrait.php @@ -57,6 +57,20 @@ protected static function assertOneTemplatedEmailSentTo(string $emailAddress = ' return $email; } + protected static function assertTemplatedEmailSentTo(string $emailAddress): TemplatedEmail + { + $emails = array_values(array_filter(static::sentTemplatedEmails(), static fn (TemplatedEmail $email): bool => $email->getHeaders()->get('to')?->getBodyAsString() === $emailAddress)); + self::assertCount(1, $emails, \sprintf('%d emails were sent to %s.', \count($emails), $emailAddress)); + $email = $emails[0]; + $context = $email->getContext(); + + self::assertSame('New Website', $context['website_name'] ?? null); + self::assertInstanceOf(User::class, $context['user'] ?? null); + self::assertSame('test@website.com', $email->getHeaders()->get('from')?->getBodyAsString()); + + return $email; + } + protected static function sentEmailLink(): string { $emails = static::sentTemplatedEmails(); diff --git a/tests/Functional/Trait/EmailTraitTest.php b/tests/Functional/Trait/EmailTraitTest.php index 0a7963a5..d3537feb 100644 --- a/tests/Functional/Trait/EmailTraitTest.php +++ b/tests/Functional/Trait/EmailTraitTest.php @@ -18,6 +18,7 @@ final class EmailTraitTest extends FunctionalTestCase { use AuthenticationTrait; use EmailTrait; + use ResourceTrait; private const array HEADERS = ['accept' => 'application/ld+json', 'referer' => 'http://www.website.com']; @@ -59,6 +60,31 @@ public function test_a_request_for_an_unknown_user_sends_no_email(): void self::assertSame([], self::sentTemplatedEmails()); } + public function test_each_of_several_emails_is_found_by_its_recipient(): void + { + $client = static::createClient(); + $user = self::loginAsUser($client); + + $client->request('PATCH', self::iriOf($user), ['headers' => self::HEADERS + ['content-type' => 'application/merge-patch+json'], 'json' => ['newEmailAddress' => 'new@example.com', 'oldPassword' => 'password']]); + + self::assertResponseStatusCodeSame(200); + self::assertCount(2, self::sentTemplatedEmails()); + self::assertSame('Please confirm your new email address', self::assertTemplatedEmailSentTo('new@example.com')->getSubject()); + self::assertSame('Your email address is being changed', self::assertTemplatedEmailSentTo('user@example.com')->getSubject()); + } + + public function test_no_email_to_an_address_fails_the_recipient_assertion(): void + { + $client = static::createClient(); + self::createUser(['ROLE_USER'], 'my_username', 'test.user@example.com'); + self::rebootKernel(); + $client->request('GET', '/password/reset/request/my_username', ['headers' => self::HEADERS]); + + $this->expectException(AssertionFailedError::class); + + self::assertTemplatedEmailSentTo('someone.else@example.com'); + } + public function test_an_email_to_another_address_fails_the_assertion(): void { $client = static::createClient(); diff --git a/tests/Functional/User/EmailLinkHostTest.php b/tests/Functional/User/EmailLinkHostTest.php index 16266f77..bec9c1b0 100644 --- a/tests/Functional/User/EmailLinkHostTest.php +++ b/tests/Functional/User/EmailLinkHostTest.php @@ -241,17 +241,17 @@ public function test_enabling_an_account_with_an_origin_header_that_is_not_allow self::assertRefusedEmailLinkLogged('user@user.co'); } - public function test_an_email_change_request_with_an_origin_header_that_is_not_allowed_is_saved_sends_no_email_and_logs_the_refusal(): void + public function test_an_email_change_request_with_an_origin_header_that_is_not_allowed_is_saved_sends_only_the_notice_and_logs_the_refusal(): void { $client = static::createClient(); self::loginAsUser($client); $formIri = self::createForm(NewEmailAddressType::class); - self::requestJsonLd($client, 'POST', $formIri . '/submit', ['new_email_address' => ['newEmailAddress' => 'new@example.com']], ['origin' => 'https://evil.example']); + self::requestJsonLd($client, 'POST', $formIri . '/submit', ['new_email_address' => ['newEmailAddress' => 'new@example.com', 'oldPassword' => 'password']], ['origin' => 'https://evil.example']); self::assertResponseStatusCodeSame(201); self::assertJsonNodeSame('newEmailAddress', 'new@example.com'); - self::assertNoEmailSent(); + self::assertOnlyEmailChangeNoticeSent(); self::assertRefusedEmailLinkLogged('new_user'); } diff --git a/tests/Functional/User/EmailRequestThrottleTest.php b/tests/Functional/User/EmailRequestThrottleTest.php index 1776c31b..0a1d265b 100644 --- a/tests/Functional/User/EmailRequestThrottleTest.php +++ b/tests/Functional/User/EmailRequestThrottleTest.php @@ -105,7 +105,7 @@ public function test_a_resend_of_the_new_email_address_confirmation_after_its_th self::requestEmail($client, '/resend-verify-new-email/my_username'); self::assertResponseStatusCodeSame(200); - self::assertEmailOfTypeSent('change_email_confirmation'); + self::assertEmailOfTypeSent('change_email_confirmation', 'new@example.com'); } public function test_the_new_email_address_confirmation_is_not_throttled_by_a_recent_password_reset_request(): void @@ -118,7 +118,7 @@ public function test_the_new_email_address_confirmation_is_not_throttled_by_a_re self::requestEmail($client, '/resend-verify-new-email/my_username'); self::assertResponseStatusCodeSame(200); - self::assertEmailOfTypeSent('change_email_confirmation'); + self::assertEmailOfTypeSent('change_email_confirmation', 'new@example.com'); } public function test_the_email_verification_is_not_throttled_by_a_recent_password_reset_request(): void diff --git a/tests/Functional/User/NewEmailAddressTest.php b/tests/Functional/User/NewEmailAddressTest.php index c3d7427f..1f1ec076 100644 --- a/tests/Functional/User/NewEmailAddressTest.php +++ b/tests/Functional/User/NewEmailAddressTest.php @@ -21,6 +21,7 @@ final class NewEmailAddressTest extends FunctionalTestCase use UserTrait; private const array REFERER = ['referer' => 'http://www.website.com']; + private const string WRONG_PASSWORD = 'You have not entered your current password correctly. Please try again.'; /** * @param array $headers @@ -32,14 +33,15 @@ public function test_submit_a_successful_change_email_request(array $headers, st self::loginAsUser($client); $formIri = self::createForm(NewEmailAddressType::class); - self::submitNewEmailAddress($client, $formIri . $postfix, 'new@example.com', $headers); + self::submitNewEmailAddress($client, $formIri . $postfix, 'new@example.com', $headers, 'password'); self::assertResponseStatusCodeSame(201); self::assertJsonMatchesSchemaFile('user.schema.json'); self::assertJsonNodeSame('newEmailAddress', 'new@example.com'); self::assertJsonNodeSame('emailAddress', 'user@example.com'); self::assertJsonNodeNotExists('newEmailConfirmationToken'); - self::assertEmailOfTypeSent($expectedEmail); + self::assertJsonNodeNotExists('oldPassword'); + self::assertEmailChangeEmailsSent($expectedEmail); } public static function successfulChangeRequests(): iterable @@ -53,17 +55,17 @@ public static function successfulChangeRequests(): iterable * @param array $headers */ #[DataProvider('invalidOrMissingHeaders')] - public function test_an_invalid_or_missing_referer_and_origin_still_saves_the_change_request_sends_no_email_and_logs_the_refusal(array $headers): void + public function test_an_invalid_or_missing_referer_and_origin_still_saves_the_change_request_sends_only_the_notice_and_logs_the_refusal(array $headers): void { $client = static::createClient(); self::loginAsUser($client); $formIri = self::createForm(NewEmailAddressType::class); - self::submitNewEmailAddress($client, $formIri . '/submit', 'new@example.com', $headers); + self::submitNewEmailAddress($client, $formIri . '/submit', 'new@example.com', $headers, 'password'); self::assertResponseStatusCodeSame(201); self::assertJsonNodeSame('newEmailAddress', 'new@example.com'); - self::assertNoEmailSent(); + self::assertOnlyEmailChangeNoticeSent(); self::assertRefusedEmailLinkLogged('new_user'); } @@ -84,7 +86,7 @@ public function test_i_get_an_invalid_response_if_i_try_to_change_my_email_addre self::loginAsUser($client); $formIri = self::createForm(NewEmailAddressType::class); - self::submitNewEmailAddress($client, $formIri . '/submit', 'user@example.com', self::REFERER); + self::submitNewEmailAddress($client, $formIri . '/submit', 'user@example.com', self::REFERER, 'password'); self::assertResponseStatusCodeSame(422); self::assertJsonNodeSame('formView.children[0].vars.errors[0]', 'Your new email address should be different.'); @@ -99,7 +101,7 @@ public function test_i_get_an_invalid_response_if_i_try_to_change_my_email_addre $formIri = self::createForm(NewEmailAddressType::class); self::createUserWithRole('another_user', self::generatedSecret(), 'ROLE_USER'); - self::submitNewEmailAddress($client, $formIri . '/submit', 'test.user@example.com', self::REFERER); + self::submitNewEmailAddress($client, $formIri . '/submit', 'test.user@example.com', self::REFERER, 'password'); self::assertResponseStatusCodeSame(422); self::assertJsonNodeSame('formView.children[0].vars.errors[0]', 'Someone else is already registered with that email address.'); @@ -192,10 +194,10 @@ public function test_cancelling_a_pending_request_to_change_email_address_clears $reloaded = self::findUserByUsername('new_user'); self::assertNotNull($reloaded); self::addBearerToken($client, $reloaded); - self::submitNewEmailAddress($client, $formIri . '/submit', 'new@example.com', self::REFERER); + self::submitNewEmailAddress($client, $formIri . '/submit', 'new@example.com', self::REFERER, 'password'); self::assertResponseStatusCodeSame(201); - self::assertEmailOfTypeSent('change_email_confirmation'); + self::assertEmailChangeEmailsSent(); } public function test_i_can_resend_a_new_email_address_confirmation_email_with_a_new_token(): void @@ -209,7 +211,7 @@ public function test_i_can_resend_a_new_email_address_confirmation_email_with_a_ self::requestJsonLd($client, 'GET', '/resend-verify-new-email/my_username', null, self::REFERER); self::assertResponseStatusCodeSame(200); - self::assertEmailOfTypeSent('change_email_confirmation'); + self::assertEmailOfTypeSent('change_email_confirmation', 'new@example.com'); } public function test_requesting_a_new_email_address_confirmation_for_an_unknown_username_is_a_404_not_an_error(): void @@ -252,11 +254,113 @@ public function test_email_verification_is_reset_if_another_user_now_has_confirm self::assertNoEmailSent(); } + public function test_a_change_request_without_the_current_password_is_refused_and_changes_nothing(): void + { + $client = static::createClient(); + self::loginAsUser($client); + $formIri = self::createForm(NewEmailAddressType::class); + + self::submitNewEmailAddress($client, $formIri . '/submit', 'new@example.com', self::REFERER); + + self::assertResponseStatusCodeSame(422); + self::assertJsonNodeSame('formView.children[1].vars.full_name', 'new_email_address[oldPassword]'); + self::assertJsonNodeSame('formView.children[1].vars.errors[0]', self::WRONG_PASSWORD); + self::assertNoEmailSent(); + self::assertNoNewEmailAddress('new_user'); + } + + public function test_a_change_request_with_a_wrong_current_password_is_refused_and_changes_nothing(): void + { + $client = static::createClient(); + self::loginAsUser($client); + $formIri = self::createForm(NewEmailAddressType::class); + + self::submitNewEmailAddress($client, $formIri . '/submit', 'new@example.com', self::REFERER, 'not-my-password'); + + self::assertResponseStatusCodeSame(422); + self::assertJsonNodeSame('formView.children[1].vars.errors[0]', self::WRONG_PASSWORD); + self::assertNoEmailSent(); + self::assertNoNewEmailAddress('new_user'); + } + + public function test_my_own_patch_of_a_new_email_address_needs_my_current_password(): void + { + $client = static::createClient(); + $user = self::loginAsUser($client); + + self::requestJsonLd($client, 'PATCH', self::iriOf($user), ['newEmailAddress' => 'new@example.com'], self::REFERER); + + self::assertResponseStatusCodeSame(422); + self::assertJsonNodeSame('violations[0].propertyPath', 'oldPassword'); + self::assertJsonNodeSame('violations[0].message', self::WRONG_PASSWORD); + self::assertNoEmailSent(); + self::assertNoNewEmailAddress('new_user'); + } + + public function test_my_own_patch_of_a_new_email_address_with_my_current_password_sends_the_confirmation_to_the_new_address_and_a_notice_to_the_current_one(): void + { + $client = static::createClient(); + $user = self::loginAsUser($client); + + self::requestJsonLd($client, 'PATCH', self::iriOf($user), ['newEmailAddress' => 'new@example.com', 'oldPassword' => 'password'], self::REFERER); + + self::assertResponseStatusCodeSame(200); + self::assertJsonNodeSame('newEmailAddress', 'new@example.com'); + self::assertJsonNodeSame('emailAddress', 'user@example.com'); + self::assertEmailChangeEmailsSent(); + } + + public function test_a_super_admin_changing_another_users_new_email_address_enters_their_own_password(): void + { + $client = static::createClient(); + self::loginAsSuperAdmin($client); + $userPassword = self::generatedSecret(); + $user = self::createUserWithRole('another_user', $userPassword, 'ROLE_USER'); + + self::requestJsonLd($client, 'PATCH', self::iriOf($user), ['newEmailAddress' => 'new@example.com', 'oldPassword' => $userPassword], self::REFERER); + + self::assertResponseStatusCodeSame(422); + self::assertJsonNodeSame('violations[0].propertyPath', 'oldPassword'); + self::assertNoNewEmailAddress('another_user'); + + self::requestJsonLd($client, 'PATCH', self::iriOf($user), ['newEmailAddress' => 'new@example.com', 'oldPassword' => 'password'], self::REFERER); + + self::assertResponseStatusCodeSame(200); + self::assertJsonNodeSame('newEmailAddress', 'new@example.com'); + self::assertEmailChangeEmailsSent(currentEmailAddress: 'test.user@example.com'); + } + + public function test_a_write_that_leaves_a_pending_new_email_address_unchanged_needs_no_password(): void + { + $client = static::createClient(); + self::loginAsSuperAdmin($client); + $user = self::createUserWithRole('another_user', self::generatedSecret(), 'ROLE_USER'); + self::giveNewEmailAddress($user, 'new@example.com', self::generatedSecret()); + + self::requestJsonLd($client, 'PATCH', self::iriOf($user), ['username' => 'renamed_user', 'newEmailAddress' => 'new@example.com'], self::REFERER); + + self::assertResponseStatusCodeSame(200); + self::assertJsonNodeSame('newEmailAddress', 'new@example.com'); + self::assertEmailOfTypeSent('username_changed_notification', 'test.user@example.com'); + } + + private static function assertNoNewEmailAddress(string $username): void + { + $stored = self::findUserByUsername($username); + self::assertNotNull($stored); + self::assertNull($stored->getNewEmailAddress()); + self::assertNull($stored->getNewEmailConfirmationToken()); + } + /** * @param array $headers */ - private static function submitNewEmailAddress(Client $client, string $url, ?string $newEmailAddress, array $headers): void + private static function submitNewEmailAddress(Client $client, string $url, ?string $newEmailAddress, array $headers, ?string $oldPassword = null): void { - self::requestJsonLd($client, 'POST', $url, ['new_email_address' => ['newEmailAddress' => $newEmailAddress]], $headers); + $data = ['newEmailAddress' => $newEmailAddress]; + if (null !== $oldPassword) { + $data['oldPassword'] = $oldPassword; + } + self::requestJsonLd($client, 'POST', $url, ['new_email_address' => $data], $headers); } } diff --git a/tests/Functional/User/UserPutTest.php b/tests/Functional/User/UserPutTest.php index 968147c5..2b4c09f1 100644 --- a/tests/Functional/User/UserPutTest.php +++ b/tests/Functional/User/UserPutTest.php @@ -52,7 +52,7 @@ public function test_a_users_own_put_with_only_a_new_email_address_keeps_their_o $password = self::generatedSecret(); $user = self::loginAsUserWithPassword($client, $password); - self::requestJsonLd($client, 'PUT', self::iriOf($user), ['newEmailAddress' => 'changed@example.com']); + self::requestJsonLd($client, 'PUT', self::iriOf($user), ['newEmailAddress' => 'changed@example.com', 'oldPassword' => $password]); self::assertResponseStatusCodeSame(200); self::assertPutResponseIsTheMergedUser(self::iriOf($user), 'new_user', 'user@example.com', ['ROLE_USER'], false); diff --git a/tests/Functional/User/UserTrait.php b/tests/Functional/User/UserTrait.php index 327cca1e..10463a6f 100644 --- a/tests/Functional/User/UserTrait.php +++ b/tests/Functional/User/UserTrait.php @@ -18,6 +18,7 @@ use Silverback\ApiComponentsBundle\Entity\Component\Form; use Silverback\ApiComponentsBundle\Exception\UnparseableRequestHeaderException; use Silverback\ApiComponentsBundle\Factory\User\Mailer\ChangeEmailConfirmationEmailFactory; +use Silverback\ApiComponentsBundle\Factory\User\Mailer\ChangeEmailNoticeEmailFactory; use Silverback\ApiComponentsBundle\Factory\User\Mailer\PasswordChangedEmailFactory; use Silverback\ApiComponentsBundle\Factory\User\Mailer\PasswordResetEmailFactory; use Silverback\ApiComponentsBundle\Factory\User\Mailer\UserEnabledEmailFactory; @@ -197,7 +198,23 @@ protected static function assertSentEmailLinkStartsWith(string $prefix): void protected static function assertEmailOfTypeSent(string $type, string $emailAddress = 'user@example.com'): TemplatedEmail { - $email = static::assertOneTemplatedEmailSentTo($emailAddress); + return static::assertEmailIsOfType(static::assertOneTemplatedEmailSentTo($emailAddress), $type); + } + + protected static function assertEmailChangeEmailsSent(string $confirmationType = 'change_email_confirmation', string $currentEmailAddress = 'user@example.com'): void + { + self::assertCount(2, static::sentTemplatedEmails()); + static::assertEmailIsOfType(static::assertTemplatedEmailSentTo('new@example.com'), $confirmationType); + static::assertEmailIsOfType(static::assertTemplatedEmailSentTo($currentEmailAddress), 'change_email_notice'); + } + + protected static function assertOnlyEmailChangeNoticeSent(): void + { + static::assertEmailIsOfType(static::assertOneTemplatedEmailSentTo('user@example.com'), 'change_email_notice'); + } + + protected static function assertEmailIsOfType(TemplatedEmail $email, string $type): TemplatedEmail + { $context = $email->getContext(); $headers = $email->getHeaders(); $subject = $headers->get('subject')?->getBodyAsString(); @@ -210,6 +227,7 @@ protected static function assertEmailOfTypeSent(string $type, string $emailAddre 'enabled_notification' => ['Your account has been enabled', UserEnabledEmailFactory::MESSAGE_ID_PREFIX, '/^http:\/\/www\.website\.com\/login$/i'], 'custom_change_email_confirmation' => ['Please confirm your new email address', ChangeEmailConfirmationEmailFactory::MESSAGE_ID_PREFIX, '/^http:\/\/www\.website\.com\/another-path\/new_user\/new%40example.com\/' . $token . '$/i'], 'change_email_confirmation' => ['Please confirm your new email address', ChangeEmailConfirmationEmailFactory::MESSAGE_ID_PREFIX, '/^http:\/\/www\.website\.com\/confirm-new-email\/' . preg_quote($context['user']->getUsername(), '/') . '\/new%40example.com\/' . $token . '$/i'], + 'change_email_notice' => ['Your email address is being changed', ChangeEmailNoticeEmailFactory::MESSAGE_ID_PREFIX, null], 'change_password_notification', 'password_changed' => ['Your password has been changed', PasswordChangedEmailFactory::MESSAGE_ID_PREFIX, null], 'user_welcome' => ['Welcome to New Website', WelcomeEmailFactory::MESSAGE_ID_PREFIX, '/^http:\/\/www.website.com\/verify-email\/new_user\/' . $token . '$/i'], 'password_reset' => ['Your password reset request', PasswordResetEmailFactory::MESSAGE_ID_PREFIX, '/^http:\/\/www.website.com\/reset-password\/my_username\/' . $token . '$/i'], @@ -221,6 +239,10 @@ protected static function assertEmailOfTypeSent(string $type, string $emailAddre if (\in_array($type, ['change_email_confirmation', 'custom_change_email_confirmation'], true)) { self::assertIsString($context['user']->getNewEmailConfirmationToken()); } + if ('change_email_notice' === $type) { + self::assertSame('n***@example.com', $context['masked_new_email_address'] ?? null); + self::assertArrayNotHasKey('redirect_url', $context); + } if (null !== $link) { self::assertMatchesRegularExpression($link, $context['enabled_notification' === $type ? 'login_url' : 'redirect_url']); } diff --git a/tests/Helper/User/UserMailerTest.php b/tests/Helper/User/UserMailerTest.php index b83e64ee..9d2ebe40 100644 --- a/tests/Helper/User/UserMailerTest.php +++ b/tests/Helper/User/UserMailerTest.php @@ -25,6 +25,7 @@ use Silverback\ApiComponentsBundle\Exception\MailerTransportException; use Silverback\ApiComponentsBundle\Factory\User\Mailer\AbstractUserEmailFactory; use Silverback\ApiComponentsBundle\Factory\User\Mailer\ChangeEmailConfirmationEmailFactory; +use Silverback\ApiComponentsBundle\Factory\User\Mailer\ChangeEmailNoticeEmailFactory; use Silverback\ApiComponentsBundle\Factory\User\Mailer\PasswordChangedEmailFactory; use Silverback\ApiComponentsBundle\Factory\User\Mailer\PasswordResetEmailFactory; use Silverback\ApiComponentsBundle\Factory\User\Mailer\UserEnabledEmailFactory; @@ -235,6 +236,17 @@ public function test_send_password_changed_email(): void $this->assertTrue($result); } + public function test_send_change_email_notice_email(): void + { + $user = new class extends AbstractUser { + protected ?string $username = 'test_send_change_email_notice_email'; + }; + + $this->expectFactoryCallAndSendMailerMethod(ChangeEmailNoticeEmailFactory::class, $user); + + $this->assertTrue($this->userMailer->sendChangeEmailNoticeEmail($user)); + } + public static function afterWriteEmails(): iterable { yield 'welcome' => ['sendWelcomeEmail', WelcomeEmailFactory::class]; @@ -243,6 +255,7 @@ public static function afterWriteEmails(): iterable yield 'password changed' => ['sendPasswordChangedEmail', PasswordChangedEmailFactory::class]; yield 'email verify' => ['sendEmailVerifyEmailAfterWrite', VerifyEmailFactory::class]; yield 'change email confirmation' => ['sendChangeEmailConfirmationEmailAfterWrite', ChangeEmailConfirmationEmailFactory::class]; + yield 'change email notice' => ['sendChangeEmailNoticeEmail', ChangeEmailNoticeEmailFactory::class]; } public static function jobEmails(): iterable diff --git a/tests/Helper/User/UserWriteNotifierTest.php b/tests/Helper/User/UserWriteNotifierTest.php index 6be086a0..9acc973b 100644 --- a/tests/Helper/User/UserWriteNotifierTest.php +++ b/tests/Helper/User/UserWriteNotifierTest.php @@ -26,6 +26,7 @@ class UserWriteNotifierTest extends TestCase 'sendPasswordChangedEmail', 'sendEmailVerifyEmailAfterWrite', 'sendChangeEmailConfirmationEmailAfterWrite', + 'sendChangeEmailNoticeEmail', ]; public function test_a_new_user_gets_only_the_welcome_email(): void @@ -65,7 +66,7 @@ public static function changes(): iterable }, []]; yield 'new email change confirmation token' => [static function (User $user): void { $user->setNewEmailConfirmationToken('hashed'); - }, ['sendChangeEmailConfirmationEmailAfterWrite']]; + }, ['sendChangeEmailConfirmationEmailAfterWrite', 'sendChangeEmailNoticeEmail']]; yield 'enabled and renamed' => [static function (User $user): void { $user->setEnabled(true); $user->setUsername('renamed'); diff --git a/tests/Utility/MaskedEmailAddressTest.php b/tests/Utility/MaskedEmailAddressTest.php new file mode 100644 index 00000000..bc48f405 --- /dev/null +++ b/tests/Utility/MaskedEmailAddressTest.php @@ -0,0 +1,35 @@ + + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ + +namespace Silverback\ApiComponentsBundle\Tests\Utility; + +use PHPUnit\Framework\Attributes\DataProvider; +use PHPUnit\Framework\TestCase; +use Silverback\ApiComponentsBundle\Utility\MaskedEmailAddress; + +class MaskedEmailAddressTest extends TestCase +{ + #[DataProvider('addresses')] + public function test_an_address_keeps_only_its_first_character_and_its_domain(string $emailAddress, string $masked): void + { + self::assertSame($masked, MaskedEmailAddress::of($emailAddress)); + } + + public static function addresses(): iterable + { + yield 'an address' => ['new@example.com', 'n***@example.com']; + yield 'a one character local part' => ['n@example.com', 'n***@example.com']; + yield 'a quoted local part holding an at sign' => ['"a@b"@example.com', '"***@example.com']; + yield 'a multibyte first character' => ['élan@example.com', 'é***@example.com']; + yield 'no at sign' => ['nobody', 'n***']; + yield 'a multibyte value with no at sign' => ['éa', 'é***']; + } +} diff --git a/tests/Validator/Constraints/NewEmailAddressRequiresPasswordValidatorTest.php b/tests/Validator/Constraints/NewEmailAddressRequiresPasswordValidatorTest.php new file mode 100644 index 00000000..264cee72 --- /dev/null +++ b/tests/Validator/Constraints/NewEmailAddressRequiresPasswordValidatorTest.php @@ -0,0 +1,163 @@ + + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ + +namespace Silverback\ApiComponentsBundle\Tests\Validator\Constraints; + +use Doctrine\ORM\EntityManagerInterface; +use Doctrine\ORM\UnitOfWork; +use Doctrine\Persistence\ManagerRegistry; +use Doctrine\Persistence\ObjectManager; +use Silverback\ApiComponentsBundle\Tests\Functional\TestBundle\Entity\User; +use Silverback\ApiComponentsBundle\Validator\Constraints\NewEmailAddress; +use Silverback\ApiComponentsBundle\Validator\Constraints\NewEmailAddressRequiresPassword; +use Silverback\ApiComponentsBundle\Validator\Constraints\NewEmailAddressRequiresPasswordValidator; +use Symfony\Component\Security\Core\Validator\Constraints\UserPassword; +use Symfony\Component\Validator\ConstraintValidatorInterface; +use Symfony\Component\Validator\Exception\UnexpectedTypeException; +use Symfony\Component\Validator\Test\ConstraintValidatorTestCase; + +class NewEmailAddressRequiresPasswordValidatorTest extends ConstraintValidatorTestCase +{ + private ?ObjectManager $manager = null; + + protected function createValidator(): ConstraintValidatorInterface + { + $registry = $this->createStub(ManagerRegistry::class); + $registry->method('getManagerForClass')->willReturnCallback(fn (string $class): ?ObjectManager => User::class === $class ? $this->manager : null); + + return new NewEmailAddressRequiresPasswordValidator($registry); + } + + public function test_a_new_email_address_that_differs_from_the_stored_one_validates_the_current_password_in_the_constraints_groups(): void + { + $user = $this->user('new@example.com', 'my-password'); + $this->storeOriginalData($user, ['newEmailAddress' => 'old-pending@example.com']); + $constraint = new NewEmailAddressRequiresPassword(groups: ['User:emailAddress', 'Default']); + + $this->expectValidateValueAt(0, 'oldPassword', 'my-password', new UserPassword(message: 'You have not entered your current password correctly. Please try again.', groups: ['User:emailAddress', 'Default'])); + + $this->validator->validate($user, $constraint); + + $this->assertNoViolation(); + } + + public function test_the_constraints_message_is_the_password_violations_message(): void + { + $user = $this->user('new@example.com', null); + $this->storeOriginalData($user, ['newEmailAddress' => null]); + $constraint = new NewEmailAddressRequiresPassword(); + $constraint->message = 'Enter your password.'; + + $this->expectValidateValueAt(0, 'oldPassword', null, new UserPassword(message: 'Enter your password.', groups: ['Default'])); + + $this->validator->validate($user, $constraint); + } + + public function test_a_user_with_no_original_data_validates_the_current_password(): void + { + $user = $this->user('new@example.com', 'my-password'); + $this->storeOriginalData($user, []); + + $this->expectValidateValueAt(0, 'oldPassword', 'my-password', new UserPassword(message: 'You have not entered your current password correctly. Please try again.', groups: ['Default'])); + + $this->validator->validate($user, new NewEmailAddressRequiresPassword()); + } + + public function test_a_user_with_no_orm_manager_validates_the_current_password(): void + { + $user = $this->user('new@example.com', 'my-password'); + $this->manager = $this->createStub(ObjectManager::class); + + $this->expectValidateValueAt(0, 'oldPassword', 'my-password', new UserPassword(message: 'You have not entered your current password correctly. Please try again.', groups: ['Default'])); + + $this->validator->validate($user, new NewEmailAddressRequiresPassword()); + } + + public function test_a_stored_new_email_address_that_is_not_a_string_counts_as_none(): void + { + $user = $this->user('new@example.com', 'my-password'); + $this->storeOriginalData($user, ['newEmailAddress' => 5]); + + $this->expectValidateValueAt(0, 'oldPassword', 'my-password', new UserPassword(message: 'You have not entered your current password correctly. Please try again.', groups: ['Default'])); + + $this->validator->validate($user, new NewEmailAddressRequiresPassword()); + } + + public function test_an_unchanged_new_email_address_needs_no_password(): void + { + $user = $this->user('new@example.com', null); + $this->storeOriginalData($user, ['newEmailAddress' => 'new@example.com']); + + $this->expectNoValidate(); + + $this->validator->validate($user, new NewEmailAddressRequiresPassword()); + + $this->assertNoViolation(); + } + + public function test_cancelling_with_null_needs_no_password(): void + { + $user = $this->user(null, null); + $this->storeOriginalData($user, ['newEmailAddress' => 'new@example.com']); + + $this->expectNoValidate(); + + $this->validator->validate($user, new NewEmailAddressRequiresPassword()); + + $this->assertNoViolation(); + } + + public function test_cancelling_with_a_blank_string_needs_no_password(): void + { + $user = $this->user('', null); + $this->storeOriginalData($user, ['newEmailAddress' => 'new@example.com']); + + $this->expectNoValidate(); + + $this->validator->validate($user, new NewEmailAddressRequiresPassword()); + + $this->assertNoViolation(); + } + + public function test_another_constraint_is_refused(): void + { + $this->expectException(UnexpectedTypeException::class); + + $this->validator->validate($this->user('new@example.com', null), new NewEmailAddress()); + } + + public function test_a_value_that_is_not_a_user_is_refused(): void + { + $this->expectException(UnexpectedTypeException::class); + + $this->validator->validate(new \stdClass(), new NewEmailAddressRequiresPassword()); + } + + private function user(?string $newEmailAddress, ?string $oldPassword): User + { + $user = new User(); + $user->setNewEmailAddress($newEmailAddress)->setOldPassword($oldPassword); + + return $user; + } + + /** + * @param array $originalData + */ + private function storeOriginalData(User $user, array $originalData): void + { + $unitOfWork = $this->createStub(UnitOfWork::class); + $unitOfWork->method('getOriginalEntityData')->willReturnCallback(static fn (object $entity): array => $entity === $user ? $originalData : ['newEmailAddress' => 'new@example.com']); + $manager = $this->createStub(EntityManagerInterface::class); + $manager->method('getUnitOfWork')->willReturn($unitOfWork); + $this->manager = $manager; + } +}