Skip to content

Commit ff74cb8

Browse files
author
Joan He
committed
MAGETWO-52441: Customer account lockout works incorrectly if this account is unlocked previously
1 parent ee61591 commit ff74cb8

File tree

2 files changed

+89
-52
lines changed

2 files changed

+89
-52
lines changed

‎app/code/Magento/Customer/Model/Authentication.php

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -92,10 +92,12 @@ public function processAuthenticationFailure($customerId)
9292
}
9393

9494
$lockThreshInterval = new \DateInterval('PT' . $lockThreshold . 'S');
95-
// set first failure date when this is first failure or last first failure expired
96-
if (1 === $failuresNum || !$firstFailureDate || $now->diff($firstFailureDate) > $lockThreshInterval) {
95+
$lockExpires = new \DateTime($customerSecure->getLockExpires());
96+
// set first failure date when this is the first failure or the lock is expired
97+
if (1 === $failuresNum || !$firstFailureDate || $now > $lockExpires) {
9798
$customerSecure->setFirstFailure($this->dateTime->formatDate($now));
9899
$failuresNum = 1;
100+
$customerSecure->setLockExpires(null);
99101
// otherwise lock customer
100102
} elseif ($failuresNum >= $maxFailures) {
101103
$customerSecure->setLockExpires($this->dateTime->formatDate($now->add($lockThreshInterval)));

‎app/code/Magento/Customer/Test/Unit/Model/AuthenticationTest.php

Lines changed: 85 additions & 50 deletions
Original file line numberDiff line numberDiff line change
@@ -5,40 +5,41 @@
55
*/
66
namespace Magento\Customer\Test\Unit\Model;
77

8+
use Magento\Backend\App\ConfigInterface;
89
use Magento\Customer\Api\CustomerRepositoryInterface;
910
use Magento\Customer\Api\Data\CustomerInterface;
1011
use Magento\Customer\Model\Authentication;
1112
use Magento\Customer\Model\AccountManagement;
13+
use Magento\Customer\Model\CustomerRegistry;
14+
use Magento\Customer\Model\Data\CustomerSecure;
1215
use Magento\Framework\TestFramework\Unit\Helper\ObjectManager as ObjectManagerHelper;
1316

1417
class AuthenticationTest extends \PHPUnit_Framework_TestCase
1518
{
1619
/**
17-
* Backend configuration interface
18-
*
19-
* @var \Magento\Backend\App\ConfigInterface
20+
* @var \Magento\Backend\App\ConfigInterface | \PHPUnit_Framework_MockObject_MockObject
2021
*/
2122
private $backendConfigMock;
2223

2324
/**
24-
* @var \Magento\Customer\Model\CustomerRegistry
25+
* @var \Magento\Customer\Model\CustomerRegistry | \PHPUnit_Framework_MockObject_MockObject
2526
*/
2627
private $customerRegistryMock;
2728

2829
/**
29-
* @var \Magento\Framework\Encryption\EncryptorInterface
30+
* @var \Magento\Framework\Encryption\EncryptorInterface | \PHPUnit_Framework_MockObject_MockObject
3031
*/
3132
protected $encryptorMock;
3233

3334
/**
34-
* @var CustomerRepositoryInterface
35+
* @var CustomerRepositoryInterface | \PHPUnit_Framework_MockObject_MockObject
3536
*/
3637
private $customerRepositoryMock;
3738

3839
/**
39-
* @var \Magento\Customer\Model\Data\CustomerSecure
40+
* @var \Magento\Customer\Model\Data\CustomerSecure | \PHPUnit_Framework_MockObject_MockObject
4041
*/
41-
private $customerSecure;
42+
private $customerSecureMock;
4243

4344
/**
4445
* @var \Magento\Customer\Model\Authentication
@@ -47,12 +48,12 @@ class AuthenticationTest extends \PHPUnit_Framework_TestCase
4748

4849
protected function setUp()
4950
{
50-
$this->backendConfigMock = $this->getMockBuilder('Magento\Backend\App\ConfigInterface')
51+
$this->backendConfigMock = $this->getMockBuilder(ConfigInterface::class)
5152
->disableOriginalConstructor()
5253
->setMethods(['getValue'])
5354
->getMockForAbstractClass();
5455
$this->customerRegistryMock = $this->getMock(
55-
'Magento\Customer\Model\CustomerRegistry',
56+
CustomerRegistry::class,
5657
['retrieveSecureData', 'retrieve'],
5758
[],
5859
'',
@@ -64,14 +65,15 @@ protected function setUp()
6465
$this->encryptorMock = $this->getMockBuilder(\Magento\Framework\Encryption\EncryptorInterface::class)
6566
->disableOriginalConstructor()
6667
->getMock();
67-
$this->customerSecure = $this->getMock(
68-
'Magento\Customer\Model\Data\CustomerSecure',
68+
$this->customerSecureMock = $this->getMock(
69+
CustomerSecure::class,
6970
[
7071
'getId',
7172
'getPasswordHash',
7273
'isCustomerLocked',
7374
'getFailuresNum',
7475
'getFirstFailure',
76+
'getLockExpires',
7577
'setFirstFailure',
7678
'setFailuresNum',
7779
'setLockExpires'
@@ -93,10 +95,7 @@ protected function setUp()
9395
);
9496
}
9597

96-
/**
97-
* @return void
98-
*/
99-
public function testLockingIsDisabled()
98+
public function testProcessAuthenticationFailureLockingIsDisabled()
10099
{
101100
$customerId = 1;
102101
$this->backendConfigMock->expects($this->exactly(2))
@@ -109,14 +108,11 @@ public function testLockingIsDisabled()
109108
$this->customerRegistryMock->expects($this->once())
110109
->method('retrieveSecureData')
111110
->with($customerId)
112-
->willReturn($this->customerSecure);
111+
->willReturn($this->customerSecureMock);
113112
$this->authentication->processAuthenticationFailure($customerId);
114113
}
115114

116-
/**
117-
* @return void
118-
*/
119-
public function testCustomerFailedFirstAttempt()
115+
public function testProcessAuthenticationFailureFirstAttempt()
120116
{
121117
$customerId = 1;
122118
$this->backendConfigMock->expects($this->exactly(2))
@@ -130,7 +126,7 @@ public function testCustomerFailedFirstAttempt()
130126
$this->customerRegistryMock->expects($this->once())
131127
->method('retrieveSecureData')
132128
->with($customerId)
133-
->willReturn($this->customerSecure);
129+
->willReturn($this->customerSecureMock);
134130
$customerMock = $this->getMockBuilder(CustomerInterface::class)
135131
->disableOriginalConstructor()
136132
->getMock();
@@ -142,18 +138,60 @@ public function testCustomerFailedFirstAttempt()
142138
->method('save')
143139
->with($customerMock);
144140

145-
$this->customerSecure->expects($this->once())->method('getFailuresNum')->willReturn(0);
146-
$this->customerSecure->expects($this->once())->method('getFirstFailure')->willReturn(0);
147-
$this->customerSecure->expects($this->once())->method('setFirstFailure');
148-
$this->customerSecure->expects($this->once())->method('setFailuresNum');
141+
$this->customerSecureMock->expects($this->once())->method('getFailuresNum')->willReturn(0);
142+
$this->customerSecureMock->expects($this->once())->method('getFirstFailure')->willReturn(0);
143+
$this->customerSecureMock->expects($this->once())->method('setFirstFailure');
144+
$this->customerSecureMock->expects($this->once())->method('setFailuresNum');
149145

150146
$this->authentication->processAuthenticationFailure($customerId);
151147
}
152148

153-
/**
154-
* @return void
155-
*/
156-
public function testCustomerHasFailedMaxNumberOfAttempts()
149+
public function testProcessAuthenticationFailureLockExpired()
150+
{
151+
$customerId = 1;
152+
$this->backendConfigMock->expects($this->exactly(2))
153+
->method('getValue')
154+
->withConsecutive(
155+
[\Magento\Customer\Model\Authentication::LOCKOUT_THRESHOLD_PATH],
156+
[\Magento\Customer\Model\Authentication::MAX_FAILURES_PATH]
157+
)
158+
->willReturnOnConsecutiveCalls(10, 5);
159+
160+
$this->customerRegistryMock->expects($this->once())
161+
->method('retrieveSecureData')
162+
->with($customerId)
163+
->willReturn($this->customerSecureMock);
164+
$customerMock = $this->getMockBuilder(CustomerInterface::class)
165+
->disableOriginalConstructor()
166+
->getMock();
167+
$this->customerRepositoryMock->expects($this->once())
168+
->method('getById')
169+
->with($customerId)
170+
->willReturn($customerMock);
171+
$this->customerRepositoryMock->expects($this->once())
172+
->method('save')
173+
->with($customerMock);
174+
175+
$date = new \DateTime();
176+
$date->modify('-100 second');
177+
$lockExpiresDate = $date->format('Y-m-d H:i:s');
178+
$date->modify('-400 second');
179+
$firstFailureDate = $date->format('Y-m-d H:i:s');
180+
$this->customerSecureMock->expects($this->once())->method('getFailuresNum')->willReturn(5);
181+
$this->customerSecureMock->expects($this->once())
182+
->method('getFirstFailure')
183+
->willReturn($firstFailureDate);
184+
$this->customerSecureMock->expects($this->once())
185+
->method('getLockExpires')
186+
->willReturn($lockExpiresDate);
187+
$this->customerSecureMock->expects($this->once())->method('setFirstFailure');
188+
$this->customerSecureMock->expects($this->once())->method('setLockExpires')->with(null);
189+
$this->customerSecureMock->expects($this->once())->method('setFailuresNum')->with(1);
190+
191+
$this->authentication->processAuthenticationFailure($customerId);
192+
}
193+
194+
public function testProcessAuthenticationFailureMaxNumberOfAttempts()
157195
{
158196
$customerId = 1;
159197
$date = new \DateTime();
@@ -170,7 +208,7 @@ public function testCustomerHasFailedMaxNumberOfAttempts()
170208
$this->customerRegistryMock->expects($this->once())
171209
->method('retrieveSecureData')
172210
->with($customerId)
173-
->willReturn($this->customerSecure);
211+
->willReturn($this->customerSecureMock);
174212
$customerMock = $this->getMockBuilder(CustomerInterface::class)
175213
->disableOriginalConstructor()
176214
->getMock();
@@ -182,26 +220,23 @@ public function testCustomerHasFailedMaxNumberOfAttempts()
182220
->method('save')
183221
->with($customerMock);
184222

185-
$this->customerSecure->expects($this->once())->method('getFailuresNum')->willReturn(5);
186-
$this->customerSecure->expects($this->once())
223+
$this->customerSecureMock->expects($this->once())->method('getFailuresNum')->willReturn(4);
224+
$this->customerSecureMock->expects($this->once())
187225
->method('getFirstFailure')
188226
->willReturn($formattedDate);
189-
$this->customerSecure->expects($this->once())->method('setLockExpires');
190-
$this->customerSecure->expects($this->once())->method('setFailuresNum');
227+
$this->customerSecureMock->expects($this->once())->method('setLockExpires');
228+
$this->customerSecureMock->expects($this->once())->method('setFailuresNum')->with(5);
191229

192230
$this->authentication->processAuthenticationFailure($customerId);
193231
}
194232

195-
/**
196-
* @return void
197-
*/
198-
public function testProcessUnlockData()
233+
public function testUnlock()
199234
{
200235
$customerId = 1;
201236
$this->customerRegistryMock->expects($this->once())
202237
->method('retrieveSecureData')
203238
->with($customerId)
204-
->willReturn($this->customerSecure);
239+
->willReturn($this->customerSecureMock);
205240
$customerMock = $this->getMockBuilder(CustomerInterface::class)
206241
->disableOriginalConstructor()
207242
->getMock();
@@ -212,9 +247,9 @@ public function testProcessUnlockData()
212247
$this->customerRepositoryMock->expects($this->once())
213248
->method('save')
214249
->with($customerMock);
215-
$this->customerSecure->expects($this->once())->method('setFailuresNum')->with(0);
216-
$this->customerSecure->expects($this->once())->method('setFirstFailure')->with(null);
217-
$this->customerSecure->expects($this->once())->method('setLockExpires')->with(null);
250+
$this->customerSecureMock->expects($this->once())->method('setFailuresNum')->with(0);
251+
$this->customerSecureMock->expects($this->once())->method('setFirstFailure')->with(null);
252+
$this->customerSecureMock->expects($this->once())->method('setLockExpires')->with(null);
218253
$this->authentication->unlock($customerId);
219254
}
220255

@@ -229,7 +264,7 @@ public function validatePasswordAndLockStatusDataProvider()
229264
/**
230265
* @return void
231266
*/
232-
public function testCheckIfLocked()
267+
public function testIsLocked()
233268
{
234269
$customerId = 7;
235270

@@ -250,7 +285,7 @@ public function testCheckIfLocked()
250285
* @param bool $result
251286
* @dataProvider validateCustomerPassword
252287
*/
253-
public function testValidateCustomerPassword($result)
288+
public function testAuthenticate($result)
254289
{
255290
$customerId = 7;
256291
$password = '1234567';
@@ -267,18 +302,18 @@ public function testValidateCustomerPassword($result)
267302
->method('getById')
268303
->willReturn($customerMock);
269304

270-
$this->customerSecure->expects($this->any())
305+
$this->customerSecureMock->expects($this->any())
271306
->method('getId')
272307
->willReturn($customerId);
273308

274-
$this->customerSecure->expects($this->once())
309+
$this->customerSecureMock->expects($this->once())
275310
->method('getPasswordHash')
276311
->willReturn($hash);
277312

278313
$this->customerRegistryMock->expects($this->any())
279314
->method('retrieveSecureData')
280315
->with($customerId)
281-
->willReturn($this->customerSecure);
316+
->willReturn($this->customerSecureMock);
282317

283318
$this->encryptorMock->expects($this->once())
284319
->method('validateHash')
@@ -295,14 +330,14 @@ public function testValidateCustomerPassword($result)
295330
[\Magento\Customer\Model\Authentication::MAX_FAILURES_PATH]
296331
)
297332
->willReturnOnConsecutiveCalls(1, 1);
298-
$this->customerSecure->expects($this->once())
333+
$this->customerSecureMock->expects($this->once())
299334
->method('isCustomerLocked')
300335
->willReturn(false);
301336

302337
$this->customerRegistryMock->expects($this->once())
303338
->method('retrieve')
304339
->with($customerId)
305-
->willReturn($this->customerSecure);
340+
->willReturn($this->customerSecureMock);
306341

307342
$this->customerRepositoryMock->expects($this->once())
308343
->method('save')

0 commit comments

Comments
 (0)