FriendsOfSymfony · dbu · Oct 5, 2018 · Oct 2, 2018
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -1,6 +1,15 @@
 Changelog
 =========
 
+2.6.0
+-----
+
+### Changed
+
+* User context lookup now tags the hash lookup response. The logout listener can now invalidate that tag instead of
+  doing a BAN request. The previous varnish BAN request has been incorrect and banned all cache entries on Varnish.
+  The logout handler is now also activated by default for the Symfony HttpCache in addition to Varnish and Noop.
+
 2.5.1
 -----
 

diff --git a/Resources/doc/reference/configuration/user-context.rst b/Resources/doc/reference/configuration/user-context.rst
@@ -139,11 +139,12 @@ or you will end up with mixed up caches.
 ~~~~~~~~~~~~~~~~~~
 
 The logout handler will invalidate any cached user hashes when the user logs
-out.
+out. This will make sure that the session cookie of the logged out session can
+not be abused to see protected cached content.
 
 For the handler to work:
 
-* your caching proxy should be :ref:`configured for BANs <foshttpcache:proxy-configuration>`
+* your caching proxy must be :ref:`configured for tag invalidation <foshttpcache:proxy-configuration>`
 * Symfony’s default behavior of regenerating the session id when users log in
   and out must be enabled (``invalidate_session``).
 

diff --git a/src/DependencyInjection/Configuration.php b/src/DependencyInjection/Configuration.php
@@ -120,12 +120,17 @@ function ($v) {
                         return $v;
                     }
 
