security-package: Added try/catch to Magento\ReCaptchaUi\Model\RequesttHandler

ReCaptchaContact/Test/Integration/ContactFormTest.php

@@ -9,7 +9,6 @@
 
 use Magento\Framework\App\Request\Http as HttpRequest;
 use Magento\Framework\Data\Form\FormKey;
-use Magento\Framework\Exception\InputException;
 use Magento\Framework\Message\MessageInterface;
 use Magento\Framework\Validation\ValidationResult;
 use Magento\ReCaptchaUi\Model\CaptchaResponseResolverInterface;
@@ -43,7 +42,7 @@ class ContactFormTest extends AbstractController
     /**
      * @inheritDoc
      */
-    protected function setUp()
+    protected function setUp(): void
     {
         parent::setUp();
         $this->formKey = $this->_objectManager->get(FormKey::class);
@@ -62,7 +61,7 @@ protected function setUp()
      * @magentoConfigFixture base_website recaptcha_frontend/type_invisible/public_key test_public_key
      * @magentoConfigFixture base_website recaptcha_frontend/type_invisible/private_key test_private_key
      */
-    public function testGetRequestIfReCaptchaIsDisabled()
+    public function testGetRequestIfReCaptchaIsDisabled(): void
     {
         $this->setConfig(false, 'test_public_key', 'test_private_key');
 
@@ -76,7 +75,7 @@ public function testGetRequestIfReCaptchaIsDisabled()
      * It's  needed for proper work of "ifconfig" in layout during tests running
      * @magentoConfigFixture default_store recaptcha_frontend/type_for/contact invisible
      */
-    public function testGetRequestIfReCaptchaKeysAreNotConfigured()
+    public function testGetRequestIfReCaptchaKeysAreNotConfigured(): void
     {
         $this->setConfig(true, null, null);
 
@@ -92,7 +91,7 @@ public function testGetRequestIfReCaptchaKeysAreNotConfigured()
      * It's  needed for proper work of "ifconfig" in layout during tests running
      * @magentoConfigFixture default_store recaptcha_frontend/type_for/contact invisible
      */
-    public function testGetRequestIfReCaptchaIsEnabled()
+    public function testGetRequestIfReCaptchaIsEnabled(): void
     {
         $this->setConfig(true, 'test_public_key', 'test_private_key');
 
@@ -104,22 +103,22 @@ public function testGetRequestIfReCaptchaIsEnabled()
      * @magentoConfigFixture base_website recaptcha_frontend/type_invisible/public_key test_public_key
      * @magentoConfigFixture base_website recaptcha_frontend/type_invisible/private_key test_private_key
      */
-    public function testPostRequestIfReCaptchaIsDisabled()
+    public function testPostRequestIfReCaptchaIsDisabled(): void
     {
         $this->setConfig(false, 'test_public_key', 'test_private_key');
 
-        $this->checkPostResponse(true);
+        $this->checkSuccessfulPostResponse();
     }
 
     /**
      * @magentoConfigFixture default_store customer/captcha/enable 0
      * @magentoConfigFixture base_website recaptcha_frontend/type_for/contact invisible
      */
-    public function testPostRequestIfReCaptchaKeysAreNotConfigured()
+    public function testPostRequestIfReCaptchaKeysAreNotConfigured(): void
     {
         $this->setConfig(true, null, null);
 
-        $this->checkPostResponse(true);
+        $this->checkSuccessfulPostResponse();
     }
 
     /**
@@ -128,13 +127,12 @@ public function testPostRequestIfReCaptchaKeysAreNotConfigured()
      * @magentoConfigFixture base_website recaptcha_frontend/type_invisible/private_key test_private_key
      * @magentoConfigFixture base_website recaptcha_frontend/type_for/contact invisible
      */
-    public function testPostRequestWithSuccessfulReCaptchaValidation()
+    public function testPostRequestWithSuccessfulReCaptchaValidation(): void
     {
         $this->setConfig(true, 'test_public_key', 'test_private_key');
         $this->captchaValidationResultMock->expects($this->once())->method('isValid')->willReturn(true);
 
-        $this->checkPostResponse(
-            true,
+        $this->checkSuccessfulPostResponse(
             [CaptchaResponseResolverInterface::PARAM_RECAPTCHA => 'test']
         );
     }
@@ -145,14 +143,11 @@ public function testPostRequestWithSuccessfulReCaptchaValidation()
      * @magentoConfigFixture base_website recaptcha_frontend/type_invisible/private_key test_private_key
      * @magentoConfigFixture base_website recaptcha_frontend/type_for/contact invisible
      */
-    public function testPostRequestIfReCaptchaParameterIsMissed()
+    public function testPostRequestIfReCaptchaParameterIsMissed(): void
     {
         $this->setConfig(true, 'test_public_key', 'test_private_key');
 
-        $this->expectException(InputException::class);
-        $this->expectExceptionMessage('Can not resolve reCAPTCHA parameter.');
-
-        $this->checkPostResponse(false);
+        $this->checkFailedPostResponse();
     }
 
     /**
@@ -161,21 +156,21 @@ public function testPostRequestIfReCaptchaParameterIsMissed()
      * @magentoConfigFixture base_website recaptcha_frontend/type_invisible/private_key test_private_key
      * @magentoConfigFixture base_website recaptcha_frontend/type_for/contact invisible
      */
-    public function testPostRequestWithFailedReCaptchaValidation()
+    public function testPostRequestWithFailedReCaptchaValidation(): void
     {
         $this->setConfig(true, 'test_public_key', 'test_private_key');
         $this->captchaValidationResultMock->expects($this->once())->method('isValid')->willReturn(false);
 
-        $this->checkPostResponse(
-            false,
+        $this->checkFailedPostResponse(
             [CaptchaResponseResolverInterface::PARAM_RECAPTCHA => 'test']
         );
     }
 
     /**
      * @param bool $shouldContainReCaptcha
+     * @return void
      */
-    private function checkSuccessfulGetResponse($shouldContainReCaptcha = false)
+    private function checkSuccessfulGetResponse($shouldContainReCaptcha = false): void
     {
         $this->dispatch('contact/index');
         $content = $this->getResponse()->getBody();
@@ -190,10 +185,41 @@ private function checkSuccessfulGetResponse($shouldContainReCaptcha = false)
     }
 
     /**
-     * @param bool $isSuccessfulRequest
      * @param array $postValues
+     * @return void
+     */
+    private function checkSuccessfulPostResponse(array $postValues = []): void
+    {
+        $this->makePostRequest($postValues);
+
+        $this->assertSessionMessages(
+            self::contains(
+                "Thanks for contacting us with your comments and questions. We'll respond to you very soon."
+            ),
+            MessageInterface::TYPE_SUCCESS
+        );
+        self::assertEmpty($this->getSessionMessages(MessageInterface::TYPE_ERROR));
+    }
+
+    /**
+     * @param array $postValues
+     * @return void
+     */
+    private function checkFailedPostResponse(array $postValues = []): void
+    {
+        $this->makePostRequest($postValues);
+
+        $this->assertSessionMessages(
+            $this->equalTo(['reCAPTCHA verification failed']),
+            MessageInterface::TYPE_ERROR
+        );
+    }
+
+    /**
+     * @param array $postValues
+     * @return void
      */
-    private function checkPostResponse(bool $isSuccessfulRequest, array $postValues = [])
+    private function makePostRequest(array $postValues = []): void
     {
         $this->getRequest()
             ->setMethod(HttpRequest::METHOD_POST)
@@ -208,29 +234,14 @@ private function checkPostResponse(bool $isSuccessfulRequest, array $postValues
             ));
 
         $this->dispatch('contact/index/post');
-
         $this->assertRedirect(self::stringContains('contact/index'));
-
-        if ($isSuccessfulRequest) {
-            $this->assertSessionMessages(
-                self::contains(
-                    "Thanks for contacting us with your comments and questions. We'll respond to you very soon."
-                ),
-                MessageInterface::TYPE_SUCCESS
-            );
-            self::assertEmpty($this->getSessionMessages(MessageInterface::TYPE_ERROR));
-        } else {
-            $this->assertSessionMessages(
-                $this->equalTo(['reCAPTCHA verification failed']),
-                MessageInterface::TYPE_ERROR
-            );
-        }
     }
 
     /**
      * @param bool $isEnabled
      * @param string|null $public
      * @param string|null $private
+     * @return void
      */
     private function setConfig(bool $isEnabled, ?string $public, ?string $private): void
     {
@@ -250,4 +261,23 @@ private function setConfig(bool $isEnabled, ?string $public, ?string $private):
             ScopeInterface::SCOPE_WEBSITE
         );
     }
+
+    public function tearDown(): void
+    {
+        $this->mutableScopeConfig->setValue(
+            'recaptcha_frontend/type_for/contact',
+            null,
+            ScopeInterface::SCOPE_WEBSITE
+        );
+        $this->mutableScopeConfig->setValue(
+            'recaptcha_frontend/type_invisible/public_key',
+            null,
+            ScopeInterface::SCOPE_WEBSITE
+        );
+        $this->mutableScopeConfig->setValue(
+            'recaptcha_frontend/type_invisible/private_key',
+            null,
+            ScopeInterface::SCOPE_WEBSITE
+        );
+    }
 }

ReCaptchaCustomer/Observer/AjaxLoginObserver.php

@@ -9,14 +9,17 @@
 
 use Magento\Framework\App\Action\Action;
 use Magento\Framework\App\ActionFlag;
+use Magento\Framework\App\ResponseInterface;
 use Magento\Framework\Event\Observer;
 use Magento\Framework\Event\ObserverInterface;
+use Magento\Framework\Exception\InputException;
 use Magento\Framework\Exception\LocalizedException;
 use Magento\Framework\Serialize\SerializerInterface;
-use Magento\ReCaptchaUi\Model\IsCaptchaEnabledInterface;
 use Magento\ReCaptchaUi\Model\CaptchaResponseResolverInterface;
+use Magento\ReCaptchaUi\Model\IsCaptchaEnabledInterface;
 use Magento\ReCaptchaUi\Model\ValidationConfigResolverInterface;
 use Magento\ReCaptchaValidationApi\Api\ValidatorInterface;
+use Psr\Log\LoggerInterface;
 
 /**
  * AjaxLoginObserver
@@ -53,28 +56,36 @@ class AjaxLoginObserver implements ObserverInterface
      */
     private $isCaptchaEnabled;
 
+    /**
+     * @var LoggerInterface
+     */
+    private $logger;
+
     /**
      * @param CaptchaResponseResolverInterface $captchaResponseResolver
      * @param ValidationConfigResolverInterface $validationConfigResolver
      * @param ValidatorInterface $captchaValidator
      * @param ActionFlag $actionFlag
      * @param SerializerInterface $serializer
      * @param IsCaptchaEnabledInterface $isCaptchaEnabled
+     * @param LoggerInterface $logger
      */
     public function __construct(
         CaptchaResponseResolverInterface $captchaResponseResolver,
         ValidationConfigResolverInterface $validationConfigResolver,
         ValidatorInterface $captchaValidator,
         ActionFlag $actionFlag,
         SerializerInterface $serializer,
-        IsCaptchaEnabledInterface $isCaptchaEnabled
+        IsCaptchaEnabledInterface $isCaptchaEnabled,
+        LoggerInterface $logger
     ) {
         $this->captchaResponseResolver = $captchaResponseResolver;
         $this->validationConfigResolver = $validationConfigResolver;
         $this->captchaValidator = $captchaValidator;
         $this->actionFlag = $actionFlag;
         $this->serializer = $serializer;
         $this->isCaptchaEnabled = $isCaptchaEnabled;
+        $this->logger = $logger;
     }
 
     /**
@@ -91,19 +102,36 @@ public function execute(Observer $observer): void
             $request = $controller->getRequest();
             $response = $controller->getResponse();
 
-            $reCaptchaResponse = $this->captchaResponseResolver->resolve($request);
             $validationConfig = $this->validationConfigResolver->get($key);
 
+            try {
+                $reCaptchaResponse = $this->captchaResponseResolver->resolve($request);
+            } catch (InputException $e) {
+                $this->logger->error($e);
+                $this->processError($response, $validationConfig->getValidationFailureMessage());
+                return;
+            }
+
             $validationResult = $this->captchaValidator->isValid($reCaptchaResponse, $validationConfig);
             if (false === $validationResult->isValid()) {
-                $this->actionFlag->set('', Action::FLAG_NO_DISPATCH, true);
-
-                $jsonPayload = $this->serializer->serialize([
-                    'errors' => true,
-                    'message' => $validationConfig->getValidationFailureMessage(),
-                ]);
-                $response->representJson($jsonPayload);
+                $this->processError($response, $validationConfig->getValidationFailureMessage());
             }
         }
     }
+
+    /**
+     * @param ResponseInterface $response
+     * @param string $message
+     * @return void
+     */
+    private function processError(ResponseInterface $response, string $message): void
+    {
+        $this->actionFlag->set('', Action::FLAG_NO_DISPATCH, true);
+
+        $jsonPayload = $this->serializer->serialize([
+            'errors' => true,
+            'message' => $message,
+        ]);
+        $response->representJson($jsonPayload);
+    }
 }