feature #33663 [Security] Make stateful firewalls turn responses priv…

…ate only when needed (nicolas-grekas)

This PR was merged into the 4.4 branch.

Discussion
----------

[Security] Make stateful firewalls turn responses private only when needed

| Q             | A
| ------------- | ---
| Branch?       | 4.4
| Bug fix?      | no
| New feature?  | yes
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets | #26769 *et al.*
| License       | MIT
| Doc PR        | -

Replaces #28089

By taking over session usage tracking and replacing it with token usage tracking, we can prevent responses that don't actually use the token from turning responses private without changing anything to the lifecycle of security listeners. This makes the behavior much more seamless, allowing to still log the user with the monolog processor, and display it in the profiler toolbar.

This works by using two separate token storage services:
- `security.token_storage` now tracks access to the token and increments the session usage tracker when needed. This is the service that is injected in userland.
- `security.untracked_token_storage` is a raw token storage that just stores the token and is disconnected from the session. This service is injected in places where reading the session doesn't impact the generated output in any way (as e.g. in Monolog processors, etc.)

Commits
-------

20df3a1 [Security] Make stateful firewalls turn responses private only when needed

src/Symfony/Bundle/SecurityBundle/DependencyInjection/Compiler/RegisterTokenUsageTrackingPass.php

@@ -0,0 +1,51 @@
+<?php
+
+/*
+ * This file is part of the Symfony package.
+ *
+ * (c) Fabien Potencier <fabien@symfony.com>
+ *
+ * For the full copyright and license information, please view the LICENSE
+ * file that was distributed with this source code.
+ */
+
+namespace Symfony\Bundle\SecurityBundle\DependencyInjection\Compiler;
+
+use Symfony\Bridge\Monolog\Processor\ProcessorInterface;
+use Symfony\Component\DependencyInjection\Argument\BoundArgument;
+use Symfony\Component\DependencyInjection\Compiler\CompilerPassInterface;
+use Symfony\Component\DependencyInjection\ContainerBuilder;
+use Symfony\Component\DependencyInjection\Reference;
+use Symfony\Component\Security\Core\Authentication\Token\Storage\TokenStorageInterface;
+
+/**
+ * Injects the session tracker enabler in "security.context_listener" + binds "security.untracked_token_storage" to ProcessorInterface instances.
+ *
+ * @author Nicolas Grekas <p@tchwork.com>
+ *
+ * @internal
+ */
+class RegisterTokenUsageTrackingPass implements CompilerPassInterface
+{
+    /**
+     * {@inheritdoc}
+     */
+    public function process(ContainerBuilder $container)
+    {
+        if (!$container->has('security.untracked_token_storage')) {
+            return;
+        }
+
+        $processorAutoconfiguration = $container->registerForAutoconfiguration(ProcessorInterface::class);
+        $processorAutoconfiguration->setBindings($processorAutoconfiguration->getBindings() + [
+            TokenStorageInterface::class => new BoundArgument(new Reference('security.untracked_token_storage'), false),
+        ]);
+
+        if (!$container->has('session')) {
+            $container->setAlias('security.token_storage', 'security.untracked_token_storage')->setPublic(true);
+        } elseif ($container->hasDefinition('security.context_listener')) {
+            $container->getDefinition('security.context_listener')
+                ->setArgument(6, [new Reference('security.token_storage'), 'enableUsageTracking']);
+        }
+    }
+}

src/Symfony/Bundle/SecurityBundle/Resources/config/collectors.xml

@@ -9,7 +9,7 @@
 
         <service id="data_collector.security" class="Symfony\Bundle\SecurityBundle\DataCollector\SecurityDataCollector">
             <tag name="data_collector" template="@Security/Collector/security.html.twig" id="security" priority="270" />
-            <argument type="service" id="security.token_storage" on-invalid="ignore" />
+            <argument type="service" id="security.untracked_token_storage" />
             <argument type="service" id="security.role_hierarchy" />
             <argument type="service" id="security.logout_url_generator" />
             <argument type="service" id="security.access.decision_manager" />

