Skip to content

Commit 2083bac

Browse files
dmannersdmanners
committed
MAGETWO-82633: #11409: Too many password reset requests even when disabled in settings #11434
- Merge Pull Request #11434 from adrian-martinez-interactiv4/magento2:FR#11409-TOO-MANY-PASSWORD-RESET-REQUESTS - Merged commits: 1. 3963446 2. 86fe123
2 parents 7ec352a + 86fe123 commit 2083bac

File tree

7 files changed

+140
-45
lines changed

7 files changed

+140
-45
lines changed

‎app/code/Magento/Security/Model/Config.php

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -24,10 +24,17 @@ class Config implements ConfigInterface
2424
*/
2525
const XML_PATH_ADMIN_AREA = 'admin/security/';
2626

27+
/**
28+
* Configuration path to frontend area
29+
*/
30+
const XML_PATH_FRONTEND_AREA = 'customer/password/';
31+
2732
/**
2833
* Configuration path to fronted area
34+
* @deprecated
35+
* @see \Magento\Security\Model\Config::XML_PATH_FRONTEND_AREA
2936
*/
30-
const XML_PATH_FRONTED_AREA = 'customer/password/';
37+
const XML_PATH_FRONTED_AREA = self::XML_PATH_FRONTEND_AREA;
3138

3239
/**
3340
* Configuration path to admin account sharing
@@ -134,7 +141,7 @@ protected function getXmlPathPrefix()
134141
if ($this->scope->getCurrentScope() == \Magento\Framework\App\Area::AREA_ADMINHTML) {
135142
return self::XML_PATH_ADMIN_AREA;
136143
}
137-
return self::XML_PATH_FRONTED_AREA;
144+
return self::XML_PATH_FRONTEND_AREA;
138145
}
139146

140147
/**

‎app/code/Magento/Security/Model/Plugin/AccountManagement.php

Lines changed: 20 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -5,10 +5,12 @@
55
*/
66
namespace Magento\Security\Model\Plugin;
77

8-
use Magento\Security\Model\SecurityManager;
98
use Magento\Customer\Model\AccountManagement as AccountManagementOriginal;
9+
use Magento\Framework\App\ObjectManager;
10+
use Magento\Framework\Config\ScopeInterface;
1011
use Magento\Framework\Exception\SecurityViolationException;
1112
use Magento\Security\Model\PasswordResetRequestEvent;
13+
use Magento\Security\Model\SecurityManager;
1214

1315
/**
1416
* Magento\Customer\Model\AccountManagement decorator
@@ -30,21 +32,29 @@ class AccountManagement
3032
*/
3133
protected $passwordRequestEvent;
3234

35+
/**
36+
* @var ScopeInterface
37+
*/
38+
private $scope;
39+
3340
/**
3441
* AccountManagement constructor.
3542
*
3643
* @param \Magento\Framework\App\RequestInterface $request
3744
* @param SecurityManager $securityManager
3845
* @param int $passwordRequestEvent
46+
* @param ScopeInterface $scope
3947
*/
4048
public function __construct(
4149
\Magento\Framework\App\RequestInterface $request,
4250
\Magento\Security\Model\SecurityManager $securityManager,
43-
$passwordRequestEvent = PasswordResetRequestEvent::CUSTOMER_PASSWORD_RESET_REQUEST
51+
$passwordRequestEvent = PasswordResetRequestEvent::CUSTOMER_PASSWORD_RESET_REQUEST,
52+
ScopeInterface $scope = null
4453
) {
4554
$this->request = $request;
4655
$this->securityManager = $securityManager;
4756
$this->passwordRequestEvent = $passwordRequestEvent;
57+
$this->scope = $scope ?: ObjectManager::getInstance()->get(ScopeInterface::class);
4858
}
4959

