diff --git a/DependencyInjection/FOSHttpCacheExtension.php b/DependencyInjection/FOSHttpCacheExtension.php index 20fabc42..1a49b545 100644 --- a/DependencyInjection/FOSHttpCacheExtension.php +++ b/DependencyInjection/FOSHttpCacheExtension.php @@ -192,6 +192,9 @@ private function loadUserContext(ContainerBuilder $container, XmlFileLoader $loa ->replaceArgument(3, $config['user_hash_header']) ->replaceArgument(4, $config['hash_cache_ttl']); + $container->getDefinition($this->getAlias().'.user_context.anonymous_request_matcher') + ->replaceArgument(0, $config['user_identifier_headers']); + if ($config['logout_handler']['enabled']) { $container->getDefinition($this->getAlias().'.user_context.logout_handler') ->replaceArgument(1, $config['user_identifier_headers']) diff --git a/EventListener/UserContextSubscriber.php b/EventListener/UserContextSubscriber.php index 4ea6ac88..ebb31a7a 100644 --- a/EventListener/UserContextSubscriber.php +++ b/EventListener/UserContextSubscriber.php @@ -13,6 +13,7 @@ use FOS\HttpCache\UserContext\HashGenerator; use Symfony\Component\EventDispatcher\EventSubscriberInterface; +use Symfony\Component\HttpFoundation\Request; use Symfony\Component\HttpFoundation\RequestMatcherInterface; use Symfony\Component\HttpKernel\Event\FilterResponseEvent; use Symfony\Component\HttpKernel\Event\GetResponseEvent; @@ -61,12 +62,22 @@ class UserContextSubscriber implements EventSubscriberInterface */ private $ttl; + /** + * Used to exclude anonymous requests (no authentication nor session) from user hash sanity check. + * 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 + */ + private $anonymousRequestMatcher; + public function __construct( RequestMatcherInterface $requestMatcher, HashGenerator $hashGenerator, array $userIdentifierHeaders = array('Cookie', 'Authorization'), $hashHeader = 'X-User-Context-Hash', - $ttl = 0 + $ttl = 0, + RequestMatcherInterface $anonymousRequestMatcher = null ) { if (!count($userIdentifierHeaders)) { throw new \InvalidArgumentException('The user context must vary on some request headers'); @@ -76,6 +87,7 @@ public function __construct( $this->userIdentifierHeaders = $userIdentifierHeaders; $this->hashHeader = $hashHeader; $this->ttl = $ttl; + $this->anonymousRequestMatcher = $anonymousRequestMatcher; } /** @@ -93,7 +105,7 @@ public function onKernelRequest(GetResponseEvent $event) } if (!$this->requestMatcher->matches($event->getRequest())) { - if ($event->getRequest()->headers->has($this->hashHeader)) { + if ($event->getRequest()->headers->has($this->hashHeader) && !$this->isAnonymous($event->getRequest())) { $this->hash = $this->hashGenerator->generateHash(); } @@ -120,6 +132,20 @@ public function onKernelRequest(GetResponseEvent $event) $event->setResponse($response); } + /** + * Tests if $request is an anonymous request or not. + * + * For backward compatibility reasons, true will be returned if no anonymous request matcher was provided. + * + * @param Request $request + * + * @return bool + */ + private function isAnonymous(Request $request) + { + return $this->anonymousRequestMatcher ? $this->anonymousRequestMatcher->matches($request) : false; + } + /** * Add the context hash header to the headers to vary on if the header was * present in the request. diff --git a/Resources/config/user_context.xml b/Resources/config/user_context.xml index 243132c2..773e4bd4 100644 --- a/Resources/config/user_context.xml +++ b/Resources/config/user_context.xml @@ -27,6 +27,7 @@ + @@ -39,5 +40,9 @@ + + + + diff --git a/Tests/Unit/EventListener/UserContextSubscriberTest.php b/Tests/Unit/EventListener/UserContextSubscriberTest.php index 3758698b..2627b208 100644 --- a/Tests/Unit/EventListener/UserContextSubscriberTest.php +++ b/Tests/Unit/EventListener/UserContextSubscriberTest.php @@ -198,6 +198,39 @@ public function testFullRequestHashOk() $this->assertContains('X-Hash', $event->getResponse()->headers->get('Vary')); } + /** + * If the request is an anonymous one, no hash should be generated/validated. + */ + public function testFullAnonymousRequestHashNotGenerated() + { + $request = new Request(); + $request->setMethod('GET'); + $request->headers->set('X-Hash', 'anonymous-hash'); + + $requestMatcher = $this->getRequestMatcher($request, false); + $hashGenerator = \Mockery::mock('\FOS\HttpCache\UserContext\HashGenerator'); + $hashGenerator->shouldReceive('generateHash')->never(); + + $anonymousRequestMatcher = \Mockery::mock('\Symfony\Component\HttpFoundation\RequestMatcherInterface'); + $anonymousRequestMatcher->shouldReceive('matches')->andReturn(true); + + // onKernelRequest + $userContextSubscriber = new UserContextSubscriber($requestMatcher, $hashGenerator, array('X-SessionId'), 'X-Hash', 0, $anonymousRequestMatcher); + $event = $this->getKernelRequestEvent($request); + + $userContextSubscriber->onKernelRequest($event); + + $response = $event->getResponse(); + + $this->assertNull($response); + + // onKernelResponse + $event = $this->getKernelResponseEvent($request); + $userContextSubscriber->onKernelResponse($event); + + $this->assertContains('X-Hash', $event->getResponse()->headers->get('Vary')); + } + /** * If there is no hash in the requests but session changed, prevent setting bad cache. */ diff --git a/Tests/Unit/UserContext/AnonymousRequestMatcherTest.php b/Tests/Unit/UserContext/AnonymousRequestMatcherTest.php new file mode 100644 index 00000000..f4de981e --- /dev/null +++ b/Tests/Unit/UserContext/AnonymousRequestMatcherTest.php @@ -0,0 +1,69 @@ + + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ + +namespace FOS\HttpCacheBundle\Tests\Unit\UserContext; + +use FOS\HttpCacheBundle\UserContext\AnonymousRequestMatcher; +use PHPUnit_Framework_TestCase; +use Symfony\Component\HttpFoundation\Request; + +class AnonymousRequestMatcherTest extends PHPUnit_Framework_TestCase +{ + public function testMatchAnonymousRequest() + { + $request = new Request(); + + $requestMatcher = new AnonymousRequestMatcher(['Cookie', 'Authorization']); + + $this->assertTrue($requestMatcher->matches($request)); + } + + public function testNoMatchIfCookie() + { + $request = new Request(); + $request->headers->set('Cookie', 'PHPSESSID7e476fc9f29f69d2ad6f11dbcd663b42=25f6d9c5a843e3c948cd26902385a527'); + $request->cookies->set('PHPSESSID7e476fc9f29f69d2ad6f11dbcd663b42', '25f6d9c5a843e3c948cd26902385a527'); + + $requestMatcher = new AnonymousRequestMatcher(['Cookie', 'Authorization']); + + $this->assertFalse($requestMatcher->matches($request)); + } + + public function testNoMatchIfEmptyCookieHeader() + { + $request = new Request(); + $request->headers->set('Cookie', ''); + + $requestMatcher = new AnonymousRequestMatcher(['Cookie', 'Authorization']); + + $this->assertTrue($requestMatcher->matches($request)); + } + + public function testNoMatchIfAuthenticationHeader() + { + $request = new Request(); + $request->headers->set('Authorization', 'foo: bar'); + + $requestMatcher = new AnonymousRequestMatcher(['Cookie', 'Authorization']); + + $this->assertFalse($requestMatcher->matches($request)); + } + + public function testMatchEmptyCookieHeaderHeader() + { + $request = new Request(); + $request->headers->set('Cookie', ''); + + $requestMatcher = new AnonymousRequestMatcher(['Cookie', 'Authorization']); + + $this->assertTrue($requestMatcher->matches($request)); + } +} diff --git a/UserContext/AnonymousRequestMatcher.php b/UserContext/AnonymousRequestMatcher.php new file mode 100644 index 00000000..4d251b17 --- /dev/null +++ b/UserContext/AnonymousRequestMatcher.php @@ -0,0 +1,47 @@ + + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ + +namespace FOS\HttpCacheBundle\UserContext; + +use Symfony\Component\HttpFoundation\Request; +use Symfony\Component\HttpFoundation\RequestMatcherInterface; + +/** + * Matches anonymous requests using a list of identification headers. + */ +class AnonymousRequestMatcher implements RequestMatcherInterface +{ + private $userIdentifierHeaders; + + /** + * @param array $userIdentifierHeaders List of request headers that authenticate a non-anonymous request + */ + public function __construct(array $userIdentifierHeaders) + { + $this->userIdentifierHeaders = $userIdentifierHeaders; + } + + public function matches(Request $request) + { + foreach ($this->userIdentifierHeaders as $header) { + if ($request->headers->has($header)) { + if (strtolower($header) === 'cookie' && 0 === $request->cookies->count()) { + // ignore empty cookie header + continue; + } + + return false; + } + } + + return true; + } +} diff --git a/composer.json b/composer.json index 98ff0ee4..bcc9cdb0 100644 --- a/composer.json +++ b/composer.json @@ -29,7 +29,7 @@ "mockery/mockery": "0.9.*", "monolog/monolog": "*", "sensio/framework-extra-bundle": "^2.3||^3.0", - "symfony/symfony": "^2.3||^3.0", + "symfony/symfony": "^2.3.4||^3.0", "symfony/phpunit-bridge": "^2.7||^3.0", "symfony/expression-language": "^2.4||^3.0", "symfony/monolog-bundle": "^2.3||^3.0",