From f920c13f6da59351e0aa156d0b8d6cf6cedf4f86 Mon Sep 17 00:00:00 2001 From: Jason Tolhurst Date: Wed, 24 Oct 2018 09:02:02 -0400 Subject: [PATCH 1/2] Fixed an issue where GET query parameters get stripped from the afterLoginReferer url during the login check. --- Controller/LoginCheck.php | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/Controller/LoginCheck.php b/Controller/LoginCheck.php index 6f30819..fb39e9b 100644 --- a/Controller/LoginCheck.php +++ b/Controller/LoginCheck.php @@ -116,7 +116,8 @@ public function execute() } $url = $this->_url->getCurrentUrl(); - $path = \parse_url($url, PHP_URL_PATH); + $urlParts = \parse_url($url); + $path = $urlParts['path']; $targetUrl = $this->getTargetUrl(); // current path is already pointing to target url, no redirect needed @@ -133,6 +134,11 @@ public function execute() } } + // Add any GET query parameters back to the path after making our url checks. + if(isset($urlParts['query']) && !empty($urlParts['query'])) { + $path .= '?' . $urlParts['query']; + } + if (!$this->isAjaxRequest()) { $this->session->setAfterLoginReferer($path); } From fb923aa0c37a6bbb9ce3ea3296140def67f011eb Mon Sep 17 00:00:00 2001 From: Jason Tolhurst Date: Tue, 13 Nov 2018 09:25:10 -0500 Subject: [PATCH 2/2] Added test for setAfterLoginReferer to ensure query params are retained. --- Test/Unit/Controller/LoginCheckUnitTest.php | 144 ++++++++++++++++++++ 1 file changed, 144 insertions(+) diff --git a/Test/Unit/Controller/LoginCheckUnitTest.php b/Test/Unit/Controller/LoginCheckUnitTest.php index 3ba8c60..463c8ed 100644 --- a/Test/Unit/Controller/LoginCheckUnitTest.php +++ b/Test/Unit/Controller/LoginCheckUnitTest.php @@ -912,4 +912,148 @@ public function ruleMatchingFailsAjaxCheckUsesHttpObject() $loginCheck->execute(); } + + /** + * Run test with default request object and with data not listed on the whitelist, so redirecting is forced and + * "isAjax" method is hit. + * + * @test + * @depends testConstructor + */ + public function redirectMatchesReferrerUrlWithQueryParameters() + { + $baseUrl = 'http://example.tld'; + $referrerUrl = 'http://example.tld/foo/bar?q=apples'; + $targetUrl = '/customer/account/login'; + $expectedTargetUrl = 'http://example.tld/customer/account/login'; + $expectedAfterLoginRedirect = '/foo/bar?q=apples'; + + // --- Scope Config + $scopeConfig = $this->getScopeConfig(); + $scopeConfig->expects($this->once()) + ->method('getValue') + ->with( + LoginCheckInterface::MODULE_CONFIG_TARGET, + ScopeInterface::SCOPE_STORE + ) + ->will($this->returnValue($targetUrl)); + + // --- StoreManager + $store = $this->getMockBuilder(StoreInterface::class) + ->setMethods([ + 'getBaseUrl', + 'getId', + 'setId', + 'getCode', + 'setCode', + 'getName', + 'setName', + 'getWebsiteId', + 'setWebsiteId', + 'getStoreGroupId', + 'setStoreGroupId', + 'getExtensionAttributes', + 'setExtensionAttributes' + ]) + ->getMock(); + $store->expects($this->once()) + ->method('getBaseUrl') + ->with(\Magento\Framework\UrlInterface::URL_TYPE_WEB, true) + ->will($this->returnValue($baseUrl)); + $storeManager = $this->getStoreManager(); + $storeManager->expects($this->once()) + ->method('getStore') + ->will($this->returnValue($store)); + + // --- Context + $url = $this->getUrl(); + $url->expects($this->once()) + ->method('getCurrentUrl') + ->will($this->returnValue($referrerUrl)); + + $request = $this->getRequestObject(); + $response = $this->getResponse(); + $redirect = $this->getRedirect(); + + $context = $this->getContext(); + $context->expects($this->exactly(1)) + ->method('getUrl') + ->will($this->returnValue($url)); + $context->expects($this->once()) + ->method('getRequest') + ->will($this->returnValue($request)); + $context->expects($this->once()) + ->method('getResponse') + ->will($this->returnValue($response)); + $context->expects($this->once()) + ->method('getRedirect') + ->will($this->returnValue($redirect)); + + // --- Response + $responseHttp = $this->getResponseHttp(); + $responseHttp->expects($this->once()) + ->method('setNoCacheHeaders'); + $responseHttp->expects($this->once()) + ->method('setRedirect') + ->with($expectedTargetUrl); + $responseHttp->expects($this->once()) + ->method('sendResponse'); + + // --- Request + $request->expects($this->once()) + ->method('isAjax') + ->willReturn(false); + + // --- Whitelist Entries + $whitelistEntityOne = $this->getMockBuilder(WhitelistEntry::class) + ->disableOriginalConstructor() + ->getMock(); + $whitelistEntityOne->expects($this->once()) + ->method('getStrategy') + ->will($this->returnValue('default')); + $whitelistCollection = $this + ->getMockBuilder(Collection::class) + ->disableOriginalConstructor() + ->getMock(); + $whitelistCollection->expects($this->once()) + ->method('getItems') + ->will($this->returnValue([$whitelistEntityOne])); + $whitelistRepository = $this->getWhitelistRepository(); + $whitelistRepository->expects($this->once()) + ->method('getCollection') + ->will($this->returnValue($whitelistCollection)); + + // --- Strategy + $strategy = $this->createMock(StrategyInterface::class); + $strategy->expects($this->once()) + ->method('isMatch') + ->with('/foo/bar', $whitelistEntityOne) + ->willReturn(false); + + $strategyManager = $this->getStrategyManager(); + $strategyManager->expects($this->once()) + ->method('get') + ->with('default') + ->will($this->returnValue($strategy)); + + // -- Session + $session = $this->getSession(); + $session->expects($this->once()) + ->method('setAfterLoginReferer') + ->with($expectedAfterLoginRedirect); + + $loginCheck = new LoginCheck( + $context, + $this->getCustomerSession(), + $session, + $storeManager, + $scopeConfig, + $whitelistRepository, + $strategyManager, + $this->getModuleCheck(), + $responseHttp + ); + + $loginCheck->execute(); + } }