Skip to content

Commit ba1a099

Browse files
dvoskoboinikovdvoskoboinikov
authored
Merge MC-29565 into MC-29564
2 parents d001f7b + 0a5a07e commit ba1a099

File tree

14 files changed

+207
-162
lines changed

14 files changed

+207
-162
lines changed

‎app/code/Magento/Customer/Model/Session.php

Lines changed: 6 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@
1919
* @method string getNoReferer()
2020
* @SuppressWarnings(PHPMD.CouplingBetweenObjects)
2121
* @SuppressWarnings(PHPMD.CookieAndSessionMisuse)
22+
* @SuppressWarnings(PHPMD.TooManyFields)
2223
* @since 100.0.2
2324
*/
2425
class Session extends \Magento\Framework\Session\SessionManager
@@ -108,6 +109,11 @@ class Session extends \Magento\Framework\Session\SessionManager
108109
*/
109110
protected $response;
110111

112+
/**
113+
* @var AccountConfirmation
114+
*/
115+
private $accountConfirmation;
116+
111117
/**
112118
* Session constructor.
113119
*
@@ -511,13 +517,6 @@ public function authenticate($loginUrl = null)
511517
$this->response->setRedirect($loginUrl);
512518
} else {
513519
$arguments = $this->_customerUrl->getLoginUrlParams();
514-
if ($this->_createUrl()->getUseSession()) {
515-
$arguments += [
516-
'_query' => [
517-
$this->sidResolver->getSessionIdQueryParam($this->_session) => $this->_session->getSessionId(),
518-
]
519-
];
520-
}
521520
$this->response->setRedirect(
522521
$this->_createUrl()->getUrl(\Magento\Customer\Model\Url::ROUTE_ACCOUNT_LOGIN, $arguments)
523522
);
@@ -535,8 +534,6 @@ public function authenticate($loginUrl = null)
535534
*/
536535
protected function _setAuthUrl($key, $url)
537536
{
538-
$url = $this->_coreUrl->removeRequestParam($url, $this->sidResolver->getSessionIdQueryParam($this));
539-
// Add correct session ID to URL if needed
540537
$url = $this->_createUrl()->getRebuiltUrl($url);
541538
return $this->storage->setData($key, $url);
542539
}

‎app/code/Magento/Customer/Test/Unit/Model/SessionTest.php

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -147,10 +147,10 @@ public function testAuthenticate()
147147
$urlMock->expects($this->once())
148148
->method('getRebuiltUrl')
149149
->willReturn('');
150-
$this->urlFactoryMock->expects($this->exactly(4))
150+
$this->urlFactoryMock->expects($this->exactly(3))
151151
->method('create')
152152
->willReturn($urlMock);
153-
$urlMock->expects($this->once())
153+
$urlMock->expects($this->never())
154154
->method('getUseSession')
155155
->willReturn(false);
156156

‎app/code/Magento/Store/Controller/Store/Redirect.php

Lines changed: 7 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -11,19 +11,15 @@
1111
use Magento\Framework\App\Action\Context;
1212
use Magento\Framework\App\Action\HttpGetActionInterface;
1313
use Magento\Framework\App\Action\HttpPostActionInterface;
14-
use Magento\Framework\App\ResponseInterface;
15-
use Magento\Framework\Controller\ResultInterface;
1614
use Magento\Framework\Exception\NoSuchEntityException;
17-
use Magento\Framework\Session\Generic as Session;
18-
use Magento\Framework\Session\SidResolverInterface;
1915
use Magento\Store\Api\StoreRepositoryInterface;
2016
use Magento\Store\Api\StoreResolverInterface;
2117
use Magento\Store\Model\Store;
2218
use Magento\Store\Model\StoreResolver;
2319
use Magento\Store\Model\StoreSwitcher\HashGenerator;
2420

