Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Backport 2.1-develop] #11409: Too many password reset requests even when disabled in settings #11436

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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