Skip to content

Commit

Permalink
MAGETWO-84861: [Backport 2.1-develop] #11409: Too many password reset…
Browse files Browse the repository at this point in the history
… requests even when disabled in settings #11436
  • Loading branch information
ishakhsuvarov authored Dec 7, 2017
2 parents 0d1a000 + d4d3f3d commit bebeb53
Show file tree
Hide file tree
Showing 7 changed files with 152 additions and 68 deletions.
11 changes: 9 additions & 2 deletions app/code/Magento/Security/Model/Config.php
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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;
}

/**
Expand Down
26 changes: 20 additions & 6 deletions app/code/Magento/Security/Model/Plugin/AccountManagement.php
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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);
}

/**
Expand All @@ -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];
}
}
2 changes: 1 addition & 1 deletion app/code/Magento/Security/Test/Unit/Model/ConfigTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -176,7 +176,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;
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,10 +3,13 @@
* Copyright © 2013-2017 Magento, Inc. All rights reserved.
* See COPYING.txt for license details.
*/

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
Expand All @@ -19,20 +22,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
*/
Expand All @@ -46,50 +54,49 @@ public function setUp()
{
$this->objectManager = new ObjectManager($this);

$this->request = $this->getMock(
'\Magento\Framework\App\RequestInterface',
[],
[],
'',
false
);
$this->request = $this->getMock(\Magento\Framework\App\RequestInterface::class);

$this->securityManager = $this->getMock(
'\Magento\Security\Model\SecurityManager',
['performSecurityCheck'],
[],
'',
false
);
$this->securityManager = $this->getMockBuilder(
\Magento\Security\Model\SecurityManager::class
)->setMethods(
['performSecurityCheck']
)->disableOriginalConstructor()->getMock();

$this->accountManagement = $this->getMock(
'\Magento\Customer\Model\AccountManagement',
[],
[],
'',
false
);
$this->accountManagement = $this->getMockBuilder(
AccountManagement::class
)->disableOriginalConstructor()->getMock();

$this->model = $this->objectManager->getObject(
'\Magento\Security\Model\Plugin\AccountManagement',
[
'request' => $this->request,
'securityManager' => $this->securityManager
]
);
$this->scope = $this->getMock(ScopeInterface::class);
}

/**
* @return void
* @param $area
* @param $passwordRequestEvent
* @param $expectedTimes
* @dataProvider beforeInitiatePasswordResetDataProvider
*/
public function testBeforeInitiatePasswordReset()
public function testBeforeInitiatePasswordReset($area, $passwordRequestEvent, $expectedTimes)
{
$email = 'test@example.com';
$template = \Magento\Customer\Model\AccountManagement::EMAIL_RESET;
$template = AccountManagement::EMAIL_RESET;

$this->securityManager->expects($this->once())
$this->model = $this->objectManager->getObject(
\Magento\Security\Model\Plugin\AccountManagement::class,
[
'passwordRequestEvent' => $passwordRequestEvent,
'request' => $this->request,
'securityManager' => $this->securityManager,
'scope' => $this->scope
]
);

$this->scope->expects($this->once())
->method('getCurrentScope')
->willReturn($area);

$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(
Expand All @@ -98,4 +105,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],
];
}
}
2 changes: 1 addition & 1 deletion app/code/Magento/Security/etc/adminhtml/di.xml
Original file line number Diff line number Diff line change
Expand Up @@ -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">
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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();
Expand Down Expand Up @@ -150,15 +146,16 @@ 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();
unset($this->accountManagement);
unset($this->subscriber);
}

private function initSubscriber() {
private function initSubscriber()
{
$this->subscriber = Bootstrap::getObjectManager()->create(
'Magento\Newsletter\Model\Subscriber'
);
Expand Down
Loading

0 comments on commit bebeb53

Please sign in to comment.