src/Symfony/Bundle/SecurityBundle/Resources/config/security.xml

@@ -21,11 +21,18 @@
         </service>
         <service id="Symfony\Component\Security\Core\Authorization\AuthorizationCheckerInterface" alias="security.authorization_checker" />
 
-        <service id="security.token_storage" class="Symfony\Component\Security\Core\Authentication\Token\Storage\TokenStorage" public="true">
+        <service id="security.token_storage" class="Symfony\Component\Security\Core\Authentication\Token\Storage\UsageTrackingTokenStorage" public="true">
+            <tag name="kernel.reset" method="disableUsageTracking" />
             <tag name="kernel.reset" method="setToken" />
+            <argument type="service" id="security.untracked_token_storage" />
+            <argument type="service_locator">
+                <argument key="session" type="service" id="session" />
+            </argument>
         </service>
         <service id="Symfony\Component\Security\Core\Authentication\Token\Storage\TokenStorageInterface" alias="security.token_storage" />
 
+        <service id="security.untracked_token_storage" class="Symfony\Component\Security\Core\Authentication\Token\Storage\TokenStorage" />
+
         <service id="security.helper" class="Symfony\Component\Security\Core\Security">
             <argument type="service_locator">
                 <argument key="security.token_storage" type="service" id="security.token_storage" />
@@ -162,7 +169,7 @@
         <service id="security.logout_url_generator" class="Symfony\Component\Security\Http\Logout\LogoutUrlGenerator">
             <argument type="service" id="request_stack" on-invalid="null" />
             <argument type="service" id="router" on-invalid="null" />
-            <argument type="service" id="security.token_storage" on-invalid="null" />
+            <argument type="service" id="security.token_storage" />
         </service>
 
         <!-- Provisioning -->

src/Symfony/Bundle/SecurityBundle/Resources/config/security_listeners.xml

@@ -9,7 +9,7 @@
 
         <service id="security.authentication.listener.anonymous" class="Symfony\Component\Security\Http\Firewall\AnonymousAuthenticationListener">
             <tag name="monolog.logger" channel="security" />
-            <argument type="service" id="security.token_storage" />
+            <argument type="service" id="security.untracked_token_storage" />
             <argument /> <!-- Key -->
             <argument type="service" id="logger" on-invalid="null" />
             <argument type="service" id="security.authentication.manager" />
@@ -37,7 +37,7 @@
 
         <service id="security.context_listener" class="Symfony\Component\Security\Http\Firewall\ContextListener">
             <tag name="monolog.logger" channel="security" />
-            <argument type="service" id="security.token_storage" />
+            <argument type="service" id="security.untracked_token_storage" />
             <argument type="collection" />
             <argument /> <!-- Provider Key -->
             <argument type="service" id="logger" on-invalid="null" />
@@ -128,7 +128,7 @@
 
         <service id="security.authentication.listener.simple_preauth" class="Symfony\Component\Security\Http\Firewall\SimplePreAuthenticationListener" abstract="true">
             <tag name="monolog.logger" channel="security" />
-            <argument type="service" id="security.token_storage" />
+            <argument type="service" id="security.untracked_token_storage" />
             <argument type="service" id="security.authentication.manager" />
             <argument /> <!-- Provider-shared Key -->
             <argument /> <!-- Authenticator -->

src/Symfony/Bundle/SecurityBundle/Resources/config/security_rememberme.xml

@@ -9,7 +9,7 @@
 
         <service id="security.authentication.listener.rememberme" class="Symfony\Component\Security\Http\Firewall\RememberMeListener" abstract="true">
             <tag name="monolog.logger" channel="security" />
-            <argument type="service" id="security.token_storage" />
+            <argument type="service" id="security.untracked_token_storage" />
             <argument type="service" id="security.authentication.rememberme" />
             <argument type="service" id="security.authentication.manager" />
             <argument type="service" id="logger" on-invalid="null" />

