Anonymous user context lookup (#330)

* make anonymous hash header same between varnish and symfony
* always do a hash lookup, even for anonymous requests

Author: dbu

CHANGELOG.md

@@ -36,6 +36,8 @@ See also the [GitHub releases page](https://github.com/FriendsOfSymfony/FOSHttpC
 * Moved Varnish 4 and 5 configuration files from `resources/config/varnish-4/`
   to `resources/config/varnish/`.
 * Changed default Varnish version to 5.
+* Removed special case for anonymous users in user context behaviour. Varnish
+  now does a hash lookup for anonymous users as well.
 
 ### NGINX
 
@@ -49,6 +51,8 @@ See also the [GitHub releases page](https://github.com/FriendsOfSymfony/FOSHttpC
   options array for customization.
 * Provide a trait for the event dispatching kernel, instead of a base class.
   The trait offers both the addSubscriber and the addListener methods.
+* The user context by default does not use a hardcoded hash for anonymous users
+  but does a hash lookup. You can still configure a hardcoded hash.
 
 ### Testing
 

doc/symfony-cache-configuration.rst

@@ -180,8 +180,10 @@ based on session cookies or authorization headers. If the default settings are
 right for you, you don't need to do anything more. You can customize a number of
 options through the constructor:
 
-* **anonymous_hash**: Hash used for anonymous user. This is a performance
-  optimization to not do a backend request for users that are not logged in.
+* **anonymous_hash**: Hard-coded hash to use for anonymous users. This is a
+  performance optimization to not do a backend request for users that are not
+  logged in. If you specify a non-empty value for this field, that is used as
+  context hash header instead of doing a hash lookup for anonymous users.
 
 * **user_hash_accept_header**: Accept header value to be used to request the
   user hash to the backend application. Must match the setup of the backend

doc/user-context.rst

@@ -62,6 +62,19 @@ client, moving step 2-4 into the cache. After the page is in cache, subsequent
 requests from clients that got the same hash can be served from the cache as
 well.
 
+.. note::
+
+    If your application starts sessions for anonymous users, you will get one
+    hash lookup request for each of those users. Your application can return
+    the same hash for authenticated users with no special privileges as for
+    anonymous users with a session cookie.
+
+    If there is no cookie and no authentication information, the hash lookup is
+    skipped and no hash header added to the request. However, we can not avoid
+    the initial hash lookup request per different cookie, as the caching proxy
+    can not know which session cookies indicate a logged in user and which an
+    anonymous session.
+
 Proxy Client Configuration
 --------------------------
 

doc/varnish-configuration.rst

@@ -55,7 +55,7 @@ Purge removes a specific URL (including query strings) in all its variants (as
 specified by the ``Vary`` header).
 
 Subroutines are provided in ``resources/config/varnish-[version]/fos_purge.vcl``.
-To enable support add the following to ``your_varnish.vcl``:
+To enable this feature, add the following to ``your_varnish.vcl``:
 
 .. configuration-block::
 
@@ -96,7 +96,7 @@ Refreshing applies only to a specific URL including the query string, but *not*
 its variants.
 
 Subroutines are provided in ``resources/config/varnish-[version]/fos_refresh.vcl``.
-To enable support, add the following to ``your_varnish.vcl``:
+To enable this feature, add the following to ``your_varnish.vcl``:
 
 .. configuration-block::
 
@@ -125,7 +125,7 @@ Ban
 Banning invalidates whole groups of cached entries with regular expressions.
 
 Subroutines are provided in ``resources/config/varnish-[version]/fos_ban.vcl``
-To enable support add the following to ``your_varnish.vcl``:
+To enable this feature, add the following to ``your_varnish.vcl``:
 
 .. configuration-block::
 
@@ -202,11 +202,26 @@ User Context
 
 Feature: :doc:`user context hashing <user-context>`
 
-The ``fos_user_context.vcl`` needs the ``user_context_hash_url`` subroutine that sets a URL to the request lookup URL. The default URL is ``/_fos_user_context_hash`` and you can simply include ``resources/config/varnish-[version]/fos_user_context_url.vcl`` in your configuration to provide this. If you need a different URL, include a custom file implementing the ``user_context_hash_url`` subroutine.
+The ``fos_user_context.vcl`` needs the ``user_context_hash_url`` subroutine
+that sets the URL to do the hash lookup. The default URL is
+``/_fos_user_context_hash`` and you can simply include
+``resources/config/varnish-[version]/fos_user_context_url.vcl`` in your
+configuration to provide this. If you need a different URL, write your own
+``user_context_hash_url`` subroutine instead.
 
+.. tip::
+
+    The provided VCL to fetch the user hash restarts GET/HEAD requests. It
+    would be more efficient to do the hash lookup request with curl, using the
+    `curl Varnish plugin`_. If you can enable curl support, the recommended way
+    is to implement your own VCL to do a curl request for the hash lookup
+    instead of using the VCL provided here.
 
-To enable support add the following to ``your_varnish.vcl``:
+    Also note that restarting a GET request leads to Varnish discarding the
+    body of the request. If you have some special case where you have GET
+    requests with a body, use curl.
 
+To enable this feature, add the following to ``your_varnish.vcl``:
 
 .. configuration-block::
 
@@ -262,13 +277,6 @@ To enable support add the following to ``your_varnish.vcl``:
 Your backend application needs to respond to the ``application/vnd.fos.user-context-hash``
 request with :ref:`a proper user hash <return context hash>`.
 
-.. note::
-
-    We do not use ``X-Original-Url`` here, as the header will be sent to the
-    backend and the header has semantical meaning for some applications, which
-    would lead to problems. For example, the Microsoft IIS rewriting module
-    uses it, and consequently Symfony also looks into it to support IIS.
-
 .. tip::
 
     The provided VCL assumes that you want the context hash to be cached, so we
@@ -358,7 +366,7 @@ sends an ``X-Cache-Debug`` header:
 
 Subroutines are provided in ``fos_debug.vcl``.
 
-To enable support add the following to ``your_varnish.vcl``:
+To enable this feature, add the following to ``your_varnish.vcl``:
 
 .. configuration-block::
 
@@ -388,5 +396,6 @@ To enable support add the following to ``your_varnish.vcl``:
 .. _banning for Varnish 3: https://www.varnish-software.com/book/3/Cache_invalidation.html#banning
 .. _ban lurker: https://www.varnish-software.com/blog/ban-lurker
 .. _explained in the Varnish documentation: https://www.varnish-cache.org/trac/wiki/VCLExampleRemovingSomeCookies#RemovingallBUTsomecookies
-.. _`builtin VCL`: https://www.varnish-cache.org/trac/browser/bin/varnishd/builtin.vcl?rev=4.0
-.. _`default VCL`: https://www.varnish-cache.org/trac/browser/bin/varnishd/default.vcl?rev=3.0
+.. _curl Varnish plugin: https://github.com/varnish/libvmod-curl
+.. _`builtin VCL`: https://github.com/varnishcache/varnish-cache/blob/5.0/bin/varnishd/builtin.vcl
+.. _`default VCL`: https://github.com/varnishcache/varnish-cache/blob/3.0/bin/varnishd/default.vcl

resources/config/varnish-3/fos_user_context.vcl

@@ -19,10 +19,9 @@ sub fos_user_context_recv {
     }
 
     # Lookup the context hash if there are credentials on the request
-    # Only do this for cacheable requests. Returning a hash lookup discards the request body.
+    # Note that the hash lookup discards the request body.
     # https://www.varnish-cache.org/trac/ticket/652
     if (req.restarts == 0
-        && (req.http.cookie || req.http.authorization)
         && (req.request == "GET" || req.request == "HEAD")
     ) {
         # Backup accept header, if set
@@ -31,9 +30,15 @@ sub fos_user_context_recv {
         }
         set req.http.accept = "application/vnd.fos.user-context-hash";
 
-        # Backup original URL
+        # Backup original URL.
+        #
+        # We do not use X-Original-Url here, as the header will be sent to the
+        # backend and X-Original-Url has semantical meaning for some applications.
+        # For example, the Microsoft IIS rewriting module uses it, and thus
+        # frameworks like Symfony also have to handle that header to integrate with IIS.
+
         set req.http.X-Fos-Original-Url = req.url;
-        
+
         call user_context_hash_url;
 
         # Force the lookup, the backend must tell not to cache or vary on all

resources/config/varnish/fos_user_context.vcl

@@ -19,10 +19,9 @@ sub fos_user_context_recv {
     }
 
     # Lookup the context hash if there are credentials on the request
-    # Only do this for cacheable requests. Returning a hash lookup discards the request body.
+    # Note that the hash lookup discards the request body.
     # https://www.varnish-cache.org/trac/ticket/652
     if (req.restarts == 0
-        && (req.http.cookie || req.http.authorization)
         && (req.method == "GET" || req.method == "HEAD")
     ) {
         # Backup accept header, if set
@@ -31,9 +30,15 @@ sub fos_user_context_recv {
         }
         set req.http.accept = "application/vnd.fos.user-context-hash";
 
-        # Backup original URL
+        # Backup original URL.
+        #
+        # We do not use X-Original-Url here, as the header will be sent to the
+        # backend and X-Original-Url has semantical meaning for some applications.
+        # For example, the Microsoft IIS rewriting module uses it, and thus
+        # frameworks like Symfony also have to handle that header to integrate with IIS.
+
         set req.http.X-Fos-Original-Url = req.url;
-        
+
         call user_context_hash_url;
 
         # Force the lookup, the backend must tell not to cache or vary on all

src/SymfonyCache/UserContextListener.php

@@ -43,7 +43,7 @@ class UserContextListener implements EventSubscriberInterface
     /**
      * When creating this listener, you can configure a number of options.
      *
-     * - anonymous_hash:          Hash used for anonymous user.
+     * - anonymous_hash:          Hash used for anonymous user. Hash lookup skipped for anonymous if this is set.
      * - user_hash_accept_header: Accept header value to be used to request the user hash to the
      *                            backend application. Must match the setup of the backend application.
      * - user_hash_header:        Name of the header the user context hash will be stored into. Must
@@ -60,7 +60,7 @@ public function __construct(array $options = [])
     {
         $resolver = new OptionsResolver();
         $resolver->setDefaults([
-            'anonymous_hash' => '38015b703d82206ebc01d17a39c727e5',
+            'anonymous_hash' => null,
             'user_hash_accept_header' => 'application/vnd.fos.user-context-hash',
             'user_hash_header' => 'X-User-Context-Hash',
             'user_hash_uri' => '/_fos_user_context_hash',
@@ -104,8 +104,8 @@ public function preHandle(CacheEvent $event)
                 return;
             }
 
-            if ($request->isMethodSafe()) {
-                $request->headers->set($this->options['user_hash_header'], $this->getUserHash($event->getKernel(), $request));
+            if ($request->isMethodSafe() && $hash = $this->getUserHash($event->getKernel(), $request)) {
+                $request->headers->set($this->options['user_hash_header'], $hash);
             }
         }
 
@@ -162,7 +162,7 @@ private function getUserHash(HttpKernelInterface $kernel, Request $request)
             return $this->userHash;
         }
 
-        if ($this->isAnonymous($request)) {
+        if ($this->options['anonymous_hash'] && $this->isAnonymous($request)) {
             return $this->userHash = $this->options['anonymous_hash'];
         }
 

tests/Functional/Symfony/EventDispatchingHttpCacheTest.php

@@ -50,7 +50,11 @@ public function testEventListeners()
         $kernel->addSubscriber(new DebugListener());
         $kernel->addSubscriber(new PurgeListener());
         $kernel->addSubscriber(new RefreshListener());
-        $kernel->addSubscriber(new UserContextListener());
+        $kernel->addSubscriber(new UserContextListener([
+            // avoid having to provide mocking for the hash lookup
+            // we already test anonymous hash lookup in the UserContextListener unit test
+            'anonymous_hash' => 'abcdef',
+        ]));
 
         $response = $kernel->handle($request);
         $this->assertSame($expectedResponse, $response);

tests/Unit/SymfonyCache/UserContextListenerTest.php

@@ -98,17 +98,60 @@ public function testPassingUserHashNotAllowed($arg, $options)
     public function testUserHashAnonymous($arg, $options)
     {
         $userContextListener = new UserContextListener($arg);
-
         $request = new Request();
 
-        $event = new CacheEvent($this->kernel, $request);
+        if ($options['anonymous_hash']) {
+            $event = new CacheEvent($this->kernel, $request);
+            $userContextListener->preHandle($event);
+
+            $this->assertTrue($request->headers->has($options['user_hash_header']));
+            $this->assertSame($options['anonymous_hash'], $request->headers->get($options['user_hash_header']));
+        } else {
+            $hashRequest = Request::create($options['user_hash_uri'], $options['user_hash_method'], [], [], [], $request->server->all());
+            $hashRequest->attributes->set('internalRequest', true);
+            $hashRequest->headers->set('Accept', $options['user_hash_accept_header']);
+            // Ensure request properties have been filled up.
+            $hashRequest->getPathInfo();
+            $hashRequest->getMethod();
+
+            $expectedContextHash = 'my_generated_hash';
+            // Just avoid the response to modify the request object, otherwise it's impossible to test objects equality.
+            /** @var \Symfony\Component\HttpFoundation\Response|\PHPUnit_Framework_MockObject_MockObject $hashResponse */
+            $hashResponse = $this->getMockBuilder('\Symfony\Component\HttpFoundation\Response')
+                ->setMethods(['prepare'])
+                ->getMock();
+            $hashResponse->headers->set($options['user_hash_header'], $expectedContextHash);
+
+            $that = $this;
+            $kernel = $this->kernel
+                ->shouldReceive('handle')
+                ->once()
+                ->with(
+                    \Mockery::on(
+                        function (Request $request) use ($that, $hashRequest) {
+                            // we need to call some methods to get the internal fields initialized
+                            $request->getMethod();
+                            $request->getPathInfo();
+                            $that->assertEquals($hashRequest, $request);
+                            $that->assertCount(0, $request->cookies->all());
+
+                            return true;
+                        }
+                    )
+                )
+                ->andReturn($hashResponse)
+                ->getMock();
+
+            $event = new CacheEvent($kernel, $request);
+            $userContextListener->preHandle($event);
+
+            $this->assertTrue($request->headers->has($options['user_hash_header']));
+            $this->assertSame($expectedContextHash, $request->headers->get($options['user_hash_header']));
+        }
 
-        $userContextListener->preHandle($event);
         $response = $event->getResponse();
 
         $this->assertNull($response);
-        $this->assertTrue($request->headers->has($options['user_hash_header']));
-        $this->assertSame($options['anonymous_hash'], $request->headers->get($options['user_hash_header']));
     }
 
     /**