Skip to content

Commit 34a21e8

Browse files
JacobBrownAustinJacobBrownAustin
committed
Merge commit '246ae08aef3ad42be7e9c69f442e7680584e2880' into ACPT-1718
2 parents 0a3b3a4 + 246ae08 commit 34a21e8

File tree

8 files changed

+108
-14
lines changed

8 files changed

+108
-14
lines changed

‎app/code/Magento/Cookie/etc/di.xml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,5 +7,6 @@
77
-->
88
<config xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:noNamespaceSchemaLocation="urn:magento:framework:ObjectManager/etc/config.xsd">
99
<preference for="Magento\Framework\Stdlib\CookieManagerInterface" type="Magento\Framework\Stdlib\Cookie\PhpCookieManager" />
10+
<preference for="Magento\Framework\Stdlib\CookieDisablerInterface" type="Magento\Framework\Stdlib\Cookie\PhpCookieDisabler" />
1011
</config>
1112

‎app/etc/di.xml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -94,6 +94,7 @@
9494
<preference for="Magento\Framework\Stdlib\Cookie\CookieScopeInterface" type="Magento\Framework\Stdlib\Cookie\CookieScope" />
9595
<preference for="Magento\Framework\Stdlib\Cookie\CookieReaderInterface" type="Magento\Framework\Stdlib\Cookie\PhpCookieReader" />
9696
<preference for="Magento\Framework\Stdlib\CookieManagerInterface" type="Magento\Framework\Stdlib\Cookie\PhpCookieManager" />
97+
<preference for="Magento\Framework\Stdlib\CookieDisablerInterface" type="Magento\Framework\Stdlib\Cookie\PhpCookieDisabler" />
9798
<preference for="Magento\Framework\TranslateInterface" type="Magento\Framework\Translate" />
9899
<preference for="Magento\Framework\Config\ScopeListInterface" type="interceptionConfigScope" />
99100
<preference for="Magento\Framework\View\Design\Theme\Label\ListInterface" type="Magento\Theme\Model\ResourceModel\Theme\Collection" />

‎dev/tests/api-functional/testsuite/Magento/GraphQl/GraphQl/GraphQlSessionTest.php

