From 8f014b145f1949cfca315f9019e4d63e67a4de9a Mon Sep 17 00:00:00 2001 From: Sergey Semenov Date: Thu, 12 Nov 2015 19:03:30 +0200 Subject: [PATCH 01/12] MAGETWO-45027: CSRF on Removing Item from Wishlist --- .../Wishlist/Controller/Index/Remove.php | 24 ++++++++--- .../Test/Unit/Controller/Index/RemoveTest.php | 41 ++++++++++++++++++- 2 files changed, 59 insertions(+), 6 deletions(-) diff --git a/app/code/Magento/Wishlist/Controller/Index/Remove.php b/app/code/Magento/Wishlist/Controller/Index/Remove.php index 7d7b396535b48..20c9da8e4bff1 100644 --- a/app/code/Magento/Wishlist/Controller/Index/Remove.php +++ b/app/code/Magento/Wishlist/Controller/Index/Remove.php @@ -6,25 +6,35 @@ namespace Magento\Wishlist\Controller\Index; use Magento\Framework\App\Action; +use Magento\Framework\Data\Form\FormKey\Validator; use Magento\Framework\Exception\NotFoundException; use Magento\Framework\Controller\ResultFactory; +use Magento\Wishlist\Controller\WishlistProviderInterface; class Remove extends \Magento\Wishlist\Controller\AbstractIndex { /** - * @var \Magento\Wishlist\Controller\WishlistProviderInterface + * @var WishlistProviderInterface */ protected $wishlistProvider; + /** + * @var Validator + */ + protected $formKeyValidator; + /** * @param Action\Context $context - * @param \Magento\Wishlist\Controller\WishlistProviderInterface $wishlistProvider + * @param WishlistProviderInterface $wishlistProvider + * @param Validator $formKeyValidator */ public function __construct( Action\Context $context, - \Magento\Wishlist\Controller\WishlistProviderInterface $wishlistProvider + WishlistProviderInterface $wishlistProvider, + Validator $formKeyValidator ) { $this->wishlistProvider = $wishlistProvider; + $this->formKeyValidator = $formKeyValidator; parent::__construct($context); } @@ -36,6 +46,12 @@ public function __construct( */ public function execute() { + /** @var \Magento\Framework\Controller\Result\Redirect $resultRedirect */ + $resultRedirect = $this->resultFactory->create(ResultFactory::TYPE_REDIRECT); + if (!$this->formKeyValidator->validate($this->getRequest())) { + return $resultRedirect->setPath('*/*/'); + } + $id = (int)$this->getRequest()->getParam('item'); $item = $this->_objectManager->create('Magento\Wishlist\Model\Item')->load($id); if (!$item->getId()) { @@ -68,8 +84,6 @@ public function execute() } else { $redirectUrl = $this->_redirect->getRedirectUrl($this->_url->getUrl('*/*')); } - /** @var \Magento\Framework\Controller\Result\Redirect $resultRedirect */ - $resultRedirect = $this->resultFactory->create(ResultFactory::TYPE_REDIRECT); $resultRedirect->setUrl($redirectUrl); return $resultRedirect; } diff --git a/app/code/Magento/Wishlist/Test/Unit/Controller/Index/RemoveTest.php b/app/code/Magento/Wishlist/Test/Unit/Controller/Index/RemoveTest.php index 6b2a6b9a5959f..6bd0bfd1418b3 100644 --- a/app/code/Magento/Wishlist/Test/Unit/Controller/Index/RemoveTest.php +++ b/app/code/Magento/Wishlist/Test/Unit/Controller/Index/RemoveTest.php @@ -54,6 +54,11 @@ class RemoveTest extends \PHPUnit_Framework_TestCase */ protected $resultRedirectMock; + /** + * @var \Magento\Framework\Data\Form\FormKey\Validator|\PHPUnit_Framework_MockObject_MockObject + */ + protected $formKeyValidator; + protected function setUp() { $this->context = $this->getMock('Magento\Framework\App\Action\Context', [], [], '', false); @@ -74,6 +79,10 @@ protected function setUp() ->method('create') ->with(ResultFactory::TYPE_REDIRECT, []) ->willReturn($this->resultRedirectMock); + + $this->formKeyValidator = $this->getMockBuilder('Magento\Framework\Data\Form\FormKey\Validator') + ->disableOriginalConstructor() + ->getMock(); } public function tearDown() @@ -130,10 +139,40 @@ protected function prepareContext() public function getController() { $this->prepareContext(); + + $this->formKeyValidator->expects($this->once()) + ->method('validate') + ->with($this->request) + ->willReturn(true); + return new \Magento\Wishlist\Controller\Index\Remove( $this->context, - $this->wishlistProvider + $this->wishlistProvider, + $this->formKeyValidator + ); + } + + public function testExecuteWithInvalidFormKey() + { + $this->prepareContext(); + + $this->formKeyValidator->expects($this->once()) + ->method('validate') + ->with($this->request) + ->willReturn(false); + + $this->resultRedirectMock->expects($this->once()) + ->method('setPath') + ->with('*/*/') + ->willReturnSelf(); + + $controller = new \Magento\Wishlist\Controller\Index\Remove( + $this->context, + $this->wishlistProvider, + $this->formKeyValidator ); + + $this->assertSame($this->resultRedirectMock, $controller->execute()); } /** From 7dc9e60ac6d25da4db6ae518583293cdb3740540 Mon Sep 17 00:00:00 2001 From: Oleksandr Karpenko Date: Fri, 13 Nov 2015 11:34:15 +0200 Subject: [PATCH 02/12] MAGETWO-43188: Unable to rename theme by editing theme.xml file --- .../Theme/Model/Theme/Plugin/Registration.php | 33 +++++++++++- .../Model/Theme/Plugin/RegistrationTest.php | 53 +++++++++++++++---- 2 files changed, 76 insertions(+), 10 deletions(-) diff --git a/app/code/Magento/Theme/Model/Theme/Plugin/Registration.php b/app/code/Magento/Theme/Model/Theme/Plugin/Registration.php index 0f469442c15de..f7c7c84335d69 100644 --- a/app/code/Magento/Theme/Model/Theme/Plugin/Registration.php +++ b/app/code/Magento/Theme/Model/Theme/Plugin/Registration.php @@ -11,12 +11,21 @@ use Magento\Framework\Exception\LocalizedException; use Psr\Log\LoggerInterface; use Magento\Framework\App\State as AppState; +use Magento\Theme\Model\Theme\Collection as ThemeCollection; +use Magento\Theme\Model\ResourceModel\Theme\Collection as ThemeLoader; +use Magento\Framework\Config\Theme; class Registration { /** @var ThemeRegistration */ protected $themeRegistration; + /** @var ThemeCollection */ + protected $themeCollection; + + /** @var ThemeLoader */ + protected $themeLoader; + /** @var LoggerInterface */ protected $logger; @@ -25,21 +34,27 @@ class Registration /** * @param ThemeRegistration $themeRegistration + * @param ThemeCollection $themeCollection + * @param ThemeLoader $themeLoader * @param LoggerInterface $logger * @param AppState $appState */ public function __construct( ThemeRegistration $themeRegistration, + ThemeCollection $themeCollection, + ThemeLoader $themeLoader, LoggerInterface $logger, AppState $appState ) { $this->themeRegistration = $themeRegistration; + $this->themeCollection = $themeCollection; + $this->themeLoader = $themeLoader; $this->logger = $logger; $this->appState = $appState; } /** - * Add new theme from filesystem + * Add new theme from filesystem and update exists * * @param AbstractAction $subject * @param RequestInterface $request @@ -54,9 +69,25 @@ public function beforeDispatch( try { if ($this->appState->getMode() != AppState::MODE_PRODUCTION) { $this->themeRegistration->register(); + $this->updateThemeData(); } } catch (LocalizedException $e) { $this->logger->critical($e); } } + + protected function updateThemeData() + { + $themesData = $this->themeCollection->loadData(); + /** @var \Magento\Theme\Model\Theme $themeData */ + foreach ($themesData as $themeData) { + /** @var \Magento\Theme\Model\Theme $theme */ + $theme = $this->themeLoader->getThemeByFullPath( + $themeData->getArea() + . Theme::THEME_PATH_SEPARATOR + . $themeData->getThemePath() + ); + $theme->addData($themeData->toArray())->save(); + } + } } diff --git a/app/code/Magento/Theme/Test/Unit/Model/Theme/Plugin/RegistrationTest.php b/app/code/Magento/Theme/Test/Unit/Model/Theme/Plugin/RegistrationTest.php index faf0bdf983871..05427a9aae9f4 100644 --- a/app/code/Magento/Theme/Test/Unit/Model/Theme/Plugin/RegistrationTest.php +++ b/app/code/Magento/Theme/Test/Unit/Model/Theme/Plugin/RegistrationTest.php @@ -26,6 +26,15 @@ class RegistrationTest extends \PHPUnit_Framework_TestCase /** @var \Magento\Framework\App\State|\PHPUnit_Framework_MockObject_MockObject */ protected $appState; + /** @var \Magento\Theme\Model\Theme\Collection|\PHPUnit_Framework_MockObject_MockObject */ + protected $themeCollection; + + /** @var \Magento\Theme\Model\ResourceModel\Theme\Collection|\PHPUnit_Framework_MockObject_MockObject */ + protected $themeLoader; + + /** @var Registration */ + protected $plugin; + public function setUp() { $this->themeRegistration = $this->getMock('Magento\Theme\Model\Theme\Registration', [], [], '', false); @@ -33,24 +42,50 @@ public function setUp() $this->abstractAction = $this->getMockForAbstractClass('Magento\Backend\App\AbstractAction', [], '', false); $this->request = $this->getMockForAbstractClass('Magento\Framework\App\RequestInterface', [], '', false); $this->appState = $this->getMock('Magento\Framework\App\State', [], [], '', false); + $this->themeCollection = $this->getMock('Magento\Theme\Model\Theme\Collection', [], [], '', false); + $this->themeLoader = $this->getMock('Magento\Theme\Model\ResourceModel\Theme\Collection', [], [], '', false); + $this->plugin = new Registration( + $this->themeRegistration, + $this->themeCollection, + $this->themeLoader, + $this->logger, + $this->appState + ); } public function testBeforeDispatch() { + $theme = $this->getMock('Magento\Theme\Model\Theme', [], [], '', false); $this->appState->expects($this->once())->method('getMode')->willReturn('default'); $this->themeRegistration->expects($this->once())->method('register'); - $this->logger->expects($this->never())->method('critical'); - $object = new Registration($this->themeRegistration, $this->logger, $this->appState); - $object->beforeDispatch($this->abstractAction, $this->request); + $this->themeCollection->expects($this->once())->method('loadData')->willReturn([$theme]); + $theme->expects($this->once())->method('getArea')->willReturn('frontend'); + $theme->expects($this->once())->method('getThemePath')->willReturn('Magento/luma'); + $this->themeLoader->expects($this->once()) + ->method('getThemeByFullPath') + ->with('frontend/Magento/luma') + ->willReturn($theme); + $theme->expects($this->once()) + ->method('toArray') + ->willReturn([ + 'title' => 'Magento Luma' + ]); + $theme->expects($this->once()) + ->method('addData') + ->with([ + 'title' => 'Magento Luma' + ]) + ->willReturnSelf(); + $theme->expects($this->once()) + ->method('save'); + + $this->plugin->beforeDispatch($this->abstractAction, $this->request); } public function testBeforeDispatchWithProductionMode() { $this->appState->expects($this->once())->method('getMode')->willReturn('production'); - $this->themeRegistration->expects($this->never())->method('register'); - $this->logger->expects($this->never())->method('critical'); - $object = new Registration($this->themeRegistration, $this->logger, $this->appState); - $object->beforeDispatch($this->abstractAction, $this->request); + $this->plugin->beforeDispatch($this->abstractAction, $this->request); } public function testBeforeDispatchWithException() @@ -58,7 +93,7 @@ public function testBeforeDispatchWithException() $exception = new LocalizedException(new Phrase('Phrase')); $this->themeRegistration->expects($this->once())->method('register')->willThrowException($exception); $this->logger->expects($this->once())->method('critical'); - $object = new Registration($this->themeRegistration, $this->logger, $this->appState); - $object->beforeDispatch($this->abstractAction, $this->request); + + $this->plugin->beforeDispatch($this->abstractAction, $this->request); } } From 1bf4d46ed33c03a566ae74a83a2da74f5b459bd0 Mon Sep 17 00:00:00 2001 From: Oleksandr Karpenko Date: Fri, 13 Nov 2015 11:36:40 +0200 Subject: [PATCH 03/12] MAGETWO-43188: Unable to rename theme by editing theme.xml file --- app/code/Magento/Theme/Model/Theme/Plugin/Registration.php | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/app/code/Magento/Theme/Model/Theme/Plugin/Registration.php b/app/code/Magento/Theme/Model/Theme/Plugin/Registration.php index f7c7c84335d69..8b470d1ccb8f6 100644 --- a/app/code/Magento/Theme/Model/Theme/Plugin/Registration.php +++ b/app/code/Magento/Theme/Model/Theme/Plugin/Registration.php @@ -76,6 +76,11 @@ public function beforeDispatch( } } + /** + * Update theme data + * + * @return void + */ protected function updateThemeData() { $themesData = $this->themeCollection->loadData(); From 73e46c1a0c2b3b8d23b89fdc2b563653652f8261 Mon Sep 17 00:00:00 2001 From: Oleksandr Karpenko Date: Fri, 13 Nov 2015 11:40:21 +0200 Subject: [PATCH 04/12] MAGETWO-43188: Unable to rename theme by editing theme.xml file --- app/code/Magento/Theme/Model/Theme/Plugin/Registration.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/code/Magento/Theme/Model/Theme/Plugin/Registration.php b/app/code/Magento/Theme/Model/Theme/Plugin/Registration.php index 8b470d1ccb8f6..7cc11c6ebd12f 100644 --- a/app/code/Magento/Theme/Model/Theme/Plugin/Registration.php +++ b/app/code/Magento/Theme/Model/Theme/Plugin/Registration.php @@ -54,7 +54,7 @@ public function __construct( } /** - * Add new theme from filesystem and update exists + * Add new theme from filesystem and update existing * * @param AbstractAction $subject * @param RequestInterface $request From 300465c11c48565224c34b86caf46d7bb9e291cb Mon Sep 17 00:00:00 2001 From: Oleksandr Karpenko Date: Fri, 13 Nov 2015 15:05:42 +0200 Subject: [PATCH 05/12] MAGETWO-43188: Unable to rename theme by editing theme.xml file --- app/code/Magento/Theme/Model/Theme/Plugin/Registration.php | 3 +++ 1 file changed, 3 insertions(+) diff --git a/app/code/Magento/Theme/Model/Theme/Plugin/Registration.php b/app/code/Magento/Theme/Model/Theme/Plugin/Registration.php index 7cc11c6ebd12f..3d77e359bf2a0 100644 --- a/app/code/Magento/Theme/Model/Theme/Plugin/Registration.php +++ b/app/code/Magento/Theme/Model/Theme/Plugin/Registration.php @@ -15,6 +15,9 @@ use Magento\Theme\Model\ResourceModel\Theme\Collection as ThemeLoader; use Magento\Framework\Config\Theme; +/** + * @SuppressWarnings(PHPMD.CouplingBetweenObjects) + */ class Registration { /** @var ThemeRegistration */ From 23da851814315d7f3290bee60e07d8a927436537 Mon Sep 17 00:00:00 2001 From: Sergey Semenov Date: Fri, 13 Nov 2015 18:04:32 +0200 Subject: [PATCH 06/12] MAGETWO-45027: CSRF on Removing Item from Wishlist --- .../Magento/Wishlist/Controller/Index/Add.php | 21 +++++-- .../Wishlist/Controller/Index/Cart.php | 18 +++++- .../Wishlist/Controller/Index/Fromcart.php | 20 +++++-- .../Controller/Index/UpdateItemOptions.php | 33 ++++++++--- .../Test/Unit/Controller/Index/CartTest.php | 58 ++++++++++++++++++- .../Unit/Controller/Index/FromcartTest.php | 48 ++++++++++++++- .../Index/UpdateItemOptionsTest.php | 41 ++++++++++++- 7 files changed, 215 insertions(+), 24 deletions(-) diff --git a/app/code/Magento/Wishlist/Controller/Index/Add.php b/app/code/Magento/Wishlist/Controller/Index/Add.php index e3bd753ce7589..61820f634c4cf 100644 --- a/app/code/Magento/Wishlist/Controller/Index/Add.php +++ b/app/code/Magento/Wishlist/Controller/Index/Add.php @@ -7,6 +7,7 @@ use Magento\Catalog\Api\ProductRepositoryInterface; use Magento\Framework\App\Action; +use Magento\Framework\Data\Form\FormKey\Validator; use Magento\Framework\Exception\NotFoundException; use Magento\Framework\Exception\NoSuchEntityException; use Magento\Framework\Controller\ResultFactory; @@ -31,22 +32,30 @@ class Add extends \Magento\Wishlist\Controller\AbstractIndex */ protected $productRepository; + /** + * @var Validator + */ + protected $formKeyValidator; + /** * @param Action\Context $context * @param \Magento\Customer\Model\Session $customerSession * @param \Magento\Wishlist\Controller\WishlistProviderInterface $wishlistProvider * @param ProductRepositoryInterface $productRepository + * @param Validator $formKeyValidator */ public function __construct( Action\Context $context, \Magento\Customer\Model\Session $customerSession, \Magento\Wishlist\Controller\WishlistProviderInterface $wishlistProvider, - ProductRepositoryInterface $productRepository + ProductRepositoryInterface $productRepository, + Validator $formKeyValidator ) { $this->_customerSession = $customerSession; $this->wishlistProvider = $wishlistProvider; - parent::__construct($context); $this->productRepository = $productRepository; + $this->formKeyValidator = $formKeyValidator; + parent::__construct($context); } /** @@ -60,6 +69,12 @@ public function __construct( */ public function execute() { + /** @var \Magento\Framework\Controller\Result\Redirect $resultRedirect */ + $resultRedirect = $this->resultFactory->create(ResultFactory::TYPE_REDIRECT); + if (!$this->formKeyValidator->validate($this->getRequest())) { + return $resultRedirect->setPath('*/'); + } + $wishlist = $this->wishlistProvider->getWishlist(); if (!$wishlist) { throw new NotFoundException(__('Page not found.')); @@ -75,8 +90,6 @@ public function execute() } $productId = isset($requestParams['product']) ? (int)$requestParams['product'] : null; - /** @var \Magento\Framework\Controller\Result\Redirect $resultRedirect */ - $resultRedirect = $this->resultFactory->create(ResultFactory::TYPE_REDIRECT); if (!$productId) { $resultRedirect->setPath('*/'); return $resultRedirect; diff --git a/app/code/Magento/Wishlist/Controller/Index/Cart.php b/app/code/Magento/Wishlist/Controller/Index/Cart.php index 2be0f7dc01fb4..9240954b302bf 100644 --- a/app/code/Magento/Wishlist/Controller/Index/Cart.php +++ b/app/code/Magento/Wishlist/Controller/Index/Cart.php @@ -59,17 +59,23 @@ class Cart extends \Magento\Wishlist\Controller\AbstractIndex */ protected $helper; + /** + * @var \Magento\Framework\Data\Form\FormKey\Validator + */ + protected $formKeyValidator; + /** * @param Action\Context $context * @param \Magento\Wishlist\Controller\WishlistProviderInterface $wishlistProvider * @param \Magento\Wishlist\Model\LocaleQuantityProcessor $quantityProcessor * @param \Magento\Wishlist\Model\ItemFactory $itemFactory * @param \Magento\Checkout\Model\Cart $cart - * @param \Magento\Wishlist\Model\Item\OptionFactory $ + * @param \Magento\Wishlist\Model\Item\OptionFactory $optionFactory * @param \Magento\Catalog\Helper\Product $productHelper * @param \Magento\Framework\Escaper $escaper * @param \Magento\Wishlist\Helper\Data $helper * @param \Magento\Checkout\Helper\Cart $cartHelper + * @param \Magento\Framework\Data\Form\FormKey\Validator $formKeyValidator * @SuppressWarnings(PHPMD.ExcessiveParameterList) */ public function __construct( @@ -82,7 +88,8 @@ public function __construct( \Magento\Catalog\Helper\Product $productHelper, \Magento\Framework\Escaper $escaper, \Magento\Wishlist\Helper\Data $helper, - \Magento\Checkout\Helper\Cart $cartHelper + \Magento\Checkout\Helper\Cart $cartHelper, + \Magento\Framework\Data\Form\FormKey\Validator $formKeyValidator ) { $this->wishlistProvider = $wishlistProvider; $this->quantityProcessor = $quantityProcessor; @@ -93,6 +100,7 @@ public function __construct( $this->escaper = $escaper; $this->helper = $helper; $this->cartHelper = $cartHelper; + $this->formKeyValidator = $formKeyValidator; parent::__construct($context); } @@ -108,9 +116,13 @@ public function __construct( */ public function execute() { - $itemId = (int)$this->getRequest()->getParam('item'); /** @var \Magento\Framework\Controller\Result\Redirect $resultRedirect */ $resultRedirect = $this->resultFactory->create(ResultFactory::TYPE_REDIRECT); + if (!$this->formKeyValidator->validate($this->getRequest())) { + return $resultRedirect->setPath('*/*/'); + } + + $itemId = (int)$this->getRequest()->getParam('item'); /* @var $item \Magento\Wishlist\Model\Item */ $item = $this->itemFactory->create()->load($itemId); if (!$item->getId()) { diff --git a/app/code/Magento/Wishlist/Controller/Index/Fromcart.php b/app/code/Magento/Wishlist/Controller/Index/Fromcart.php index c5ff1ef21c27a..9afecb231788b 100644 --- a/app/code/Magento/Wishlist/Controller/Index/Fromcart.php +++ b/app/code/Magento/Wishlist/Controller/Index/Fromcart.php @@ -9,6 +9,7 @@ use Magento\Checkout\Model\Cart as CheckoutCart; use Magento\Customer\Model\Session; use Magento\Framework\App\Action; +use Magento\Framework\Data\Form\FormKey\Validator; use Magento\Framework\Escaper; use Magento\Framework\Exception\NotFoundException; use Magento\Framework\Exception\LocalizedException; @@ -46,6 +47,11 @@ class Fromcart extends \Magento\Wishlist\Controller\AbstractIndex */ protected $escaper; + /** + * @var Validator + */ + protected $formKeyValidator; + /** * @param Action\Context $context * @param WishlistProviderInterface $wishlistProvider @@ -53,6 +59,7 @@ class Fromcart extends \Magento\Wishlist\Controller\AbstractIndex * @param CheckoutCart $cart * @param CartHelper $cartHelper * @param Escaper $escaper + * @param Validator $formKeyValidator */ public function __construct( Action\Context $context, @@ -60,13 +67,15 @@ public function __construct( WishlistHelper $wishlistHelper, CheckoutCart $cart, CartHelper $cartHelper, - Escaper $escaper + Escaper $escaper, + Validator $formKeyValidator ) { $this->wishlistProvider = $wishlistProvider; $this->wishlistHelper = $wishlistHelper; $this->cart = $cart; $this->cartHelper = $cartHelper; $this->escaper = $escaper; + $this->formKeyValidator = $formKeyValidator; parent::__construct($context); } @@ -79,6 +88,12 @@ public function __construct( */ public function execute() { + /** @var \Magento\Framework\Controller\Result\Redirect $resultRedirect */ + $resultRedirect = $this->resultFactory->create(ResultFactory::TYPE_REDIRECT); + if (!$this->formKeyValidator->validate($this->getRequest())) { + return $resultRedirect->setPath('*/*/'); + } + $wishlist = $this->wishlistProvider->getWishlist(); if (!$wishlist) { throw new NotFoundException(__('Page not found.')); @@ -112,9 +127,6 @@ public function execute() } catch (\Exception $e) { $this->messageManager->addExceptionMessage($e, __('We can\'t move the item to the wish list.')); } - - /** @var \Magento\Framework\Controller\Result\Redirect $resultRedirect */ - $resultRedirect = $this->resultFactory->create(ResultFactory::TYPE_REDIRECT); return $resultRedirect->setUrl($this->cartHelper->getCartUrl()); } } diff --git a/app/code/Magento/Wishlist/Controller/Index/UpdateItemOptions.php b/app/code/Magento/Wishlist/Controller/Index/UpdateItemOptions.php index 3c8dcedd80426..c1cb5cbb88019 100644 --- a/app/code/Magento/Wishlist/Controller/Index/UpdateItemOptions.php +++ b/app/code/Magento/Wishlist/Controller/Index/UpdateItemOptions.php @@ -6,19 +6,22 @@ namespace Magento\Wishlist\Controller\Index; use Magento\Catalog\Api\ProductRepositoryInterface; +use Magento\Customer\Model\Session; use Magento\Framework\App\Action; +use Magento\Framework\Data\Form\FormKey\Validator; use Magento\Framework\Exception\NoSuchEntityException; use Magento\Framework\Controller\ResultFactory; +use Magento\Wishlist\Controller\WishlistProviderInterface; class UpdateItemOptions extends \Magento\Wishlist\Controller\AbstractIndex { /** - * @var \Magento\Wishlist\Controller\WishlistProviderInterface + * @var WishlistProviderInterface */ protected $wishlistProvider; /** - * @var \Magento\Customer\Model\Session + * @var Session */ protected $_customerSession; @@ -27,22 +30,30 @@ class UpdateItemOptions extends \Magento\Wishlist\Controller\AbstractIndex */ protected $productRepository; + /** + * @var Validator + */ + protected $formKeyValidator; + /** * @param Action\Context $context - * @param \Magento\Customer\Model\Session $customerSession - * @param \Magento\Wishlist\Controller\WishlistProviderInterface $wishlistProvider + * @param Session $customerSession + * @param WishlistProviderInterface $wishlistProvider * @param ProductRepositoryInterface $productRepository + * @param Validator $formKeyValidator */ public function __construct( Action\Context $context, - \Magento\Customer\Model\Session $customerSession, - \Magento\Wishlist\Controller\WishlistProviderInterface $wishlistProvider, - ProductRepositoryInterface $productRepository + Session $customerSession, + WishlistProviderInterface $wishlistProvider, + ProductRepositoryInterface $productRepository, + Validator $formKeyValidator ) { $this->_customerSession = $customerSession; $this->wishlistProvider = $wishlistProvider; - parent::__construct($context); $this->productRepository = $productRepository; + $this->formKeyValidator = $formKeyValidator; + parent::__construct($context); } /** @@ -52,9 +63,13 @@ public function __construct( */ public function execute() { - $productId = (int)$this->getRequest()->getParam('product'); /** @var \Magento\Framework\Controller\Result\Redirect $resultRedirect */ $resultRedirect = $this->resultFactory->create(ResultFactory::TYPE_REDIRECT); + if (!$this->formKeyValidator->validate($this->getRequest())) { + return $resultRedirect->setPath('*/*/'); + } + + $productId = (int)$this->getRequest()->getParam('product'); if (!$productId) { $resultRedirect->setPath('*/'); return $resultRedirect; diff --git a/app/code/Magento/Wishlist/Test/Unit/Controller/Index/CartTest.php b/app/code/Magento/Wishlist/Test/Unit/Controller/Index/CartTest.php index cdd8ea3b33372..a26ae8cb29ab2 100644 --- a/app/code/Magento/Wishlist/Test/Unit/Controller/Index/CartTest.php +++ b/app/code/Magento/Wishlist/Test/Unit/Controller/Index/CartTest.php @@ -110,6 +110,11 @@ class CartTest extends \PHPUnit_Framework_TestCase */ protected $resultJsonMock; + /** + * @var \Magento\Framework\Data\Form\FormKey\Validator|\PHPUnit_Framework_MockObject_MockObject + */ + protected $formKeyValidator; + /** * @SuppressWarnings(PHPMD.ExcessiveMethodLength) */ @@ -216,6 +221,9 @@ protected function setUp() ] ); + $this->formKeyValidator = $this->getMockBuilder('Magento\Framework\Data\Form\FormKey\Validator') + ->disableOriginalConstructor() + ->getMock(); $this->model = new Cart( $this->contextMock, @@ -227,14 +235,35 @@ protected function setUp() $this->productHelperMock, $this->escaperMock, $this->helperMock, - $this->cartHelperMock + $this->cartHelperMock, + $this->formKeyValidator ); } + public function testExecuteWithInvalidFormKey() + { + $this->formKeyValidator->expects($this->once()) + ->method('validate') + ->with($this->requestMock) + ->willReturn(false); + + $this->resultRedirectMock->expects($this->once()) + ->method('setPath') + ->with('*/*/') + ->willReturnSelf(); + + $this->assertSame($this->resultRedirectMock, $this->model->execute()); + } + public function testExecuteWithNoItem() { $itemId = false; + $this->formKeyValidator->expects($this->once()) + ->method('validate') + ->with($this->requestMock) + ->willReturn(true); + $itemMock = $this->getMockBuilder('Magento\Wishlist\Model\Item') ->disableOriginalConstructor() ->getMock(); @@ -267,6 +296,11 @@ public function testExecuteWithNoWishlist() $itemId = 2; $wishlistId = 1; + $this->formKeyValidator->expects($this->once()) + ->method('validate') + ->with($this->requestMock) + ->willReturn(true); + $itemMock = $this->getMockBuilder('Magento\Wishlist\Model\Item') ->disableOriginalConstructor() ->setMethods(['load', 'getId', 'getWishlistId']) @@ -306,7 +340,12 @@ public function testExecuteWithNoWishlist() public function testExecuteWithQuantityArray() { $refererUrl = $this->prepareExecuteWithQuantityArray(); - + + $this->formKeyValidator->expects($this->once()) + ->method('validate') + ->with($this->requestMock) + ->willReturn(true); + $this->resultRedirectMock->expects($this->once()) ->method('setUrl') ->with($refererUrl) @@ -319,6 +358,11 @@ public function testExecuteWithQuantityArrayAjax() { $refererUrl = $this->prepareExecuteWithQuantityArray(true); + $this->formKeyValidator->expects($this->once()) + ->method('validate') + ->with($this->requestMock) + ->willReturn(true); + $this->resultJsonMock->expects($this->once()) ->method('setData') ->with(['backUrl' => $refererUrl]) @@ -554,6 +598,11 @@ public function testExecuteWithoutQuantityArrayAndOutOfStock() $options = [5 => 'option']; $params = ['item' => $itemId, 'qty' => $qty]; + $this->formKeyValidator->expects($this->once()) + ->method('validate') + ->with($this->requestMock) + ->willReturn(true); + $itemMock = $this->getMockBuilder('Magento\Wishlist\Model\Item') ->disableOriginalConstructor() ->setMethods( @@ -715,6 +764,11 @@ public function testExecuteWithoutQuantityArrayAndConfigurable() $options = [5 => 'option']; $params = ['item' => $itemId, 'qty' => $qty]; + $this->formKeyValidator->expects($this->once()) + ->method('validate') + ->with($this->requestMock) + ->willReturn(true); + $itemMock = $this->getMockBuilder('Magento\Wishlist\Model\Item') ->disableOriginalConstructor() ->setMethods( diff --git a/app/code/Magento/Wishlist/Test/Unit/Controller/Index/FromcartTest.php b/app/code/Magento/Wishlist/Test/Unit/Controller/Index/FromcartTest.php index 58109d6dc3d25..6c8124af90800 100644 --- a/app/code/Magento/Wishlist/Test/Unit/Controller/Index/FromcartTest.php +++ b/app/code/Magento/Wishlist/Test/Unit/Controller/Index/FromcartTest.php @@ -11,6 +11,7 @@ use Magento\Framework\App\Request\Http; use Magento\Framework\Controller\Result\Redirect as ResultRedirect; use Magento\Framework\Controller\ResultFactory; +use Magento\Framework\Data\Form\FormKey\Validator; use Magento\Framework\DataObject; use Magento\Framework\Escaper; use Magento\Framework\Message\Manager as MessageManager; @@ -78,6 +79,11 @@ class FromcartTest extends \PHPUnit_Framework_TestCase */ protected $resultRedirect; + /** + * @var Validator|\PHPUnit_Framework_MockObject_MockObject + */ + protected $formKeyValidator; + protected function setUp() { $this->prepareContext(); @@ -101,22 +107,47 @@ protected function setUp() ->disableOriginalConstructor() ->getMock(); + $this->formKeyValidator = $this->getMockBuilder('Magento\Framework\Data\Form\FormKey\Validator') + ->disableOriginalConstructor() + ->getMock(); + $this->controller = new Fromcart( $this->context, $this->wishlistProvider, $this->wishlistHelper, $this->cart, $this->cartHelper, - $this->escaper + $this->escaper, + $this->formKeyValidator ); } + public function testExecuteWithInvalidFormKey() + { + $this->formKeyValidator->expects($this->once()) + ->method('validate') + ->with($this->request) + ->willReturn(false); + + $this->resultRedirect->expects($this->once()) + ->method('setPath') + ->with('*/*/') + ->willReturnSelf(); + + $this->assertSame($this->resultRedirect, $this->controller->execute()); + } + /** * @expectedException \Magento\Framework\Exception\NotFoundException * @expectedExceptionMessage Page not found */ public function testExecutePageNotFound() { + $this->formKeyValidator->expects($this->once()) + ->method('validate') + ->with($this->request) + ->willReturn(true); + $this->wishlistProvider->expects($this->once()) ->method('getWishlist') ->willReturn(null); @@ -129,6 +160,11 @@ public function testExecuteNoCartItem() $itemId = 1; $cartUrl = 'cart_url'; + $this->formKeyValidator->expects($this->once()) + ->method('validate') + ->with($this->request) + ->willReturn(true); + $wishlistMock = $this->getMockBuilder('Magento\Wishlist\Model\Wishlist') ->disableOriginalConstructor() ->getMock(); @@ -179,6 +215,11 @@ public function testExecute() $productId = 1; $productName = 'product_name'; + $this->formKeyValidator->expects($this->once()) + ->method('validate') + ->with($this->request) + ->willReturn(true); + $dataObjectMock = $this->getMockBuilder('Magento\Framework\DataObject') ->disableOriginalConstructor() ->getMock(); @@ -244,6 +285,11 @@ public function testExecuteWithException() $exceptionMessage = 'exception_message'; $exception = new \Exception($exceptionMessage); + $this->formKeyValidator->expects($this->once()) + ->method('validate') + ->with($this->request) + ->willReturn(true); + $wishlistMock = $this->getMockBuilder('Magento\Wishlist\Model\Wishlist') ->disableOriginalConstructor() ->getMock(); diff --git a/app/code/Magento/Wishlist/Test/Unit/Controller/Index/UpdateItemOptionsTest.php b/app/code/Magento/Wishlist/Test/Unit/Controller/Index/UpdateItemOptionsTest.php index f70306b60257a..ec44ab755bee1 100644 --- a/app/code/Magento/Wishlist/Test/Unit/Controller/Index/UpdateItemOptionsTest.php +++ b/app/code/Magento/Wishlist/Test/Unit/Controller/Index/UpdateItemOptionsTest.php @@ -67,6 +67,11 @@ class UpdateItemOptionsTest extends \PHPUnit_Framework_TestCase */ protected $resultRedirectMock; + /** + * @var \Magento\Framework\Data\Form\FormKey\Validator|\PHPUnit_Framework_MockObject_MockObject + */ + protected $formKeyValidator; + /** * SetUp method * @@ -94,6 +99,10 @@ protected function setUp() ->method('create') ->with(ResultFactory::TYPE_REDIRECT, []) ->willReturn($this->resultRedirectMock); + + $this->formKeyValidator = $this->getMockBuilder('Magento\Framework\Data\Form\FormKey\Validator') + ->disableOriginalConstructor() + ->getMock(); } /** @@ -161,12 +170,42 @@ public function prepareContext() protected function getController() { $this->prepareContext(); + + $this->formKeyValidator->expects($this->once()) + ->method('validate') + ->with($this->request) + ->willReturn(true); + return new \Magento\Wishlist\Controller\Index\UpdateItemOptions( $this->context, $this->customerSession, $this->wishlistProvider, - $this->productRepository + $this->productRepository, + $this->formKeyValidator + ); + } + + public function testExecuteWithInvalidFormKey() + { + $this->prepareContext(); + + $this->formKeyValidator->expects($this->once()) + ->method('validate') + ->with($this->request) + ->willReturn(false); + + $this->resultRedirectMock->expects($this->once()) + ->method('setPath') + ->with('*/*/') + ->willReturnSelf(); + + $controller = new \Magento\Wishlist\Controller\Index\Remove( + $this->context, + $this->wishlistProvider, + $this->formKeyValidator ); + + $this->assertSame($this->resultRedirectMock, $controller->execute()); } /** From 0aeac1bf8592dc99637e1eb829d1a91c4b49e36a Mon Sep 17 00:00:00 2001 From: Oleksandr Karpenko Date: Fri, 13 Nov 2015 18:39:16 +0200 Subject: [PATCH 07/12] MAGETWO-43188: Unable to rename theme by editing theme.xml file --- .../Theme/Model/Theme/Plugin/Registration.php | 7 ++++ .../Model/Theme/Plugin/RegistrationTest.php | 34 ++++++++++++++++--- 2 files changed, 37 insertions(+), 4 deletions(-) diff --git a/app/code/Magento/Theme/Model/Theme/Plugin/Registration.php b/app/code/Magento/Theme/Model/Theme/Plugin/Registration.php index 3d77e359bf2a0..947d7ceca6d05 100644 --- a/app/code/Magento/Theme/Model/Theme/Plugin/Registration.php +++ b/app/code/Magento/Theme/Model/Theme/Plugin/Registration.php @@ -89,6 +89,13 @@ protected function updateThemeData() $themesData = $this->themeCollection->loadData(); /** @var \Magento\Theme\Model\Theme $themeData */ foreach ($themesData as $themeData) { + if ($themeData->getParentTheme()) { + $parentTheme = $this->themeLoader->getThemeByFullPath( + $themeData->getParentTheme()->getFullPath() + ); + $themeData->setParentId($parentTheme->getId()); + } + /** @var \Magento\Theme\Model\Theme $theme */ $theme = $this->themeLoader->getThemeByFullPath( $themeData->getArea() diff --git a/app/code/Magento/Theme/Test/Unit/Model/Theme/Plugin/RegistrationTest.php b/app/code/Magento/Theme/Test/Unit/Model/Theme/Plugin/RegistrationTest.php index 05427a9aae9f4..3512086977d35 100644 --- a/app/code/Magento/Theme/Test/Unit/Model/Theme/Plugin/RegistrationTest.php +++ b/app/code/Magento/Theme/Test/Unit/Model/Theme/Plugin/RegistrationTest.php @@ -55,16 +55,42 @@ public function setUp() public function testBeforeDispatch() { - $theme = $this->getMock('Magento\Theme\Model\Theme', [], [], '', false); + $theme = $this->getMock( + 'Magento\Theme\Model\Theme', + [ + 'setParentId', + 'getArea', + 'getThemePath', + 'getParentTheme', + 'getId', + 'getFullPath', + 'toArray', + 'addData', + 'save', + ], + [], + '', + false + ); $this->appState->expects($this->once())->method('getMode')->willReturn('default'); $this->themeRegistration->expects($this->once())->method('register'); $this->themeCollection->expects($this->once())->method('loadData')->willReturn([$theme]); $theme->expects($this->once())->method('getArea')->willReturn('frontend'); $theme->expects($this->once())->method('getThemePath')->willReturn('Magento/luma'); - $this->themeLoader->expects($this->once()) + $theme->expects($this->exactly(2))->method('getParentTheme')->willReturnSelf(); + $theme->expects($this->once())->method('getId')->willReturn(1); + $theme->expects($this->once())->method('getFullPath')->willReturn('frontend/Magento/blank'); + $theme->expects($this->once())->method('setParentId')->with(1); + $this->themeLoader->expects($this->exactly(2)) ->method('getThemeByFullPath') - ->with('frontend/Magento/luma') - ->willReturn($theme); + ->withConsecutive( + ['frontend/Magento/blank'], + ['frontend/Magento/luma'] + ) + ->will($this->onConsecutiveCalls( + $theme, + $theme + )); $theme->expects($this->once()) ->method('toArray') ->willReturn([ From 766aa845aa16a2a68827722f0d30450a7b4efc0b Mon Sep 17 00:00:00 2001 From: Sergey Semenov Date: Mon, 16 Nov 2015 19:34:45 +0200 Subject: [PATCH 08/12] MAGETWO-45027: CSRF on Removing Item from Wishlist --- .../Magento/Wishlist/Test/Unit/Controller/Index/RemoveTest.php | 3 +++ 1 file changed, 3 insertions(+) diff --git a/app/code/Magento/Wishlist/Test/Unit/Controller/Index/RemoveTest.php b/app/code/Magento/Wishlist/Test/Unit/Controller/Index/RemoveTest.php index 6bd0bfd1418b3..9ade108968009 100644 --- a/app/code/Magento/Wishlist/Test/Unit/Controller/Index/RemoveTest.php +++ b/app/code/Magento/Wishlist/Test/Unit/Controller/Index/RemoveTest.php @@ -7,6 +7,9 @@ use Magento\Framework\Controller\ResultFactory; +/** + * @SuppressWarnings(PHPMD.CouplingBetweenObjects) + */ class RemoveTest extends \PHPUnit_Framework_TestCase { /** From a0ac37f0349320f4c36328d4ad44bc2cf946aa77 Mon Sep 17 00:00:00 2001 From: Sergey Semenov Date: Wed, 18 Nov 2015 18:35:48 +0200 Subject: [PATCH 09/12] MAGETWO-45027: CSRF on Removing Item from Wishlist --- .../testsuite/Magento/Wishlist/Controller/IndexTest.php | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/dev/tests/integration/testsuite/Magento/Wishlist/Controller/IndexTest.php b/dev/tests/integration/testsuite/Magento/Wishlist/Controller/IndexTest.php index 1f17a766c424f..8a86f51af9241 100644 --- a/dev/tests/integration/testsuite/Magento/Wishlist/Controller/IndexTest.php +++ b/dev/tests/integration/testsuite/Magento/Wishlist/Controller/IndexTest.php @@ -83,6 +83,12 @@ public function testItemColumnBlock() */ public function testAddActionProductNameXss() { + /** @var \Magento\Framework\Data\Form\FormKey $formKey */ + $formKey = $this->_objectManager->get('Magento\Framework\Data\Form\FormKey'); + $this->getRequest()->setPostValue([ + 'form_key' => $formKey->getFormKey(), + ]); + $this->dispatch('wishlist/index/add/product/1?nocookie=1'); $messages = $this->_messages->getMessages()->getItems(); $isProductNamePresent = false; From 7d610f0b3b17ceb100b72fd6a89120d4c82b1a3a Mon Sep 17 00:00:00 2001 From: Sergey Semenov Date: Fri, 20 Nov 2015 13:19:42 +0200 Subject: [PATCH 10/12] MAGETWO-45027: CSRF on Removing Item from Wishlist --- .../Mtf/Util/Protocol/CurlTransport/FrontendDecorator.php | 3 +++ 1 file changed, 3 insertions(+) diff --git a/dev/tests/functional/lib/Magento/Mtf/Util/Protocol/CurlTransport/FrontendDecorator.php b/dev/tests/functional/lib/Magento/Mtf/Util/Protocol/CurlTransport/FrontendDecorator.php index 0eedeef274490..8f0d5333965b4 100644 --- a/dev/tests/functional/lib/Magento/Mtf/Util/Protocol/CurlTransport/FrontendDecorator.php +++ b/dev/tests/functional/lib/Magento/Mtf/Util/Protocol/CurlTransport/FrontendDecorator.php @@ -117,6 +117,9 @@ protected function initCookies() */ public function write($url, $params = [], $method = CurlInterface::POST, $headers = []) { + if ($this->formKey) { + $params['form_key'] = $this->formKey; + } $headers = ['Set-Cookie:' . $this->cookies]; $this->transport->write($url, http_build_query($params), $method, $headers); } From 1bc4f34a7270293511807e3aac122c0d59365390 Mon Sep 17 00:00:00 2001 From: Sviatoslav Mankivskyi Date: Fri, 27 Nov 2015 11:26:21 +0200 Subject: [PATCH 11/12] MAGETWO-45887: Persistent XSS on Create User Account --- .../Magento/Framework/View/Page/Config/Renderer.php | 2 +- .../Framework/View/Test/Unit/Page/Config/RendererTest.php | 8 ++++++-- 2 files changed, 7 insertions(+), 3 deletions(-) diff --git a/lib/internal/Magento/Framework/View/Page/Config/Renderer.php b/lib/internal/Magento/Framework/View/Page/Config/Renderer.php index 660d1d9c5aa6b..c117472325c06 100644 --- a/lib/internal/Magento/Framework/View/Page/Config/Renderer.php +++ b/lib/internal/Magento/Framework/View/Page/Config/Renderer.php @@ -106,7 +106,7 @@ public function renderHeadContent() */ public function renderTitle() { - return '' . $this->pageConfig->getTitle()->get() . '' . "\n"; + return '' . $this->escaper->escapeHtml($this->pageConfig->getTitle()->get()) . '' . "\n"; } /** diff --git a/lib/internal/Magento/Framework/View/Test/Unit/Page/Config/RendererTest.php b/lib/internal/Magento/Framework/View/Test/Unit/Page/Config/RendererTest.php index 62c52b45cc818..8164bd3bb18a2 100644 --- a/lib/internal/Magento/Framework/View/Test/Unit/Page/Config/RendererTest.php +++ b/lib/internal/Magento/Framework/View/Test/Unit/Page/Config/RendererTest.php @@ -179,11 +179,15 @@ public function testRenderTitle() $this->pageConfigMock->expects($this->any()) ->method('getTitle') - ->will($this->returnValue($this->titleMock)); + ->willReturn($this->titleMock); $this->titleMock->expects($this->once()) ->method('get') - ->will($this->returnValue($title)); + ->willReturn($title); + + $this->escaperMock->expects($this->once()) + ->method('escapeHtml') + ->willReturnArgument(0); $this->assertEquals($expected, $this->renderer->renderTitle()); } From 9be4939db0adaa3134fbe77c557bcb1651979a35 Mon Sep 17 00:00:00 2001 From: Sviatoslav Mankivskyi Date: Wed, 9 Dec 2015 10:17:25 +0200 Subject: [PATCH 12/12] MAGETWO-43188: Unable to rename theme by editing theme.xml file --- .../Newsletter/Controller/Adminhtml/NewsletterTemplateTest.php | 1 - 1 file changed, 1 deletion(-) diff --git a/dev/tests/integration/testsuite/Magento/Newsletter/Controller/Adminhtml/NewsletterTemplateTest.php b/dev/tests/integration/testsuite/Magento/Newsletter/Controller/Adminhtml/NewsletterTemplateTest.php index b9cb424179fea..64e5e25abeaee 100644 --- a/dev/tests/integration/testsuite/Magento/Newsletter/Controller/Adminhtml/NewsletterTemplateTest.php +++ b/dev/tests/integration/testsuite/Magento/Newsletter/Controller/Adminhtml/NewsletterTemplateTest.php @@ -93,7 +93,6 @@ public function testSaveActionEditTemplateAndVerifySuccessMessage() /** * @magentoAppIsolation enabled - * @magentoDbIsolation enabled */ public function testSaveActionTemplateWithInvalidDataAndVerifySuccessMessage() {