Skip to content

Commit 4ecde67

Browse files
committed
Fixes for password resets and tests
1 parent 74d3278 commit 4ecde67

File tree

16 files changed

+141
-114
lines changed

16 files changed

+141
-114
lines changed

features/bootstrap/DoctrineContext.php

Lines changed: 5 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,7 @@
4242
use Silverback\ApiComponentsBundle\Form\Type\User\UserLoginType;
4343
use Silverback\ApiComponentsBundle\Form\Type\User\UserRegisterType;
4444
use Silverback\ApiComponentsBundle\Helper\Timestamped\TimestampedDataPersister;
45+
use Silverback\ApiComponentsBundle\Repository\User\UserRepositoryInterface;
4546
use Silverback\ApiComponentsBundle\Tests\Functional\TestBundle\Entity\DummyComponent;
4647
use Silverback\ApiComponentsBundle\Tests\Functional\TestBundle\Entity\DummyCustomTimestamped;
4748
use Silverback\ApiComponentsBundle\Tests\Functional\TestBundle\Entity\DummyPublishableComponent;
@@ -125,7 +126,8 @@ private function login(array $roles = [], $useAuthHeader = false): void
125126
$user = new User();
126127
$user
127128
->setRoles($roles)
128-
->setUsername('user@example.com')
129+
->setUsername('new_user')
130+
->setEmailAddress('user@example.com')
129131
->setPassword($this->passwordHasher->hashPassword($user, 'password'))
130132
->setEnabled(true)
131133
->setEmailAddressVerified(true);
@@ -919,13 +921,10 @@ public function theResourceShouldExist(string $name): void
919921
*/
920922
public function thePasswordShouldBeEqualTo(string $password, string $username): void
921923
{
924+
/** @var UserRepositoryInterface $repository */
922925
$repository = $this->manager->getRepository(User::class);
923926
/** @var AbstractUser $user */
924-
$user = $repository->findOneBy(
925-
[
926-
'username' => $username,
927-
]
928-
);
927+
$user = $repository->loadUserByIdentifier($username);
929928
Assert::assertTrue($this->passwordHasher->isPasswordValid($user, $password));
930929
}
931930

features/bootstrap/ProfilerContext.php