Lines changed: 47 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -48,6 +48,10 @@ public function setUp(): void
4848
/**
4949
* Test for checking if graphQL query sets session cookies
5050
*
51+
* Note: The reason why the first response doesn't have cookies, but the subsequent responses do is
52+
* because Magento/Framework/App/PageCache/Kernel.php removes Set-Cookie headers when the response has a
53+
* public Cache-Control. This test asserts that behaviour.
54+
*
5155
* @magentoApiDataFixture Magento/Catalog/_files/categories.php
5256
* @magentoConfigFixture graphql/session/disable 0
5357
*/
@@ -71,8 +75,7 @@ public function testCheckSessionCookieWithGetCategoryList(): void
7175
$result = $this->graphQlClient->getWithResponseHeaders($query, [], '', [], true);
7276
$this->assertEmpty($result['cookies']);
7377
// perform secondary request after cookies have been flushed
74-
$result = $this->graphQlClient->getWithResponseHeaders($query, [], '', []);
75-
78+
$result = $this->graphQlClient->getWithResponseHeaders($query, [], '', [], true);
7679
// may have other cookies than session
7780
$this->assertNotEmpty($result['cookies']);
7881
$this->assertAnyCookieMatchesRegex('/PHPSESSID=[a-z0-9]+;/', $result['cookies']);
@@ -280,4 +283,46 @@ private function assertNoCookiesMatchRegex(string $pattern, array $cookies): voi
280283
}
281284
$this->assertTrue($result, 'Failed assertion. At least one cookie in the array matches pattern: ' . $pattern);
282285
}
286+
287+
/**
288+
* Tests that Magento\Customer\Model\Session works properly when graphql/session/disable=0
289+
*
290+
* @magentoApiDataFixture Magento/Customer/_files/customer.php
291+
* @magentoConfigFixture graphql/session/disable 0
292+
*/
293+
public function testCustomerCanQueryOwnEmailUsingSession() : void
294+
{
295+
$query = '{customer{email}}';
296+
$result = $this->graphQlClient->postWithResponseHeaders($query, [], '', $this->getAuthHeaders(), true);
297+
// cookies are never empty and session is restarted for the authorized customer regardless current session
298+
$this->assertNotEmpty($result['cookies']);
299+
$this->assertAnyCookieMatchesRegex('/PHPSESSID=[a-z0-9]+;/', $result['cookies']);
300+
$this->assertEquals('customer@example.com', $result['body']['customer']['email'] ?? '');
301+
$result = $this->graphQlClient->postWithResponseHeaders($query, [], '', $this->getAuthHeaders());
302+
// cookies are never empty and session is restarted for the authorized customer
303+
// regardless current session and missing flush
304+
$this->assertNotEmpty($result['cookies']);
305+
$this->assertAnyCookieMatchesRegex('/PHPSESSID=[a-z0-9]+;/', $result['cookies']);
306+
$this->assertEquals('customer@example.com', $result['body']['customer']['email'] ?? '');
307+
/* Note: This third request is the actual one that tests that the session cookie is properly used.
308+
* This time we don't send the Authorization header and rely on Cookie header instead.
309+
* Because of bug in postWithResponseHeaders's $flushCookies parameter not being properly used,
310+
* We have to manually set cookie header ourselves. :-(
311+
*/
312+
$cookiesToSend = '';
313+
foreach ($result['cookies'] as $cookie) {
314+
preg_match('/^([^;]*);/', $cookie, $matches);
315+
if (!strlen($matches[1] ?? '')) {
316+
continue;
317+
}
318+
if (!empty($cookiesToSend)) {
319+
$cookiesToSend .= '; ';
320+
}
321+
$cookiesToSend .= $matches[1];
322+
}
323+
$result = $this->graphQlClient->postWithResponseHeaders($query, [], '', ['Cookie: ' . $cookiesToSend]);
324+
$this->assertNotEmpty($result['cookies']);
325+
$this->assertAnyCookieMatchesRegex('/PHPSESSID=[a-z0-9]+;/', $result['cookies']);
326+
$this->assertEquals('customer@example.com', $result['body']['customer']['email'] ?? '');
327+
}
283328
}

‎dev/tests/static/framework/Magento/PhpStan/Formatters/FilteredErrorFormatter.php

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -76,7 +76,8 @@ public function formatErrors(AnalysisResult $analysisResult, Output $output): in
7676
$analysisResult->isDefaultLevelUsed(),
7777
$analysisResult->getProjectConfigFile(),
7878
$analysisResult->isResultCacheSaved(),
79-
$analysisResult->getPeakMemoryUsageBytes()
79+
$analysisResult->getPeakMemoryUsageBytes(),
80+
$analysisResult->isResultCacheUsed(),
8081
);
8182

8283
return $this->tableErrorFormatter->formatErrors($clearedAnalysisResult, $output);

‎lib/internal/Magento/Framework/App/PageCache/Kernel.php

Lines changed: 9 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77

88
use Magento\Framework\App\State as AppState;
99
use Magento\Framework\App\ObjectManager;
10+
use Magento\Framework\Stdlib\CookieDisablerInterface;
1011

1112
/**
1213
* Builtin cache processor
@@ -68,6 +69,9 @@ class Kernel
6869
*/
6970
private $identifierForSave;
7071

