Skip to content

Commit c6dcbb0

Browse files
author
Lysenko Olexandr
authored
Merge pull request #2655 from magento-chaika/pr1
[chaika] bugfixes
2 parents 00c6b48 + 5899dc1 commit c6dcbb0

File tree

4 files changed

+91
-38
lines changed

4 files changed

+91
-38
lines changed

‎app/code/Magento/Captcha/Model/Customer/Plugin/AjaxLogin.php‎

Lines changed: 27 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
* Copyright © Magento, Inc. All rights reserved.
44
* See COPYING.txt for license details.
55
*/
6+
67
namespace Magento\Captcha\Model\Customer\Plugin;
78

89
use Magento\Captcha\Helper\Data as CaptchaHelper;
@@ -81,27 +82,37 @@ public function aroundExecute(
8182
if ($content) {
8283
$loginParams = $this->serializer->unserialize($content);
8384
}
84-
$username = isset($loginParams['username']) ? $loginParams['username'] : null;
85-
$captchaString = isset($loginParams[$captchaInputName]) ? $loginParams[$captchaInputName] : null;
86-
$loginFormId = isset($loginParams[$captchaFormIdField]) ? $loginParams[$captchaFormIdField] : null;
85+
$username = $loginParams['username'] ?? null;
86+
$captchaString = $loginParams[$captchaInputName] ?? null;
87+
$loginFormId = $loginParams[$captchaFormIdField] ?? null;
8788

88-
foreach ($this->formIds as $formId) {
89-
$captchaModel = $this->helper->getCaptcha($formId);
90-
if ($captchaModel->isRequired($username) && !in_array($loginFormId, $this->formIds)) {
91-
$resultJson = $this->resultJsonFactory->create();
92-
return $resultJson->setData(['errors' => true, 'message' => __('Provided form does not exist')]);
93-
}
89+
if (!in_array($loginFormId, $this->formIds) && $this->helper->getCaptcha($loginFormId)->isRequired($username)) {
90+
return $this->returnJsonError(__('Provided form does not exist'));
91+
}
9492

95-
if ($formId == $loginFormId) {
96-
$captchaModel->logAttempt($username);
97-
if (!$captchaModel->isCorrect($captchaString)) {
98-
$this->sessionManager->setUsername($username);
99-
/** @var \Magento\Framework\Controller\Result\Json $resultJson */
100-
$resultJson = $this->resultJsonFactory->create();
101-
return $resultJson->setData(['errors' => true, 'message' => __('Incorrect CAPTCHA')]);
93+
foreach ($this->formIds as $formId) {
94+
if ($formId === $loginFormId) {
95+
$captchaModel = $this->helper->getCaptcha($formId);
96+
if ($captchaModel->isRequired($username)) {
97+
$captchaModel->logAttempt($username);
98+
if (!$captchaModel->isCorrect($captchaString)) {
99+
$this->sessionManager->setUsername($username);
100+
return $this->returnJsonError(__('Incorrect CAPTCHA'));
101+
}
102102
}
103103
}
104104
}
105105
return $proceed();
106106
}
107+
108+
/**
109+
*
110+
* @param \Magento\Framework\Phrase $phrase
111+
* @return \Magento\Framework\Controller\Result\Json
112+
*/
113+
private function returnJsonError(\Magento\Framework\Phrase $phrase): \Magento\Framework\Controller\Result\Json
114+
{
115+
$resultJson = $this->resultJsonFactory->create();
116+
return $resultJson->setData(['errors' => true, 'message' => $phrase]);
117+
}
107118
}

‎app/code/Magento/Captcha/Model/DefaultModel.php‎

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

8+
use Magento\Captcha\Helper\Data;
9+
810
/**
911
* Implementation of \Zend\Captcha\Image
1012
*
@@ -29,7 +31,7 @@ class DefaultModel extends \Zend\Captcha\Image implements \Magento\Captcha\Model
2931
const DEFAULT_WORD_LENGTH_TO = 5;
3032

3133
/**
32-
* @var \Magento\Captcha\Helper\Data
34+
* @var Data
3335
* @since 100.2.0
3436
*/
3537
protected $captchaData;
@@ -125,8 +127,8 @@ public function getBlockName()
125127
*/
126128
public function isRequired($login = null)
127129
{
128-
if ($this->isUserAuth()
129-
&& !$this->isShownToLoggedInUser()
130+
if (($this->isUserAuth()
131+
&& !$this->isShownToLoggedInUser())
130132
|| !$this->isEnabled()
131133
|| !in_array(
132134
$this->formId,
@@ -431,12 +433,14 @@ public function getWordLen()
431433
*/
432434
private function isShowAlways()
433435
{
434-
if ((string)$this->captchaData->getConfig('mode') == \Magento\Captcha\Helper\Data::MODE_ALWAYS) {
436+
$captchaMode = (string)$this->captchaData->getConfig('mode');
437+
438+
if ($captchaMode === Data::MODE_ALWAYS) {
435439
return true;
436440
}
437441

438-
if ((string)$this->captchaData->getConfig('mode') == \Magento\Captcha\Helper\Data::MODE_AFTER_FAIL
439-
&& $this->getAllowedAttemptsForSameLogin() == 0
442+
if ($captchaMode === Data::MODE_AFTER_FAIL
443+
&& $this->getAllowedAttemptsForSameLogin() === 0
440444
) {
441445
return true;
442446
}

‎app/code/Magento/Captcha/Test/Unit/Model/Customer/Plugin/AjaxLoginTest.php‎

Lines changed: 53 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -3,22 +3,23 @@
33
* Copyright © Magento, Inc. All rights reserved.
44
* See COPYING.txt for license details.
55
*/
6+
67
namespace Magento\Captcha\Test\Unit\Model\Customer\Plugin;
78

89
class AjaxLoginTest extends \PHPUnit\Framework\TestCase
910
{
1011
/**
11-
* @var \PHPUnit_Framework_MockObject_MockObject
12+
* @var \PHPUnit_Framework_MockObject_MockObject | \Magento\Checkout\Model\Session
1213
*/
1314
protected $sessionManagerMock;
1415

1516
/**
16-
* @var \PHPUnit_Framework_MockObject_MockObject
17+
* @var \PHPUnit_Framework_MockObject_MockObject | \Magento\Captcha\Helper\Data
1718
*/
1819
protected $captchaHelperMock;
1920

2021
/**
21-
* @var \PHPUnit_Framework_MockObject_MockObject
22+
* @var \PHPUnit_Framework_MockObject_MockObject | \Magento\Framework\Controller\Result\JsonFactory
2223
*/
2324
protected $jsonFactoryMock;
2425

@@ -38,12 +39,12 @@ class AjaxLoginTest extends \PHPUnit\Framework\TestCase
3839
protected $requestMock;
3940

4041
/**
41-
* @var \PHPUnit_Framework_MockObject_MockObject
42+
* @var \PHPUnit_Framework_MockObject_MockObject | \Magento\Customer\Controller\Ajax\Login
4243
*/
4344
protected $loginControllerMock;
4445

4546
/**
46-
* @var \PHPUnit_Framework_MockObject_MockObject
47+
* @var \PHPUnit_Framework_MockObject_MockObject | \Magento\Framework\Serialize\Serializer\Json
4748
*/
4849
protected $serializerMock;
4950

@@ -72,8 +73,12 @@ protected function setUp()
7273

7374
$this->loginControllerMock->expects($this->any())->method('getRequest')
7475
->will($this->returnValue($this->requestMock));
75-
$this->captchaHelperMock->expects($this->once())->method('getCaptcha')
76-
->with('user_login')->will($this->returnValue($this->captchaMock));
76+
77+
$this->captchaHelperMock
78+
->expects($this->exactly(1))
79+
->method('getCaptcha')
80+
->will($this->returnValue($this->captchaMock));
81+
7782
$this->formIds = ['user_login'];
7883
$this->serializerMock = $this->createMock(\Magento\Framework\Serialize\Serializer\Json::class);
7984

@@ -103,11 +108,18 @@ public function testAroundExecute()
103108
$this->captchaMock->expects($this->once())->method('logAttempt')->with($username);
104109
$this->captchaMock->expects($this->once())->method('isCorrect')->with($captchaString)
105110
->will($this->returnValue(true));
106-
$this->serializerMock->expects(($this->once()))->method('unserialize')->will($this->returnValue($requestData));
111+
$this->serializerMock->expects($this->once())->method('unserialize')->will($this->returnValue($requestData));
107112

108113
$closure = function () {
109114
return 'result';
110115
};
116+
117+
$this->captchaHelperMock
118+
->expects($this->exactly(1))
119+
->method('getCaptcha')
120+
->with('user_login')
121+
->will($this->returnValue($this->captchaMock));
122+
111123
$this->assertEquals('result', $this->model->aroundExecute($this->loginControllerMock, $closure));
112124
}
113125

@@ -128,18 +140,21 @@ public function testAroundExecuteIncorrectCaptcha()
128140
$this->captchaMock->expects($this->once())->method('logAttempt')->with($username);
129141
$this->captchaMock->expects($this->once())->method('isCorrect')
130142
->with($captchaString)->will($this->returnValue(false));
131-
$this->serializerMock->expects(($this->once()))->method('unserialize')->will($this->returnValue($requestData));
143+
$this->serializerMock->expects($this->once())->method('unserialize')->will($this->returnValue($requestData));
132144

133145
$this->sessionManagerMock->expects($this->once())->method('setUsername')->with($username);
134146
$this->jsonFactoryMock->expects($this->once())->method('create')
135147
->will($this->returnValue($this->resultJsonMock));
136148

137-
$this->resultJsonMock->expects($this->once())->method('setData')
138-
->with(['errors' => true, 'message' => __('Incorrect CAPTCHA')])->will($this->returnValue('response'));
149+
$this->resultJsonMock
150+
->expects($this->once())
151+
->method('setData')
152+
->with(['errors' => true, 'message' => __('Incorrect CAPTCHA')])
153+
->will($this->returnSelf());
139154

140155
$closure = function () {
141156
};
142-
$this->assertEquals('response', $this->model->aroundExecute($this->loginControllerMock, $closure));
157+
$this->assertEquals($this->resultJsonMock, $this->model->aroundExecute($this->loginControllerMock, $closure));
143158
}
144159

145160
/**
@@ -151,7 +166,7 @@ public function testAroundExecuteCaptchaIsNotRequired($username, $requestContent
151166
{
152167
$this->requestMock->expects($this->once())->method('getContent')
153168
->will($this->returnValue(json_encode($requestContent)));
154-
$this->serializerMock->expects(($this->once()))->method('unserialize')
169+
$this->serializerMock->expects($this->once())->method('unserialize')
155170
->will($this->returnValue($requestContent));
156171

157172
$this->captchaMock->expects($this->once())->method('isRequired')->with($username)
@@ -168,16 +183,39 @@ public function testAroundExecuteCaptchaIsNotRequired($username, $requestContent
168183
/**
169184
* @return array
170185
*/
171-
public function aroundExecuteCaptchaIsNotRequired()
186+
public function aroundExecuteCaptchaIsNotRequired(): array
172187
{
173188
return [
174189
[
175190
'username' => 'name',
176191
'requestData' => ['username' => 'name', 'captcha_string' => 'string'],
177192
],
193+
[
194+
'username' => 'name',
195+
'requestData' =>
196+
[
197+
'username' => 'name',
198+
'captcha_string' => 'string',
199+
'captcha_form_id' => $this->formIds[0]
200+
],
201+
],
178202
[
179203
'username' => null,
180-
'requestData' => ['captcha_string' => 'string'],
204+
'requestData' =>
205+
[
206+
'username' => null,
207+
'captcha_string' => 'string',
208+
'captcha_form_id' => $this->formIds[0]
209+
],
210+
],
211+
[
212+
'username' => 'name',
213+
'requestData' =>
214+
[
215+
'username' => 'name',
216+
'captcha_string' => 'string',
217+
'captcha_form_id' => null
218+
],
181219
],
182220
];
183221
}

‎app/code/Magento/Customer/Block/Account/Dashboard/Info.php‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -102,7 +102,7 @@ public function getSubscriptionObject()
102102
$this->_subscription = $this->_createSubscriber();
103103
$customer = $this->getCustomer();
104104
if ($customer) {
105-
$this->_subscription->loadByEmail($customer->getEmail());
105+
$this->_subscription->loadByCustomerId($customer->getId());
106106
}
107107
}
108108
return $this->_subscription;

0 commit comments

Comments
 (0)