Lines changed: 7 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,7 @@
3434
use Symfony\Component\HttpClient\DataCollector\HttpClientDataCollector;
3535
use Symfony\Component\HttpKernel\Profiler\Profile as HttpProfile;
3636
use Symfony\Component\Mailer\DataCollector\MessageDataCollector;
37+
use Symfony\Component\Mailer\Event\MessageEvent;
3738
use Symfony\Component\Mercure\Update;
3839
use Symfony\Component\Mime\Header\Headers;
3940
use Symfony\Component\VarDumper\Cloner\Data;
@@ -209,16 +210,11 @@ public function iShouldGetAnEmail(string $emailType, string $emailAddress = 'use
209210
/** @var TemplatedEmail[] $messages */
210211
$messages = iterator_to_array($templatedEmailMessageEventSubscriber->getMessages());
211212

212-
// $events = $collector->getEvents()->getEvents();
213-
// /** @var TemplatedEmail[] $messages */
214-
// $messages = [];
215-
// foreach ($events as $event) {
216-
// if (!$event->isQueued()) {
217-
// $messages[] = $event->getMessage();
218-
// }
219-
// }
213+
$subjects = array_map(static function(TemplatedEmail $email) {
214+
return $email->getSubject();
215+
}, $messages);
220216

221-
Assert::assertCount(1, $messages);
217+
Assert::assertCount(1, $messages, sprintf("%d messages were sent but only 1 was expected. Messages were sent with subjects '%s'", count($messages), implode("', '", $subjects)));
222218
Assert::assertInstanceOf(TemplatedEmail::class, $email = $messages[0]);
223219

224220
/** @var TemplatedEmail $email */
@@ -294,7 +290,7 @@ private function validateChangeEmailVerification(array $context, Headers $header
294290
Assert::assertEquals('Please confirm your new email address', $headers->get('subject')->getBodyAsString());
295291
Assert::assertStringStartsWith(ChangeEmailConfirmationEmailFactory::MESSAGE_ID_PREFIX, $headers->get('x-message-id')->getBodyAsString());
296292
Assert::assertIsString($context['user']->getNewEmailConfirmationToken());
297-
Assert::assertRegExp('/^http:\/\/www.website.com\/' . $pathInsert . '\/user%40example.com\/new%40example.com\/([a-z0-9]+)$/i', $context['redirect_url']);
293+
Assert::assertRegExp('/^http:\/\/www.website.com\/' . $pathInsert . '\/new_user\/new%40example.com\/([a-z0-9]+)$/i', $context['redirect_url']);
298294
}
299295

300296
private function validateChangePasswordNotification(Headers $headers): void
@@ -307,7 +303,7 @@ private function validateUserWelcomeEmail(array $context, Headers $headers): voi
307303
{
308304
Assert::assertEquals('Welcome to New Website', $headers->get('subject')->getBodyAsString());
309305
Assert::assertStringStartsWith(WelcomeEmailFactory::MESSAGE_ID_PREFIX, $headers->get('x-message-id')->getBodyAsString());
310-
Assert::assertRegExp('/^http:\/\/www.website.com\/verify-email\/user%40example.com\/([a-z0-9]+)$/i', $context['redirect_url']);
306+
Assert::assertRegExp('/^http:\/\/www.website.com\/verify-email\/new_user\/([a-z0-9]+)$/i', $context['redirect_url']);
311307
}
312308

313309
private function validatePasswordReset(array $context, Headers $headers, bool $customPath = false): void

features/user/change_password_form.feature

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -89,5 +89,6 @@ Feature: Register process via a form
8989
"""
9090
Then the response status code should be 201
9191
And the JSON should be valid according to the schema file "user.schema.json"
92-
And the JSON node "username" should be equal to "user@example.com"
92+
And the JSON node "username" should be equal to "new_user"
93+
And the JSON node "emailAddress" should be equal to "user@example.com"
9394
And the password should be "new_password" for username "user@example.com"

features/user/forgot_password.feature

Lines changed: 34 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,9 @@ Feature: Forgot password system
2828
Given there is a "password_update" form
2929
When I send a "GET" request to the resource "password_update_form" and the postfix "<postfix>"
3030
Then the response status code should be 200
31+
And the JSON node "formView.children[0].vars.name" should be equal to "username"
3132
And the JSON node "formView.children[0].vars.value" should be equal to "<expectedUsername>"
33+
And the JSON node "formView.children[1].vars.name" should be equal to "plainNewPasswordConfirmationToken"
3234
And the JSON node "formView.children[1].vars.value" should be equal to "<expectedToken>"
3335
Examples:
3436
| postfix | expectedUsername | expectedToken |
@@ -44,17 +46,41 @@ Feature: Forgot password system
4446
{
4547
"password_update": {
4648
"username": "username",
47-
"newPasswordConfirmationToken": "abc123",
49+
"plainNewPasswordConfirmationToken": "abc123",
4850
"plainPassword": {
4951
"first": "mynewpassword",
5052
"second": "mynewpassword"
5153
}
5254
}
5355
}
5456
"""
55-
Then the response status code should be 200
57+
Then the response status code should be 201
5658
And I should get a "password_changed" email sent to the email address "test.user@example.com"
5759

60+
Scenario Outline: I cannot reset my password with an invalid token
61+
Given there is a "password_update" form
62+
And there is a user with the username "username" password "password" and role "ROLE_USER"
63+
And the user has the newPasswordConfirmationToken "abc123" requested at "now"
64+
When I send a "POST" request to the resource "password_update_form" and the postfix "/submit" with body:
65+
"""
66+
{
67+
"password_update": {
68+
"username": "username",
69+
"plainNewPasswordConfirmationToken": "INVALID",
70+
"plainPassword": {
71+
"first": "<password>",
72+
"second": "<password>"
73+
}
74+
}
75+
}
76+
"""
77+
Then the response status code should be <statusCode>
78+
And I should not receive any emails
79+
Examples:
80+
| password | statusCode |
81+
| a | 422 |
82+
| mynewpassword | 404 |
83+
5884
Scenario Outline: I cannot reset my password with invalid data
5985
Given there is a "password_update" form
6086
And there is a user with the username "username" password "password" and role "ROLE_USER"
@@ -64,7 +90,7 @@ Feature: Forgot password system
6490
{
6591
"password_update": {
6692
"username": "<username>",
67-
"newPasswordConfirmationToken": "<token>",
93+
"plainNewPasswordConfirmationToken": "<token>",
6894
"plainPassword": {
6995
"first": "mynewpassword",
7096
"second": "mynewpassword"
@@ -90,7 +116,7 @@ Feature: Forgot password system
90116
{
91117
"password_update": {
92118
"username": "username",
93-
"newPasswordConfirmationToken": "abc123",
119+
"plainNewPasswordConfirmationToken": "abc123",
94120
"plainPassword": {
95121
"first": "<passwordFirst>",
96122
"second": "<passwordSecond>"
@@ -110,18 +136,19 @@ Feature: Forgot password system
110136
Scenario: I can reset my password successfully without a specific password update form component being required
111137
Given there is a user with the username "username" password "password" and role "ROLE_USER"
112138
And the user has the newPasswordConfirmationToken "abc123" requested at "now"
113-
When I send a "PATCH" request to "/component/forms/password_reset/submit" with body:
139+
When I send a "POST" request to "/component/forms/password_reset/submit" with body:
114140
"""
115141
{
116142
"password_update": {
117143
"username": "username",
118-
"newPasswordConfirmationToken": "abc123",
144+
"plainNewPasswordConfirmationToken": "abc123",
119145
"plainPassword": {
120146
"first": "mynewpassword",
121147
"second": "mynewpassword"
122148
}
123149
}
124150
}
125151
"""
126-
Then the response status code should be 200
152+
Then the response status code should be 201
127153
And I should get a "password_changed" email sent to the email address "test.user@example.com"
154+
And the JSON should be valid according to the schema file "form.schema.json"

features/user/register_form.feature

Lines changed: 15 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,8 @@ Feature: Register process via a form
1515
"""
1616
{
1717
"user_register": {
18-
"username": "user@example.com",
18+
"username": "new_user",
19+
"emailAddress": "user@example.com",
1920
"plainPassword": {
2021
"first": "password",
2122
"second": "password"
@@ -31,7 +32,7 @@ Feature: Register process via a form
3132
{
3233
"@context": "/contexts/User",
3334
"@type": "User",
34-
"username": "user@example.com",
35+
"username": "new_user",
3536
"emailAddress": "user@example.com",
3637
"_metadata": {
3738
"persisted": true
@@ -41,14 +42,15 @@ Feature: Register process via a form
4142
And the JSON should be valid according to the schema file "user.schema.json"
4243
And I should get a "user_welcome" email sent
4344

44-
Scenario: Submit a duplicate user registration form
45+
Scenario Outline: Submit a duplicate user registration form
4546
Given there is a "register" form
46-
And there is a user with the username "user@email.com" password "password" and role "ROLE_USER"
47+
And there is a user with the username "new_user" password "password" and role "ROLE_USER" and the email address "user@example.com"
4748
When I send a "POST" request to the resource "register_form" and the postfix "/submit" with body:
4849
"""
4950
{
5051
"user_register": {
51-
"username": "user@email.com",
52+
"username": "<username>",
53+
"emailAddress": "<emailAddress>",
5254
"plainPassword": {
5355
"first": "pw",
5456
"second": "pw"
@@ -58,9 +60,13 @@ Feature: Register process via a form
5860
"""
5961
And the response status code should be 422
6062
And the response should be in JSON
61-
And the JSON node "formView.children[0].vars.errors[0]" should be equal to "Sorry, that user already exists in the database."
62-
And the JSON node "formView.children[1].children[0].vars.errors[0]" should be equal to "Your password must be more than 6 characters long."
63+
And the JSON node "<errorPath>" should be equal to "<errorMessage>"
64+
And the JSON node "formView.children[2].children[0].vars.errors[0]" should be equal to "Your password must be more than 6 characters long."
6365
And the JSON should be valid according to the schema file "form.schema.json"
66+
Examples:
67+
| username | emailAddress | errorPath | errorMessage |
68+
| new_user | different@email.com | formView.children[0].vars.errors[0] | Sorry, that user already exists in the database. |
69+
| different_user | user@example.com | formView.children[1].vars.errors[0] | Sorry, that email address already exists in the database. |
6470

6571
Scenario: Submit an invalid user registration form
6672
Given there is a "register" form
@@ -76,5 +82,6 @@ Feature: Register process via a form
7682
And the response status code should be 422
7783
And the response should be in JSON
7884
And the JSON node "formView.children[0].vars.errors[0]" should be equal to "Please enter a username."
79-
And the JSON node "formView.children[1].children[0].vars.errors[0]" should be equal to "Please enter your desired password."
85+
And the JSON node "formView.children[1].vars.errors[0]" should be equal to "Please enter your email address."
86+
And the JSON node "formView.children[2].children[0].vars.errors[0]" should be equal to "Please enter your desired password."
8087
And the JSON should be valid according to the schema file "form.schema.json"

src/Entity/User/AbstractUser.php

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -39,11 +39,11 @@ abstract class AbstractUser implements SymfonyUserInterface, PasswordAuthenticat
3939
use IdTrait;
4040
use TimestampedTrait;
4141

42-
#[Assert\NotBlank(message: 'Please enter a username.', groups: ['Default'])]
42+
#[Assert\NotBlank(message: 'Please enter a username.', allowNull: false, groups: ['Default'])]
4343
#[Groups(['User:superAdmin', 'User:output', 'Form:cwa_resource:read'])]
4444
protected ?string $username;
4545

46-
#[Assert\NotBlank(groups: ['Default'])]
46+
#[Assert\NotBlank(message: 'Please enter your email address.', allowNull: false, groups: ['Default'])]
4747
#[Assert\Email]
4848
#[Groups(['User:superAdmin', 'User:output', 'Form:cwa_resource:read'])]
4949
protected ?string $emailAddress;

src/EventListener/Form/EntityPersistFormListener.php

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@
2121
use Silverback\ApiComponentsBundle\Exception\InvalidArgumentException;
2222
use Silverback\ApiComponentsBundle\Helper\Timestamped\TimestampedDataPersister;
2323
use Silverback\ApiComponentsBundle\Helper\User\UserDataProcessor;
24+
use Silverback\ApiComponentsBundle\Serializer\Normalizer\UserNormalizer;
2425
use Silverback\ApiComponentsBundle\Utility\ClassMetadataTrait;
2526
use Symfony\Component\Serializer\Normalizer\DenormalizerInterface;
2627
use Symfony\Component\Serializer\Normalizer\NormalizerInterface;
@@ -95,7 +96,9 @@ public function __invoke(FormSuccessEvent $event): void
9596
if (\count($oldData)) {
9697
$normalized = $this->normalizer->normalize($oldData);
9798
/** @var AbstractUser $oldUser */
98-
$oldUser = $this->normalizer->denormalize($normalized, \get_class($data));
99+
$oldUser = $this->normalizer->denormalize($normalized, \get_class($data), null, [
100+
UserNormalizer::ALREADY_CALLED => true
101+
]);
99102
}
100103

101104
$this->userDataProcessor->processChanges($data, $oldUser);

src/EventListener/Form/User/PasswordUpdateListener.php

Lines changed: 8 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,9 @@
1515

1616
use Silverback\ApiComponentsBundle\Entity\User\AbstractUser;
1717
use Silverback\ApiComponentsBundle\Event\FormSuccessEvent;
18+
use Silverback\ApiComponentsBundle\EventListener\Form\EntityPersistFormListener;
1819
use Silverback\ApiComponentsBundle\EventListener\Form\FormSuccessEventListenerInterface;
20+
use Silverback\ApiComponentsBundle\Form\Type\User\ChangePasswordType;
1921
use Silverback\ApiComponentsBundle\Form\Type\User\PasswordUpdateType;
2022
use Silverback\ApiComponentsBundle\Helper\User\UserDataProcessor;
2123
use Silverback\ApiComponentsBundle\Helper\User\UserMailer;
@@ -24,15 +26,10 @@
2426
/**
2527
* @author Daniel West <daniel@silverback.is>
2628
*/
27-
class PasswordUpdateListener implements FormSuccessEventListenerInterface
28-
{
29-
private UserDataProcessor $userDataProcessor;
30-
private UserMailer $userMailer;
31-
32-
public function __construct(UserDataProcessor $userDataProcessor, UserMailer $userMailer)
29+
class PasswordUpdateListener extends EntityPersistFormListener {
30+
public function __construct(private readonly UserDataProcessor $userDataProcessor)
3331
{
34-
$this->userDataProcessor = $userDataProcessor;
35-
$this->userMailer = $userMailer;
32+
parent::__construct(PasswordUpdateType::class, AbstractUser::class, false);
3633
}
3734

3835
public function __invoke(FormSuccessEvent $event): void
@@ -45,14 +42,13 @@ public function __invoke(FormSuccessEvent $event): void
4542
return;
4643
}
4744

48-
$user = $this->userDataProcessor->passwordReset((string) $formDataUser->getUsername(), (string) $formDataUser->getNewPasswordConfirmationToken(), (string) $formDataUser->getPlainPassword());
45+
$user = $this->userDataProcessor->passwordReset($formDataUser->getUsername(), (string) $formDataUser->plainNewPasswordConfirmationToken, (string) $formDataUser->getPlainPassword());
46+
4947
if (!$user) {
5048
$event->result = new Response(null, Response::HTTP_NOT_FOUND);
51-
5249
return;
5350
}
5451

55-
$this->userMailer->sendPasswordChangedEmail($user);
56-
$event->result = new Response(null, Response::HTTP_OK);
52+
parent::__invoke($event);
5753
}
5854
}

src/Factory/Form/FormViewFactory.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,7 @@ public function create(Form $form): FormView
3939
{
4040
$builder = $this->formFactory->createBuilder($form->formType);
4141

42-
if (!($currentAction = $builder->getAction()) || '' === $currentAction) {
42+
if (!$builder->getAction()) {
4343
$builder->setAction($this->getFormAction($form));
4444
}
4545

0 commit comments

Comments
 (0)