72+
// phpcs:disable Magento2.Commenting.ClassPropertyPHPDocFormatting
73+
private readonly CookieDisablerInterface $cookieDisabler;
74+
7175
/**
7276
* @param Cache $cache
7377
* @param \Magento\Framework\App\PageCache\IdentifierInterface $identifier
@@ -79,6 +83,7 @@ class Kernel
7983
* @param AppState|null $state
8084
* @param \Magento\PageCache\Model\Cache\Type|null $fullPageCache
8185
* @param \Magento\Framework\App\PageCache\IdentifierInterface|null $identifierForSave
86+
* @param CookieDisablerInterface|null $cookieDisabler
8287
* @SuppressWarnings(PHPMD.ExcessiveParameterList)
8388
*/
8489
public function __construct(
@@ -91,7 +96,8 @@ public function __construct(
9196
\Magento\Framework\Serialize\SerializerInterface $serializer = null,
9297
AppState $state = null,
9398
\Magento\PageCache\Model\Cache\Type $fullPageCache = null,
94-
\Magento\Framework\App\PageCache\IdentifierInterface $identifierForSave = null
99+
\Magento\Framework\App\PageCache\IdentifierInterface $identifierForSave = null,
100+
?CookieDisablerInterface $cookieDisabler = null,
95101
) {
96102
$this->cache = $cache;
97103
$this->identifier = $identifier;
@@ -113,6 +119,7 @@ public function __construct(
113119
$this->identifierForSave = $identifierForSave ?? ObjectManager::getInstance()->get(
114120
\Magento\Framework\App\PageCache\IdentifierInterface::class
115121
);
122+
$this->cookieDisabler = $cookieDisabler ?? ObjectManager::getInstance()->get(CookieDisablerInterface::class);
116123
}
117124

118125
/**
@@ -163,9 +170,7 @@ public function process(\Magento\Framework\App\Response\Http $response)
163170
if ($this->state->getMode() != AppState::MODE_DEVELOPER) {
164171
$response->clearHeader('X-Magento-Tags');
165172
}
166-
if (!headers_sent()) {
167-
header_remove('Set-Cookie');
168-
}
173+
$this->cookieDisabler->setCookiesDisabled(true);
169174

170175
$this->fullPageCache->save(
171176
$this->serializer->serialize($this->getPreparedData($response)),

‎lib/internal/Magento/Framework/Session/SessionManager.php

Lines changed: 0 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -638,13 +638,6 @@ private function initIniOptions()
638638
*/
639639
public function _resetState(): void
640640
{
641-
if (session_status() === PHP_SESSION_ACTIVE) {
642-
session_write_close();
643-
session_id('');
644-
}
645-
session_name('PHPSESSID');
646-
session_unset();
647641
static::$urlHostCache = [];
648-
$_SESSION = [];
649642
}
650643
}

‎lib/internal/Magento/Framework/Stdlib/Cookie/PhpCookieDisabler.php

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,26 @@
1+
<?php
2+
/**
3+
* Copyright © Magento, Inc. All rights reserved.
4+
* See COPYING.txt for license details.
5+
*/
6+
declare(strict_types=1);
7+
8+
namespace Magento\Framework\Stdlib\Cookie;
9+
10+
use Magento\Framework\Stdlib\CookieDisablerInterface;
11+
12+
/**
13+
* Disables sending the cookies that are currently set.
14+
*/
15+
class PhpCookieDisabler implements CookieDisablerInterface
16+
{
17+
/**
18+
* @inheritDoc
19+
*/
20+
public function setCookiesDisabled(bool $disabled) : void
21+
{
22+
if ($disabled && !headers_sent()) {
23+
header_remove('Set-Cookie');
24+
}
25+
}
26+
}

‎lib/internal/Magento/Framework/Stdlib/CookieDisablerInterface.php

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,22 @@
1+
<?php
2+
/**
3+
* Copyright © Magento, Inc. All rights reserved.
4+
* See COPYING.txt for license details.
5+
*/
6+
declare(strict_types=1);
7+
8+
namespace Magento\Framework\Stdlib;
9+
10+
/**
11+
* This interface is for when you need to disable all cookies from being sent in the HTTP response
12+
*/
13+
interface CookieDisablerInterface
14+
{
15+
/**
16+
* Set Cookies Disabled. If true, cookies won't be sent.
17+
*
18+
* @param bool $disabled
19+
* @return void
20+
*/
21+
public function setCookiesDisabled(bool $disabled) : void;
22+
}

0 commit comments

Comments
 (0)