Skip to content

Commit 52ea72b

Browse files
dsikkema-magentodsikkema-magento
committed
Merge remote-tracking branch 'origin/MAGETWO-52441-password' into develop
2 parents 284aec3 + ca032ca commit 52ea72b

File tree

2 files changed

+93
-82
lines changed

2 files changed

+93
-82
lines changed

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

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -82,7 +82,7 @@ public function processAuthenticationFailure($customerId)
8282
$customerSecure = $this->customerRegistry->retrieveSecureData($customerId);
8383

8484
if (!($lockThreshold && $maxFailures)) {
85-
return false;
85+
return;
8686
}
8787
$failuresNum = (int)$customerSecure->getFailuresNum() + 1;
8888

@@ -92,10 +92,13 @@ 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 = $customerSecure->getLockExpires();
96+
$lockExpired = ($lockExpires !== null) && ($now > new \DateTime($lockExpires));
97+
// set first failure date when this is the first failure or the lock is expired
98+
if (1 === $failuresNum || !$firstFailureDate || $lockExpired) {
9799
$customerSecure->setFirstFailure($this->dateTime->formatDate($now));
98100
$failuresNum = 1;
101+
$customerSecure->setLockExpires(null);
99102
// otherwise lock customer
100103
} elseif ($failuresNum >= $maxFailures) {
101104
$customerSecure->setLockExpires($this->dateTime->formatDate($now->add($lockThreshInterval)));

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

Lines changed: 87 additions & 79 deletions
Original file line numberDiff line numberDiff line change
@@ -5,54 +5,64 @@
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;
15+
use Magento\Framework\Stdlib\DateTime;
1216
use Magento\Framework\TestFramework\Unit\Helper\ObjectManager as ObjectManagerHelper;
1317

18+
/**
19+
* @SuppressWarnings(PHPMD.CouplingBetweenObjects)
20+
*/
1421
class AuthenticationTest extends \PHPUnit_Framework_TestCase
1522
{
1623
/**
17-
* Backend configuration interface
18-
*
19-
* @var \Magento\Backend\App\ConfigInterface
24+
* @var \Magento\Backend\App\ConfigInterface | \PHPUnit_Framework_MockObject_MockObject
2025
*/
2126
private $backendConfigMock;
2227

2328
/**
24-
* @var \Magento\Customer\Model\CustomerRegistry
29+
* @var \Magento\Customer\Model\CustomerRegistry | \PHPUnit_Framework_MockObject_MockObject
2530
*/
2631
private $customerRegistryMock;
2732

2833
/**
29-
* @var \Magento\Framework\Encryption\EncryptorInterface
34+
* @var \Magento\Framework\Encryption\EncryptorInterface | \PHPUnit_Framework_MockObject_MockObject
3035
*/
3136
protected $encryptorMock;
3237

3338
/**
34-
* @var CustomerRepositoryInterface
39+
* @var CustomerRepositoryInterface | \PHPUnit_Framework_MockObject_MockObject
3540
*/
3641
private $customerRepositoryMock;
3742

3843
/**
39-
* @var \Magento\Customer\Model\Data\CustomerSecure
44+
* @var \Magento\Customer\Model\Data\CustomerSecure | \PHPUnit_Framework_MockObject_MockObject
4045
*/
41-
private $customerSecure;
46+
private $customerSecureMock;
4247

4348
/**
4449
* @var \Magento\Customer\Model\Authentication
4550
*/
4651
private $authentication;
4752

53+
/**
54+
* @var DateTime
55+
*/
56+
private $dateTimeMock;
57+
4858
protected function setUp()
4959
{
50-
$this->backendConfigMock = $this->getMockBuilder('Magento\Backend\App\ConfigInterface')
60+
$this->backendConfigMock = $this->getMockBuilder(ConfigInterface::class)
5161
->disableOriginalConstructor()
5262
->setMethods(['getValue'])
5363
->getMockForAbstractClass();
5464
$this->customerRegistryMock = $this->getMock(
55-
'Magento\Customer\Model\CustomerRegistry',
65+
CustomerRegistry::class,
5666
['retrieveSecureData', 'retrieve'],
5767
[],
5868
'',
@@ -64,14 +74,21 @@ protected function setUp()
6474
$this->encryptorMock = $this->getMockBuilder(\Magento\Framework\Encryption\EncryptorInterface::class)
6575
->disableOriginalConstructor()
6676
->getMock();
67-
$this->customerSecure = $this->getMock(
68-
'Magento\Customer\Model\Data\CustomerSecure',
77+
$this->dateTimeMock = $this->getMockBuilder(DateTime::class)
78+
->disableOriginalConstructor()
79+
->getMock();
80+
$this->dateTimeMock->expects($this->any())
81+
->method('formatDate')
82+
->willReturn('formattedDate');
83+
$this->customerSecureMock = $this->getMock(
84+
CustomerSecure::class,
6985
[
7086
'getId',
7187
'getPasswordHash',
7288
'isCustomerLocked',
7389
'getFailuresNum',
7490
'getFirstFailure',
91+
'getLockExpires',
7592
'setFirstFailure',
7693
'setFailuresNum',
7794
'setLockExpires'
@@ -80,6 +97,7 @@ protected function setUp()
8097
'',
8198
false
8299
);
100+
83101
$objectManagerHelper = new ObjectManagerHelper($this);
84102

85103
$this->authentication = $objectManagerHelper->getObject(
@@ -89,14 +107,12 @@ protected function setUp()
89107
'backendConfig' => $this->backendConfigMock,
90108
'customerRepository' => $this->customerRepositoryMock,
91109
'encryptor' => $this->encryptorMock,
110+
'dateTime' => $this->dateTimeMock,
92111
]
93112
);
94113
}
95114

96-
/**
97-
* @return void
98-
*/
99-
public function testLockingIsDisabled()
115+
public function testProcessAuthenticationFailureLockingIsDisabled()
100116
{
101117
$customerId = 1;
102118
$this->backendConfigMock->expects($this->exactly(2))
@@ -109,15 +125,32 @@ public function testLockingIsDisabled()
109125
$this->customerRegistryMock->expects($this->once())
110126
->method('retrieveSecureData')
111127
->with($customerId)
112-
->willReturn($this->customerSecure);
128+
->willReturn($this->customerSecureMock);
113129
$this->authentication->processAuthenticationFailure($customerId);
114130
}
115131

116132
/**
117-
* @return void
133+
* @param int $failureNum
134+
* @param string $firstFailure
135+
* @param string $lockExpires
136+
* @param int $setFailureNumCallCtr
137+
* @param int $setFailureNumValue
138+
* @param int $setFirstFailureCallCtr
139+
* @param int $setFirstFailureValue
140+
* @param int $setLockExpiresCallCtr
141+
* @param int $setLockExpiresValue
142+
* @dataProvider processAuthenticationFailureDataProvider
118143
*/
119-
public function testCustomerFailedFirstAttempt()
120-
{
144+
public function testProcessAuthenticationFailureFirstAttempt(
145+
$failureNum,
146+
$firstFailure,
147+
$lockExpires,
148+
$setFailureNumCallCtr,
149+
$setFailureNumValue,
150+
$setFirstFailureCallCtr,
151+
$setLockExpiresCallCtr,
152+
$setLockExpiresValue
153+
) {
121154
$customerId = 1;
122155
$this->backendConfigMock->expects($this->exactly(2))
123156
->method('getValue')
@@ -130,7 +163,7 @@ public function testCustomerFailedFirstAttempt()
130163
$this->customerRegistryMock->expects($this->once())
131164
->method('retrieveSecureData')
132165
->with($customerId)
133-
->willReturn($this->customerSecure);
166+
->willReturn($this->customerSecureMock);
134167
$customerMock = $this->getMockBuilder(CustomerInterface::class)
135168
->disableOriginalConstructor()
136169
->getMock();
@@ -142,66 +175,41 @@ public function testCustomerFailedFirstAttempt()
142175
->method('save')
143176
->with($customerMock);
144177

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');
178+
$this->customerSecureMock->expects($this->once())->method('getFailuresNum')->willReturn($failureNum);
179+
$this->customerSecureMock->expects($this->once())
180+
->method('getFirstFailure')
181+
->willReturn($firstFailure ? (new \DateTime())->modify($firstFailure)->format('Y-m-d H:i:s') : null);
182+
$this->customerSecureMock->expects($this->once())
183+
->method('getLockExpires')
184+
->willReturn($lockExpires ? (new \DateTime())->modify($lockExpires)->format('Y-m-d H:i:s') : null);
185+
$this->customerSecureMock->expects($this->exactly($setFirstFailureCallCtr))->method('setFirstFailure');
186+
$this->customerSecureMock->expects($this->exactly($setFailureNumCallCtr))
187+
->method('setFailuresNum')
188+
->with($setFailureNumValue);
189+
$this->customerSecureMock->expects($this->exactly($setLockExpiresCallCtr))
190+
->method('setLockExpires')
191+
->with($setLockExpiresValue);
149192

150193
$this->authentication->processAuthenticationFailure($customerId);
151194
}
152195

153-
/**
154-
* @return void
155-
*/
156-
public function testCustomerHasFailedMaxNumberOfAttempts()
196+
public function processAuthenticationFailureDataProvider()
157197
{
158-
$customerId = 1;
159-
$date = new \DateTime();
160-
$date->modify('-500 second');
161-
$formattedDate = $date->format('Y-m-d H:i:s');
162-
$this->backendConfigMock->expects($this->exactly(2))
163-
->method('getValue')
164-
->withConsecutive(
165-
[\Magento\Customer\Model\Authentication::LOCKOUT_THRESHOLD_PATH],
166-
[\Magento\Customer\Model\Authentication::MAX_FAILURES_PATH]
167-
)
168-
->willReturnOnConsecutiveCalls(10, 5);
169-
170-
$this->customerRegistryMock->expects($this->once())
171-
->method('retrieveSecureData')
172-
->with($customerId)
173-
->willReturn($this->customerSecure);
174-
$customerMock = $this->getMockBuilder(CustomerInterface::class)
175-
->disableOriginalConstructor()
176-
->getMock();
177-
$this->customerRepositoryMock->expects($this->once())
178-
->method('getById')
179-
->with($customerId)
180-
->willReturn($customerMock);
181-
$this->customerRepositoryMock->expects($this->once())
182-
->method('save')
183-
->with($customerMock);
184-
185-
$this->customerSecure->expects($this->once())->method('getFailuresNum')->willReturn(5);
186-
$this->customerSecure->expects($this->once())
187-
->method('getFirstFailure')
188-
->willReturn($formattedDate);
189-
$this->customerSecure->expects($this->once())->method('setLockExpires');
190-
$this->customerSecure->expects($this->once())->method('setFailuresNum');
191-
192-
$this->authentication->processAuthenticationFailure($customerId);
198+
return [
199+
'first attempt' => [0, null, null, 1, 1, 1, 1, null],
200+
'not locked' => [3, '-400 second', null, 1, 4, 0, 0, null],
201+
'lock expired' => [5, '-400 second', '-100 second', 1, 1, 1, 1, null],
202+
'max attempt' => [4, '-400 second', null, 1, 5, 0, 1, 'formattedDate'],
203+
];
193204
}
194205

195-
/**
196-
* @return void
197-
*/
198-
public function testProcessUnlockData()
206+
public function testUnlock()
199207
{
200208
$customerId = 1;
201209
$this->customerRegistryMock->expects($this->once())
202210
->method('retrieveSecureData')
203211
->with($customerId)
204-
->willReturn($this->customerSecure);
212+
->willReturn($this->customerSecureMock);
205213
$customerMock = $this->getMockBuilder(CustomerInterface::class)
206214
->disableOriginalConstructor()
207215
->getMock();
@@ -212,9 +220,9 @@ public function testProcessUnlockData()
212220
$this->customerRepositoryMock->expects($this->once())
213221
->method('save')
214222
->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);
223+
$this->customerSecureMock->expects($this->once())->method('setFailuresNum')->with(0);
224+
$this->customerSecureMock->expects($this->once())->method('setFirstFailure')->with(null);
225+
$this->customerSecureMock->expects($this->once())->method('setLockExpires')->with(null);
218226
$this->authentication->unlock($customerId);
219227
}
220228

@@ -229,7 +237,7 @@ public function validatePasswordAndLockStatusDataProvider()
229237
/**
230238
* @return void
231239
*/
232-
public function testCheckIfLocked()
240+
public function testIsLocked()
233241
{
234242
$customerId = 7;
235243

@@ -250,7 +258,7 @@ public function testCheckIfLocked()
250258
* @param bool $result
251259
* @dataProvider validateCustomerPassword
252260
*/
253-
public function testValidateCustomerPassword($result)
261+
public function testAuthenticate($result)
254262
{
255263
$customerId = 7;
256264
$password = '1234567';
@@ -267,18 +275,18 @@ public function testValidateCustomerPassword($result)
267275
->method('getById')
268276
->willReturn($customerMock);
269277

270-
$this->customerSecure->expects($this->any())
278+
$this->customerSecureMock->expects($this->any())
271279
->method('getId')
272280
->willReturn($customerId);
273281

274-
$this->customerSecure->expects($this->once())
282+
$this->customerSecureMock->expects($this->once())
275283
->method('getPasswordHash')
276284
->willReturn($hash);
277285

278286
$this->customerRegistryMock->expects($this->any())
279287
->method('retrieveSecureData')
280288
->with($customerId)
281-
->willReturn($this->customerSecure);
289+
->willReturn($this->customerSecureMock);
282290

283291
$this->encryptorMock->expects($this->once())
284292
->method('validateHash')
@@ -295,14 +303,14 @@ public function testValidateCustomerPassword($result)
295303
[\Magento\Customer\Model\Authentication::MAX_FAILURES_PATH]
296304
)
297305
->willReturnOnConsecutiveCalls(1, 1);
298-
$this->customerSecure->expects($this->once())
306+
$this->customerSecureMock->expects($this->once())
299307
->method('isCustomerLocked')
300308
->willReturn(false);
301309

302310
$this->customerRegistryMock->expects($this->once())
303311
->method('retrieve')
304312
->with($customerId)
305-
->willReturn($this->customerSecure);
313+
->willReturn($this->customerSecureMock);
306314

307315
$this->customerRepositoryMock->expects($this->once())
308316
->method('save')

0 commit comments

Comments
 (0)