From cbf8d88ec30cf6474cf1804bf4e1bd7d5a3d08de Mon Sep 17 00:00:00 2001 From: Alexander Steshuk Date: Mon, 2 Mar 2020 16:48:15 +0200 Subject: [PATCH 1/4] security-package/issues/86: Updated composer.json files. --- ReCaptcha/Model/CaptchaValidator.php | 42 +++++++++++++++++++++++----- ReCaptchaApi/Model/ErrorLabels.php | 35 +++++++++++++++++++++++ ReCaptchaApi/etc/di.xml | 21 ++++++++++++++ 3 files changed, 91 insertions(+), 7 deletions(-) create mode 100644 ReCaptchaApi/Model/ErrorLabels.php diff --git a/ReCaptcha/Model/CaptchaValidator.php b/ReCaptcha/Model/CaptchaValidator.php index 85670699..b41d53a1 100644 --- a/ReCaptcha/Model/CaptchaValidator.php +++ b/ReCaptcha/Model/CaptchaValidator.php @@ -7,16 +7,40 @@ namespace Magento\ReCaptcha\Model; -use Magento\Framework\Exception\LocalizedException; +use Magento\Framework\Validation\ValidationException; +use Magento\Framework\Validation\ValidationResultFactory; use Magento\ReCaptchaApi\Api\CaptchaValidatorInterface; use Magento\ReCaptchaApi\Api\Data\ValidationConfigInterface; use ReCaptcha\ReCaptcha; +use Magento\ReCaptchaApi\Model\ErrorLabels; /** * @inheritDoc */ class CaptchaValidator implements CaptchaValidatorInterface { + /** + * @var ErrorLabels + */ + private $errorLabels; + + /** + * @var ValidationResultFactory + */ + private $validationResultFactory; + + /** + * @param ValidationResultFactory $validationResultFactory + * @param ErrorLabels $errorLabels + */ + public function __construct( + ValidationResultFactory $validationResultFactory, + ErrorLabels $errorLabels + ) { + $this->validationResultFactory = $validationResultFactory; + $this->errorLabels = $errorLabels; + } + /** * @inheritdoc */ @@ -38,15 +62,19 @@ public function isValid( } $res = $reCaptcha->verify($reCaptchaResponse, $validationConfig->getRemoteIp()); - if (($validationConfig->getCaptchaType() === 'recaptcha_v3') && ($res->getScore() === null)) { - throw new LocalizedException(__('Internal error: Make sure you are using reCaptcha V3 api keys')); + $validationErrors = []; + + if ($res->getErrorCodes() && is_array($res->getErrorCodes()) && count($res->getErrorCodes()) > 0) { + foreach ($res->getErrorCodes() as $errorCode) { + $validationErrors[] = __($this->errorLabels->getErrorCodeLabel($errorCode)); + } } - if ($res->isSuccess()) { - return true; + $validationResult = $this->validationResultFactory->create(['errors' => $validationErrors]); + + if (false === $validationResult->isValid()) { + throw new ValidationException(__('Validation Failed.'), null, 0, $validationResult); } } - - return false; } } diff --git a/ReCaptchaApi/Model/ErrorLabels.php b/ReCaptchaApi/Model/ErrorLabels.php new file mode 100644 index 00000000..31542087 --- /dev/null +++ b/ReCaptchaApi/Model/ErrorLabels.php @@ -0,0 +1,35 @@ +errorCodes = $errorCodes; + } + + /** + * Get error label + * + * @param string $key + * @return string + */ + public function getErrorCodeLabel(string $key): string + { + return isset($this->errorCodes[$key]) ? $this->errorCodes[$key] : $key; + } +} diff --git a/ReCaptchaApi/etc/di.xml b/ReCaptchaApi/etc/di.xml index 174b5ad4..33deaad0 100644 --- a/ReCaptchaApi/etc/di.xml +++ b/ReCaptchaApi/etc/di.xml @@ -17,4 +17,25 @@ + + + + Invalid JSON received. + Could not connect to service. + Did not receive a 200 from the service. + Not a success, but no error codes received! + ReCAPTCHA response not provided. + Expected hostname did not match. + Expected APK package name did not match. + Expected action did not match. + Score threshold not met. + Challenge timeout. + The secret parameter is missing. + The secret parameter is invalid or malformed. + The response parameter is invalid or malformed. + The request is invalid or malformed. + The response is no longer valid: either is too old or has been used previously. + + + From 38fccabc57c527d8177f5978f6f29803770109cf Mon Sep 17 00:00:00 2001 From: Alexander Steshuk Date: Mon, 2 Mar 2020 16:50:05 +0200 Subject: [PATCH 2/4] security-package/recaptcha-captcha-validator: Updated CaptchaValidator.php, added Validation result Errors labels. --- ReCaptcha/Model/CaptchaValidator.php | 1 + 1 file changed, 1 insertion(+) diff --git a/ReCaptcha/Model/CaptchaValidator.php b/ReCaptcha/Model/CaptchaValidator.php index b41d53a1..323df204 100644 --- a/ReCaptcha/Model/CaptchaValidator.php +++ b/ReCaptcha/Model/CaptchaValidator.php @@ -77,4 +77,5 @@ public function isValid( } } } + } From 20894df80ca727c93c066c60a457f8bc195ca892 Mon Sep 17 00:00:00 2001 From: Alexander Steshuk Date: Tue, 3 Mar 2020 13:20:28 +0200 Subject: [PATCH 3/4] security-package/recaptcha-captcha-validator: Format code. --- ReCaptcha/Model/CaptchaValidator.php | 13 +++++++------ ReCaptchaApi/Model/ErrorLabels.php | 2 +- 2 files changed, 8 insertions(+), 7 deletions(-) diff --git a/ReCaptcha/Model/CaptchaValidator.php b/ReCaptcha/Model/CaptchaValidator.php index 323df204..a40600ce 100644 --- a/ReCaptcha/Model/CaptchaValidator.php +++ b/ReCaptcha/Model/CaptchaValidator.php @@ -60,20 +60,21 @@ public function isValid( $reCaptcha->setScoreThreshold($validationConfig->getScoreThreshold()); } } - $res = $reCaptcha->verify($reCaptchaResponse, $validationConfig->getRemoteIp()); - $validationErrors = []; + $res = $reCaptcha->verify($reCaptchaResponse, $validationConfig->getRemoteIp()); if ($res->getErrorCodes() && is_array($res->getErrorCodes()) && count($res->getErrorCodes()) > 0) { + $validationErrors = []; + foreach ($res->getErrorCodes() as $errorCode) { $validationErrors[] = __($this->errorLabels->getErrorCodeLabel($errorCode)); } - } - $validationResult = $this->validationResultFactory->create(['errors' => $validationErrors]); + $validationResult = $this->validationResultFactory->create(['errors' => $validationErrors]); - if (false === $validationResult->isValid()) { - throw new ValidationException(__('Validation Failed.'), null, 0, $validationResult); + if (false === $validationResult->isValid()) { + throw new ValidationException(__('Validation Failed.'), null, 0, $validationResult); + } } } } diff --git a/ReCaptchaApi/Model/ErrorLabels.php b/ReCaptchaApi/Model/ErrorLabels.php index 31542087..ae871870 100644 --- a/ReCaptchaApi/Model/ErrorLabels.php +++ b/ReCaptchaApi/Model/ErrorLabels.php @@ -30,6 +30,6 @@ public function __construct(array $errorCodes = []) */ public function getErrorCodeLabel(string $key): string { - return isset($this->errorCodes[$key]) ? $this->errorCodes[$key] : $key; + return $this->errorCodes[$key] ?? $key; } } From 06ba41aeb6767cf5526f562643517e7521d80466 Mon Sep 17 00:00:00 2001 From: Valerii Naida Date: Mon, 9 Mar 2020 15:56:40 -0500 Subject: [PATCH 4/4] security-package/issues/128: reCAPTCHA validation refactoring --- ReCaptcha/Model/CaptchaValidator.php | 82 ------------------- ReCaptchaCheckout/composer.json | 1 - ReCaptchaContact/composer.json | 1 - .../Observer/AjaxLoginObserver.php | 3 +- ReCaptchaNewsletter/composer.json | 1 - ReCaptchaReview/composer.json | 1 - ReCaptchaSendFriend/composer.json | 1 - ReCaptchaUi/Model/RequestHandler.php | 3 +- ReCaptchaUser/Observer/LoginObserver.php | 3 +- ReCaptchaValidation/Model/Validator.php | 57 +++++++++---- .../Api/ValidatorInterface.php | 7 +- ReCaptchaValidationApi/Model/ErrorLabels.php | 35 -------- .../Model/ErrorMessages.php | 40 +++++++++ .../etc/di.xml | 14 +--- _metapackage/composer.json | 11 ++- 15 files changed, 100 insertions(+), 160 deletions(-) delete mode 100644 ReCaptcha/Model/CaptchaValidator.php delete mode 100644 ReCaptchaValidationApi/Model/ErrorLabels.php create mode 100644 ReCaptchaValidationApi/Model/ErrorMessages.php rename {ReCaptchaApi => ReCaptchaValidationApi}/etc/di.xml (79%) diff --git a/ReCaptcha/Model/CaptchaValidator.php b/ReCaptcha/Model/CaptchaValidator.php deleted file mode 100644 index a40600ce..00000000 --- a/ReCaptcha/Model/CaptchaValidator.php +++ /dev/null @@ -1,82 +0,0 @@ -validationResultFactory = $validationResultFactory; - $this->errorLabels = $errorLabels; - } - - /** - * @inheritdoc - */ - public function isValid( - string $reCaptchaResponse, - ValidationConfigInterface $validationConfig - ): bool { - $secret = $validationConfig->getPrivateKey(); - - if ($reCaptchaResponse) { - // @codingStandardsIgnoreStart - $reCaptcha = new ReCaptcha($secret); - // @codingStandardsIgnoreEmd - - if ($validationConfig->getCaptchaType() === 'recaptcha_v3') { - if (isset($options['threshold'])) { - $reCaptcha->setScoreThreshold($validationConfig->getScoreThreshold()); - } - } - - $res = $reCaptcha->verify($reCaptchaResponse, $validationConfig->getRemoteIp()); - - if ($res->getErrorCodes() && is_array($res->getErrorCodes()) && count($res->getErrorCodes()) > 0) { - $validationErrors = []; - - foreach ($res->getErrorCodes() as $errorCode) { - $validationErrors[] = __($this->errorLabels->getErrorCodeLabel($errorCode)); - } - - $validationResult = $this->validationResultFactory->create(['errors' => $validationErrors]); - - if (false === $validationResult->isValid()) { - throw new ValidationException(__('Validation Failed.'), null, 0, $validationResult); - } - } - } - } - -} diff --git a/ReCaptchaCheckout/composer.json b/ReCaptchaCheckout/composer.json index be2c373a..3c75eb20 100644 --- a/ReCaptchaCheckout/composer.json +++ b/ReCaptchaCheckout/composer.json @@ -5,7 +5,6 @@ "php": "~7.1.3||~7.2.0||~7.3.0", "magento/framework": "102.0.*", "magento/module-checkout": "*", - "magento/module-re-captcha-api": "*", "magento/module-re-captcha-ui": "*" }, "type": "magento2-module", diff --git a/ReCaptchaContact/composer.json b/ReCaptchaContact/composer.json index 37a02f40..094e102a 100644 --- a/ReCaptchaContact/composer.json +++ b/ReCaptchaContact/composer.json @@ -4,7 +4,6 @@ "require": { "php": "~7.1.3||~7.2.0||~7.3.0", "magento/framework": "102.0.*", - "magento/module-re-captcha-api": "*", "magento/module-re-captcha-ui": "*" }, "type": "magento2-module", diff --git a/ReCaptchaCustomer/Observer/AjaxLoginObserver.php b/ReCaptchaCustomer/Observer/AjaxLoginObserver.php index 1af97117..4540b93a 100644 --- a/ReCaptchaCustomer/Observer/AjaxLoginObserver.php +++ b/ReCaptchaCustomer/Observer/AjaxLoginObserver.php @@ -94,7 +94,8 @@ public function execute(Observer $observer): void $reCaptchaResponse = $this->captchaResponseResolver->resolve($request); $validationConfig = $this->validationConfigResolver->get($key); - if (!$this->captchaValidator->isValid($reCaptchaResponse, $validationConfig)) { + $validationResult = $this->captchaValidator->isValid($reCaptchaResponse, $validationConfig); + if (false === $validationResult->isValid()) { $this->actionFlag->set('', Action::FLAG_NO_DISPATCH, true); $jsonPayload = $this->serializer->serialize([ diff --git a/ReCaptchaNewsletter/composer.json b/ReCaptchaNewsletter/composer.json index 82f33bf0..00d0c7ae 100644 --- a/ReCaptchaNewsletter/composer.json +++ b/ReCaptchaNewsletter/composer.json @@ -4,7 +4,6 @@ "require": { "php": "~7.1.3||~7.2.0||~7.3.0", "magento/framework": "102.0.*", - "magento/module-re-captcha-api": "*", "magento/module-re-captcha-ui": "*" }, "type": "magento2-module", diff --git a/ReCaptchaReview/composer.json b/ReCaptchaReview/composer.json index 9db49439..f0c07c88 100644 --- a/ReCaptchaReview/composer.json +++ b/ReCaptchaReview/composer.json @@ -4,7 +4,6 @@ "require": { "php": "~7.1.3||~7.2.0||~7.3.0", "magento/framework": "102.0.*", - "magento/module-re-captcha-api": "*", "magento/module-re-captcha-ui": "*" }, "type": "magento2-module", diff --git a/ReCaptchaSendFriend/composer.json b/ReCaptchaSendFriend/composer.json index 166b7050..1379709a 100644 --- a/ReCaptchaSendFriend/composer.json +++ b/ReCaptchaSendFriend/composer.json @@ -4,7 +4,6 @@ "require": { "php": "~7.1.3||~7.2.0||~7.3.0", "magento/framework": "102.0.*", - "magento/module-re-captcha-api": "*", "magento/module-re-captcha-ui": "*" }, "type": "magento2-module", diff --git a/ReCaptchaUi/Model/RequestHandler.php b/ReCaptchaUi/Model/RequestHandler.php index 54f55f08..530c8ed8 100644 --- a/ReCaptchaUi/Model/RequestHandler.php +++ b/ReCaptchaUi/Model/RequestHandler.php @@ -77,7 +77,8 @@ public function execute( $reCaptchaResponse = $this->captchaResponseResolver->resolve($request); $validationConfig = $this->validationConfigResolver->get($key); - if (false === $this->captchaValidator->isValid($reCaptchaResponse, $validationConfig)) { + $validationResult = $this->captchaValidator->isValid($reCaptchaResponse, $validationConfig); + if (false === $validationResult->isValid()) { $this->messageManager->addErrorMessage($validationConfig->getValidationFailureMessage()); $this->actionFlag->set('', Action::FLAG_NO_DISPATCH, true); diff --git a/ReCaptchaUser/Observer/LoginObserver.php b/ReCaptchaUser/Observer/LoginObserver.php index f581ec77..8145af63 100644 --- a/ReCaptchaUser/Observer/LoginObserver.php +++ b/ReCaptchaUser/Observer/LoginObserver.php @@ -91,7 +91,8 @@ public function execute(Observer $observer): void $reCaptchaResponse = $this->captchaResponseResolver->resolve($this->request); $validationConfig = $this->validationConfigResolver->get($key); - if (false === $this->captchaValidator->isValid($reCaptchaResponse, $validationConfig)) { + $validationResult = $this->captchaValidator->isValid($reCaptchaResponse, $validationConfig); + if (false === $validationResult->isValid()) { throw new AuthenticationException(__($validationConfig->getValidationFailureMessage())); } } diff --git a/ReCaptchaValidation/Model/Validator.php b/ReCaptchaValidation/Model/Validator.php index a1b6de2b..c2edb47b 100644 --- a/ReCaptchaValidation/Model/Validator.php +++ b/ReCaptchaValidation/Model/Validator.php @@ -7,40 +7,67 @@ namespace Magento\ReCaptchaValidation\Model; +use Magento\Framework\Validation\ValidationResult; +use Magento\Framework\Validation\ValidationResultFactory; use Magento\ReCaptchaValidationApi\Api\ValidatorInterface; use Magento\ReCaptchaValidationApi\Api\Data\ValidationConfigInterface; +use Magento\ReCaptchaValidationApi\Model\ErrorMessages; use ReCaptcha\ReCaptcha; /** - * @inheritDoc + * @inheritdoc */ class Validator implements ValidatorInterface { + /** + * @var ValidationResultFactory + */ + private $validationResultFactory; + + /** + * @var ErrorMessages + */ + private $errorMessages; + + /** + * @param ValidationResultFactory $validationResultFactory + * @param ErrorMessages $errorMessages + */ + public function __construct( + ValidationResultFactory $validationResultFactory, + ErrorMessages $errorMessages + ) { + $this->validationResultFactory = $validationResultFactory; + $this->errorMessages = $errorMessages; + } + /** * @inheritdoc */ public function isValid( string $reCaptchaResponse, ValidationConfigInterface $validationConfig - ): bool { + ): ValidationResult { $secret = $validationConfig->getPrivateKey(); - if ($reCaptchaResponse) { - // @codingStandardsIgnoreStart - $reCaptcha = new ReCaptcha($secret); - // @codingStandardsIgnoreEnd + // @codingStandardsIgnoreStart + $reCaptcha = new ReCaptcha($secret); + // @codingStandardsIgnoreEnd - // Should use $validationConfig->getExtensionAttributes() - if (isset($options['threshold'])) { - $reCaptcha->setScoreThreshold($options['threshold']); - } - $res = $reCaptcha->verify($reCaptchaResponse, $validationConfig->getRemoteIp()); + $extensionAttributes = $validationConfig->getExtensionAttributes(); + if ($extensionAttributes && (null !== $extensionAttributes->getScoreThreshold())) { + $reCaptcha->setScoreThreshold($extensionAttributes->getScoreThreshold()); + } + $result = $reCaptcha->verify($reCaptchaResponse, $validationConfig->getRemoteIp()); - if ($res->isSuccess()) { - return true; + if ($result->isSuccess()) { + $validationResult = $this->validationResultFactory->create(); + } else { + foreach ($result->getErrorCodes() as $errorCode) { + $validationErrors[] = $this->errorMessages->getErrorMessage($errorCode); } + $validationResult = $this->validationResultFactory->create(['errors' => $validationErrors]); } - - return false; + return $validationResult; } } diff --git a/ReCaptchaValidationApi/Api/ValidatorInterface.php b/ReCaptchaValidationApi/Api/ValidatorInterface.php index f9b0a7a6..6e580935 100644 --- a/ReCaptchaValidationApi/Api/ValidatorInterface.php +++ b/ReCaptchaValidationApi/Api/ValidatorInterface.php @@ -7,7 +7,7 @@ namespace Magento\ReCaptchaValidationApi\Api; -use Magento\Framework\Exception\LocalizedException; +use Magento\Framework\Validation\ValidationResult; use Magento\ReCaptchaValidationApi\Api\Data\ValidationConfigInterface; /** @@ -22,11 +22,10 @@ interface ValidatorInterface * * @param string $reCaptchaResponse * @param ValidationConfigInterface $validationConfig - * @return bool - * @throws LocalizedException + * @return ValidationResult */ public function isValid( string $reCaptchaResponse, ValidationConfigInterface $validationConfig - ): bool; + ): ValidationResult; } diff --git a/ReCaptchaValidationApi/Model/ErrorLabels.php b/ReCaptchaValidationApi/Model/ErrorLabels.php deleted file mode 100644 index ae871870..00000000 --- a/ReCaptchaValidationApi/Model/ErrorLabels.php +++ /dev/null @@ -1,35 +0,0 @@ -errorCodes = $errorCodes; - } - - /** - * Get error label - * - * @param string $key - * @return string - */ - public function getErrorCodeLabel(string $key): string - { - return $this->errorCodes[$key] ?? $key; - } -} diff --git a/ReCaptchaValidationApi/Model/ErrorMessages.php b/ReCaptchaValidationApi/Model/ErrorMessages.php new file mode 100644 index 00000000..4ba15083 --- /dev/null +++ b/ReCaptchaValidationApi/Model/ErrorMessages.php @@ -0,0 +1,40 @@ +errors = $errors; + } + + /** + * Get error label + * + * @param string $key + * @return string + */ + public function getErrorMessage(string $key): string + { + return $this->errors[$key] ?? $key; + } +} diff --git a/ReCaptchaApi/etc/di.xml b/ReCaptchaValidationApi/etc/di.xml similarity index 79% rename from ReCaptchaApi/etc/di.xml rename to ReCaptchaValidationApi/etc/di.xml index 33deaad0..9b0bbb21 100644 --- a/ReCaptchaApi/etc/di.xml +++ b/ReCaptchaValidationApi/etc/di.xml @@ -7,19 +7,9 @@ --> - - - + - - invisible - recaptcha_v3 - - - - - - + Invalid JSON received. Could not connect to service. Did not receive a 200 from the service. diff --git a/_metapackage/composer.json b/_metapackage/composer.json index ceee1365..13192e59 100644 --- a/_metapackage/composer.json +++ b/_metapackage/composer.json @@ -23,18 +23,21 @@ "magento/module-notifier-template": "*", "magento/module-notifier-template-admin-ui": "*", "magento/module-notifier-template-api": "*", - "magento/module-re-captcha-ui": "*", - "magento/module-re-captcha": "*", "magento/module-re-captcha-admin-ui": "*", - "magento/module-re-captcha-api": "*", "magento/module-re-captcha-checkout": "*", "magento/module-re-captcha-contact": "*", - "magento/module-re-captcha-customer":"*", + "magento/module-re-captcha-customer": "*", "magento/module-re-captcha-frontend-ui": "*", "magento/module-re-captcha-newsletter": "*", "magento/module-re-captcha-review": "*", "magento/module-re-captcha-send-friend": "*", + "magento/module-re-captcha-ui": "*", "magento/module-re-captcha-user": "*", + "magento/module-re-captcha-validation": "*", + "magento/module-re-captcha-validation-api": "*", + "magento/module-re-captcha-version-2-checkbox": "*", + "magento/module-re-captcha-version-2-invisible": "*", + "magento/module-re-captcha-version-3-invisible": "*", "magento/module-securitytxt": "*" } }