Too many password reset requests even when disabled in settings #11409

magento · Oct 26, 2017 · 3963446 · 3963446
1 parent cb93fe5
commit 3963446
Show file tree

Hide file tree

Showing 4 changed files with 123 additions and 31 deletions.
diff --git a/app/code/Magento/Security/Model/Plugin/AccountManagement.php b/app/code/Magento/Security/Model/Plugin/AccountManagement.php
@@ -5,10 +5,12 @@
  */
 namespace Magento\Security\Model\Plugin;
 
-use Magento\Security\Model\SecurityManager;
 use Magento\Customer\Model\AccountManagement as AccountManagementOriginal;
+use Magento\Framework\App\ObjectManager;
+use Magento\Framework\Config\ScopeInterface;
 use Magento\Framework\Exception\SecurityViolationException;
 use Magento\Security\Model\PasswordResetRequestEvent;
+use Magento\Security\Model\SecurityManager;
 
 /**
  * Magento\Customer\Model\AccountManagement decorator
@@ -30,21 +32,29 @@ class AccountManagement
      */
     protected $passwordRequestEvent;
 
+    /**
+     * @var ScopeInterface
+     */
+    private $scope;
+
     /**
      * AccountManagement constructor.
      *
      * @param \Magento\Framework\App\RequestInterface $request
      * @param SecurityManager $securityManager
      * @param int $passwordRequestEvent
+     * @param ScopeInterface $scope
      */
     public function __construct(
         \Magento\Framework\App\RequestInterface $request,
         \Magento\Security\Model\SecurityManager $securityManager,
-        $passwordRequestEvent = PasswordResetRequestEvent::CUSTOMER_PASSWORD_RESET_REQUEST
+        $passwordRequestEvent = PasswordResetRequestEvent::CUSTOMER_PASSWORD_RESET_REQUEST,
+        ScopeInterface $scope = null
     ) {
         $this->request = $request;
         $this->securityManager = $securityManager;
         $this->passwordRequestEvent = $passwordRequestEvent;
+        $this->scope = $scope ?: ObjectManager::getInstance()->get(ScopeInterface::class);
     }
 
     /**
@@ -63,10 +73,14 @@ public function beforeInitiatePasswordReset(
         $template,
         $websiteId = null
     ) {
-        $this->securityManager->performSecurityCheck(
-            $this->passwordRequestEvent,
-            $email
-        );
+        if ($this->scope->getCurrentScope() == \Magento\Framework\App\Area::AREA_FRONTEND
+            || $this->passwordRequestEvent == PasswordResetRequestEvent::ADMIN_PASSWORD_RESET_REQUEST) {
+            $this->securityManager->performSecurityCheck(
+                $this->passwordRequestEvent,
+                $email
+            );
+        }
+
         return [$email, $template, $websiteId];
     }
 }
diff --git a/app/code/Magento/Security/Test/Unit/Model/Plugin/AccountManagementTest.php b/app/code/Magento/Security/Test/Unit/Model/Plugin/AccountManagementTest.php
@@ -6,7 +6,11 @@
 
 namespace Magento\Security\Test\Unit\Model\Plugin;
 
+use Magento\Customer\Model\AccountManagement;
+use Magento\Framework\App\Area;
+use Magento\Framework\Config\ScopeInterface;
 use Magento\Framework\TestFramework\Unit\Helper\ObjectManager;
+use Magento\Security\Model\PasswordResetRequestEvent;
 
 /**
  * Test class for \Magento\Security\Model\Plugin\AccountManagement testing
@@ -19,20 +23,25 @@ class AccountManagementTest extends \PHPUnit\Framework\TestCase
     protected $model;
 
     /**
-     * @var \Magento\Framework\App\RequestInterface
+     * @var \Magento\Framework\App\RequestInterface|\PHPUnit_Framework_MockObject_MockObject
      */
     protected $request;
 
     /**
-     * @var \Magento\Security\Model\SecurityManager
+     * @var \Magento\Security\Model\SecurityManager|\PHPUnit_Framework_MockObject_MockObject
      */
     protected $securityManager;
 
     /**
-     * @var \Magento\Customer\Model\AccountManagement
+     * @var AccountManagement|\PHPUnit_Framework_MockObject_MockObject
      */
     protected $accountManagement;
 
