Skip to content

Commit

Permalink
bug #36118 [Security/Http] don't require the session to be started wh…
Browse files Browse the repository at this point in the history
…en tracking its id (nicolas-grekas)

This PR was merged into the 4.4 branch.

Discussion
----------

[Security/Http] don't require the session to be started when tracking its id

| Q             | A
| ------------- | ---
| Branch?       | 4.4
| Bug fix?      | yes
| New feature?  | no
| Deprecations? | no
| Tickets       | -
| License       | MIT
| Doc PR        | -

`$session->getId()` returns the empty string when the session is not yet started.
When this happens, the session tracking logic wrongly detects that a new session was created and thus disables HTTP caching.

This fixes the issue by looking at the value of the session cookie instead.
(the case for `true` is when using `MockArraySessionStorage` as done in tests)

Commits
-------

c39188a [Security/Http] don't require the session to be started when tracking its id
  • Loading branch information
fabpot committed Mar 18, 2020
2 parents 7baec32 + c39188a commit abefccf
Show file tree
Hide file tree
Showing 2 changed files with 22 additions and 2 deletions.
Expand Up @@ -115,10 +115,10 @@ public function authenticate(RequestEvent $event)

if (null !== $session) {
$usageIndexValue = method_exists(Request::class, 'getPreferredFormat') && $session instanceof Session ? $usageIndexReference = &$session->getUsageIndex() : 0;
$sessionId = $session->getId();
$sessionId = $request->cookies->get($session->getName());
$token = $session->get($this->sessionKey);

if ($this->sessionTrackerEnabler && $session->getId() === $sessionId) {
if ($this->sessionTrackerEnabler && \in_array($sessionId, [true, $session->getId()], true)) {
$usageIndexReference = $usageIndexValue;
}
}
Expand Down
Expand Up @@ -344,6 +344,26 @@ public function testDeauthenticatedEvent()
$this->assertNull($tokenStorage->getToken());
}

/**
* @requires function \Symfony\Component\HttpFoundation\Request::getPreferredFormat
*/
public function testWithPreviousNotStartedSession()
{
$session = new Session(new MockArraySessionStorage());

$request = new Request();
$request->setSession($session);
$request->cookies->set('MOCKSESSID', true);

$usageIndex = $session->getUsageIndex();

$tokenStorage = new TokenStorage();
$listener = new ContextListener($tokenStorage, [], 'context_key', null, null, null, [$tokenStorage, 'getToken']);
$listener(new RequestEvent($this->getMockBuilder(HttpKernelInterface::class)->getMock(), $request, HttpKernelInterface::MASTER_REQUEST));

$this->assertSame($usageIndex, $session->getUsageIndex());
}

protected function runSessionOnKernelResponse($newToken, $original = null)
{
$session = new Session(new MockArraySessionStorage());
Expand Down

0 comments on commit abefccf

Please sign in to comment.