From c0d220827ae1136d44b4ff7cc16408d6ad8b8061 Mon Sep 17 00:00:00 2001 From: David Date: Wed, 28 Nov 2018 13:17:46 +0100 Subject: [PATCH] Forgot password with username or email or another property (#32) * reset password with username * resolve discussions on reset password with username * add unit tests on reset password with username * update configuration * return after resetPassword * resolve discussions * Fix BC Break * Fix code review * Fix tests * Fix code review * PHP-CS-Fixer * Fix phpunit test * Fix tests * Fix tests * Fix Code review * Fix Conflicts with master * use strict mode for in_array --- Controller/ForgotPasswordController.php | 7 +-- DependencyInjection/Configuration.php | 6 +++ .../CoopTilleulsForgotPasswordExtension.php | 4 +- EventListener/RequestEventListener.php | 40 +++++++++++++---- Exception/InvalidJsonHttpException.php | 22 ++++++++++ Exception/NoParameterException.php | 22 ++++++++++ Exception/UnauthorizedFieldException.php | 22 ++++++++++ Manager/ForgotPasswordManager.php | 13 +++--- Resources/config/services.xml | 4 +- features/app/AppKernel.php | 3 ++ features/app/TestBundle/Entity/User.php | 31 +++++++++---- features/bootstrap/FeatureContext.php | 16 ++++--- features/forgotPassword.feature | 20 ++++++--- .../ForgotPasswordControllerTest.php | 4 +- .../RequestEventListenerTest.php | 43 ++++++++++++++++++- tests/Manager/ForgotPasswordManagerTest.php | 11 +++-- 16 files changed, 216 insertions(+), 52 deletions(-) mode change 100644 => 100755 EventListener/RequestEventListener.php create mode 100644 Exception/InvalidJsonHttpException.php create mode 100644 Exception/NoParameterException.php create mode 100644 Exception/UnauthorizedFieldException.php diff --git a/Controller/ForgotPasswordController.php b/Controller/ForgotPasswordController.php index e8b7a7f..33e3f90 100644 --- a/Controller/ForgotPasswordController.php +++ b/Controller/ForgotPasswordController.php @@ -42,13 +42,14 @@ public function __construct( } /** - * @param string $email + * @param string $propertyName + * @param string $value * * @return Response */ - public function resetPasswordAction($email) + public function resetPasswordAction($propertyName, $value) { - $this->forgotPasswordManager->resetPassword($email); + $this->forgotPasswordManager->resetPassword($propertyName, $value); return new Response('', 204); } diff --git a/DependencyInjection/Configuration.php b/DependencyInjection/Configuration.php index 9959f75..dadb946 100644 --- a/DependencyInjection/Configuration.php +++ b/DependencyInjection/Configuration.php @@ -69,6 +69,12 @@ public function getConfigTreeBuilder() ->children() ->scalarNode('class')->cannotBeEmpty()->isRequired()->info('User class.')->end() ->scalarNode('email_field')->defaultValue('email')->cannotBeEmpty()->info('User email field name to retrieve it (email, username...).')->end() + ->arrayNode('authorized_fields') + ->defaultValue(['email']) + ->requiresAtLeastOneElement() + ->info('User fields names to retrieve it (email, username...).') + ->prototype('scalar')->end() + ->end() ->scalarNode('password_field')->defaultValue('password')->cannotBeEmpty()->info('User password field name.')->end() ->end() ->end() diff --git a/DependencyInjection/CoopTilleulsForgotPasswordExtension.php b/DependencyInjection/CoopTilleulsForgotPasswordExtension.php index 80f4691..0c70c4f 100644 --- a/DependencyInjection/CoopTilleulsForgotPasswordExtension.php +++ b/DependencyInjection/CoopTilleulsForgotPasswordExtension.php @@ -40,7 +40,9 @@ public function load(array $configs, ContainerBuilder $container) $container->setParameter('coop_tilleuls_forgot_password.password_token_serialization_groups', $config['password_token']['serialization_groups']); $container->setParameter('coop_tilleuls_forgot_password.user_class', $config['user']['class']); - $container->setParameter('coop_tilleuls_forgot_password.user_email_field', $config['user']['email_field']); + $config['user']['authorized_fields'] = array_unique(array_merge($config['user']['authorized_fields'], [$config['user']['email_field']])); + unset($config['user']['email_field']); + $container->setParameter('coop_tilleuls_forgot_password.user_authorized_fields', $config['user']['authorized_fields']); $container->setParameter('coop_tilleuls_forgot_password.user_password_field', $config['user']['password_field']); $loader = new XmlFileLoader($container, new FileLocator(__DIR__.'/../Resources/config')); diff --git a/EventListener/RequestEventListener.php b/EventListener/RequestEventListener.php old mode 100644 new mode 100755 index f812ae5..0fc64fc --- a/EventListener/RequestEventListener.php +++ b/EventListener/RequestEventListener.php @@ -11,7 +11,10 @@ namespace CoopTilleuls\ForgotPasswordBundle\EventListener; +use CoopTilleuls\ForgotPasswordBundle\Exception\InvalidJsonHttpException; use CoopTilleuls\ForgotPasswordBundle\Exception\MissingFieldHttpException; +use CoopTilleuls\ForgotPasswordBundle\Exception\NoParameterException; +use CoopTilleuls\ForgotPasswordBundle\Exception\UnauthorizedFieldException; use CoopTilleuls\ForgotPasswordBundle\Manager\PasswordTokenManager; use Symfony\Component\HttpKernel\Event\GetResponseEvent; use Symfony\Component\HttpKernel\Exception\NotFoundHttpException; @@ -21,21 +24,21 @@ */ final class RequestEventListener { - private $userEmailField; + private $authorizedFields; private $userPasswordField; private $passwordTokenManager; /** - * @param string $userEmailField + * @param array $authorizedFields * @param string $userPasswordField * @param PasswordTokenManager $passwordTokenManager */ public function __construct( - $userEmailField, + array $authorizedFields, $userPasswordField, PasswordTokenManager $passwordTokenManager ) { - $this->userEmailField = $userEmailField; + $this->authorizedFields = $authorizedFields; $this->userPasswordField = $userPasswordField; $this->passwordTokenManager = $passwordTokenManager; } @@ -52,12 +55,33 @@ public function decodeRequest(GetResponseEvent $event) return; } - $data = json_decode($request->getContent(), true); - $fieldName = 'coop_tilleuls_forgot_password.reset' === $routeName ? $this->userEmailField : $this->userPasswordField; - if (!isset($data[$fieldName]) || empty($data[$fieldName])) { + $content = $request->getContent(); + $data = json_decode($content, true); + if (!empty($content) && JSON_ERROR_NONE !== json_last_error()) { + throw new InvalidJsonHttpException(); + } + if (!is_array($data) || empty($data)) { + throw new NoParameterException(); + } + + $fieldName = key($data); + if (empty($data[$fieldName])) { throw new MissingFieldHttpException($fieldName); } - $request->attributes->set($fieldName, $data[$fieldName]); + + if ('coop_tilleuls_forgot_password.reset' === $routeName) { + if (!in_array($fieldName, $this->authorizedFields, true)) { + throw new UnauthorizedFieldException($fieldName); + } + $request->attributes->set('propertyName', $fieldName); + $request->attributes->set('value', $data[$fieldName]); + } else { + if ($this->userPasswordField !== $fieldName) { + throw new MissingFieldHttpException($this->userPasswordField); + } + + $request->attributes->set($fieldName, $data[$fieldName]); + } } public function getTokenFromRequest(GetResponseEvent $event) diff --git a/Exception/InvalidJsonHttpException.php b/Exception/InvalidJsonHttpException.php new file mode 100644 index 0000000..442320a --- /dev/null +++ b/Exception/InvalidJsonHttpException.php @@ -0,0 +1,22 @@ + + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ + +namespace CoopTilleuls\ForgotPasswordBundle\Exception; + +use Symfony\Component\HttpKernel\Exception\HttpException; + +final class InvalidJsonHttpException extends HttpException implements JsonHttpExceptionInterface +{ + public function __construct() + { + parent::__construct(400, 'Invalid JSON data.'); + } +} diff --git a/Exception/NoParameterException.php b/Exception/NoParameterException.php new file mode 100644 index 0000000..a9f07ee --- /dev/null +++ b/Exception/NoParameterException.php @@ -0,0 +1,22 @@ + + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ + +namespace CoopTilleuls\ForgotPasswordBundle\Exception; + +use Symfony\Component\HttpKernel\Exception\HttpException; + +final class NoParameterException extends HttpException implements JsonHttpExceptionInterface +{ + public function __construct() + { + parent::__construct(400, 'No parameter sent.'); + } +} diff --git a/Exception/UnauthorizedFieldException.php b/Exception/UnauthorizedFieldException.php new file mode 100644 index 0000000..198e937 --- /dev/null +++ b/Exception/UnauthorizedFieldException.php @@ -0,0 +1,22 @@ + + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ + +namespace CoopTilleuls\ForgotPasswordBundle\Exception; + +use Symfony\Component\HttpKernel\Exception\HttpException; + +final class UnauthorizedFieldException extends HttpException implements JsonHttpExceptionInterface +{ + public function __construct($propertyName) + { + parent::__construct(400, sprintf('The parameter "%s" is not authorized in your configuration.', $propertyName)); + } +} diff --git a/Manager/ForgotPasswordManager.php b/Manager/ForgotPasswordManager.php index 5c064ce..07afe63 100644 --- a/Manager/ForgotPasswordManager.php +++ b/Manager/ForgotPasswordManager.php @@ -25,35 +25,32 @@ class ForgotPasswordManager private $passwordTokenManager; private $dispatcher; private $userClass; - private $userEmailField; /** * @param PasswordTokenManager $passwordTokenManager * @param EventDispatcherInterface $dispatcher * @param ManagerInterface $manager * @param string $userClass - * @param string $userEmailField */ public function __construct( PasswordTokenManager $passwordTokenManager, EventDispatcherInterface $dispatcher, ManagerInterface $manager, - $userClass, - $userEmailField + $userClass ) { $this->passwordTokenManager = $passwordTokenManager; $this->dispatcher = $dispatcher; $this->manager = $manager; $this->userClass = $userClass; - $this->userEmailField = $userEmailField; } /** - * @param string $username + * @param $propertyName + * @param $value */ - public function resetPassword($username) + public function resetPassword($propertyName, $value) { - $user = $this->manager->findOneBy($this->userClass, [$this->userEmailField => $username]); + $user = $this->manager->findOneBy($this->userClass, [$propertyName => $value]); if (null === $user) { return; } diff --git a/Resources/config/services.xml b/Resources/config/services.xml index 144c801..cfaa8af 100644 --- a/Resources/config/services.xml +++ b/Resources/config/services.xml @@ -3,7 +3,6 @@ xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:schemaLocation="http://symfony.com/schema/dic/services http://symfony.com/schema/dic/services/services-1.0.xsd"> - @@ -16,7 +15,6 @@ %coop_tilleuls_forgot_password.user_class% - %coop_tilleuls_forgot_password.user_email_field% @@ -31,7 +29,7 @@ - %coop_tilleuls_forgot_password.user_email_field% + %coop_tilleuls_forgot_password.user_authorized_fields% %coop_tilleuls_forgot_password.user_password_field% diff --git a/features/app/AppKernel.php b/features/app/AppKernel.php index c6bf8fb..b233d9f 100644 --- a/features/app/AppKernel.php +++ b/features/app/AppKernel.php @@ -55,6 +55,9 @@ protected function configureContainer(ContainerBuilder $c, LoaderInterface $load $c->loadFromExtension('coop_tilleuls_forgot_password', [ 'password_token_class' => PasswordToken::class, 'user_class' => User::class, + 'user' => [ + 'authorized_fields' => ['email', 'username'], + ], 'use_jms_serializer' => 'jmsserializer' === $this->getEnvironment(), ]); diff --git a/features/app/TestBundle/Entity/User.php b/features/app/TestBundle/Entity/User.php index 3da2a7a..4fabd26 100644 --- a/features/app/TestBundle/Entity/User.php +++ b/features/app/TestBundle/Entity/User.php @@ -37,6 +37,13 @@ final class User implements UserInterface */ private $email; + /** + * @var string + * + * @ORM\Column(type="string") + */ + private $username; + /** * @var string * @@ -52,14 +59,6 @@ public function getId() return $this->id; } - /** - * {@inheritdoc} - */ - public function getUsername() - { - return $this->getEmail(); - } - /** * {@inheritdoc} */ @@ -76,6 +75,22 @@ public function setEmail($email) $this->email = $email; } + /** + * @return string + */ + public function getUsername() + { + return $this->username; + } + + /** + * @param string $username + */ + public function setUsername($username) + { + $this->username = $username; + } + /** * {@inheritdoc} */ diff --git a/features/bootstrap/FeatureContext.php b/features/bootstrap/FeatureContext.php index b00c07f..56122ba 100644 --- a/features/bootstrap/FeatureContext.php +++ b/features/bootstrap/FeatureContext.php @@ -85,8 +85,12 @@ public function iHaveAnExpiredToken() /** * @Then I reset my password + * @Then I reset my password with my :propertyName ":value" + * + * @param string $propertyName + * @param string $value */ - public function iResetMyPassword() + public function IResetMyPassword($propertyName = 'email', $value = 'john.doe@example.com') { $this->createUser(); @@ -97,11 +101,12 @@ public function iResetMyPassword() [], [], ['CONTENT_TYPE' => 'application/json'], - <<<'JSON' + sprintf(<<<'JSON' { - "email": "john.doe@example.com" + "%s": "%s" } JSON + , $propertyName, $value) ); } @@ -194,9 +199,9 @@ public function iResetMyPasswordUsingInvalidEmailAddress() } /** - * @Then I reset my password using no email address + * @Then I reset my password using no parameter */ - public function iResetMyPasswordUsingNoEmailAddress() + public function iResetMyPasswordUsingNoParameter() { $this->client->request('POST', '/forgot_password/'); } @@ -314,6 +319,7 @@ private function createUser() { $user = new User(); $user->setEmail('john.doe@example.com'); + $user->setUsername('JohnDoe'); $user->setPassword('password'); $this->doctrine->getManager()->persist($user); $this->doctrine->getManager()->flush(); diff --git a/features/forgotPassword.feature b/features/forgotPassword.feature index 1f81b09..c72f0fa 100644 --- a/features/forgotPassword.feature +++ b/features/forgotPassword.feature @@ -1,8 +1,16 @@ Feature: I need to be able to reset my password - Scenario: I can reset my password - When I reset my password + Scenario Outline: I can reset my password + When I reset my password with my "" Then I should receive an email + Examples: + | property | value | + | username | JohnDoe | + | email | john.doe@example.com | + + Scenario: I can't reset my password with an authorized field + When I reset my password with my id "1" + Then the request should be invalid with message 'The parameter "id" is not authorized in your configuration.' Scenario: I can reset my password even if I have already requested a token and this token has not expired yet Given I have a valid token @@ -18,9 +26,9 @@ Feature: I need to be able to reset my password When I reset my password using invalid email address Then the response should be empty - Scenario: I can't reset my password with no email address specified - When I reset my password using no email address - Then the request should be invalid with message 'Parameter "email" is missing.' + Scenario: I can't reset my password with no parameter specified + When I reset my password using no parameter + Then the request should be invalid with message 'No parameter sent.' Scenario: I can't update my password using an invalid token When I update my password using an invalid token @@ -32,7 +40,7 @@ Feature: I need to be able to reset my password Scenario: I can't update my password with no password specified When I update my password using no password - Then the request should be invalid with message 'Parameter "password" is missing.' + Then the request should be invalid with message 'No parameter sent.' Scenario: I can get a password token When I get a password token diff --git a/tests/Controller/ForgotPasswordControllerTest.php b/tests/Controller/ForgotPasswordControllerTest.php index 0dd4e99..26df028 100644 --- a/tests/Controller/ForgotPasswordControllerTest.php +++ b/tests/Controller/ForgotPasswordControllerTest.php @@ -46,8 +46,8 @@ protected function setUp() public function testResetPasswordAction() { - $this->managerMock->resetPassword('foo@example.com')->shouldBeCalledTimes(1); - $response = $this->controller->resetPasswordAction('foo@example.com'); + $this->managerMock->resetPassword('email', 'foo@example.com')->shouldBeCalledTimes(1); + $response = $this->controller->resetPasswordAction('email', 'foo@example.com'); $this->assertInstanceOf(Response::class, $response); $this->assertEquals('', $response->getContent()); $this->assertEquals(204, $response->getStatusCode()); diff --git a/tests/EventListener/RequestEventListenerTest.php b/tests/EventListener/RequestEventListenerTest.php index 14951af..d0f20b0 100644 --- a/tests/EventListener/RequestEventListenerTest.php +++ b/tests/EventListener/RequestEventListenerTest.php @@ -43,7 +43,7 @@ protected function setUp() $this->requestMock->attributes = $this->parameterBagMock->reveal(); $this->listener = new RequestEventListener( - 'email', + ['email', 'username'], 'password', $this->managerMock->reveal() ); @@ -66,7 +66,46 @@ public function testDecodeRequestMissingFieldException() { $this->parameterBagMock->get('_route')->willReturn('coop_tilleuls_forgot_password.update')->shouldBeCalledTimes(1); $this->eventMock->isMasterRequest()->willReturn(true)->shouldBeCalledTimes(1); - $this->requestMock->getContent()->willReturn(json_encode(['foo' => 'bar']))->shouldBeCalledTimes(1); + $this->requestMock->getContent()->willReturn(json_encode(['password' => '']))->shouldBeCalledTimes(1); + + $this->listener->decodeRequest($this->eventMock->reveal()); + } + + /** + * @expectedException \CoopTilleuls\ForgotPasswordBundle\Exception\NoParameterException + * @expectedExceptionMessage No parameter sent. + */ + public function testDecodeRequestNoParametersException() + { + $this->parameterBagMock->get('_route')->willReturn('coop_tilleuls_forgot_password.update')->shouldBeCalledTimes(1); + $this->eventMock->isMasterRequest()->willReturn(true)->shouldBeCalledTimes(1); + $this->requestMock->getContent()->willReturn('{}')->shouldBeCalledTimes(1); + + $this->listener->decodeRequest($this->eventMock->reveal()); + } + + /** + * @expectedException \CoopTilleuls\ForgotPasswordBundle\Exception\InvalidJsonHttpException + * @expectedExceptionMessage Invalid JSON data. + */ + public function testDecodeRequestInvalidJsonHttpException() + { + $this->parameterBagMock->get('_route')->willReturn('coop_tilleuls_forgot_password.update')->shouldBeCalledTimes(1); + $this->eventMock->isMasterRequest()->willReturn(true)->shouldBeCalledTimes(1); + $this->requestMock->getContent()->willReturn('{')->shouldBeCalledTimes(1); + + $this->listener->decodeRequest($this->eventMock->reveal()); + } + + /** + * @expectedException \CoopTilleuls\ForgotPasswordBundle\Exception\UnauthorizedFieldException + * @expectedExceptionMessage The parameter "name" is not authorized in your configuration. + */ + public function testDecodeRequestUnauthorizedException() + { + $this->parameterBagMock->get('_route')->willReturn('coop_tilleuls_forgot_password.reset')->shouldBeCalledTimes(1); + $this->eventMock->isMasterRequest()->willReturn(true)->shouldBeCalledTimes(1); + $this->requestMock->getContent()->willReturn(json_encode(['name' => 'foo']))->shouldBeCalledTimes(1); $this->listener->decodeRequest($this->eventMock->reveal()); } diff --git a/tests/Manager/ForgotPasswordManagerTest.php b/tests/Manager/ForgotPasswordManagerTest.php index a423ff4..7a169e7 100644 --- a/tests/Manager/ForgotPasswordManagerTest.php +++ b/tests/Manager/ForgotPasswordManagerTest.php @@ -49,8 +49,7 @@ protected function setUp() $this->passwordManagerMock->reveal(), $this->eventDispatcherMock->reveal(), $this->managerMock->reveal(), - 'App\Entity\User', - 'email' + 'App\Entity\User' ); } @@ -59,7 +58,7 @@ public function testResetPasswordNotUser() $this->managerMock->findOneBy('App\Entity\User', ['email' => 'foo@example.com'])->shouldBeCalledTimes(1); $this->passwordManagerMock->findOneByUser(Argument::any())->shouldNotBeCalled(); - $this->manager->resetPassword('foo@example.com'); + $this->manager->resetPassword('email', 'foo@example.com'); } public function testResetPasswordWithNoPreviousToken() @@ -73,7 +72,7 @@ public function testResetPasswordWithNoPreviousToken() return $event instanceof ForgotPasswordEvent && is_null($event->getPassword()) && $tokenMock->reveal() === $event->getPasswordToken(); }))->shouldBeCalledTimes(1); - $this->manager->resetPassword('foo@example.com'); + $this->manager->resetPassword('email', 'foo@example.com'); } public function testResetPasswordWithExpiredPreviousToken() @@ -88,7 +87,7 @@ public function testResetPasswordWithExpiredPreviousToken() return $event instanceof ForgotPasswordEvent && is_null($event->getPassword()) && $tokenMock->reveal() === $event->getPasswordToken(); }))->shouldBeCalledTimes(1); - $this->manager->resetPassword('foo@example.com'); + $this->manager->resetPassword('email', 'foo@example.com'); } /** @@ -113,7 +112,7 @@ public function testResetPasswordWithUnexpiredTokenHttp() return $event instanceof ForgotPasswordEvent && is_null($event->getPassword()) && $token === $event->getPasswordToken(); }))->shouldBeCalledTimes(1); - $this->manager->resetPassword('foo@example.com'); + $this->manager->resetPassword('email', 'foo@example.com'); } public function testUpdatePassword()