+    /**
+     * @var ScopeInterface|\PHPUnit_Framework_MockObject_MockObject
+     */
+    private $scope;
+
     /**
      * @var  \Magento\Framework\TestFramework\Unit\Helper\ObjectManager
      */
@@ -53,28 +62,38 @@ public function setUp()
             ['performSecurityCheck']
         );
 
-        $this->accountManagement =  $this->createMock(\Magento\Customer\Model\AccountManagement::class);
+        $this->accountManagement =  $this->createMock(AccountManagement::class);
+        $this->scope =  $this->createMock(ScopeInterface::class);
+    }
+
+    /**
+     * @param $area
+     * @param $passwordRequestEvent
+     * @param $expectedTimes
+     * @dataProvider beforeInitiatePasswordResetDataProvider
+     */
+    public function testBeforeInitiatePasswordReset($area, $passwordRequestEvent, $expectedTimes)
+    {
+        $email = '[email protected]';
+        $template = AccountManagement::EMAIL_RESET;
 
         $this->model = $this->objectManager->getObject(
             \Magento\Security\Model\Plugin\AccountManagement::class,
             [
+                'passwordRequestEvent' => $passwordRequestEvent,
                 'request' => $this->request,
-                'securityManager' => $this->securityManager
+                'securityManager' => $this->securityManager,
+                'scope' => $this->scope
             ]
         );
-    }
 
-    /**
-     * @return void
-     */
-    public function testBeforeInitiatePasswordReset()
-    {
-        $email = '[email protected]';
-        $template = \Magento\Customer\Model\AccountManagement::EMAIL_RESET;
+        $this->scope->expects($this->once())
+            ->method('getCurrentScope')
+            ->willReturn($area);
 
-        $this->securityManager->expects($this->once())
+        $this->securityManager->expects($this->exactly($expectedTimes))
             ->method('performSecurityCheck')
-            ->with(\Magento\Security\Model\PasswordResetRequestEvent::CUSTOMER_PASSWORD_RESET_REQUEST, $email)
+            ->with($passwordRequestEvent, $email)
             ->willReturnSelf();
 
         $this->model->beforeInitiatePasswordReset(
@@ -83,4 +102,18 @@ public function testBeforeInitiatePasswordReset()
             $template
         );
     }
+
+    /**
+     * @return array
+     */
+    public function beforeInitiatePasswordResetDataProvider()
+    {
+        return [
+            [Area::AREA_ADMINHTML, PasswordResetRequestEvent::CUSTOMER_PASSWORD_RESET_REQUEST, 0],
+            [Area::AREA_ADMINHTML, PasswordResetRequestEvent::ADMIN_PASSWORD_RESET_REQUEST, 1],
+            [Area::AREA_FRONTEND, PasswordResetRequestEvent::CUSTOMER_PASSWORD_RESET_REQUEST, 1],
+            // This should never happen, but let's cover it with tests
+            [Area::AREA_FRONTEND, PasswordResetRequestEvent::ADMIN_PASSWORD_RESET_REQUEST, 1],
+        ];
+    }
 }
diff --git a/app/code/Magento/Security/etc/adminhtml/di.xml b/app/code/Magento/Security/etc/adminhtml/di.xml
@@ -17,7 +17,7 @@
     </type>
     <type name="Magento\Security\Model\Plugin\AccountManagement">
         <arguments>
-            <argument name="passwordRequestEvent" xsi:type="const">Magento\Security\Model\PasswordResetRequestEvent::ADMIN_PASSWORD_RESET_REQUEST</argument>
+            <argument name="passwordRequestEvent" xsi:type="const">Magento\Security\Model\PasswordResetRequestEvent::CUSTOMER_PASSWORD_RESET_REQUEST</argument>
         </arguments>
     </type>
     <type name="Magento\Security\Model\SecurityManager">

diff --git a/...s/integration/testsuite/Magento/Customer/Controller/Adminhtml/Index/ResetPasswordTest.php b/...s/integration/testsuite/Magento/Customer/Controller/Adminhtml/Index/ResetPasswordTest.php
@@ -20,10 +20,11 @@ class ResetPasswordTest extends \Magento\TestFramework\TestCase\AbstractBackendC
     protected $baseControllerUrl = 'http://localhost/index.php/backend/customer/index/';
 
     /**
-     * Checks reset password functionality with default settings and customer reset request event.
+     * Checks reset password functionality with no restrictive settings and customer reset request event.
+     * Admin is not affected by this security check, so reset password email must be sent.
      *
-     * @magentoConfigFixture current_store admin/security/limit_password_reset_requests_method 1
-     * @magentoConfigFixture current_store admin/security/min_time_between_password_reset_requests 10
+     * @magentoConfigFixture current_store customer/password/limit_password_reset_requests_method 0
+     * @magentoConfigFixture current_store customer/password/min_time_between_password_reset_requests 0
      * @magentoDataFixture Magento/Customer/_files/customer.php
      */
     public function testResetPasswordSuccess()
