diff --git a/CHANGELOG.md b/CHANGELOG.md index 8f1f9bd7..bc27966b 100644 --- a/CHANGELOG.md +++ b/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 diff --git a/doc/symfony-cache-configuration.rst b/doc/symfony-cache-configuration.rst index 160c2eae..9a468f99 100644 --- a/doc/symfony-cache-configuration.rst +++ b/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 diff --git a/doc/user-context.rst b/doc/user-context.rst index d7dcc919..4acf9ba0 100644 --- a/doc/user-context.rst +++ b/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 -------------------------- diff --git a/doc/varnish-configuration.rst b/doc/varnish-configuration.rst index 21126089..b5571329 100644 --- a/doc/varnish-configuration.rst +++ b/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 ` -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 `. -.. 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 diff --git a/resources/config/varnish-3/fos_user_context.vcl b/resources/config/varnish-3/fos_user_context.vcl index 68a11d03..9fab26f9 100644 --- a/resources/config/varnish-3/fos_user_context.vcl +++ b/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 diff --git a/resources/config/varnish/fos_user_context.vcl b/resources/config/varnish/fos_user_context.vcl index 1e6c9b54..4bbdc4d2 100644 --- a/resources/config/varnish/fos_user_context.vcl +++ b/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 diff --git a/src/SymfonyCache/UserContextListener.php b/src/SymfonyCache/UserContextListener.php index 6d38ca16..f794e34e 100644 --- a/src/SymfonyCache/UserContextListener.php +++ b/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']; } diff --git a/tests/Functional/Symfony/EventDispatchingHttpCacheTest.php b/tests/Functional/Symfony/EventDispatchingHttpCacheTest.php index d2f7799f..8b5681ae 100644 --- a/tests/Functional/Symfony/EventDispatchingHttpCacheTest.php +++ b/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); diff --git a/tests/Unit/SymfonyCache/UserContextListenerTest.php b/tests/Unit/SymfonyCache/UserContextListenerTest.php index 6e4fb91d..094c0bd0 100644 --- a/tests/Unit/SymfonyCache/UserContextListenerTest.php +++ b/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'])); } /**