src/Symfony/Bundle/SecurityBundle/SecurityBundle.php

@@ -15,6 +15,7 @@
 use Symfony\Bundle\SecurityBundle\DependencyInjection\Compiler\AddSecurityVotersPass;
 use Symfony\Bundle\SecurityBundle\DependencyInjection\Compiler\AddSessionDomainConstraintPass;
 use Symfony\Bundle\SecurityBundle\DependencyInjection\Compiler\RegisterCsrfTokenClearingLogoutHandlerPass;
+use Symfony\Bundle\SecurityBundle\DependencyInjection\Compiler\RegisterTokenUsageTrackingPass;
 use Symfony\Bundle\SecurityBundle\DependencyInjection\Security\Factory\AnonymousFactory;
 use Symfony\Bundle\SecurityBundle\DependencyInjection\Security\Factory\FormLoginFactory;
 use Symfony\Bundle\SecurityBundle\DependencyInjection\Security\Factory\FormLoginLdapFactory;
@@ -66,5 +67,6 @@ public function build(ContainerBuilder $container)
         $container->addCompilerPass(new AddSecurityVotersPass());
         $container->addCompilerPass(new AddSessionDomainConstraintPass(), PassConfig::TYPE_BEFORE_REMOVING);
         $container->addCompilerPass(new RegisterCsrfTokenClearingLogoutHandlerPass());
+        $container->addCompilerPass(new RegisterTokenUsageTrackingPass(), PassConfig::TYPE_BEFORE_OPTIMIZATION, 200);
     }
 }

src/Symfony/Component/HttpFoundation/Session/Session.php

@@ -136,10 +136,7 @@ public function count()
         return \count($this->getAttributeBag()->all());
     }
 
-    /**
-     * @internal
-     */
-    public function getUsageIndex(): int
+    public function &getUsageIndex(): int
     {
         return $this->usageIndex;
     }

src/Symfony/Component/Security/Core/Authentication/Token/Storage/UsageTrackingTokenStorage.php

@@ -0,0 +1,73 @@
+<?php
+
+/*
+ * This file is part of the Symfony package.
+ *
+ * (c) Fabien Potencier <fabien@symfony.com>
+ *
+ * For the full copyright and license information, please view the LICENSE
+ * file that was distributed with this source code.
+ */
+
+namespace Symfony\Component\Security\Core\Authentication\Token\Storage;
+
+use Psr\Container\ContainerInterface;
+use Symfony\Component\HttpFoundation\Session\SessionInterface;
+use Symfony\Component\Security\Core\Authentication\Token\TokenInterface;
+use Symfony\Contracts\Service\ServiceSubscriberInterface;
+
+/**
+ * A token storage that increments the session usage index when the token is accessed.
+ *
+ * @author Nicolas Grekas <p@tchwork.com>
+ */
+final class UsageTrackingTokenStorage implements TokenStorageInterface, ServiceSubscriberInterface
+{
+    private $storage;
+    private $sessionLocator;
+    private $enableUsageTracking = false;
+
+    public function __construct(TokenStorageInterface $storage, ContainerInterface $sessionLocator)
+    {
+        $this->storage = $storage;
+        $this->sessionLocator = $sessionLocator;
+    }
+
+    /**
+     * {@inheritdoc}
+     */
+    public function getToken(): ?TokenInterface
+    {
+        if ($this->enableUsageTracking) {
+            // increments the internal session usage index
+            $this->sessionLocator->get('session')->getMetadataBag();
+        }
+
+        return $this->storage->getToken();
+    }
+
+    /**
+     * {@inheritdoc}
+     */
+    public function setToken(TokenInterface $token = null): void
+    {
+        $this->storage->setToken($token);
+    }
+
+    public function enableUsageTracking(): void
+    {
+        $this->enableUsageTracking = true;
+    }
+
+    public function disableUsageTracking(): void
+    {
+        $this->enableUsageTracking = false;
+    }
+
+    public static function getSubscribedServices(): array
+    {
+        return [
+            'session' => SessionInterface::class,
+        ];
+    }
+}

