Skip to content

Commit 12e8fd2

Browse files
naydavnaydav
authored
Merge pull request #175 from magento/security-package/try-catch-for-handler
security-package: Added try/catch to Magento\ReCaptchaUi\Model\RequesttHandler
2 parents b209b88 + d330813 commit 12e8fd2

File tree

14 files changed

+788
-461
lines changed

14 files changed

+788
-461
lines changed

‎ReCaptchaContact/Test/Integration/ContactFormTest.php‎

Lines changed: 69 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,6 @@
99

1010
use Magento\Framework\App\Request\Http as HttpRequest;
1111
use Magento\Framework\Data\Form\FormKey;
12-
use Magento\Framework\Exception\InputException;
1312
use Magento\Framework\Message\MessageInterface;
1413
use Magento\Framework\Validation\ValidationResult;
1514
use Magento\ReCaptchaUi\Model\CaptchaResponseResolverInterface;
@@ -43,7 +42,7 @@ class ContactFormTest extends AbstractController
4342
/**
4443
* @inheritDoc
4544
*/
46-
protected function setUp()
45+
protected function setUp(): void
4746
{
4847
parent::setUp();
4948
$this->formKey = $this->_objectManager->get(FormKey::class);
@@ -62,7 +61,7 @@ protected function setUp()
6261
* @magentoConfigFixture base_website recaptcha_frontend/type_invisible/public_key test_public_key
6362
* @magentoConfigFixture base_website recaptcha_frontend/type_invisible/private_key test_private_key
6463
*/
65-
public function testGetRequestIfReCaptchaIsDisabled()
64+
public function testGetRequestIfReCaptchaIsDisabled(): void
6665
{
6766
$this->setConfig(false, 'test_public_key', 'test_private_key');
6867

@@ -76,7 +75,7 @@ public function testGetRequestIfReCaptchaIsDisabled()
7675
* It's needed for proper work of "ifconfig" in layout during tests running
7776
* @magentoConfigFixture default_store recaptcha_frontend/type_for/contact invisible
7877
*/
79-
public function testGetRequestIfReCaptchaKeysAreNotConfigured()
78+
public function testGetRequestIfReCaptchaKeysAreNotConfigured(): void
8079
{
8180
$this->setConfig(true, null, null);
8281

@@ -92,7 +91,7 @@ public function testGetRequestIfReCaptchaKeysAreNotConfigured()
9291
* It's needed for proper work of "ifconfig" in layout during tests running
9392
* @magentoConfigFixture default_store recaptcha_frontend/type_for/contact invisible
9493
*/
95-
public function testGetRequestIfReCaptchaIsEnabled()
94+
public function testGetRequestIfReCaptchaIsEnabled(): void
9695
{
9796
$this->setConfig(true, 'test_public_key', 'test_private_key');
9897

@@ -104,22 +103,22 @@ public function testGetRequestIfReCaptchaIsEnabled()
104103
* @magentoConfigFixture base_website recaptcha_frontend/type_invisible/public_key test_public_key
105104
* @magentoConfigFixture base_website recaptcha_frontend/type_invisible/private_key test_private_key
106105
*/
107-
public function testPostRequestIfReCaptchaIsDisabled()
106+
public function testPostRequestIfReCaptchaIsDisabled(): void
108107
{
109108
$this->setConfig(false, 'test_public_key', 'test_private_key');
110109

111-
$this->checkPostResponse(true);
110+
$this->checkSuccessfulPostResponse();
112111
}
113112

114113
/**
115114
* @magentoConfigFixture default_store customer/captcha/enable 0
116115
* @magentoConfigFixture base_website recaptcha_frontend/type_for/contact invisible
117116
*/
118-
public function testPostRequestIfReCaptchaKeysAreNotConfigured()
117+
public function testPostRequestIfReCaptchaKeysAreNotConfigured(): void
119118
{
120119
$this->setConfig(true, null, null);
121120

122-
$this->checkPostResponse(true);
121+
$this->checkSuccessfulPostResponse();
123122
}
124123

125124
/**
@@ -128,13 +127,12 @@ public function testPostRequestIfReCaptchaKeysAreNotConfigured()
128127
* @magentoConfigFixture base_website recaptcha_frontend/type_invisible/private_key test_private_key
129128
* @magentoConfigFixture base_website recaptcha_frontend/type_for/contact invisible
130129
*/
131-
public function testPostRequestWithSuccessfulReCaptchaValidation()
130+
public function testPostRequestWithSuccessfulReCaptchaValidation(): void
132131
{
133132
$this->setConfig(true, 'test_public_key', 'test_private_key');
134133
$this->captchaValidationResultMock->expects($this->once())->method('isValid')->willReturn(true);
135134

136-
$this->checkPostResponse(
137-
true,
135+
$this->checkSuccessfulPostResponse(
138136
[CaptchaResponseResolverInterface::PARAM_RECAPTCHA => 'test']
139137
);
140138
}
@@ -145,14 +143,11 @@ public function testPostRequestWithSuccessfulReCaptchaValidation()
145143
* @magentoConfigFixture base_website recaptcha_frontend/type_invisible/private_key test_private_key
146144
* @magentoConfigFixture base_website recaptcha_frontend/type_for/contact invisible
147145
*/
148-
public function testPostRequestIfReCaptchaParameterIsMissed()
146+
public function testPostRequestIfReCaptchaParameterIsMissed(): void
149147
{
150148
$this->setConfig(true, 'test_public_key', 'test_private_key');
151149

152-
$this->expectException(InputException::class);
153-
$this->expectExceptionMessage('Can not resolve reCAPTCHA parameter.');
154-
155-
$this->checkPostResponse(false);
150+
$this->checkFailedPostResponse();
156151
}
157152

158153
/**
@@ -161,21 +156,21 @@ public function testPostRequestIfReCaptchaParameterIsMissed()
161156
* @magentoConfigFixture base_website recaptcha_frontend/type_invisible/private_key test_private_key
162157
* @magentoConfigFixture base_website recaptcha_frontend/type_for/contact invisible
163158
*/
164-
public function testPostRequestWithFailedReCaptchaValidation()
159+
public function testPostRequestWithFailedReCaptchaValidation(): void
165160
{
166161
$this->setConfig(true, 'test_public_key', 'test_private_key');
167162
$this->captchaValidationResultMock->expects($this->once())->method('isValid')->willReturn(false);
168163

169-
$this->checkPostResponse(
170-
false,
164+
$this->checkFailedPostResponse(
171165
[CaptchaResponseResolverInterface::PARAM_RECAPTCHA => 'test']
172166
);
173167
}
174168

175169
/**
176170
* @param bool $shouldContainReCaptcha
171+
* @return void
177172
*/
178-
private function checkSuccessfulGetResponse($shouldContainReCaptcha = false)
173+
private function checkSuccessfulGetResponse($shouldContainReCaptcha = false): void
179174
{
180175
$this->dispatch('contact/index');
181176
$content = $this->getResponse()->getBody();
@@ -190,10 +185,41 @@ private function checkSuccessfulGetResponse($shouldContainReCaptcha = false)
190185
}
191186

192187
/**
193-
* @param bool $isSuccessfulRequest
194188
* @param array $postValues
189+
* @return void
190+
*/
191+
private function checkSuccessfulPostResponse(array $postValues = []): void
192+
{
193+
$this->makePostRequest($postValues);
194+
195+
$this->assertSessionMessages(
196+
self::contains(
197+
"Thanks for contacting us with your comments and questions. We'll respond to you very soon."
198+
),
199+
MessageInterface::TYPE_SUCCESS
200+
);
201+
self::assertEmpty($this->getSessionMessages(MessageInterface::TYPE_ERROR));
202+
}
203+
204+
/**
205+
* @param array $postValues
206+
* @return void
207+
*/
208+
private function checkFailedPostResponse(array $postValues = []): void
209+
{
210+
$this->makePostRequest($postValues);
211+
212+
$this->assertSessionMessages(
213+
$this->equalTo(['reCAPTCHA verification failed']),
214+
MessageInterface::TYPE_ERROR
215+
);
216+
}
217+
218+
/**
219+
* @param array $postValues
220+
* @return void
195221
*/
196-
private function checkPostResponse(bool $isSuccessfulRequest, array $postValues = [])
222+
private function makePostRequest(array $postValues = []): void
197223
{
198224
$this->getRequest()
199225
->setMethod(HttpRequest::METHOD_POST)
@@ -208,29 +234,14 @@ private function checkPostResponse(bool $isSuccessfulRequest, array $postValues
208234
));
209235

210236
$this->dispatch('contact/index/post');
211-
212237
$this->assertRedirect(self::stringContains('contact/index'));
213-
214-
if ($isSuccessfulRequest) {
215-
$this->assertSessionMessages(
216-
self::contains(
217-
"Thanks for contacting us with your comments and questions. We'll respond to you very soon."
218-
),
219-
MessageInterface::TYPE_SUCCESS
220-
);
221-
self::assertEmpty($this->getSessionMessages(MessageInterface::TYPE_ERROR));
222-
} else {
223-
$this->assertSessionMessages(
224-
$this->equalTo(['reCAPTCHA verification failed']),
225-
MessageInterface::TYPE_ERROR
226-
);
227-
}
228238
}
229239

230240
/**
231241
* @param bool $isEnabled
232242
* @param string|null $public
233243
* @param string|null $private
244+
* @return void
234245
*/
235246
private function setConfig(bool $isEnabled, ?string $public, ?string $private): void
236247
{
@@ -250,4 +261,23 @@ private function setConfig(bool $isEnabled, ?string $public, ?string $private):
250261
ScopeInterface::SCOPE_WEBSITE
251262
);
252263
}
264+
265+
public function tearDown(): void
266+
{
267+
$this->mutableScopeConfig->setValue(
268+
'recaptcha_frontend/type_for/contact',
269+
null,
270+
ScopeInterface::SCOPE_WEBSITE
271+
);
272+
$this->mutableScopeConfig->setValue(
273+
'recaptcha_frontend/type_invisible/public_key',
274+
null,
275+
ScopeInterface::SCOPE_WEBSITE
276+
);
277+
$this->mutableScopeConfig->setValue(
278+
'recaptcha_frontend/type_invisible/private_key',
279+
null,
280+
ScopeInterface::SCOPE_WEBSITE
281+
);
282+
}
253283
}

‎ReCaptchaCustomer/Observer/AjaxLoginObserver.php‎

Lines changed: 38 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -9,14 +9,17 @@
99

1010
use Magento\Framework\App\Action\Action;
1111
use Magento\Framework\App\ActionFlag;
12+
use Magento\Framework\App\ResponseInterface;
1213
use Magento\Framework\Event\Observer;
1314
use Magento\Framework\Event\ObserverInterface;
15+
use Magento\Framework\Exception\InputException;
1416
use Magento\Framework\Exception\LocalizedException;
1517
use Magento\Framework\Serialize\SerializerInterface;
16-
use Magento\ReCaptchaUi\Model\IsCaptchaEnabledInterface;
1718
use Magento\ReCaptchaUi\Model\CaptchaResponseResolverInterface;
19+
use Magento\ReCaptchaUi\Model\IsCaptchaEnabledInterface;
1820
use Magento\ReCaptchaUi\Model\ValidationConfigResolverInterface;
1921
use Magento\ReCaptchaValidationApi\Api\ValidatorInterface;
22+
use Psr\Log\LoggerInterface;
2023

2124
/**
2225
* AjaxLoginObserver
@@ -53,28 +56,36 @@ class AjaxLoginObserver implements ObserverInterface
5356
*/
5457
private $isCaptchaEnabled;
5558

59+
/**
60+
* @var LoggerInterface
61+
*/
62+
private $logger;
63+
5664
/**
5765
* @param CaptchaResponseResolverInterface $captchaResponseResolver
5866
* @param ValidationConfigResolverInterface $validationConfigResolver
5967
* @param ValidatorInterface $captchaValidator
6068
* @param ActionFlag $actionFlag
6169
* @param SerializerInterface $serializer
6270
* @param IsCaptchaEnabledInterface $isCaptchaEnabled
71+
* @param LoggerInterface $logger
6372
*/
6473
public function __construct(
6574
CaptchaResponseResolverInterface $captchaResponseResolver,
6675
ValidationConfigResolverInterface $validationConfigResolver,
6776
ValidatorInterface $captchaValidator,
6877
ActionFlag $actionFlag,
6978
SerializerInterface $serializer,
70-
IsCaptchaEnabledInterface $isCaptchaEnabled
79+
IsCaptchaEnabledInterface $isCaptchaEnabled,
80+
LoggerInterface $logger
7181
) {
7282
$this->captchaResponseResolver = $captchaResponseResolver;
7383
$this->validationConfigResolver = $validationConfigResolver;
7484
$this->captchaValidator = $captchaValidator;
7585
$this->actionFlag = $actionFlag;
7686
$this->serializer = $serializer;
7787
$this->isCaptchaEnabled = $isCaptchaEnabled;
88+
$this->logger = $logger;
7889
}
7990

8091
/**
@@ -91,19 +102,36 @@ public function execute(Observer $observer): void
91102
$request = $controller->getRequest();
92103
$response = $controller->getResponse();
93104

94-
$reCaptchaResponse = $this->captchaResponseResolver->resolve($request);
95105
$validationConfig = $this->validationConfigResolver->get($key);
96106

107+
try {
108+
$reCaptchaResponse = $this->captchaResponseResolver->resolve($request);
109+
} catch (InputException $e) {
110+
$this->logger->error($e);
111+
$this->processError($response, $validationConfig->getValidationFailureMessage());
112+
return;
113+
}
114+
97115
$validationResult = $this->captchaValidator->isValid($reCaptchaResponse, $validationConfig);
98116
if (false === $validationResult->isValid()) {
99-
$this->actionFlag->set('', Action::FLAG_NO_DISPATCH, true);
100-
101-
$jsonPayload = $this->serializer->serialize([
102-
'errors' => true,
103-
'message' => $validationConfig->getValidationFailureMessage(),
104-
]);
105-
$response->representJson($jsonPayload);
117+
$this->processError($response, $validationConfig->getValidationFailureMessage());
106118
}
107119
}
108120
}
121+
122+
/**
123+
* @param ResponseInterface $response
124+
* @param string $message
125+
* @return void
126+
*/
127+
private function processError(ResponseInterface $response, string $message): void
128+
{
129+
$this->actionFlag->set('', Action::FLAG_NO_DISPATCH, true);
130+
131+
$jsonPayload = $this->serializer->serialize([
132+
'errors' => true,
133+
'message' => $message,
134+
]);
135+
$response->representJson($jsonPayload);
136+
}
109137
}

0 commit comments

Comments
 (0)