[SecurityBundle] Fix the session listener under the new authenticatio…

…n manager Fixes the logic that adds session listeners for firewalls to properly add them only for statefull firewalls. Adds tests to confirm that it is only added to statefull ones. Also remove unused abstract field on session listener
symfony · Jun 7, 2020 · fdc7fd9 · fdc7fd9
1 parent e778ea6
commit fdc7fd9
Show file tree

Hide file tree

Showing 3 changed files with 49 additions and 11 deletions.
diff --git a/src/Symfony/Bundle/SecurityBundle/DependencyInjection/SecurityExtension.php b/src/Symfony/Bundle/SecurityBundle/DependencyInjection/SecurityExtension.php
@@ -168,14 +168,6 @@ public function load(array $configs, ContainerBuilder $container)
         $container->getDefinition('security.authentication.guard_handler')
             ->replaceArgument(2, $this->statelessFirewallKeys);
 
-        if ($this->authenticatorManagerEnabled) {
-            foreach ($this->statelessFirewallKeys as $statelessFirewallId) {
-                $container
-                    ->setDefinition('security.listener.session.'.$statelessFirewallId, new ChildDefinition('security.listener.session'))
-                    ->addTag('kernel.event_subscriber', ['dispatcher' => 'security.event_dispatcher.'.$statelessFirewallId]);
-            }
-        }
-
         if ($config['encoders']) {
             $this->createEncoders($config['encoders'], $container);
         }
@@ -373,6 +365,12 @@ private function createFirewall(ContainerBuilder $container, string $id, array $
             $contextKey = $firewall['context'] ?? $id;
             $listeners[] = new Reference($contextListenerId = $this->createContextListener($container, $contextKey));
             $sessionStrategyId = 'security.authentication.session_strategy';
+
+            if ($this->authenticatorManagerEnabled) {
+                $container
+                    ->setDefinition('security.listener.session.'.$id, new ChildDefinition('security.listener.session'))
+                    ->addTag('kernel.event_subscriber', ['dispatcher' => $firewallEventDispatcherId]);
+            }
         } else {
             $this->statelessFirewallKeys[] = $id;
             $sessionStrategyId = 'security.authentication.session_strategy_noop';

diff --git a/src/Symfony/Bundle/SecurityBundle/Resources/config/security_authenticator.xml b/src/Symfony/Bundle/SecurityBundle/Resources/config/security_authenticator.xml
@@ -60,10 +60,8 @@
         </service>
 
         <service id="security.listener.session"
-                 class="Symfony\Component\Security\Http\EventListener\SessionStrategyListener"
-                 abstract="true">
+                 class="Symfony\Component\Security\Http\EventListener\SessionStrategyListener">
             <argument type="service" id="security.authentication.session_strategy" />
-            <argument type="abstract">stateless firewall keys</argument>
         </service>
 
         <service id="security.listener.remember_me"

diff --git a/src/Symfony/Bundle/SecurityBundle/Tests/DependencyInjection/SecurityExtensionTest.php b/src/Symfony/Bundle/SecurityBundle/Tests/DependencyInjection/SecurityExtensionTest.php
@@ -559,6 +559,48 @@ public function provideConfigureCustomAuthenticatorData()
         ];
     }
 
+    public function testCompilesWithoutSessionListenerWithStatelessFirewallWithAuthenticationManager()
+    {
+        $container = $this->getRawContainer();
+
+        $firewallId = 'stateless_firewall';
+        $container->loadFromExtension('security', [
+            'enable_authenticator_manager' => true,
+            'firewalls' => [
+                $firewallId => [
+                    'pattern' => '/.*',
+                    'stateless' => true,
+                    'http_basic' => null,
+                ],
+            ],
+        ]);
+
+        $container->compile();
+
+        $this->assertFalse($container->has('security.listener.session.'.$firewallId));
+    }
+
+    public function testCompilesWithSessionListenerWithStatefulllFirewallWithAuthenticationManager()
+    {
+        $container = $this->getRawContainer();
+
+        $firewallId = 'statefull_firewall';
+        $container->loadFromExtension('security', [
+            'enable_authenticator_manager' => true,
+            'firewalls' => [
+                $firewallId => [
+                    'pattern' => '/.*',
+                    'stateless' => false,
+                    'http_basic' => null,
+                ],
+            ],
+        ]);
+
+        $container->compile();
+
+        $this->assertTrue($container->has('security.listener.session.'.$firewallId));
+    }
+
     protected function getRawContainer()
     {
         $container = new ContainerBuilder();