5060
/**
@@ -63,10 +73,14 @@ public function beforeInitiatePasswordReset(
6373
$template,
6474
$websiteId = null
6575
) {
66-
$this->securityManager->performSecurityCheck(
67-
$this->passwordRequestEvent,
68-
$email
69-
);
76+
if ($this->scope->getCurrentScope() == \Magento\Framework\App\Area::AREA_FRONTEND
77+
|| $this->passwordRequestEvent == PasswordResetRequestEvent::ADMIN_PASSWORD_RESET_REQUEST) {
78+
$this->securityManager->performSecurityCheck(
79+
$this->passwordRequestEvent,
80+
$email
81+
);
82+
}
83+
7084
return [$email, $template, $websiteId];
7185
}
7286
}

‎app/code/Magento/Security/Test/Unit/Model/ConfigTest.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -167,7 +167,7 @@ protected function getXmlPathPrefix($scope)
167167
if ($scope == \Magento\Framework\App\Area::AREA_ADMINHTML) {
168168
return \Magento\Security\Model\Config::XML_PATH_ADMIN_AREA;
169169
}
170-
return \Magento\Security\Model\Config::XML_PATH_FRONTED_AREA;
170+
return \Magento\Security\Model\Config::XML_PATH_FRONTEND_AREA;
171171
}
172172

173173
/**

‎app/code/Magento/Security/Test/Unit/Model/Plugin/AccountManagementTest.php

Lines changed: 48 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,11 @@
66

77
namespace Magento\Security\Test\Unit\Model\Plugin;
88

9+
use Magento\Customer\Model\AccountManagement;
10+
use Magento\Framework\App\Area;
11+
use Magento\Framework\Config\ScopeInterface;
912
use Magento\Framework\TestFramework\Unit\Helper\ObjectManager;
13+
use Magento\Security\Model\PasswordResetRequestEvent;
1014

1115
/**
1216
* Test class for \Magento\Security\Model\Plugin\AccountManagement testing
@@ -19,20 +23,25 @@ class AccountManagementTest extends \PHPUnit\Framework\TestCase
1923
protected $model;
2024

2125
/**
22-
* @var \Magento\Framework\App\RequestInterface
26+
* @var \Magento\Framework\App\RequestInterface|\PHPUnit_Framework_MockObject_MockObject
2327
*/
2428
protected $request;
2529

2630
/**
27-
* @var \Magento\Security\Model\SecurityManager
31+
* @var \Magento\Security\Model\SecurityManager|\PHPUnit_Framework_MockObject_MockObject
2832
*/
2933
protected $securityManager;
3034

3135
/**
32-
* @var \Magento\Customer\Model\AccountManagement
36+
* @var AccountManagement|\PHPUnit_Framework_MockObject_MockObject
3337
*/
3438
protected $accountManagement;
3539

40+
/**
41+
* @var ScopeInterface|\PHPUnit_Framework_MockObject_MockObject
42+
*/
43+
private $scope;
44+
3645
/**
3746
* @var \Magento\Framework\TestFramework\Unit\Helper\ObjectManager
3847
*/
@@ -53,28 +62,38 @@ public function setUp()
5362
['performSecurityCheck']
5463
);
5564

56-
$this->accountManagement = $this->createMock(\Magento\Customer\Model\AccountManagement::class);
65+
$this->accountManagement = $this->createMock(AccountManagement::class);
66+
$this->scope = $this->createMock(ScopeInterface::class);
67+
}
68+
69+
/**
70+
* @param $area
71+
* @param $passwordRequestEvent
72+
* @param $expectedTimes
73+
* @dataProvider beforeInitiatePasswordResetDataProvider
74+
*/
75+
public function testBeforeInitiatePasswordReset($area, $passwordRequestEvent, $expectedTimes)
76+
{
77+
$email = '[email protected]';
78+
$template = AccountManagement::EMAIL_RESET;
5779

5880
$this->model = $this->objectManager->getObject(
5981
\Magento\Security\Model\Plugin\AccountManagement::class,
6082
[
83+
'passwordRequestEvent' => $passwordRequestEvent,
6184
'request' => $this->request,
62-
'securityManager' => $this->securityManager
85+
'securityManager' => $this->securityManager,
86+
'scope' => $this->scope
6387
]
6488
);
65-
}
6689

67-
/**
68-
* @return void
69-
*/
70-
public function testBeforeInitiatePasswordReset()
71-
{
72-
$email = '[email protected]';
73-
$template = \Magento\Customer\Model\AccountManagement::EMAIL_RESET;
90+
$this->scope->expects($this->once())
91+
->method('getCurrentScope')
92+
->willReturn($area);
7493

75-
$this->securityManager->expects($this->once())
94+
$this->securityManager->expects($this->exactly($expectedTimes))
7695
->method('performSecurityCheck')
77-
->with(\Magento\Security\Model\PasswordResetRequestEvent::CUSTOMER_PASSWORD_RESET_REQUEST, $email)
96+
->with($passwordRequestEvent, $email)
7897
->willReturnSelf();
7998

8099
$this->model->beforeInitiatePasswordReset(
@@ -83,4 +102,18 @@ public function testBeforeInitiatePasswordReset()
83102
$template
84103
);
85104
}
105+
106+
/**
107+
* @return array
108+
*/
109+
public function beforeInitiatePasswordResetDataProvider()
110+
{
111+
return [
112+
[Area::AREA_ADMINHTML, PasswordResetRequestEvent::CUSTOMER_PASSWORD_RESET_REQUEST, 0],
113+
[Area::AREA_ADMINHTML, PasswordResetRequestEvent::ADMIN_PASSWORD_RESET_REQUEST, 1],
114+
[Area::AREA_FRONTEND, PasswordResetRequestEvent::CUSTOMER_PASSWORD_RESET_REQUEST, 1],
115+
// This should never happen, but let's cover it with tests
116+
[Area::AREA_FRONTEND, PasswordResetRequestEvent::ADMIN_PASSWORD_RESET_REQUEST, 1],
117+
];
118+
}
86119
}