src/Symfony/Component/Security/Core/Tests/Authentication/Token/Storage/UsageTrackingTokenStorageTest.php

@@ -0,0 +1,57 @@
+<?php
+
+/*
+ * This file is part of the Symfony package.
+ *
+ * (c) Fabien Potencier <fabien@symfony.com>
+ *
+ * For the full copyright and license information, please view the LICENSE
+ * file that was distributed with this source code.
+ */
+
+namespace Symfony\Component\Security\Core\Tests\Authentication\Token\Storage;
+
+use PHPUnit\Framework\TestCase;
+use Psr\Container\ContainerInterface;
+use Symfony\Component\HttpFoundation\Session\SessionInterface;
+use Symfony\Component\Security\Core\Authentication\Token\Storage\TokenStorage;
+use Symfony\Component\Security\Core\Authentication\Token\Storage\UsageTrackingTokenStorage;
+use Symfony\Component\Security\Core\Authentication\Token\TokenInterface;
+use Symfony\Contracts\Service\ServiceLocatorTrait;
+
+class UsageTrackingTokenStorageTest extends TestCase
+{
+    public function testGetSetToken()
+    {
+        $sessionAccess = 0;
+        $sessionLocator = new class(['session' => function () use (&$sessionAccess) {
+            ++$sessionAccess;
+
+            $session = $this->createMock(SessionInterface::class);
+            $session->expects($this->once())
+                    ->method('getMetadataBag');
+
+            return $session;
+        }]) implements ContainerInterface {
+            use ServiceLocatorTrait;
+        };
+        $tokenStorage = new TokenStorage();
+        $trackingStorage = new UsageTrackingTokenStorage($tokenStorage, $sessionLocator);
+
+        $this->assertNull($trackingStorage->getToken());
+        $token = $this->getMockBuilder(TokenInterface::class)->getMock();
+
+        $trackingStorage->setToken($token);
+        $this->assertSame($token, $trackingStorage->getToken());
+        $this->assertSame($token, $tokenStorage->getToken());
+        $this->assertSame(0, $sessionAccess);
+
+        $trackingStorage->enableUsageTracking();
+        $this->assertSame($token, $trackingStorage->getToken());
+        $this->assertSame(1, $sessionAccess);
+
+        $trackingStorage->disableUsageTracking();
+        $this->assertSame($token, $trackingStorage->getToken());
+        $this->assertSame(1, $sessionAccess);
+    }
+}

src/Symfony/Component/Security/Core/composer.json

@@ -18,7 +18,7 @@
     "require": {
         "php": "^7.1.3",
         "symfony/event-dispatcher-contracts": "^1.1|^2",
-        "symfony/service-contracts": "^1.1|^2"
+        "symfony/service-contracts": "^1.1.6|^2"
     },
     "require-dev": {
         "psr/container": "^1.0",

src/Symfony/Component/Security/Http/Firewall/AccessListener.php

@@ -51,18 +51,18 @@ public function __construct(TokenStorageInterface $tokenStorage, AccessDecisionM
      */
     public function __invoke(RequestEvent $event)
     {
-        if (null === $token = $this->tokenStorage->getToken()) {
-            throw new AuthenticationCredentialsNotFoundException('A Token was not found in the TokenStorage.');
-        }
-
         $request = $event->getRequest();
 
         list($attributes) = $this->map->getPatterns($request);
 
-        if (null === $attributes) {
+        if (!$attributes) {
             return;
         }
 
+        if (null === $token = $this->tokenStorage->getToken()) {
+            throw new AuthenticationCredentialsNotFoundException('A Token was not found in the TokenStorage.');
+        }
+
         if (!$token->isAuthenticated()) {
             $token = $this->authManager->authenticate($token);
             $this->tokenStorage->setToken($token);