From 2bc192a69c638605a4a6ac07e7140554d2e31407 Mon Sep 17 00:00:00 2001 From: Jesse Rushlow Date: Tue, 5 Mar 2024 03:34:07 -0500 Subject: [PATCH] require a user object --- README.md | 42 +++++++++++++++---- .../FakeResetPasswordInternalRepository.php | 2 +- .../ResetPasswordRequestRepositoryTrait.php | 16 ++----- ...esetPasswordRequestRepositoryInterface.php | 2 +- .../ResetPasswordRequestRepositoryTest.php | 12 ------ ...akeResetPasswordInternalRepositoryTest.php | 2 +- 6 files changed, 42 insertions(+), 34 deletions(-) diff --git a/README.md b/README.md index 20415c6c..4117f738 100644 --- a/README.md +++ b/README.md @@ -112,21 +112,49 @@ requests that may have been left in persistence. The `ResetPasswordRequestRepositoryInterface::removeRequests()` method, which is implemented in the [ResetPasswordRequestRepositoryTrait](https://github.com/SymfonyCasts/reset-password-bundle/blob/main/src/Persistence/Repository/ResetPasswordRequestRepositoryTrait.php), -can be used to remove request objects from persistence for a single user or all -users. This differs from the +can be used to remove all request objects from persistence for a single user. This +differs from the [garbage collection mechanism](https://github.com/SymfonyCasts/reset-password-bundle/blob/df64d82cca2ee371da5e8c03c227457069ae663e/src/Persistence/Repository/ResetPasswordRequestRepositoryTrait.php#L73) which only removes _expired_ request objects for _all_ users automatically. Typically, you'd call this method when you need to remove request object(s) for a user who changed their email address due to suspicious activity and potentially has valid request objects in persistence with their "old" compromised email address. -In such cases, pass the `user` object to `removeRequest()`. - -Passing `null` or omitting a `user` object will remove all request objects from -persistence - even if any of those requests have not expired. This method relies on the user objects `primary key` being `immutable`. -E.g. `User::id = UUID` not something like `User::id = 'john@example.com'` +E.g. `User::id = UUID` not something like `User::id = 'john@example.com'`. + +```php +// ProfileController + +#[Route(path: '/profile/{id}', name: 'app_update_profile', methods: ['GET', 'POST'])] +public function profile(Request $request, User $user, ResetPasswordRequestRepositoryInterface $repository): Response +{ + $form = $this->createFormBuilder($user) + ->add('email', EmailType::class) + ->add('save', SubmitType::class, ['label' => 'Save Profile']) + ->getForm() + ; + + $form->handleRequest($request); + + if ($form->isSubmitted() && $form->isValid()) { + $newEmail = $form->get('email')->getData(); + + if ($newEmail !== $user->getEmail()) { + // The user changed their email address. + // Remove any old reset requests for the user. + $repository->removeRequests($user); + } + + // Persist the user object... + + return $this->render('success.html.twig'); + } + + return $this->render('profile.html.twig', ['form' => $form]); +} +``` ## Support diff --git a/src/Persistence/Fake/FakeResetPasswordInternalRepository.php b/src/Persistence/Fake/FakeResetPasswordInternalRepository.php index adf6b773..ec9d465e 100644 --- a/src/Persistence/Fake/FakeResetPasswordInternalRepository.php +++ b/src/Persistence/Fake/FakeResetPasswordInternalRepository.php @@ -61,7 +61,7 @@ public function removeExpiredResetPasswordRequests(): int throw new FakeRepositoryException(); } - public function removeRequests(?object $user = null): void + public function removeRequests(object $user): void { throw new FakeRepositoryException(); } diff --git a/src/Persistence/Repository/ResetPasswordRequestRepositoryTrait.php b/src/Persistence/Repository/ResetPasswordRequestRepositoryTrait.php index 87e5d60f..f446f5cf 100644 --- a/src/Persistence/Repository/ResetPasswordRequestRepositoryTrait.php +++ b/src/Persistence/Repository/ResetPasswordRequestRepositoryTrait.php @@ -84,30 +84,22 @@ public function removeExpiredResetPasswordRequests(): int } /** - * Remove ResetPasswordRequest objects from persistence. + * Remove a users ResetPasswordRequest objects from persistence. * * Warning - This is a destructive operation. Calling this method * may have undesired consequences for users who have valid * ResetPasswordRequests but have not "checked their email" yet. * - * If a "user" object is passed, only the requests for that user - * will be removed. - * * @see https://github.com/SymfonyCasts/reset-password-bundle?tab=readme-ov-file#advanced-usage */ - public function removeRequests(?object $user = null): void + public function removeRequests(object $user): void { $query = $this->createQueryBuilder('t') ->delete() + ->where('t.user = :user') + ->setParameter('user', $user) ; - if (null !== $user) { - $query - ->where('t.user = :user') - ->setParameter('user', $user) - ; - } - $query->getQuery()->execute(); } } diff --git a/src/Persistence/ResetPasswordRequestRepositoryInterface.php b/src/Persistence/ResetPasswordRequestRepositoryInterface.php index 173c27cd..ede76311 100644 --- a/src/Persistence/ResetPasswordRequestRepositoryInterface.php +++ b/src/Persistence/ResetPasswordRequestRepositoryInterface.php @@ -15,7 +15,7 @@ * @author Jesse Rushlow * @author Ryan Weaver * - * @method void removeRequests(?object $user = null) Remove ResetPasswordRequest objects from persistence. + * @method void removeRequests(object $user) Remove a users ResetPasswordRequest objects from persistence. */ interface ResetPasswordRequestRepositoryInterface { diff --git a/tests/FunctionalTests/Persistence/ResetPasswordRequestRepositoryTest.php b/tests/FunctionalTests/Persistence/ResetPasswordRequestRepositoryTest.php index 312f2443..1cbe9314 100644 --- a/tests/FunctionalTests/Persistence/ResetPasswordRequestRepositoryTest.php +++ b/tests/FunctionalTests/Persistence/ResetPasswordRequestRepositoryTest.php @@ -208,18 +208,6 @@ public function testRemovedExpiredResetPasswordRequestsOnlyRemovedExpiredRequest self::assertSame($futureFixture, $result[0]); } - public function testRemoveRequestsRemovesAllRequestsFromPersistence(): void - { - $this->manager->persist(new ResetPasswordTestFixtureRequest()); - $this->manager->persist(new ResetPasswordTestFixtureRequest()); - - $this->manager->flush(); - - $this->repository->removeRequests(); - - self::assertCount(0, $this->repository->findAll()); - } - public function testRemoveRequestsRemovesAllRequestsForASingleUser(): void { $this->manager->persist($userFixture = new ResetPasswordTestFixtureUser()); diff --git a/tests/UnitTests/Persistence/FakeResetPasswordInternalRepositoryTest.php b/tests/UnitTests/Persistence/FakeResetPasswordInternalRepositoryTest.php index 5d2a71cb..0246202f 100644 --- a/tests/UnitTests/Persistence/FakeResetPasswordInternalRepositoryTest.php +++ b/tests/UnitTests/Persistence/FakeResetPasswordInternalRepositoryTest.php @@ -28,7 +28,7 @@ public function methodDataProvider(): \Generator yield ['findResetPasswordRequest', ['']]; yield ['getMostRecentNonExpiredRequestDate', [new \stdClass()]]; yield ['removeResetPasswordRequest', [$this->createMock(ResetPasswordRequestInterface::class)]]; - yield ['removeRequests', []]; + yield ['removeRequests', [new \stdClass()]]; } /**