‎app/code/Magento/Security/etc/adminhtml/di.xml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@
1717
</type>
1818
<type name="Magento\Security\Model\Plugin\AccountManagement">
1919
<arguments>
20-
<argument name="passwordRequestEvent" xsi:type="const">Magento\Security\Model\PasswordResetRequestEvent::ADMIN_PASSWORD_RESET_REQUEST</argument>
20+
<argument name="passwordRequestEvent" xsi:type="const">Magento\Security\Model\PasswordResetRequestEvent::CUSTOMER_PASSWORD_RESET_REQUEST</argument>
2121
</arguments>
2222
</type>
2323
<type name="Magento\Security\Model\SecurityManager">

‎dev/tests/api-functional/testsuite/Magento/Customer/Api/AccountManagementTest.php

Lines changed: 7 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -8,16 +8,12 @@
88
use Magento\Customer\Api\Data\CustomerInterface as Customer;
99
use Magento\Customer\Model\AccountManagement;
1010
use Magento\Framework\Exception\InputException;
11-
use Magento\Framework\Exception\NoSuchEntityException;
11+
use Magento\Framework\Webapi\Exception as HTTPExceptionCodes;
12+
use Magento\Newsletter\Model\Subscriber;
13+
use Magento\Security\Model\Config;
1214
use Magento\TestFramework\Helper\Bootstrap;
1315
use Magento\TestFramework\Helper\Customer as CustomerHelper;
1416
use Magento\TestFramework\TestCase\WebapiAbstract;
15-
use Magento\Framework\Webapi\Exception as HTTPExceptionCodes;
16-
use Magento\Security\Model\Config;
17-
use Magento\Newsletter\Model\Plugin\CustomerPlugin;
18-
use Magento\Framework\Webapi\Rest\Request as RestRequest;
19-
use Magento\Newsletter\Model\Subscriber;
20-
use Magento\Customer\Model\Data\Customer as CustomerData;
2117

2218
/**
2319
* Test class for Magento\Customer\Api\AccountManagementInterface
@@ -112,16 +108,16 @@ public function setUp()
112108
$this->initSubscriber();
113109

114110
if ($this->config->getConfigDataValue(
115-
Config::XML_PATH_FRONTED_AREA .
111+
Config::XML_PATH_FRONTEND_AREA .
116112
Config::XML_PATH_PASSWORD_RESET_PROTECTION_TYPE
117113
) != 0) {
118114
$this->configValue = $this->config
119115
->getConfigDataValue(
120-
Config::XML_PATH_FRONTED_AREA .
116+
Config::XML_PATH_FRONTEND_AREA .
121117
Config::XML_PATH_PASSWORD_RESET_PROTECTION_TYPE
122118
);
123119
$this->config->setDataByPath(
124-
Config::XML_PATH_FRONTED_AREA . Config::XML_PATH_PASSWORD_RESET_PROTECTION_TYPE,
120+
Config::XML_PATH_FRONTEND_AREA . Config::XML_PATH_PASSWORD_RESET_PROTECTION_TYPE,
125121
0
126122
);
127123
$this->config->save();
@@ -150,7 +146,7 @@ public function tearDown()
150146
}
151147
}
152148
$this->config->setDataByPath(
153-
Config::XML_PATH_FRONTED_AREA . Config::XML_PATH_PASSWORD_RESET_PROTECTION_TYPE,
149+
Config::XML_PATH_FRONTEND_AREA . Config::XML_PATH_PASSWORD_RESET_PROTECTION_TYPE,
154150
$this->configValue
155151
);
156152
$this->config->save();

‎dev/tests/integration/testsuite/Magento/Customer/Controller/Adminhtml/Index/ResetPasswordTest.php

Lines changed: 54 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -20,10 +20,11 @@ class ResetPasswordTest extends \Magento\TestFramework\TestCase\AbstractBackendC
2020
protected $baseControllerUrl = 'http://localhost/index.php/backend/customer/index/';
2121

2222
/**
23-
* Checks reset password functionality with default settings and customer reset request event.
23+
* Checks reset password functionality with no restrictive settings and customer reset request event.
24+
* Admin is not affected by this security check, so reset password email must be sent.
2425
*
25-
* @magentoConfigFixture current_store admin/security/limit_password_reset_requests_method 1
26-
* @magentoConfigFixture current_store admin/security/min_time_between_password_reset_requests 10
26+
* @magentoConfigFixture current_store customer/password/limit_password_reset_requests_method 0
27+
* @magentoConfigFixture current_store customer/password/min_time_between_password_reset_requests 0
2728
* @magentoDataFixture Magento/Customer/_files/customer.php
2829
*/
2930
public function testResetPasswordSuccess()
@@ -40,11 +41,57 @@ public function testResetPasswordSuccess()
4041
$this->assertRedirect($this->stringStartsWith($this->baseControllerUrl . 'edit'));
4142
}
4243