2521
/**
26-
* Builds correct url to target store and performs redirect.
22+
* Builds correct url to target store (group) and performs redirect.
2723
*/
2824
class Redirect extends Action implements HttpGetActionInterface, HttpPostActionInterface
2925
{
@@ -37,16 +33,6 @@ class Redirect extends Action implements HttpGetActionInterface, HttpPostActionI
3733
*/
3834
private $storeResolver;
3935

40-
/**
41-
* @var SidResolverInterface
42-
*/
43-
private $sidResolver;
44-
45-
/**
46-
* @var Session
47-
*/
48-
private $session;
49-
5036
/**
5137
* @var HashGenerator
5238
*/
@@ -56,30 +42,28 @@ class Redirect extends Action implements HttpGetActionInterface, HttpPostActionI
5642
* @param Context $context
5743
* @param StoreRepositoryInterface $storeRepository
5844
* @param StoreResolverInterface $storeResolver
59-
* @param Session $session
60-
* @param SidResolverInterface $sidResolver
45+
* @param \Magento\Framework\Session\Generic $session
46+
* @param \Magento\Framework\Session\SidResolverInterface $sidResolver
6147
* @param HashGenerator $hashGenerator
48+
* @SuppressWarnings(PHPMD.UnusedFormalParameter)
6249
*/
6350
public function __construct(
6451
Context $context,
6552
StoreRepositoryInterface $storeRepository,
6653
StoreResolverInterface $storeResolver,
67-
Session $session,
68-
SidResolverInterface $sidResolver,
54+
\Magento\Framework\Session\Generic $session,
55+
\Magento\Framework\Session\SidResolverInterface $sidResolver,
6956
HashGenerator $hashGenerator
7057
) {
7158
parent::__construct($context);
7259
$this->storeRepository = $storeRepository;
7360
$this->storeResolver = $storeResolver;
74-
$this->session = $session;
75-
$this->sidResolver = $sidResolver;
7661
$this->hashGenerator = $hashGenerator;
7762
}
7863

7964
/**
80-
* Performs store redirect
65+
* @inheritDoc
8166
*
82-
* @return ResponseInterface|ResultInterface
8367
* @throws NoSuchEntityException
8468
*/
8569
public function execute()
@@ -113,12 +97,6 @@ public function execute()
11397
\Magento\Framework\App\ActionInterface::PARAM_NAME_URL_ENCODED => $encodedUrl,
11498
];
11599

116-
if ($this->sidResolver->getUseSessionInUrl()) {
117-
// allow customers to stay logged in during store switching
118-
$sidName = $this->sidResolver->getSessionIdQueryParam($this->session);
119-
$query[$sidName] = $this->session->getSessionId();
120-
}
121-
122100
$customerHash = $this->hashGenerator->generateHash($fromStore);
123101
$query = array_merge($query, $customerHash);
124102

‎app/code/Magento/Store/Model/Store.php

Lines changed: 1 addition & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -278,6 +278,7 @@ class Store extends AbstractExtensibleModel implements
278278

279279
/**
280280
* @var \Magento\Framework\Session\SidResolverInterface
281+
* @deprecated Not used anymore.
281282
*/
282283
protected $_sidResolver;
283284

@@ -1199,7 +1200,6 @@ public function isDefault()
11991200
*/
12001201
public function getCurrentUrl($fromStore = true)
12011202
{
1202-
$sidQueryParam = $this->_sidResolver->getSessionIdQueryParam($this->_getSession());
12031203
$requestString = $this->_url->escape(ltrim($this->_request->getRequestString(), '/'));
12041204

12051205
$storeUrl = $this->getUrl('', ['_secure' => $this->_storeManager->getStore()->isCurrentlySecure()]);
@@ -1218,12 +1218,6 @@ public function getCurrentUrl($fromStore = true)
12181218
}
12191219

12201220
$currQuery = $this->_request->getQueryValue();
1221-
if (isset($currQuery[$sidQueryParam])
1222-
&& !empty($currQuery[$sidQueryParam])
1223-
&& $this->_getSession()->getSessionIdForHost($storeUrl) != $currQuery[$sidQueryParam]
1224-
) {
1225-
unset($currQuery[$sidQueryParam]);
1226-
}
12271221

12281222
foreach ($currQuery as $key => $value) {
12291223
$storeParsedQuery[$key] = $value;

‎dev/tests/integration/testsuite/Magento/Customer/Model/SessionTest.php

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,9 +6,12 @@
66
namespace Magento\Customer\Model;
77

88
use Magento\Framework\App\PageCache\FormKey;
9+
use Magento\Framework\App\ResponseInterface;
10+
use Magento\Framework\Session\SidResolverInterface;
911
use Magento\Framework\Stdlib\Cookie\CookieMetadataFactory;
1012
use Magento\Framework\Stdlib\Cookie\PublicCookieMetadata;
1113
use Magento\TestFramework\Helper\Bootstrap;
14+
use Magento\Framework\App\Response\Http as HttpResponse;
1215

1316
/**
1417
* @magentoDataFixture Magento/Customer/_files/customer.php
@@ -29,6 +32,11 @@ class SessionTest extends \PHPUnit\Framework\TestCase
2932
/** @var PublicCookieMetadata $cookieMetadata */
3033
protected $cookieMetadata;
3134

35+
/**
36+
* @var HttpResponse
37+
*/
38+
private $response;
39+
3240
protected function setUp()
3341
{
3442
$this->_customerSession = Bootstrap::getObjectManager()->create(
@@ -48,6 +56,7 @@ protected function setUp()
4856
'form_key',
4957
$this->cookieMetadata
5058
);
59+
$this->response = Bootstrap::getObjectManager()->get(ResponseInterface::class);
5160
}
5261

5362
public function testLoginById()
@@ -100,4 +109,26 @@ public function testLogoutActionFlushesFormKey()
100109

101110
$this->assertNotEquals($beforeKey, $afterKey);
102111
}
112+
113+
/**
114+
* Check that SID is not used in redirects.
115+
*
116+
* @return void
117+
* @magentoConfigFixture current_store web/session/use_frontend_sid 1
118+
*/
119+
public function testNoSid(): void
120+
{
121+
$this->_customerSession->authenticate();
122+
$location = (string)$this->response->getHeader('Location');
123+
$this->assertNotEmpty($location);
124+
$this->assertNotContains(SidResolverInterface::SESSION_ID_QUERY_PARAM .'=', $location);
125+
$beforeAuthUrl = $this->_customerSession->getData('before_auth_url');
126+
$this->assertNotEmpty($beforeAuthUrl);
127+
$this->assertNotContains(SidResolverInterface::SESSION_ID_QUERY_PARAM .'=', $beforeAuthUrl);
128+
129+
$this->_customerSession->authenticate('/customer/account');
130+
$location = (string)$this->response->getHeader('Location');
131+
$this->assertNotEmpty($location);
132+
$this->assertNotContains(SidResolverInterface::SESSION_ID_QUERY_PARAM .'=', $location);
133+
}
103134
}

