Skip to content

Commit

Permalink
Merge pull request #3531 from magento-borg/BugFixPR
Browse files Browse the repository at this point in the history
[2.3-develop] Bug Fixes
  • Loading branch information
joanhe authored Dec 13, 2018
2 parents 1773f41 + a297c29 commit 3a42052
Show file tree
Hide file tree
Showing 49 changed files with 1,300 additions and 264 deletions.
61 changes: 61 additions & 0 deletions app/code/Magento/Captcha/CustomerData/Captcha.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,61 @@
<?php
/**
* Copyright © Magento, Inc. All rights reserved.
* See COPYING.txt for license details.
*/

declare(strict_types=1);

namespace Magento\Captcha\CustomerData;

use Magento\Customer\CustomerData\SectionSourceInterface;

/**
* Captcha section
*/
class Captcha extends \Magento\Framework\DataObject implements SectionSourceInterface
{
/**
* @var array
*/
private $formIds;

/**
* @var \Magento\Captcha\Helper\Data
*/
private $helper;

/**
* @param \Magento\Captcha\Helper\Data $helper
* @param array $formIds
* @param array $data
* @codeCoverageIgnore
*/
public function __construct(
\Magento\Captcha\Helper\Data $helper,
array $formIds,
array $data = []
) {
parent::__construct($data);
$this->helper = $helper;
$this->formIds = $formIds;
}

/**
* @inheritdoc
*/
public function getSectionData() :array
{
$data = [];

foreach ($this->formIds as $formId) {
$captchaModel = $this->helper->getCaptcha($formId);
$data[$formId] = [
'isRequired' => $captchaModel->isRequired(),
'timestamp' => time()
];
}

return $data;
}
}
8 changes: 6 additions & 2 deletions app/code/Magento/Captcha/Model/Checkout/ConfigProvider.php
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,9 @@
*/
namespace Magento\Captcha\Model\Checkout;

/**
* Configuration provider for Captcha rendering.
*/
class ConfigProvider implements \Magento\Checkout\Model\ConfigProviderInterface
{
/**
Expand Down Expand Up @@ -38,7 +41,7 @@ public function __construct(
}

/**
* {@inheritdoc}
* @inheritdoc
*/
public function getConfig()
{
Expand All @@ -49,7 +52,8 @@ public function getConfig()
'imageHeight' => $this->getImageHeight($formId),
'imageSrc' => $this->getImageSrc($formId),
'refreshUrl' => $this->getRefreshUrl(),
'isRequired' => $this->isRequired($formId)
'isRequired' => $this->isRequired($formId),
'timestamp' => time()
];
}
return $config;
Expand Down
28 changes: 12 additions & 16 deletions app/code/Magento/Captcha/Model/Customer/Plugin/AjaxLogin.php
Original file line number Diff line number Diff line change
Expand Up @@ -3,18 +3,15 @@
* Copyright © Magento, Inc. All rights reserved.
* See COPYING.txt for license details.
*/
declare(strict_types=1);

namespace Magento\Captcha\Model\Customer\Plugin;

use Magento\Captcha\Helper\Data as CaptchaHelper;
use Magento\Customer\Controller\Ajax\Login;
use Magento\Framework\Controller\Result\Json;
use Magento\Framework\Controller\Result\JsonFactory;
use Magento\Framework\Session\SessionManagerInterface;
use Magento\Framework\Controller\Result\JsonFactory;

