bug #25272 [SecurityBundle] fix setLogoutOnUserChange calls for conte…

…xt listeners (dmaicher)

This PR was squashed before being merged into the 3.4 branch (closes #25272).

Discussion
----------

[SecurityBundle] fix setLogoutOnUserChange calls for context listeners

| Q             | A
| ------------- | ---
| Branch?       | 3.4
| Bug fix?      | yes
| New feature?  | no
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets | #25267
| License       | MIT
| Doc PR        | -

As pointed out in #25267 the `setLogoutOnUserChange` method calls were added to the parent definition `security.context_listener` instead of the concrete child definitions `security.context_listener.*`.

ping @iltar @chalasr

Commits
-------

4eff146 [SecurityBundle] fix setLogoutOnUserChange calls for context listeners

src/Symfony/Bundle/SecurityBundle/DependencyInjection/MainConfiguration.php

@@ -340,17 +340,6 @@ private function addFirewallsSection(ArrayNodeDefinition $rootNode, array $facto
                     return $firewall;
                 })
             ->end()
-            ->validate()
-                ->ifTrue(function ($v) {
-                    return (isset($v['stateless']) && true === $v['stateless']) || (isset($v['security']) && false === $v['security']);
-                })
-                ->then(function ($v) {
-                    // this option doesn't change behavior when true when stateless, so prevent deprecations
-                    $v['logout_on_user_change'] = true;
-
-                    return $v;
-                })
-            ->end()
         ;
     }
 

src/Symfony/Bundle/SecurityBundle/DependencyInjection/SecurityExtension.php

@@ -43,6 +43,7 @@ class SecurityExtension extends Extension
     private $factories = array();
     private $userProviderFactories = array();
     private $expressionLanguage;
+    private $logoutOnUserChangeByContextKey = array();
 
     public function __construct()
     {
@@ -276,12 +277,6 @@ private function createFirewalls($config, ContainerBuilder $container)
                 $customUserChecker = true;
             }
 
-            if (!isset($firewall['logout_on_user_change']) || !$firewall['logout_on_user_change']) {
-                @trigger_error(sprintf('Not setting "logout_on_user_change" to true on firewall "%s" is deprecated as of 3.4, it will always be true in 4.0.', $name), E_USER_DEPRECATED);
-            }
-
-            $contextListenerDefinition->addMethodCall('setLogoutOnUserChange', array($firewall['logout_on_user_change']));
-
             $configId = 'security.firewall.map.config.'.$name;
 
             list($matcher, $listeners, $exceptionListener) = $this->createFirewall($container, $name, $firewall, $authenticationProviders, $providerIds, $configId);
@@ -370,7 +365,16 @@ private function createFirewall(ContainerBuilder $container, $id, $firewall, &$a
                 $contextKey = $firewall['context'];
             }
 
-            $listeners[] = new Reference($this->createContextListener($container, $contextKey));
+            if (!$logoutOnUserChange = $firewall['logout_on_user_change']) {
+                @trigger_error(sprintf('Not setting "logout_on_user_change" to true on firewall "%s" is deprecated as of 3.4, it will always be true in 4.0.', $id), E_USER_DEPRECATED);
+            }
+
+            if (isset($this->logoutOnUserChangeByContextKey[$contextKey]) && $this->logoutOnUserChangeByContextKey[$contextKey][1] !== $logoutOnUserChange) {
+                throw new InvalidConfigurationException(sprintf('Firewalls "%s" and "%s" need to have the same value for option "logout_on_user_change" as they are sharing the context "%s"', $this->logoutOnUserChangeByContextKey[$contextKey][0], $id, $contextKey));
+            }
+
+            $this->logoutOnUserChangeByContextKey[$contextKey] = array($id, $logoutOnUserChange);
+            $listeners[] = new Reference($this->createContextListener($container, $contextKey, $logoutOnUserChange));
         }
 
         $config->replaceArgument(6, $contextKey);
@@ -481,7 +485,7 @@ private function createFirewall(ContainerBuilder $container, $id, $firewall, &$a
         return array($matcher, $listeners, $exceptionListener);
     }
 
-    private function createContextListener($container, $contextKey)
+    private function createContextListener($container, $contextKey, $logoutUserOnChange)
     {
         if (isset($this->contextListeners[$contextKey])) {
             return $this->contextListeners[$contextKey];
@@ -490,6 +494,7 @@ private function createContextListener($container, $contextKey)
         $listenerId = 'security.context_listener.'.count($this->contextListeners);
         $listener = $container->setDefinition($listenerId, new ChildDefinition('security.context_listener'));
         $listener->replaceArgument(2, $contextKey);
+        $listener->addMethodCall('setLogoutOnUserChange', array($logoutUserOnChange));
 
         return $this->contextListeners[$contextKey] = $listenerId;
     }

src/Symfony/Bundle/SecurityBundle/Tests/DependencyInjection/SecurityExtensionTest.php

@@ -127,21 +127,60 @@ public function testDisableRoleHierarchyVoter()
      * @group legacy
      * @expectedDeprecation Not setting "logout_on_user_change" to true on firewall "some_firewall" is deprecated as of 3.4, it will always be true in 4.0.
      */
-    public function testDeprecationForUserLogout()
+    public function testConfiguresLogoutOnUserChangeForContextListenersCorrectly()
     {
         $container = $this->getRawContainer();
 
         $container->loadFromExtension('security', array(
             'providers' => array(
                 'default' => array('id' => 'foo'),
             ),
+            'firewalls' => array(
+                'some_firewall' => array(
+                    'pattern' => '/.*',
+                    'http_basic' => null,
+                    'logout_on_user_change' => false,
+                ),
+                'some_other_firewall' => array(
+                    'pattern' => '/.*',
+                    'http_basic' => null,
+                    'logout_on_user_change' => true,
+                ),
+            ),
+        ));
+
+        $container->compile();
+
+        $this->assertEquals(array(array('setLogoutOnUserChange', array(false))), $container->getDefinition('security.context_listener.0')->getMethodCalls());
+        $this->assertEquals(array(array('setLogoutOnUserChange', array(true))), $container->getDefinition('security.context_listener.1')->getMethodCalls());
+    }
 
+    /**
+     * @group legacy
+     * @expectedException \Symfony\Component\Config\Definition\Exception\InvalidConfigurationException
+     * @expectedExceptionMessage Firewalls "some_firewall" and "some_other_firewall" need to have the same value for option "logout_on_user_change" as they are sharing the context "my_context"
+     */
+    public function testThrowsIfLogoutOnUserChangeDifferentForSharedContext()
+    {
+        $container = $this->getRawContainer();
+
+        $container->loadFromExtension('security', array(
+            'providers' => array(
+                'default' => array('id' => 'foo'),
+            ),
             'firewalls' => array(
                 'some_firewall' => array(
                     'pattern' => '/.*',
                     'http_basic' => null,
+                    'context' => 'my_context',
                     'logout_on_user_change' => false,
                 ),
+                'some_other_firewall' => array(
+                    'pattern' => '/.*',
+                    'http_basic' => null,
+                    'context' => 'my_context',
+                    'logout_on_user_change' => true,
+                ),
             ),
         ));