diff --git a/app/code/Magento/Security/Model/Config.php b/app/code/Magento/Security/Model/Config.php index 100f4630a45a6..2135b81eb82b5 100644 --- a/app/code/Magento/Security/Model/Config.php +++ b/app/code/Magento/Security/Model/Config.php @@ -24,10 +24,17 @@ class Config implements ConfigInterface */ const XML_PATH_ADMIN_AREA = 'admin/security/'; + /** + * Configuration path to frontend area + */ + const XML_PATH_FRONTEND_AREA = 'customer/password/'; + /** * Configuration path to fronted area + * @deprecated + * @see \Magento\Security\Model\Config::XML_PATH_FRONTEND_AREA */ - const XML_PATH_FRONTED_AREA = 'customer/password/'; + const XML_PATH_FRONTED_AREA = self::XML_PATH_FRONTEND_AREA; /** * Configuration path to admin account sharing @@ -134,7 +141,7 @@ protected function getXmlPathPrefix() if ($this->scope->getCurrentScope() == \Magento\Framework\App\Area::AREA_ADMINHTML) { return self::XML_PATH_ADMIN_AREA; } - return self::XML_PATH_FRONTED_AREA; + return self::XML_PATH_FRONTEND_AREA; } /** diff --git a/app/code/Magento/Security/Model/Plugin/AccountManagement.php b/app/code/Magento/Security/Model/Plugin/AccountManagement.php index dea54b194880d..9476bf46df338 100644 --- 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/ConfigTest.php b/app/code/Magento/Security/Test/Unit/Model/ConfigTest.php index 7186502df73b5..3ef8655539b5a 100644 --- a/app/code/Magento/Security/Test/Unit/Model/ConfigTest.php +++ b/app/code/Magento/Security/Test/Unit/Model/ConfigTest.php @@ -167,7 +167,7 @@ protected function getXmlPathPrefix($scope) if ($scope == \Magento\Framework\App\Area::AREA_ADMINHTML) { return \Magento\Security\Model\Config::XML_PATH_ADMIN_AREA; } - return \Magento\Security\Model\Config::XML_PATH_FRONTED_AREA; + return \Magento\Security\Model\Config::XML_PATH_FRONTEND_AREA; } /** diff --git a/app/code/Magento/Security/Test/Unit/Model/Plugin/AccountManagementTest.php b/app/code/Magento/Security/Test/Unit/Model/Plugin/AccountManagementTest.php index 0935dc003d5b3..ce90ad532db67 100644 --- 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 = 'test@example.com'; + $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 = 'test@example.com'; - $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 index 6f07fb580490e..c1188c2d405cf 100644 --- a/app/code/Magento/Security/etc/adminhtml/di.xml +++ b/app/code/Magento/Security/etc/adminhtml/di.xml @@ -17,7 +17,7 @@ - Magento\Security\Model\PasswordResetRequestEvent::ADMIN_PASSWORD_RESET_REQUEST + Magento\Security\Model\PasswordResetRequestEvent::CUSTOMER_PASSWORD_RESET_REQUEST diff --git a/dev/tests/api-functional/testsuite/Magento/Customer/Api/AccountManagementTest.php b/dev/tests/api-functional/testsuite/Magento/Customer/Api/AccountManagementTest.php index dec9711cda70f..e47d88183622b 100644 --- a/dev/tests/api-functional/testsuite/Magento/Customer/Api/AccountManagementTest.php +++ b/dev/tests/api-functional/testsuite/Magento/Customer/Api/AccountManagementTest.php @@ -8,16 +8,12 @@ use Magento\Customer\Api\Data\CustomerInterface as Customer; use Magento\Customer\Model\AccountManagement; use Magento\Framework\Exception\InputException; -use Magento\Framework\Exception\NoSuchEntityException; +use Magento\Framework\Webapi\Exception as HTTPExceptionCodes; +use Magento\Newsletter\Model\Subscriber; +use Magento\Security\Model\Config; use Magento\TestFramework\Helper\Bootstrap; use Magento\TestFramework\Helper\Customer as CustomerHelper; use Magento\TestFramework\TestCase\WebapiAbstract; -use Magento\Framework\Webapi\Exception as HTTPExceptionCodes; -use Magento\Security\Model\Config; -use Magento\Newsletter\Model\Plugin\CustomerPlugin; -use Magento\Framework\Webapi\Rest\Request as RestRequest; -use Magento\Newsletter\Model\Subscriber; -use Magento\Customer\Model\Data\Customer as CustomerData; /** * Test class for Magento\Customer\Api\AccountManagementInterface @@ -112,16 +108,16 @@ public function setUp() $this->initSubscriber(); if ($this->config->getConfigDataValue( - Config::XML_PATH_FRONTED_AREA . + Config::XML_PATH_FRONTEND_AREA . Config::XML_PATH_PASSWORD_RESET_PROTECTION_TYPE ) != 0) { $this->configValue = $this->config ->getConfigDataValue( - Config::XML_PATH_FRONTED_AREA . + Config::XML_PATH_FRONTEND_AREA . Config::XML_PATH_PASSWORD_RESET_PROTECTION_TYPE ); $this->config->setDataByPath( - Config::XML_PATH_FRONTED_AREA . Config::XML_PATH_PASSWORD_RESET_PROTECTION_TYPE, + Config::XML_PATH_FRONTEND_AREA . Config::XML_PATH_PASSWORD_RESET_PROTECTION_TYPE, 0 ); $this->config->save(); @@ -150,7 +146,7 @@ public function tearDown() } } $this->config->setDataByPath( - Config::XML_PATH_FRONTED_AREA . Config::XML_PATH_PASSWORD_RESET_PROTECTION_TYPE, + Config::XML_PATH_FRONTEND_AREA . Config::XML_PATH_PASSWORD_RESET_PROTECTION_TYPE, $this->configValue ); $this->config->save(); diff --git a/dev/tests/integration/testsuite/Magento/Customer/Controller/Adminhtml/Index/ResetPasswordTest.php b/dev/tests/integration/testsuite/Magento/Customer/Controller/Adminhtml/Index/ResetPasswordTest.php index fcbe7c5106617..b5ca783d68cf2 100644 --- a/dev/tests/integration/testsuite/Magento/Customer/Controller/Adminhtml/Index/ResetPasswordTest.php +++ b/dev/tests/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 hello@example.com * @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 hello@example.com.'] - ), - \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')); }