@@ -40,11 +41,57 @@ public function testResetPasswordSuccess()
         $this->assertRedirect($this->stringStartsWith($this->baseControllerUrl . 'edit'));
     }
 
+    /**
+     * Checks reset password functionality with default restrictive min time between
+     * password reset requests and customer reset request event.
+     * Admin is not affected by this security check, so reset password email must be sent.
+     *
+     * @magentoConfigFixture current_store customer/password/max_number_password_reset_requests 0
+     * @magentoConfigFixture current_store customer/password/min_time_between_password_reset_requests 10
+     * @magentoDataFixture Magento/Customer/_files/customer.php
+     */
+    public function testResetPasswordMinTimeError()
+    {
+        $this->passwordResetRequestEventCreate(
+            \Magento\Security\Model\PasswordResetRequestEvent::CUSTOMER_PASSWORD_RESET_REQUEST
+        );
+        $this->getRequest()->setPostValue(['customer_id' => '1']);
+        $this->dispatch('backend/customer/index/resetPassword');
+        $this->assertSessionMessages(
+            $this->equalTo(['The customer will receive an email with a link to reset password.']),
+            \Magento\Framework\Message\MessageInterface::TYPE_SUCCESS
+        );
+        $this->assertRedirect($this->stringStartsWith($this->baseControllerUrl . 'edit'));
+    }
+
+    /**
+     * Checks reset password functionality with default restrictive limited number
+     * password reset requests and customer reset request event.
+     * Admin is not affected by this security check, so reset password email must be sent.
+     *
+     * @magentoConfigFixture current_store customer/password/max_number_password_reset_requests 1
+     * @magentoConfigFixture current_store customer/password/min_time_between_password_reset_requests 0
+     * @magentoDataFixture Magento/Customer/_files/customer.php
+     */
+    public function testResetPasswordLimitError()
+    {
+        $this->passwordResetRequestEventCreate(
+            \Magento\Security\Model\PasswordResetRequestEvent::CUSTOMER_PASSWORD_RESET_REQUEST
+        );
+        $this->getRequest()->setPostValue(['customer_id' => '1']);
+        $this->dispatch('backend/customer/index/resetPassword');
+        $this->assertSessionMessages(
+            $this->equalTo(['The customer will receive an email with a link to reset password.']),
+            \Magento\Framework\Message\MessageInterface::TYPE_SUCCESS
+        );
+        $this->assertRedirect($this->stringStartsWith($this->baseControllerUrl . 'edit'));
+    }
+
     /**
      * Checks reset password functionality with default settings, customer and admin reset request events.
      *
-     * @magentoConfigFixture current_store admin/security/limit_password_reset_requests_method 1
-     * @magentoConfigFixture current_store admin/security/min_time_between_password_reset_requests 10
+     * @magentoConfigFixture current_store customer/password/limit_password_reset_requests_method 1
+     * @magentoConfigFixture current_store customer/password/min_time_between_password_reset_requests 10
      * @magentoConfigFixture current_store contact/email/recipient_email [email protected]
      * @magentoDataFixture Magento/Customer/_files/customer.php
      */
@@ -59,10 +106,8 @@ public function testResetPasswordWithSecurityViolationException()
         $this->getRequest()->setPostValue(['customer_id' => '1']);
         $this->dispatch('backend/customer/index/resetPassword');
         $this->assertSessionMessages(
-            $this->equalTo(
-                ['Too many password reset requests. Please wait and try again or contact [email protected].']
-            ),
-            \Magento\Framework\Message\MessageInterface::TYPE_ERROR
+            $this->equalTo(['The customer will receive an email with a link to reset password.']),
+            \Magento\Framework\Message\MessageInterface::TYPE_SUCCESS
         );
         $this->assertRedirect($this->stringStartsWith($this->baseControllerUrl . 'edit'));
     }