44+
/**
45+
* Checks reset password functionality with default restrictive min time between
46+
* password reset requests and customer reset request event.
47+
* Admin is not affected by this security check, so reset password email must be sent.
48+
*
49+
* @magentoConfigFixture current_store customer/password/max_number_password_reset_requests 0
50+
* @magentoConfigFixture current_store customer/password/min_time_between_password_reset_requests 10
51+
* @magentoDataFixture Magento/Customer/_files/customer.php
52+
*/
53+
public function testResetPasswordMinTimeError()
54+
{
55+
$this->passwordResetRequestEventCreate(
56+
\Magento\Security\Model\PasswordResetRequestEvent::CUSTOMER_PASSWORD_RESET_REQUEST
57+
);
58+
$this->getRequest()->setPostValue(['customer_id' => '1']);
59+
$this->dispatch('backend/customer/index/resetPassword');
60+
$this->assertSessionMessages(
61+
$this->equalTo(['The customer will receive an email with a link to reset password.']),
62+
\Magento\Framework\Message\MessageInterface::TYPE_SUCCESS
63+
);
64+
$this->assertRedirect($this->stringStartsWith($this->baseControllerUrl . 'edit'));
65+
}
66+
67+
/**
68+
* Checks reset password functionality with default restrictive limited number
69+
* password reset requests and customer reset request event.
70+
* Admin is not affected by this security check, so reset password email must be sent.
71+
*
72+
* @magentoConfigFixture current_store customer/password/max_number_password_reset_requests 1
73+
* @magentoConfigFixture current_store customer/password/min_time_between_password_reset_requests 0
74+
* @magentoDataFixture Magento/Customer/_files/customer.php
75+
*/
76+
public function testResetPasswordLimitError()
77+
{
78+
$this->passwordResetRequestEventCreate(
79+
\Magento\Security\Model\PasswordResetRequestEvent::CUSTOMER_PASSWORD_RESET_REQUEST
80+
);
81+
$this->getRequest()->setPostValue(['customer_id' => '1']);
82+
$this->dispatch('backend/customer/index/resetPassword');
83+
$this->assertSessionMessages(
84+
$this->equalTo(['The customer will receive an email with a link to reset password.']),
85+
\Magento\Framework\Message\MessageInterface::TYPE_SUCCESS
86+
);
87+
$this->assertRedirect($this->stringStartsWith($this->baseControllerUrl . 'edit'));
88+
}
89+
4390
/**
4491
* Checks reset password functionality with default settings, customer and admin reset request events.
4592
*
46-
* @magentoConfigFixture current_store admin/security/limit_password_reset_requests_method 1
47-
* @magentoConfigFixture current_store admin/security/min_time_between_password_reset_requests 10
93+
* @magentoConfigFixture current_store customer/password/limit_password_reset_requests_method 1
94+
* @magentoConfigFixture current_store customer/password/min_time_between_password_reset_requests 10
4895
* @magentoConfigFixture current_store contact/email/recipient_email [email protected]
4996
* @magentoDataFixture Magento/Customer/_files/customer.php
5097
*/
@@ -59,10 +106,8 @@ public function testResetPasswordWithSecurityViolationException()
59106
$this->getRequest()->setPostValue(['customer_id' => '1']);
60107
$this->dispatch('backend/customer/index/resetPassword');
61108
$this->assertSessionMessages(
62-
$this->equalTo(
63-
['Too many password reset requests. Please wait and try again or contact [email protected].']
64-
),
65-
\Magento\Framework\Message\MessageInterface::TYPE_ERROR
109+
$this->equalTo(['The customer will receive an email with a link to reset password.']),
110+
\Magento\Framework\Message\MessageInterface::TYPE_SUCCESS
66111
);
67112
$this->assertRedirect($this->stringStartsWith($this->baseControllerUrl . 'edit'));
68113
}

0 commit comments

Comments
 (0)