Merge pull request #434 from FriendsOfSymfony/anonymous-request-matcher

Use AnonymousRequestMatcher from FOSHttpCache
FriendsOfSymfony · Mar 23, 2018 · e8a825b · e8a825b
2 parents 218e9f4 + e645e38
commit e8a825b
Show file tree

Hide file tree

Showing 15 changed files with 98 additions and 125 deletions.
diff --git a/.travis.yml b/.travis.yml
@@ -31,31 +31,13 @@ matrix:
     - php: 5.4
       env:
         - COMPOSER_FLAGS="--prefer-lowest"
-        - SYMFONY_VERSION='2.3.*'
+        - SYMFONY_VERSION='2.8.*'
     - php: 5.6
       env:
         - SYMFONY_VERSION='3.2.*'
         - PHPUNIT_FLAGS="--coverage-clover=coverage.clover"
         - COVERAGE=true
         - DOCCHECK=true
-    - php: 5.6
-      env: SYMFONY_VERSION='2.3.*'
-    - php: 5.6
-      env:
-        - SYMFONY_VERSION='2.4.*'
-        - FRAMEWORK_EXTRA_VERSION='~3.0'
-    - php: 5.6
-      env:
-        - SYMFONY_VERSION='2.5.*'
-        - FRAMEWORK_EXTRA_VERSION='~3.0'
-    - php: 5.6
-      env:
-        - SYMFONY_VERSION='2.6.*'
-        - FRAMEWORK_EXTRA_VERSION='~3.0'
-    - php: 5.6
-      env:
-        - SYMFONY_VERSION='2.7.*'
-        - FRAMEWORK_EXTRA_VERSION='~3.0'
     - php: 5.6
       env:
         - SYMFONY_VERSION='2.8.*'

diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -1,6 +1,38 @@
 Changelog
 =========
 
+1.3.13
+------
+
+* Symfony HttpCache User Context: Move the AnonymousRequestMatcher to FOSHttpCache.
+
+  The recommended way to ignore cookie based sessions is to set `session_name_prefix` to
+  false rather than omit the Cookie header from `user_identifier_headers`.
+
+1.3.12
+------
+
+* Prevent potential accidental caching on user context hash mismatch (particularly with symfony HttpCache).
+
+1.3.11
+------
+
+* #395 : Compatibility with SensioFrameworkExtraBundle 4.
+
+1.3.10
+------
+
+* Avoid calling deprecated method in Symfony 3.2.
+
+1.3.9
+-----
+
+* Fix configuration handling when only custom proxy client is configured.
+
+1.3.8
+-----
+
+* Do not sanity check hash on anonymous requests.
 
 1.3.7
 -----

diff --git a/DependencyInjection/Configuration.php b/DependencyInjection/Configuration.php
@@ -647,7 +647,11 @@ private function addUserContextListenerSection(ArrayNodeDefinition $rootNode)
                         ->arrayNode('user_identifier_headers')
                             ->prototype('scalar')->end()
                             ->defaultValue(array('Cookie', 'Authorization'))
-                            ->info('List of headers that contains the unique identifier for the user in the hash request.')
+                            ->info('List of headers that contain the unique identifier for the user in the hash request.')
+                        ->end()
+                        ->scalarNode('session_name_prefix')
+                            ->defaultValue(false)
+                            ->info('Prefix for session cookies. Must match your PHP session configuration. Set to false to ignore the session in user context.')
                         ->end()
                         ->scalarNode('user_hash_header')
                             ->defaultValue('X-User-Context-Hash')