-                    if (isset($v['proxy_client']['default']) && in_array($v['proxy_client']['default'], ['varnish', 'noop'])) {
+                    if (isset($v['proxy_client']['default'])
+                        && in_array($v['proxy_client']['default'], ['varnish', 'symfony', 'noop'])
+                    ) {
                         $v['user_context']['logout_handler']['enabled'] = true;
 
                         return $v;
                     }
-                    if (isset($v['proxy_client']['varnish']) || isset($v['proxy_client']['noop'])) {
+                    if (isset($v['proxy_client']['varnish'])
+                        || isset($v['proxy_client']['symfony'])
+                        || isset($v['proxy_client']['noop'])
+                    ) {
                         $v['user_context']['logout_handler']['enabled'] = true;
 
                         return $v;

diff --git a/src/DependencyInjection/FOSHttpCacheExtension.php b/src/DependencyInjection/FOSHttpCacheExtension.php
@@ -313,10 +313,6 @@ private function loadUserContext(ContainerBuilder $container, XmlFileLoader $loa
             ->replaceArgument(0, $options);
 
         if ($config['logout_handler']['enabled']) {
-            $container->getDefinition('fos_http_cache.user_context_invalidator')
-                ->replaceArgument(1, $completeUserIdentifierHeaders)
-                ->replaceArgument(2, $config['match']['accept']);
-
             $container->setAlias('security.logout.handler.session', 'fos_http_cache.user_context.session_logout_handler');
         } else {
             $container->removeDefinition('fos_http_cache.user_context.logout_handler');

diff --git a/src/EventListener/UserContextListener.php b/src/EventListener/UserContextListener.php
@@ -11,7 +11,9 @@
 
 namespace FOS\HttpCacheBundle\EventListener;
 
+use FOS\HttpCache\ResponseTagger;
 use FOS\HttpCache\UserContext\HashGenerator;
+use FOS\HttpCacheBundle\UserContextInvalidator;
 use Symfony\Component\EventDispatcher\EventSubscriberInterface;
 use Symfony\Component\HttpFoundation\Request;
 use Symfony\Component\HttpFoundation\RequestMatcherInterface;
@@ -45,6 +47,13 @@ class UserContextListener implements EventSubscriberInterface
      */
     private $hashGenerator;
 
+    /**
+     * If the response tagger is set, the hash lookup response is tagged with the session id for later invalidation.
+     *
+     * @var ResponseTagger|null
+     */
+    private $responseTagger;
+
     /**
      * @var array
      */
@@ -60,26 +69,20 @@ class UserContextListener implements EventSubscriberInterface
      * It prevents issues when the hash generator that is used returns a customized value for anonymous users,
      * that differs from the documented, hardcoded one.
      *
-     * @var RequestMatcherInterface
+     * @var RequestMatcherInterface|null
      */
     private $anonymousRequestMatcher;
 
-    /**
-     * Constructor.
-     *
-     * @param RequestMatcherInterface      $requestMatcher
-     * @param HashGenerator                $hashGenerator
-     * @param RequestMatcherInterface|null $anonymousRequestMatcher
-     * @param array                        $options
-     */
     public function __construct(
         RequestMatcherInterface $requestMatcher,
         HashGenerator $hashGenerator,
         RequestMatcherInterface $anonymousRequestMatcher = null,
+        ResponseTagger $responseTagger = null,
         array $options = []
     ) {
         $this->requestMatcher = $requestMatcher;
         $this->hashGenerator = $hashGenerator;
+        $this->responseTagger = $responseTagger;
         $this->anonymousRequestMatcher = $anonymousRequestMatcher;
 
         $resolver = new OptionsResolver();
@@ -115,7 +118,8 @@ public function onKernelRequest(GetResponseEvent $event)
             return;
         }
 
-        if (!$this->requestMatcher->matches($event->getRequest())) {
+        $request = $event->getRequest();
+        if (!$this->requestMatcher->matches($request)) {
             if ($event->getRequest()->headers->has($this->options['user_hash_header'])
                 && !$this->isAnonymous($event->getRequest())
             ) {
@@ -127,6 +131,11 @@ public function onKernelRequest(GetResponseEvent $event)
 
         $hash = $this->hashGenerator->generateHash();
 
+        if ($this->responseTagger && $request->hasSession()) {
+            $tag = UserContextInvalidator::buildTag($request->getSession()->getId());
+            $this->responseTagger->addTags([$tag]);
+        }
+
         // status needs to be 200 as otherwise varnish will not cache the response.
         $response = new Response('', 200, [
             $this->options['user_hash_header'] => $hash,

diff --git a/src/Resources/config/user_context.xml b/src/Resources/config/user_context.xml
@@ -17,6 +17,7 @@
             <argument type="service" id="fos_http_cache.user_context.request_matcher" />
             <argument type="service" id="fos_http_cache.user_context.hash_generator" />
             <argument type="service" id="fos_http_cache.user_context.anonymous_request_matcher" />
+            <argument type="service" id="fos_http_cache.http.symfony_response_tagger" on-invalid="ignore" />
             <argument>%fos_http_cache.event_listener.user_context.options%</argument>
             <tag name="kernel.event_subscriber" />
         </service>
@@ -27,8 +28,6 @@
 
         <service id="fos_http_cache.user_context_invalidator" class="FOS\HttpCacheBundle\UserContextInvalidator">
             <argument type="service" id="fos_http_cache.default_proxy_client" />
-            <argument />
-            <argument />
         </service>
 
         <service id="fos_http_cache.user_context.logout_handler" class="FOS\HttpCacheBundle\Security\Http\Logout\ContextInvalidationLogoutHandler" public="false">

diff --git a/src/UserContextInvalidator.php b/src/UserContextInvalidator.php
@@ -11,36 +11,20 @@
 
 namespace FOS\HttpCacheBundle;
 
-use FOS\HttpCache\ProxyClient\Invalidation\BanCapable;
+use FOS\HttpCache\ProxyClient\Invalidation\TagCapable;
 
 class UserContextInvalidator
 {
-    /**
-     * Service used to ban hash request.
-     *
-     * @var BanCapable
-     */
-    private $banner;
+    const USER_CONTEXT_TAG_PREFIX = 'fos_http_cache_hashlookup-';
 
     /**
-     * Accept header.
-     *
-     * @var string
+     * @var TagCapable
      */
-    private $acceptHeader;
+    private $tagger;
 
-    /**
-     * User identifier headers.
-     *
-     * @var string[]
-     */
-    private $userIdentifierHeaders;
-
-    public function __construct(BanCapable $banner, $userIdentifierHeaders, $acceptHeader)
+    public function __construct(TagCapable $tagger)
     {
-        $this->banner = $banner;
-        $this->acceptHeader = $acceptHeader;
-        $this->userIdentifierHeaders = $userIdentifierHeaders;
+        $this->tagger = $tagger;
     }
 
     /**
@@ -50,11 +34,11 @@ public function __construct(BanCapable $banner, $userIdentifierHeaders, $acceptH
      */
     public function invalidateContext($sessionId)
     {
-        foreach ($this->userIdentifierHeaders as $header) {
-            $this->banner->ban([
-                'accept' => $this->acceptHeader,
-                $header => sprintf('.*%s.*', $sessionId),
-            ]);
-        }
+        $this->tagger->invalidateTags([static::buildTag($sessionId)]);
+    }
+
+    public static function buildTag($hash)
+    {
+        return static::USER_CONTEXT_TAG_PREFIX.$hash;
     }
 }
diff --git a/tests/Functional/EventListener/UserContextListenerTest.php b/tests/Functional/EventListener/UserContextListenerTest.php
@@ -12,6 +12,7 @@
 namespace FOS\HttpCacheBundle\Tests\Functional\EventListener;
 
 use Symfony\Bundle\FrameworkBundle\Test\WebTestCase;
+use Symfony\Component\HttpFoundation\Session\SessionInterface;
 
 class UserContextListenerTest extends WebTestCase
 {
@@ -21,6 +22,9 @@ public function testHashLookup()
             'PHP_AUTH_USER' => 'user',
             'PHP_AUTH_PW' => 'user',
         ]);
+        /** @var SessionInterface $session */
+        $session = $client->getContainer()->get('session');
+        $session->setId('test');
 
         $client->request('GET', '/secured_area/_fos_user_context_hash', [], [], [
             'HTTP_ACCEPT' => 'application/vnd.fos.user-context-hash',
@@ -29,6 +33,7 @@ public function testHashLookup()
 
         $this->assertTrue($response->headers->has('X-User-Context-Hash'), 'X-User-Context-Hash header missing on the response');
         $this->assertEquals('5224d8f5b85429624e2160e538a3376a479ec87b89251b295c44ecbf7498ea3c', $response->headers->get('X-User-Context-Hash'), 'Not the expected context hash');
+        $this->assertEquals('fos_http_cache_hashlookup-test', $response->headers->get('X-Cache-Tags'));
         $this->assertEquals('max-age=60, public', $response->headers->get('Cache-Control'));
     }
 

diff --git a/tests/Functional/Security/Http/Logout/ContextInvalidationLogoutHandlerTest.php b/tests/Functional/Security/Http/Logout/ContextInvalidationLogoutHandlerTest.php
@@ -32,30 +32,20 @@ public function testLogout()
         $session->save();
 
         $mock = \Mockery::mock(Varnish::class);
-        $mock->shouldReceive('ban')
+        $mock->shouldReceive('invalidateTags')
             ->once()
-            ->with([
-                'accept' => 'application/vnd.fos.user-context-hash',
-                'cookie' => '.*test.*',
-            ])
-        ;
-        $mock->shouldReceive('ban')
-            ->once()
-            ->with([
-                'accept' => 'application/vnd.fos.user-context-hash',
-                'authorization' => '.*test.*',
-            ])
+            ->with(['fos_http_cache_hashlookup-test'])
         ;
         $mock->shouldReceive('flush')
             ->once()
-            ->andReturn(2)
+            ->andReturn(1)
         ;
 
         $client->getContainer()->set('fos_http_cache.proxy_client.varnish', $mock);
 
         $client->getCookieJar()->set(new Cookie($session->getName(), 'test'));
         $client->request('GET', '/secured_area/logout');
 
-        $this->assertEquals(302, $client->getResponse()->getStatusCode(), $client->getResponse()->getContent());
+        $this->assertEquals(302, $client->getResponse()->getStatusCode(), substr($client->getResponse()->getContent(), 0, 2000));
     }
 }
diff --git a/tests/Unit/DependencyInjection/ConfigurationTest.php b/tests/Unit/DependencyInjection/ConfigurationTest.php
@@ -278,7 +278,7 @@ public function testSupportsSymfony()
         $expectedConfiguration['cache_manager']['generate_url_type'] = 'auto';
         $expectedConfiguration['tags']['enabled'] = 'auto';
         $expectedConfiguration['invalidation']['enabled'] = 'auto';
-        $expectedConfiguration['user_context']['logout_handler']['enabled'] = false;
+        $expectedConfiguration['user_context']['logout_handler']['enabled'] = true;
 
         $formats = array_map(function ($path) {
             return __DIR__.'/../../Resources/Fixtures/'.$path;