diff --git a/app/code/Magento/Customer/Model/Account/Redirect.php b/app/code/Magento/Customer/Model/Account/Redirect.php index ac03bd02553c762535ba575a7ca1ad398afb606a..5a1470959b60d37758bc698cdc2abaeadd3aea25 100644 --- a/app/code/Magento/Customer/Model/Account/Redirect.php +++ b/app/code/Magento/Customer/Model/Account/Redirect.php @@ -9,6 +9,7 @@ use Magento\Customer\Model\Session; use Magento\Customer\Model\Url as CustomerUrl; use Magento\Framework\App\RequestInterface; use Magento\Framework\Controller\ResultFactory; +use Magento\Framework\Url\HostChecker; use Magento\Framework\UrlInterface; use Magento\Store\Model\ScopeInterface; use Magento\Store\Model\StoreManagerInterface; @@ -53,6 +54,7 @@ class Redirect protected $customerUrl; /** + * @deprecated * @var UrlInterface */ protected $url; @@ -67,6 +69,11 @@ class Redirect */ protected $cookieManager; + /** + * @var HostChecker + */ + private $hostChecker; + /** * @param RequestInterface $request * @param Session $customerSession @@ -76,6 +83,7 @@ class Redirect * @param DecoderInterface $urlDecoder * @param CustomerUrl $customerUrl * @param ResultFactory $resultFactory + * @param HostChecker|null $hostChecker */ public function __construct( RequestInterface $request, @@ -85,7 +93,8 @@ class Redirect UrlInterface $url, DecoderInterface $urlDecoder, CustomerUrl $customerUrl, - ResultFactory $resultFactory + ResultFactory $resultFactory, + HostChecker $hostChecker = null ) { $this->request = $request; $this->session = $customerSession; @@ -95,6 +104,7 @@ class Redirect $this->urlDecoder = $urlDecoder; $this->customerUrl = $customerUrl; $this->resultFactory = $resultFactory; + $this->hostChecker = $hostChecker ?: ObjectManager::getInstance()->get(HostChecker::class); } /** @@ -196,7 +206,7 @@ class Redirect $referer = $this->request->getParam(CustomerUrl::REFERER_QUERY_PARAM_NAME); if ($referer) { $referer = $this->urlDecoder->decode($referer); - if ($this->url->isOwnOriginUrl()) { + if ($this->hostChecker->isOwnOrigin($referer)) { $this->applyRedirect($referer); } } diff --git a/app/code/Magento/Customer/Model/Url.php b/app/code/Magento/Customer/Model/Url.php index fa2ff49aafd3abfab8d69c21589b0c6366150424..470093717549a871fb3deb6d74690dcfc45f4f2b 100644 --- a/app/code/Magento/Customer/Model/Url.php +++ b/app/code/Magento/Customer/Model/Url.php @@ -56,25 +56,43 @@ class Url */ protected $urlEncoder; + /** + * @var \Magento\Framework\Url\DecoderInterface + */ + private $urlDecoder; + + /** + * @var \Magento\Framework\Url\HostChecker + */ + private $hostChecker; + /** * @param Session $customerSession * @param ScopeConfigInterface $scopeConfig * @param RequestInterface $request * @param UrlInterface $urlBuilder * @param EncoderInterface $urlEncoder + * @param \Magento\Framework\Url\DecoderInterface|null $urlDecoder + * @param \Magento\Framework\Url\HostChecker|null $hostChecker */ public function __construct( Session $customerSession, ScopeConfigInterface $scopeConfig, RequestInterface $request, UrlInterface $urlBuilder, - EncoderInterface $urlEncoder + EncoderInterface $urlEncoder, + \Magento\Framework\Url\DecoderInterface $urlDecoder = null, + \Magento\Framework\Url\HostChecker $hostChecker = null ) { $this->request = $request; $this->urlBuilder = $urlBuilder; $this->scopeConfig = $scopeConfig; $this->customerSession = $customerSession; $this->urlEncoder = $urlEncoder; + $this->urlDecoder = $urlDecoder ?: \Magento\Framework\App\ObjectManager::getInstance() + ->get(\Magento\Framework\Url\DecoderInterface::class); + $this->hostChecker = $hostChecker ?: \Magento\Framework\App\ObjectManager::getInstance() + ->get(\Magento\Framework\Url\HostChecker::class); } /** @@ -95,7 +113,7 @@ class Url public function getLoginUrlParams() { $params = []; - $referer = $this->request->getParam(self::REFERER_QUERY_PARAM_NAME); + $referer = $this->getRequestReferrer(); if (!$referer && !$this->scopeConfig->isSetFlag( self::XML_PATH_CUSTOMER_STARTUP_REDIRECT_TO_DASHBOARD, @@ -122,9 +140,10 @@ class Url public function getLoginPostUrl() { $params = []; - if ($this->request->getParam(self::REFERER_QUERY_PARAM_NAME)) { + $referer = $this->getRequestReferrer(); + if ($referer) { $params = [ - self::REFERER_QUERY_PARAM_NAME => $this->request->getParam(self::REFERER_QUERY_PARAM_NAME), + self::REFERER_QUERY_PARAM_NAME => $referer, ]; } return $this->urlBuilder->getUrl('customer/account/loginPost', $params); @@ -220,4 +239,16 @@ class Url { return $this->urlBuilder->getUrl('customer/account/confirmation', ['email' => $email]); } + + /** + * @return mixed|null + */ + private function getRequestReferrer() + { + $referer = $this->request->getParam(self::REFERER_QUERY_PARAM_NAME); + if ($referer && $this->hostChecker->isOwnOrigin($this->urlDecoder->decode($referer))) { + return $referer; + } + return null; + } } diff --git a/app/code/Magento/Customer/Test/Unit/Model/Account/RedirectTest.php b/app/code/Magento/Customer/Test/Unit/Model/Account/RedirectTest.php index 5d512bcc6bda1a4fc0f154447a9ec91d74b74228..47b7ea669de30ead287169d1170acbb2c18ae301 100644 --- a/app/code/Magento/Customer/Test/Unit/Model/Account/RedirectTest.php +++ b/app/code/Magento/Customer/Test/Unit/Model/Account/RedirectTest.php @@ -13,6 +13,7 @@ namespace Magento\Customer\Test\Unit\Model\Account; use Magento\Customer\Model\Account\Redirect; use Magento\Customer\Model\Url as CustomerUrl; use Magento\Framework\Controller\ResultFactory; +use Magento\Framework\Url\HostChecker; use Magento\Store\Model\ScopeInterface; use Magento\Framework\TestFramework\Unit\Helper\ObjectManager; @@ -81,6 +82,11 @@ class RedirectTest extends \PHPUnit_Framework_TestCase */ protected $resultFactory; + /** + * @var HostChecker | \PHPUnit_Framework_MockObject_MockObject + */ + private $hostChecker; + protected function setUp() { $this->request = $this->getMockForAbstractClass(\Magento\Framework\App\RequestInterface::class); @@ -134,6 +140,10 @@ class RedirectTest extends \PHPUnit_Framework_TestCase ->disableOriginalConstructor() ->getMock(); + $this->hostChecker = $this->getMockBuilder(HostChecker::class) + ->disableOriginalConstructor() + ->getMock(); + $objectManager = new ObjectManager($this); $this->model = $objectManager->getObject( \Magento\Customer\Model\Account\Redirect::class, @@ -145,7 +155,8 @@ class RedirectTest extends \PHPUnit_Framework_TestCase 'url' => $this->url, 'urlDecoder' => $this->urlDecoder, 'customerUrl' => $this->customerUrl, - 'resultFactory' => $this->resultFactory + 'resultFactory' => $this->resultFactory, + 'hostChecker' => $this->hostChecker ] ); } @@ -254,6 +265,7 @@ class RedirectTest extends \PHPUnit_Framework_TestCase $this->resultRedirect->expects($this->once()) ->method('setUrl') + ->with($beforeAuthUrl) ->willReturnSelf(); $this->resultFactory->expects($this->once()) @@ -286,6 +298,7 @@ class RedirectTest extends \PHPUnit_Framework_TestCase return [ // Loggend In, Redirect by Referer [1, 2, 'referer', 'base', '', '', 'account', '', '', '', true, false], + [1, 2, 'http://referer.com/', 'http://base.com/', '', '', 'account', '', '', 'dashboard', true, false], // Loggend In, Redirect by AfterAuthUrl [1, 2, 'referer', 'base', '', 'defined', 'account', '', '', '', true, true], // Not logged In, Redirect by LoginUrl diff --git a/lib/internal/Magento/Framework/Test/Unit/UrlTest.php b/lib/internal/Magento/Framework/Test/Unit/UrlTest.php index 36fce179395b5463fc3cf522defc5075038fc5c1..939e9a39bea584adc34037fcb9a899c4d3fe3e68 100644 --- a/lib/internal/Magento/Framework/Test/Unit/UrlTest.php +++ b/lib/internal/Magento/Framework/Test/Unit/UrlTest.php @@ -7,6 +7,7 @@ // @codingStandardsIgnoreFile namespace Magento\Framework\Test\Unit; +use Magento\Framework\Url\HostChecker; /** * Test class for Magento\Framework\Url @@ -59,6 +60,11 @@ class UrlTest extends \PHPUnit_Framework_TestCase */ protected $urlModifier; + /** + * @var HostChecker|\PHPUnit_Framework_MockObject_MockObject + */ + private $hostChecker; + protected function setUp() { $this->routeParamsResolverMock = $this->getMock( @@ -549,18 +555,17 @@ class UrlTest extends \PHPUnit_Framework_TestCase /** * @param bool $result - * @param string $baseUrl * @param string $referrer * @dataProvider isOwnOriginUrlDataProvider */ - public function testIsOwnOriginUrl($result, $baseUrl, $referrer) + public function testIsOwnOriginUrl($result, $referrer) { $requestMock = $this->getRequestMock(); - $model = $this->getUrlModel(['scopeResolver' => $this->scopeResolverMock, 'request' => $requestMock]); + $this->hostChecker = $this->getMockBuilder(HostChecker::class) + ->disableOriginalConstructor()->getMock(); + $this->hostChecker->expects($this->once())->method('isOwnOrigin')->with($referrer)->willReturn($result); + $model = $this->getUrlModel(['hostChecker' => $this->hostChecker, 'request' => $requestMock]); - $this->scopeMock->expects($this->any())->method('getBaseUrl')->will($this->returnValue($baseUrl)); - $this->scopeResolverMock->expects($this->any())->method('getScopes') - ->will($this->returnValue([$this->scopeMock])); $requestMock->expects($this->once())->method('getServer')->with('HTTP_REFERER') ->will($this->returnValue($referrer)); @@ -570,8 +575,8 @@ class UrlTest extends \PHPUnit_Framework_TestCase public function isOwnOriginUrlDataProvider() { return [ - 'is origin url' => [true, 'http://localhost/', 'http://localhost/'], - 'is not origin url' => [false, 'http://localhost/', 'http://example.com/'], + 'is origin url' => [true, 'http://localhost/'], + 'is not origin url' => [false, 'http://example.com/'], ]; } diff --git a/lib/internal/Magento/Framework/Url.php b/lib/internal/Magento/Framework/Url.php index 7361fdb336dd8ecd9ce58507dda027f1731f1d58..30af3528b2baba392095ecb86f9e65eb78bfd8a1 100644 --- a/lib/internal/Magento/Framework/Url.php +++ b/lib/internal/Magento/Framework/Url.php @@ -8,6 +8,8 @@ namespace Magento\Framework; +use Magento\Framework\Url\HostChecker; + /** * URL * @@ -178,6 +180,11 @@ class Url extends \Magento\Framework\DataObject implements \Magento\Framework\Ur */ private $escaper; + /** + * @var HostChecker + */ + private $hostChecker; + /** * @param \Magento\Framework\App\Route\ConfigInterface $routeConfig * @param \Magento\Framework\App\RequestInterface $request @@ -191,6 +198,7 @@ class Url extends \Magento\Framework\DataObject implements \Magento\Framework\Ur * @param \Magento\Framework\Url\RouteParamsPreprocessorInterface $routeParamsPreprocessor * @param string $scopeType * @param array $data + * @param HostChecker|null $hostChecker * @SuppressWarnings(PHPMD.ExcessiveParameterList) */ public function __construct( @@ -205,7 +213,8 @@ class Url extends \Magento\Framework\DataObject implements \Magento\Framework\Ur \Magento\Framework\App\Config\ScopeConfigInterface $scopeConfig, \Magento\Framework\Url\RouteParamsPreprocessorInterface $routeParamsPreprocessor, $scopeType, - array $data = [] + array $data = [], + HostChecker $hostChecker = null ) { $this->_request = $request; $this->_routeConfig = $routeConfig; @@ -218,6 +227,8 @@ class Url extends \Magento\Framework\DataObject implements \Magento\Framework\Ur $this->_scopeConfig = $scopeConfig; $this->routeParamsPreprocessor = $routeParamsPreprocessor; $this->_scopeType = $scopeType; + $this->hostChecker = $hostChecker ?: \Magento\Framework\App\ObjectManager::getInstance() + ->get(HostChecker::class); parent::__construct($data); } @@ -1086,17 +1097,7 @@ class Url extends \Magento\Framework\DataObject implements \Magento\Framework\Ur */ public function isOwnOriginUrl() { - $scopeDomains = []; - $referer = parse_url($this->_request->getServer('HTTP_REFERER'), PHP_URL_HOST); - foreach ($this->_scopeResolver->getScopes() as $scope) { - $scopeDomains[] = parse_url($scope->getBaseUrl(), PHP_URL_HOST); - $scopeDomains[] = parse_url($scope->getBaseUrl(UrlInterface::URL_TYPE_LINK, true), PHP_URL_HOST); - } - $scopeDomains = array_unique($scopeDomains); - if (empty($referer) || in_array($referer, $scopeDomains)) { - return true; - } - return false; + return $this->hostChecker->isOwnOrigin($this->_request->getServer('HTTP_REFERER')); } /** @@ -1163,7 +1164,7 @@ class Url extends \Magento\Framework\DataObject implements \Magento\Framework\Ur private function getUrlModifier() { if ($this->urlModifier === null) { - $this->urlModifier = \Magento\Framework\App\ObjectManager::getInstance()->get( + $this->urlModifier = \Magento\Framework\App\ObjectManager::getInstance()->get( \Magento\Framework\Url\ModifierInterface::class ); } diff --git a/lib/internal/Magento/Framework/Url/HostChecker.php b/lib/internal/Magento/Framework/Url/HostChecker.php new file mode 100644 index 0000000000000000000000000000000000000000..32546595a040dee59dfd9de3dae2577c35bbf444 --- /dev/null +++ b/lib/internal/Magento/Framework/Url/HostChecker.php @@ -0,0 +1,48 @@ +<?php +/** + * Copyright © 2016 Magento. All rights reserved. + * See COPYING.txt for license details. + */ +namespace Magento\Framework\Url; + +use Magento\Framework\UrlInterface; + +/** + * Class provides functionality for checks of a host name + */ +class HostChecker +{ + /** + * @var \Magento\Framework\Url\ScopeResolverInterface + */ + private $scopeResolver; + + /** + * @param ScopeResolverInterface $scopeResolver + */ + public function __construct(ScopeResolverInterface $scopeResolver) + { + $this->scopeResolver = $scopeResolver; + } + + /** + * Check if provided URL is one of the domain URLs assigned to scopes + * + * @param string $url + * @return bool + */ + public function isOwnOrigin($url) + { + $scopeHostNames = []; + $hostName = parse_url($url, PHP_URL_HOST); + if (empty($hostName)) { + return true; + } + foreach ($this->scopeResolver->getScopes() as $scope) { + $scopeHostNames[] = parse_url($scope->getBaseUrl(), PHP_URL_HOST); + $scopeHostNames[] = parse_url($scope->getBaseUrl(UrlInterface::URL_TYPE_LINK, true), PHP_URL_HOST); + } + $scopeHostNames = array_unique($scopeHostNames); + return in_array($hostName, $scopeHostNames); + } +} diff --git a/lib/internal/Magento/Framework/Url/Test/Unit/HostCheckerTest.php b/lib/internal/Magento/Framework/Url/Test/Unit/HostCheckerTest.php new file mode 100644 index 0000000000000000000000000000000000000000..a6be097ac38ea19c2d1bdc2f8bac4cc12306f7b6 --- /dev/null +++ b/lib/internal/Magento/Framework/Url/Test/Unit/HostCheckerTest.php @@ -0,0 +1,63 @@ +<?php +/** + * Copyright © 2016 Magento. All rights reserved. + * See COPYING.txt for license details. + */ +namespace Magento\Framework\Url\Test\Unit; + +use Magento\Framework\TestFramework\Unit\Helper\ObjectManager; + +class HostCheckerTest extends \PHPUnit_Framework_TestCase +{ + /** @var \Magento\Framework\Url\HostChecker */ + private $object; + + /** @var \Magento\Framework\Url\ScopeResolverInterface|\PHPUnit_Framework_MockObject_MockObject */ + private $scopeResolver; + + /** + * @inheritdoc + */ + protected function setUp() + { + $this->scopeResolver = $this->getMockBuilder( + \Magento\Framework\Url\ScopeResolverInterface::class + )->getMock(); + + $objectManager = new ObjectManager($this); + $this->object = $objectManager->getObject( + \Magento\Framework\Url\HostChecker::class, + [ + 'scopeResolver' => $this->scopeResolver + ] + ); + } + + /** + * @dataProvider isOwnOriginDataProvider + * @param string $url + * @param boolean $result + */ + public function testIsOwnOrigin($url, $result) + { + $scopes[0] = $this->getMockBuilder(\Magento\Framework\Url\ScopeInterface::class)->getMock(); + $scopes[0]->expects($this->any())->method('getBaseUrl')->willReturn('http://www.example.com'); + $scopes[1] = $this->getMockBuilder(\Magento\Framework\Url\ScopeInterface::class)->getMock(); + $scopes[1]->expects($this->any())->method('getBaseUrl')->willReturn('https://www.example2.com'); + + $this->scopeResolver->expects($this->atLeastOnce())->method('getScopes')->willReturn($scopes); + + $this->assertEquals($result, $this->object->isOwnOrigin($url)); + } + + /** + * @return array + */ + public function isOwnOriginDataProvider() + { + return [ + ['http://www.example.com/some/page/', true], + ['http://www.test.com/other/page/', false], + ]; + } +}