‎dev/tests/integration/testsuite/Magento/Framework/Session/SessionManagerTest.php

Lines changed: 10 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -52,7 +52,7 @@ function ini_set($varName, $newValue)
5252
SessionManagerTest::$isIniSetInvoked[$varName] = $newValue;
5353
return true;
5454
}
55-
return call_user_func_array('\ini_set', func_get_args());
55+
return call_user_func_array('\ini_set', [$varName, $newValue]);
5656
}
5757

5858
/**
@@ -213,15 +213,16 @@ public function testDestroy()
213213
public function testSetSessionId()
214214
{
215215
$this->initializeModel();
216-
$sessionId = $this->model->getSessionId();
217-
$this->appState->expects($this->atLeastOnce())
216+
$this->assertNotEmpty($this->model->getSessionId());
217+
$this->appState->expects($this->any())
218218
->method('getAreaCode')
219219
->willReturn(\Magento\Framework\App\Area::AREA_FRONTEND);
220-
$this->model->setSessionId($this->sidResolver->getSid($this->model));
221-
$this->assertEquals($sessionId, $this->model->getSessionId());
222220

223221
$this->model->setSessionId('test');
224222
$this->assertEquals('test', $this->model->getSessionId());
223+
/* Use not valid identifier */
224+
$this->model->setSessionId('test_id');
225+
$this->assertEquals('test', $this->model->getSessionId());
225226
}
226227

227228
/**
@@ -230,17 +231,14 @@ public function testSetSessionId()
230231
public function testSetSessionIdFromParam()
231232
{
232233
$this->initializeModel();
233-
$this->appState->expects($this->atLeastOnce())
234+
$this->appState->expects($this->any())
234235
->method('getAreaCode')
235236
->willReturn(\Magento\Framework\App\Area::AREA_FRONTEND);
237+
$currentId = $this->model->getSessionId();
236238
$this->assertNotEquals('test_id', $this->model->getSessionId());
237-
$this->request->getQuery()->set($this->sidResolver->getSessionIdQueryParam($this->model), 'test-id');
238-
$this->model->setSessionId($this->sidResolver->getSid($this->model));
239-
$this->assertEquals('test-id', $this->model->getSessionId());
240-
/* Use not valid identifier */
241-
$this->request->getQuery()->set($this->sidResolver->getSessionIdQueryParam($this->model), 'test_id');
239+
$this->request->getQuery()->set(SidResolverInterface::SESSION_ID_QUERY_PARAM, 'test-id');
242240
$this->model->setSessionId($this->sidResolver->getSid($this->model));
243-
$this->assertEquals('test-id', $this->model->getSessionId());
241+
$this->assertEquals($currentId, $this->model->getSessionId());
244242
}
245243

246244
public function testGetSessionIdForHost()

‎dev/tests/integration/testsuite/Magento/Framework/UrlTest.php

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -477,6 +477,8 @@ public function testGetDirectUrl()
477477
}
478478

479479
/**
480+
* Check that SID is removed from URL.
481+
*
480482
* Note: isolation flushes the URL memory cache
481483
* @magentoAppIsolation enabled
482484
*
@@ -485,11 +487,8 @@ public function testGetDirectUrl()
485487
*/
486488
public function testSessionUrlVar()
487489
{
488-
$sessionId = \Magento\TestFramework\Helper\Bootstrap::getObjectManager()->get(
489-
\Magento\Framework\Session\Generic::class
490-
)->getSessionId();
491490
$sessionUrl = $this->model->sessionUrlVar('<a href="http://example.com/?___SID=U">www.example.com</a>');
492-
$this->assertEquals('<a href="http://example.com/?SID=' . $sessionId . '">www.example.com</a>', $sessionUrl);
491+
$this->assertEquals('<a href="http://example.com/">www.example.com</a>', $sessionUrl);
493492
}
494493

495494
public function testUseSessionIdForUrl()

0 commit comments

Comments
 (0)