/**
* The plugin for ajax login controller.
* Around plugin for login action.
*/
class AjaxLogin
{
Expand Down Expand Up @@ -67,14 +64,16 @@ public function __construct(
}

/**
* Validates captcha during request execution.
* Check captcha data on login action.
*
* @param Login $subject
* @param \Magento\Customer\Controller\Ajax\Login $subject
* @param \Closure $proceed
* @return $this
* @SuppressWarnings(PHPMD.NPathComplexity)
* @SuppressWarnings(PHPMD.CyclomaticComplexity)
*/
public function aroundExecute(
Login $subject,
\Magento\Customer\Controller\Ajax\Login $subject,
\Closure $proceed
) {
$captchaFormIdField = 'captcha_form_id';
Expand All @@ -99,31 +98,28 @@ public function aroundExecute(
foreach ($this->formIds as $formId) {
if ($formId === $loginFormId) {
$captchaModel = $this->helper->getCaptcha($formId);

if ($captchaModel->isRequired($username)) {
if (!$captchaModel->isCorrect($captchaString)) {
$this->sessionManager->setUsername($username);
$captchaModel->logAttempt($username);
return $this->returnJsonError(__('Incorrect CAPTCHA'), true);
return $this->returnJsonError(__('Incorrect CAPTCHA'));
}
}

$captchaModel->logAttempt($username);
}
}
return $proceed();
}

/**
* Gets Json response.
* Format JSON response.
*
* @param \Magento\Framework\Phrase $phrase
* @param bool $isCaptchaRequired
* @return Json
* @return \Magento\Framework\Controller\Result\Json
*/
private function returnJsonError(\Magento\Framework\Phrase $phrase, bool $isCaptchaRequired = false): Json
private function returnJsonError(\Magento\Framework\Phrase $phrase): \Magento\Framework\Controller\Result\Json
{
$resultJson = $this->resultJsonFactory->create();
return $resultJson->setData(['errors' => true, 'message' => $phrase, 'captcha' => $isCaptchaRequired]);
return $resultJson->setData(['errors' => true, 'message' => $phrase]);
}
}
29 changes: 23 additions & 6 deletions app/code/Magento/Captcha/Model/DefaultModel.php
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,11 @@ class DefaultModel extends \Zend\Captcha\Image implements \Magento\Captcha\Model
*/
protected $session;

/**
* @var string
*/
private $words;

/**
* @param \Magento\Framework\Session\SessionManagerInterface $session
* @param \Magento\Captcha\Helper\Data $captchaData
Expand Down Expand Up @@ -311,18 +316,18 @@ public function getImgUrl()
*/
public function isCorrect($word)
{
$storedWord = $this->getWord();
$storedWords = $this->getWords();
$this->clearWord();

if (!$word || !$storedWord) {
if (!$word || !$storedWords) {
return false;
}

if (!$this->isCaseSensitive()) {
$storedWord = strtolower($storedWord);
$storedWords = strtolower($storedWords);
$word = strtolower($word);
}
return $word === $storedWord;
return in_array($word, explode(',', $storedWords));
}

/**
Expand Down Expand Up @@ -481,14 +486,25 @@ private function getTargetForms()
/**
* Get captcha word
*
* @return string
* @return string|null
*/
public function getWord()
{
$sessionData = $this->session->getData($this->getFormIdKey(self::SESSION_WORD));
return time() < $sessionData['expires'] ? $sessionData['data'] : null;
}

/**
* Get captcha words
*
* @return string|null
*/
private function getWords()
{
$sessionData = $this->session->getData($this->getFormIdKey(self::SESSION_WORD));
return time() < $sessionData['expires'] ? $sessionData['words'] : null;
}

/**
* Set captcha word
*
Expand All @@ -498,9 +514,10 @@ public function getWord()
*/
protected function setWord($word)
{
$this->words = $this->words ? $this->words . ',' . $word : $word;
$this->session->setData(
$this->getFormIdKey(self::SESSION_WORD),
['data' => $word, 'expires' => time() + $this->getTimeout()]
['data' => $word, 'words' => $this->words, 'expires' => time() + $this->getTimeout()]
);
$this->word = $word;
return $this;
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
<?xml version="1.0" encoding="UTF-8"?>
<!--
/**
* Copyright © Magento, Inc. All rights reserved.
* See COPYING.txt for license details.
*/
-->

<sections xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
xsi:noNamespaceSchemaLocation="urn:magento:mftf:Page/etc/SectionObject.xsd">
<section name="StorefrontCustomerSignInPopupFormSection">
<element name="captchaField" type="input" selector="#captcha_user_login"/>
<element name="captchaImg" type="block" selector=".captcha-img"/>
<element name="captchaReload" type="block" selector=".captcha-reload"/>
</section>
</sections>
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,52 @@
<!--Roll back configuration-->
<scrollToTopOfPage stepKey="ScrollToTop"/>
<click selector="{{CaptchaFormsDisplayingSection.captcha}}" stepKey="ClickToCloseCaptcha"/>

</test>
<test name="CaptchaWithDisabledGuestCheckout">
<annotations>
<features value="Captcha"/>
<stories value="MC-5602 - CAPTCHA doesn't appear in login popup after refreshing page."/>
<title value="Captcha is displaying on login form with disabled guest checkout"/>
<description value="Captcha is displaying on login form with disabled guest checkout"/>
<severity value="MAJOR"/>
<testCaseId value="MAGETWO-96691"/>
<group value="captcha"/>
</annotations>
<before>
<magentoCLI command="config:set checkout/options/guest_checkout 0" stepKey="disableGuestCheckout"/>
<magentoCLI command="config:set customer/captcha/failed_attempts_login 1" stepKey="decreaseLoginAttempt"/>
<createData entity="ApiCategory" stepKey="createCategory"/>
<createData entity="ApiSimpleProduct" stepKey="createSimpleProduct">
<requiredEntity createDataKey="createCategory"/>
</createData>
</before>
<after>
<magentoCLI command="config:set checkout/options/guest_checkout 1" stepKey="enableGuestCheckout"/>
<magentoCLI command="config:set customer/captcha/failed_attempts_login 3" stepKey="increaseLoginAttempt"/>
<deleteData createDataKey="createCategory" stepKey="deleteCategory"/>
<deleteData createDataKey="createSimpleProduct" stepKey="deleteSimpleProduct1"/>
</after>
<amOnPage url="{{StorefrontProductPage.url($$createSimpleProduct.sku$$)}}" stepKey="openProductPage"/>
<waitForPageLoad stepKey="waitForPageLoad"/>
<click selector="{{StorefrontProductActionSection.addToCart}}" stepKey="addToCart" />
<waitForText userInput="You added $$createSimpleProduct.name$$ to your shopping cart." stepKey="waitForText"/>
<click selector="{{StorefrontMinicartSection.showCart}}" stepKey="clickCart"/>
<click selector="{{StorefrontMinicartSection.goToCheckout}}" stepKey="goToCheckout"/>
<waitForElementVisible selector="{{StorefrontCustomerSignInPopupFormSection.email}}" stepKey="waitEmailFieldVisible"/>
<fillField selector="{{StorefrontCustomerSignInPopupFormSection.email}}" userInput="{{Simple_US_Customer.email}}" stepKey="fillCustomerEmail"/>
<fillField selector="{{StorefrontCustomerSignInPopupFormSection.password}}" userInput="incorrectPassword" stepKey="fillIncorrectCustomerPassword"/>
<click selector="{{StorefrontCustomerSignInPopupFormSection.signIn}}" stepKey="clickSignIn"/>
<waitForElementVisible selector="{{StorefrontCustomerSignInPopupFormSection.errorMessage}}" stepKey="seeErrorMessage"/>
<waitForElementVisible selector="{{StorefrontCustomerSignInPopupFormSection.captchaField}}" stepKey="seeCaptchaField"/>
<waitForElementVisible selector="{{StorefrontCustomerSignInPopupFormSection.captchaImg}}" stepKey="seeCaptchaImage"/>
<waitForElementVisible selector="{{StorefrontCustomerSignInPopupFormSection.captchaReload}}" stepKey="seeCaptchaReloadButton"/>
<reloadPage stepKey="refreshPage"/>
<waitForPageLoad stepKey="waitForPageLoad2"/>
<click selector="{{StorefrontMinicartSection.showCart}}" stepKey="clickCart2"/>
<click selector="{{StorefrontMinicartSection.goToCheckout}}" stepKey="goToCheckout2"/>
<waitForElementVisible selector="{{StorefrontCustomerSignInPopupFormSection.email}}" stepKey="waitEmailFieldVisible2"/>
<waitForElementVisible selector="{{StorefrontCustomerSignInPopupFormSection.captchaField}}" stepKey="seeCaptchaField2"/>
<waitForElementVisible selector="{{StorefrontCustomerSignInPopupFormSection.captchaImg}}" stepKey="seeCaptchaImage2"/>
<waitForElementVisible selector="{{StorefrontCustomerSignInPopupFormSection.captchaReload}}" stepKey="seeCaptchaReloadButton2"/>
</test>
</tests>
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,7 @@ public function testGetConfig($isRequired, $captchaGenerations, $expectedConfig)
->will($this->returnValue('https://magento.com/captcha'));

$config = $this->model->getConfig();
unset($config['captcha'][$this->formId]['timestamp']);
$this->assertEquals($config, $expectedConfig);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -158,7 +158,7 @@ public function testAroundExecuteIncorrectCaptcha()
$this->resultJsonMock
->expects($this->once())
->method('setData')
->with(['errors' => true, 'message' => __('Incorrect CAPTCHA'), 'captcha' => true])
->with(['errors' => true, 'message' => __('Incorrect CAPTCHA')])
->will($this->returnSelf());

$closure = function () {
Expand Down
18 changes: 15 additions & 3 deletions app/code/Magento/Captcha/Test/Unit/Model/DefaultTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -183,7 +183,13 @@ public function testIsCorrect()
{
self::$_defaultConfig['case_sensitive'] = '1';
$this->assertFalse($this->_object->isCorrect('abcdef5'));
$sessionData = ['user_create_word' => ['data' => 'AbCdEf5', 'expires' => time() + self::EXPIRE_FRAME]];
$sessionData = [
'user_create_word' => [
'data' => 'AbCdEf5',
'words' => 'AbCdEf5',
'expires' => time() + self::EXPIRE_FRAME
]
];
$this->_object->getSession()->setData($sessionData);
self::$_defaultConfig['case_sensitive'] = '0';
$this->assertTrue($this->_object->isCorrect('abcdef5'));
Expand Down Expand Up @@ -224,7 +230,7 @@ public function testGetWord()
{
$this->assertEquals($this->_object->getWord(), 'AbCdEf5');
$this->_object->getSession()->setData(
['user_create_word' => ['data' => 'AbCdEf5', 'expires' => time() - 360]]
['user_create_word' => ['data' => 'AbCdEf5', 'words' => 'AbCdEf5','expires' => time() - 360]]
);
$this->assertNull($this->_object->getWord());
}
Expand All @@ -247,7 +253,13 @@ protected function _getSessionStub()
->getMock();
$session->expects($this->any())->method('isLoggedIn')->will($this->returnValue(false));

$session->setData(['user_create_word' => ['data' => 'AbCdEf5', 'expires' => time() + self::EXPIRE_FRAME]]);
$session->setData([
'user_create_word' => [
'data' => 'AbCdEf5',
'words' => 'AbCdEf5',
'expires' => time() + self::EXPIRE_FRAME
]
]);
return $session;
}

Expand Down
2 changes: 1 addition & 1 deletion app/code/Magento/Captcha/etc/db_schema.xml
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@
xsi:noNamespaceSchemaLocation="urn:magento:framework:Setup/Declaration/Schema/etc/schema.xsd">
<table name="captcha_log" resource="default" engine="innodb" comment="Count Login Attempts">
<column xsi:type="varchar" name="type" nullable="false" length="32" comment="Type"/>
<column xsi:type="varchar" name="value" nullable="false" length="32" comment="Value"/>
<column xsi:type="varchar" name="value" nullable="false" length="255" comment="Value"/>
<column xsi:type="int" name="count" padding="10" unsigned="true" nullable="false" identity="false" default="0"
comment="Count"/>
<column xsi:type="timestamp" name="updated_at" on_update="false" nullable="true" comment="Update Time"/>
Expand Down
14 changes: 14 additions & 0 deletions app/code/Magento/Captcha/etc/frontend/di.xml
Original file line number Diff line number Diff line change
Expand Up @@ -20,4 +20,18 @@
</argument>
</arguments>
</type>
<type name="Magento\Captcha\CustomerData\Captcha">
<arguments>
<argument name="formIds" xsi:type="array">
<item name="user_login" xsi:type="string">user_login</item>
</argument>
</arguments>
</type>
<type name="Magento\Customer\CustomerData\SectionPoolInterface">
<arguments>
<argument name="sectionSourceMap" xsi:type="array">
<item name="captcha" xsi:type="string">Magento\Captcha\CustomerData\Captcha</item>
</argument>
</arguments>
</type>
</config>
Loading

0 comments on commit 3a42052

Please sign in to comment.