diff --git a/DependencyInjection/FOSHttpCacheExtension.php b/DependencyInjection/FOSHttpCacheExtension.php
@@ -186,6 +186,12 @@ private function createRuleMatcher(ContainerBuilder $container, Reference $reque
 
     private function loadUserContext(ContainerBuilder $container, XmlFileLoader $loader, array $config)
     {
+        $configuredUserIdentifierHeaders = array_map('strtolower', $config['user_identifier_headers']);
+        $completeUserIdentifierHeaders = $configuredUserIdentifierHeaders;
+        if (false !== $config['session_name_prefix'] && !in_array('cookie', $completeUserIdentifierHeaders)) {
+            $completeUserIdentifierHeaders[] = 'cookie';
+        }
+
         $loader->load('user_context.xml');
 
         $container->getDefinition($this->getAlias().'.user_context.request_matcher')
@@ -194,16 +200,20 @@ private function loadUserContext(ContainerBuilder $container, XmlFileLoader $loa
 
         $container->getDefinition($this->getAlias().'.event_listener.user_context')
             ->replaceArgument(0, new Reference($config['match']['matcher_service']))
-            ->replaceArgument(2, $config['user_identifier_headers'])
+            ->replaceArgument(2, $completeUserIdentifierHeaders)
             ->replaceArgument(3, $config['user_hash_header'])
             ->replaceArgument(4, $config['hash_cache_ttl']);
 
+        $options = array(
+            'user_identifier_headers' => $configuredUserIdentifierHeaders,
+            'session_name_prefix' => $config['session_name_prefix'],
+        );
         $container->getDefinition($this->getAlias().'.user_context.anonymous_request_matcher')
-            ->replaceArgument(0, $config['user_identifier_headers']);
+            ->replaceArgument(0, $options);
 
         if ($config['logout_handler']['enabled']) {
             $container->getDefinition($this->getAlias().'.user_context.logout_handler')
-                ->replaceArgument(1, $config['user_identifier_headers'])
+                ->replaceArgument(1, $completeUserIdentifierHeaders)
                 ->replaceArgument(2, $config['match']['accept']);
         } else {
             $container->removeDefinition($this->getAlias().'.user_context.logout_handler');

diff --git a/EventListener/UserContextSubscriber.php b/EventListener/UserContextSubscriber.php
@@ -165,7 +165,7 @@ public function onKernelResponse(FilterResponseEvent $event)
 
         if ($request->headers->has($this->hashHeader)) {
             // hash has changed, session has most certainly changed, prevent setting incorrect cache
-            if (!is_null($this->hash) && $this->hash !== $request->headers->get($this->hashHeader)) {
+            if (null !== $this->hash && $this->hash !== $request->headers->get($this->hashHeader)) {
                 $response->setCache([
                     'max_age' => 0,
                     's_maxage' => 0,

diff --git a/Resources/config/user_context.xml b/Resources/config/user_context.xml
@@ -41,7 +41,7 @@
             <argument />
         </service>
 
-        <service id="fos_http_cache.user_context.anonymous_request_matcher" class="FOS\HttpCacheBundle\UserContext\AnonymousRequestMatcher">
+        <service id="fos_http_cache.user_context.anonymous_request_matcher" class="FOS\HttpCache\UserContext\AnonymousRequestMatcher">
             <argument type="collection" />
         </service>
     </services>

diff --git a/Resources/doc/reference/configuration/user-context.rst b/Resources/doc/reference/configuration/user-context.rst
@@ -174,6 +174,23 @@ for 15 minutes, configure:
                 - Authorization
             hash_cache_ttl: 900
 
+The ``Cookie`` header is automatically added to this list unless ``session_name_prefix``
+is set to ``false``.
+
+``session_name_prefix``
+~~~~~~~~~~~~~~~~~~~~~~~
+
+**type**: ``string`` **default**: ``PHPSESSID``
+
+Defines which cookie is the session cookie. Normal cookies will be ignored in
+user context and only the session cookie is taken into account. It is
+recommended that you clean up the cookie header to avoid any other cookies in
+your requests.
+
+If you set this configuration to ``false``, cookies are completely ignored. If
+you add the ``Cookie`` header to ``user_identifier_headers``, any cookie will
+make the request not anonymous.
+
 ``role_provider``
 ~~~~~~~~~~~~~~~~~
 

diff --git a/Tests/Functional/Security/Http/Logout/ContextInvalidationLogoutHandlerTest.php b/Tests/Functional/Security/Http/Logout/ContextInvalidationLogoutHandlerTest.php
@@ -23,8 +23,8 @@ public function testLogout()
             'fos_http_cache.proxy_client.varnish',
             '\FOS\HttpCache\ProxyClient\Varnish'
         )
-            ->shouldReceive('ban')->once()->with(array('accept' => 'application/vnd.fos.user-context-hash', 'Cookie' => '.*test.*'))
-            ->shouldReceive('ban')->once()->with(array('accept' => 'application/vnd.fos.user-context-hash', 'Authorization' => '.*test.*'))
+            ->shouldReceive('ban')->once()->with(array('accept' => 'application/vnd.fos.user-context-hash', 'cookie' => '.*test.*'))
+            ->shouldReceive('ban')->once()->with(array('accept' => 'application/vnd.fos.user-context-hash', 'authorization' => '.*test.*'))
             ->shouldReceive('flush')->once()
         ;
 

diff --git a/Tests/Resources/Fixtures/config/full.php b/Tests/Resources/Fixtures/config/full.php
@@ -107,6 +107,7 @@
         ),
         'hash_cache_ttl' => 300,
         'user_identifier_headers' => array('Cookie', 'Authorization'),
+        'session_name_prefix' => 'PHPSESSID',
         'user_hash_header' => 'FOS-User-Context-Hash',
         'role_provider' => true,
     ),

diff --git a/Tests/Resources/Fixtures/config/full.xml b/Tests/Resources/Fixtures/config/full.xml
@@ -75,7 +75,7 @@
             </rule>
         </invalidation>
 
-        <user-context hash-cache-ttl="300" role-provider="true" user-hash-header="FOS-User-Context-Hash">
+        <user-context hash-cache-ttl="300" role-provider="true" session-name-prefix="PHPSESSID" user-hash-header="FOS-User-Context-Hash">
             <match method="GET"/>
             <user-identifier-header>Cookie</user-identifier-header>
             <user-identifier-header>Authorization</user-identifier-header>

diff --git a/Tests/Resources/Fixtures/config/full.yml b/Tests/Resources/Fixtures/config/full.yml
@@ -95,6 +95,7 @@ fos_http_cache:
         user_identifier_headers:
             - Cookie
             - Authorization
+        session_name_prefix: PHPSESSID
         user_hash_header: FOS-User-Context-Hash
 
     flash_message:

diff --git a/Tests/Unit/DependencyInjection/ConfigurationTest.php b/Tests/Unit/DependencyInjection/ConfigurationTest.php
@@ -152,6 +152,7 @@ public function testSupportsAllConfigFormats()
                 ),
                 'hash_cache_ttl' => 300,
                 'user_identifier_headers' => array('Cookie', 'Authorization'),
+                'session_name_prefix' => 'PHPSESSID',
                 'user_hash_header' => 'FOS-User-Context-Hash',
                 'role_provider' => true,
                 'logout_handler' => array(
@@ -497,6 +498,7 @@ private function getEmptyConfig()
                 ),
                 'hash_cache_ttl' => 0,
                 'user_identifier_headers' => array('Cookie', 'Authorization'),
+                'session_name_prefix' => false,
                 'user_hash_header' => 'X-User-Context-Hash',
                 'role_provider' => false,
                 'logout_handler' => array(

diff --git a/Tests/Unit/UserContext/AnonymousRequestMatcherTest.php b/Tests/Unit/UserContext/AnonymousRequestMatcherTest.php
diff --git a/UserContext/AnonymousRequestMatcher.php b/UserContext/AnonymousRequestMatcher.php
@@ -11,37 +11,30 @@
 
 namespace FOS\HttpCacheBundle\UserContext;
 
-use Symfony\Component\HttpFoundation\Request;
-use Symfony\Component\HttpFoundation\RequestMatcherInterface;
+use FOS\HttpCache\UserContext\AnonymousRequestMatcher as BaseAnonymousRequestMatcher;
 
 /**
  * Matches anonymous requests using a list of identification headers.
+ *
+ * @deprecated Use AnonymousRequestMatcher of HttpCache library
  */
-class AnonymousRequestMatcher implements RequestMatcherInterface
+class AnonymousRequestMatcher extends BaseAnonymousRequestMatcher
 {
-    private $userIdentifierHeaders;
-
-    /**
-     * @param array $userIdentifierHeaders List of request headers that authenticate a non-anonymous request
-     */
-    public function __construct(array $userIdentifierHeaders)
+    public function __construct(array $options = array())
     {
-        $this->userIdentifierHeaders = $userIdentifierHeaders;
-    }
-
-    public function matches(Request $request)
-    {
-        foreach ($this->userIdentifierHeaders as $header) {
-            if ($request->headers->has($header)) {
-                if ('cookie' === strtolower($header) && 0 === $request->cookies->count()) {
-                    // ignore empty cookie header
-                    continue;
-                }
-
-                return false;
-            }
+        @trigger_error(
+            'AnonymousRequestMatcher of HttpCacheBundle is deprecated. '.
+            'Use AnonymousRequestMatcher of HttpCache library.',
+            E_USER_DEPRECATED
+        );
+
+        if (isset($options['user_identifier_headers'], $options['session_name_prefix'])) {
+            parent::__construct($options);
+        } else {
+            parent::__construct(array(
+                'user_identifier_headers' => $options,
+                'session_name_prefix' => 'PHPSESSID',
+            ));
         }
-
-        return true;
     }
 }
diff --git a/composer.json b/composer.json
@@ -21,15 +21,15 @@
         }
     ],
     "require": {
-        "php": ">=5.3.3",
-        "friendsofsymfony/http-cache": "~1.4",
+        "php": "^5.4 || ^7.0",
+        "friendsofsymfony/http-cache": "^1.4.5",
         "symfony/framework-bundle": "^2.3||^3.0"
     },
     "require-dev": {
         "mockery/mockery": "0.9.*",
         "monolog/monolog": "*",
         "sensio/framework-extra-bundle": "^2.3||^3.0",
-        "symfony/symfony": "^2.3.4||^3.0",
+        "symfony/symfony": "^2.8||^3.0",
         "symfony/phpunit-bridge": "^3.2",
         "symfony/expression-language": "^2.4||^3.0",
         "symfony/monolog-bundle": "^2.3||^3.0",