Sylius · mamazu · Feb 3, 2020 · Jan 10, 2020 · Jan 10, 2020 · Jan 10, 2020
diff --git a/UPGRADE.md b/UPGRADE.md
@@ -7,6 +7,7 @@ access_control:
     - { path: "%sylius_shop_api.security.regex%/me", role: ROLE_USER}
 
 ```
+* The forgot passwort function now doesn't return an error if the user with this email is not found. It might give a potential attacker information about which users are registered in the shop.
 
 # UPGRADE FROM 1.0.0-rc.2 or 1.0.0-rc.3 to 1.0.0 
 

diff --git a/doc/swagger.yml b/doc/swagger.yml
@@ -709,9 +709,9 @@ paths:
                         $ref: "#/definitions/RequestPasswordResetting"
             responses:
                 204:
-                    description: "Reset password request has been sent."
-                500:
-                    description: "User with provided email has not been found."
+                    description: "Reset password request has been sent If the email exists in our system,."
+                400:
+                    description: "Email not valid"
 
     /password-reset/{token}:
         parameters:

diff --git a/spec/Handler/Customer/GenerateResetPasswordTokenHandlerSpec.php b/spec/Handler/Customer/GenerateResetPasswordTokenHandlerSpec.php
@@ -38,12 +38,4 @@ function it_handles_generating_user_verification_token(
 
         $this(new GenerateResetPasswordToken('example@customer.com'));
     }
-
-    function it_throws_an_exception_if_user_has_not_been_found(
-        UserRepositoryInterface $userRepository
-    ): void {
-        $userRepository->findOneByEmail('example@customer.com')->willReturn(null);
-
-        $this->shouldThrow(\InvalidArgumentException::class)->during('__invoke', [new GenerateResetPasswordToken('example@customer.com')]);
-    }
 }
diff --git a/spec/Handler/Customer/SendResetPasswordTokenHandlerSpec.php b/spec/Handler/Customer/SendResetPasswordTokenHandlerSpec.php
@@ -37,14 +37,6 @@ function it_handles_emailing_user_with_verification_email(
         $this(new SendResetPasswordToken('example@customer.com', 'WEB_GB'));
     }
 
-    function it_throws_an_exception_if_user_has_not_been_found(
-        UserRepositoryInterface $userRepository
-    ): void {
-        $userRepository->findOneByEmail('example@customer.com')->willReturn(null);
-
-        $this->shouldThrow(\InvalidArgumentException::class)->during('__invoke', [new SendResetPasswordToken('example@customer.com', 'WEB_GB')]);
-    }
-
     function it_throws_an_exception_if_user_has_not_verification_token(
         UserRepositoryInterface $userRepository,
         ShopUserInterface $user

diff --git a/src/Controller/Customer/RequestPasswordResettingAction.php b/src/Controller/Customer/RequestPasswordResettingAction.php
@@ -10,6 +10,7 @@
 use Sylius\Component\Core\Model\ChannelInterface;
 use Sylius\ShopApiPlugin\CommandProvider\ChannelBasedCommandProviderInterface;
 use Sylius\ShopApiPlugin\CommandProvider\CommandProviderInterface;
+use Sylius\ShopApiPlugin\Factory\ValidationErrorViewFactoryInterface;
 use Symfony\Component\HttpFoundation\Request;
 use Symfony\Component\HttpFoundation\Response;
 use Symfony\Component\Messenger\MessageBusInterface;
@@ -22,6 +23,9 @@ final class RequestPasswordResettingAction
     /** @var MessageBusInterface */
     private $bus;
 
+    /** @var ValidationErrorViewFactoryInterface */
+    private $validationErrorViewFactory;
+
     /** @var ChannelContextInterface */
     private $channelContext;
 
@@ -34,12 +38,14 @@ final class RequestPasswordResettingAction
     public function __construct(
         ViewHandlerInterface $viewHandler,
         MessageBusInterface $bus,
+        ValidationErrorViewFactoryInterface $validationErrorViewFactory,
         ChannelContextInterface $channelContext,
         CommandProviderInterface $generateResetPasswordTokenCommandProvider,
         ChannelBasedCommandProviderInterface $sendResetPasswordTokenCommandProvider
     ) {
         $this->viewHandler = $viewHandler;
         $this->bus = $bus;
+        $this->validationErrorViewFactory = $validationErrorViewFactory;
         $this->channelContext = $channelContext;
         $this->generateResetPasswordTokenCommandProvider = $generateResetPasswordTokenCommandProvider;
         $this->sendResetPasswordTokenCommandProvider = $sendResetPasswordTokenCommandProvider;
@@ -49,7 +55,13 @@ public function __invoke(Request $request): Response
     {
         /** @var ChannelInterface $channel */
         $channel = $this->channelContext->getChannel();
-
+        $validationResults = $this->generateResetPasswordTokenCommandProvider->validate($request);
+        if (0 !== count($validationResults)) {
+            return $this->viewHandler->handle(View::create(
+                $this->validationErrorViewFactory->create($validationResults),
+                Response::HTTP_BAD_REQUEST
+            ));
+        }
         $this->bus->dispatch($this->generateResetPasswordTokenCommandProvider->getCommand($request));
         $this->bus->dispatch($this->sendResetPasswordTokenCommandProvider->getCommand($request, $channel));
 

diff --git a/src/Handler/Customer/GenerateResetPasswordTokenHandler.php b/src/Handler/Customer/GenerateResetPasswordTokenHandler.php
@@ -8,7 +8,6 @@
 use Sylius\Component\User\Repository\UserRepositoryInterface;
 use Sylius\Component\User\Security\Generator\GeneratorInterface;
 use Sylius\ShopApiPlugin\Command\Customer\GenerateResetPasswordToken;
-use Webmozart\Assert\Assert;
 
 final class GenerateResetPasswordTokenHandler
 {
@@ -30,10 +29,9 @@ public function __invoke(GenerateResetPasswordToken $generateResetPasswordToken)
 
         /** @var ShopUserInterface $user */
         $user = $this->userRepository->findOneByEmail($email);
-
-        Assert::notNull($user, sprintf('User with %s email has not been found.', $email));
-
-        $user->setPasswordResetToken($this->tokenGenerator->generate());
-        $user->setPasswordRequestedAt(new \DateTime());
+        if (null !== $user) {
+            $user->setPasswordResetToken($this->tokenGenerator->generate());
+            $user->setPasswordRequestedAt(new \DateTime());
+        }
     }
 }
diff --git a/src/Handler/Customer/SendResetPasswordTokenHandler.php b/src/Handler/Customer/SendResetPasswordTokenHandler.php
@@ -31,14 +31,13 @@ public function __invoke(SendResetPasswordToken $resendResetPasswordToken): void
 
         /** @var ShopUserInterface $user */
         $user = $this->userRepository->findOneByEmail($email);
-
-        Assert::notNull($user, sprintf('User with %s email has not been found.', $email));
-        Assert::notNull($user->getPasswordResetToken(), sprintf('User with %s email has not verification token defined.', $email));
-
-        $this->sender->send(
-            Emails::EMAIL_RESET_PASSWORD_TOKEN,
-            [$email],
-            ['user' => $user, 'channelCode' => $resendResetPasswordToken->channelCode()]
-        );
+        if (null !== $user) {
+            Assert::notNull($user->getPasswordResetToken(), sprintf('User with %s email has not verification token defined.', $email));
+            $this->sender->send(
+                Emails::EMAIL_RESET_PASSWORD_TOKEN,
+                [$email],
+                ['user' => $user, 'channelCode' => $resendResetPasswordToken->channelCode()]
+            );
+        }
     }
 }
diff --git a/src/Resources/config/services/actions/customer.xml b/src/Resources/config/services/actions/customer.xml
@@ -42,6 +42,7 @@
         >
             <argument type="service" id="fos_rest.view_handler" />
             <argument type="service" id="sylius_shop_api_plugin.command_bus" />
+            <argument type="service" id="sylius.shop_api_plugin.factory.validation_error_view_factory" />
             <argument type="service" id="sylius.context.channel" />
             <argument type="service" id="sylius.shop_api_plugin.command_provider.generate_reset_password_token" />
             <argument type="service" id="sylius.shop_api_plugin.command_provider.send_reset_password_token" />

diff --git a/src/Resources/config/validation/customer/GenerateResetPasswordTokenRequest.xml b/src/Resources/config/validation/customer/GenerateResetPasswordTokenRequest.xml
@@ -0,0 +1,15 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<constraint-mapping xmlns="http://symfony.com/schema/dic/constraint-mapping"
+                    xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
+                    xsi:schemaLocation="http://symfony.com/schema/dic/constraint-mapping http://symfony.com/schema/dic/services/constraint-mapping-1.0.xsd">
+    <class name="Sylius\ShopApiPlugin\Request\Customer\GenerateResetPasswordTokenRequest">
+        <property name="email">
+            <constraint name="NotBlank">
+                <option name="message">sylius.shop_api.email.not_blank</option>
+            </constraint>
+            <constraint name="Email">
+                <option name="message">sylius.shop_api.email.invalid</option>
+            </constraint>
+        </property>
+    </class>
+</constraint-mapping>
diff --git a/tests/Controller/Customer/RequestPasswordResettingApiTest.php b/tests/Controller/Customer/RequestPasswordResettingApiTest.php
@@ -34,6 +34,51 @@ public function it_allows_to_reset_user_password(): void
         $this->assertTrue($emailChecker->hasRecipient('oliver@queen.com'));
     }
 
+    /**
+     * @test
+     */
+    public function it_does_not_allow_to_reset_user_password_without_entering_valid_email(): void
+    {
+        $this->loadFixturesFromFiles(['channel.yml', 'customer.yml']);
+
+        $data = '{"email": "oliver"}';
+
+        $this->client->request('PUT', '/shop-api/request-password-reset', [], [], self::CONTENT_TYPE_HEADER, $data);
+
+        $response = $this->client->getResponse();
+        $this->assertResponse($response, 'customer/request_reset_password_failed_email', Response::HTTP_BAD_REQUEST);
+    }
+
+    /**
+     * @test
+     */
+    public function it_does_not_allow_to_reset_user_password_without_entering_email(): void
+    {
+        $this->loadFixturesFromFiles(['channel.yml', 'customer.yml']);
+
+        $data = '{}';
+
+        $this->client->request('PUT', '/shop-api/request-password-reset', [], [], self::CONTENT_TYPE_HEADER, $data);
+
+        $response = $this->client->getResponse();
+        $this->assertResponse($response, 'customer/request_reset_password_empty_email', Response::HTTP_BAD_REQUEST);
+    }
+
+    /**
+     * @test
+     */
+    public function it_allow_to_reset_user_password_without_sending_mail_user_not_exist(): void
+    {
+        $this->loadFixturesFromFiles(['channel.yml', 'customer.yml']);
+
+        $data = '{"email": "Amr@amr.com"}';
+
+        $this->client->request('PUT', '/shop-api/request-password-reset', [], [], self::CONTENT_TYPE_HEADER, $data);
+
+        $response = $this->client->getResponse();
+        $this->assertResponseCode($response, Response::HTTP_NO_CONTENT);
+    }
+
     protected function getContainer(): ContainerInterface
     {
         return static::$sharedKernel->getContainer();

diff --git a/tests/Responses/Expected/customer/request_reset_password_empty_email.json b/tests/Responses/Expected/customer/request_reset_password_empty_email.json
@@ -0,0 +1,9 @@
+{
+  "code": 400,
+  "message": "Validation failed",
+  "errors": {
+    "email": [
+      "Please enter your email."
+    ]
+  }
+}
diff --git a/tests/Responses/Expected/customer/request_reset_password_failed_email.json b/tests/Responses/Expected/customer/request_reset_password_failed_email.json
@@ -0,0 +1,9 @@
+{
+  "code": 400,
+  "message": "Validation failed",
+  "errors": {
+    "email": [
+      "This email is invalid."
+    ]
+  }
+}
-Original file line number
+Diff line change
@@ Expand Up / @@ -7,6 +7,7 @@ access_control: @@
         - { path: "%sylius_shop_api.security.regex%/me", role: ROLE_USER}
     ```
+    * The forgot passwort function now doesn't return an error if the user with this email is not found. It might give a potential attacker information about which users are registered in the shop.
     # UPGRADE FROM 1.0.0-rc.2 or 1.0.0-rc.3 to 1.0.0
@@ Expand Down @@