bug symfony#37366 [SecurityBundle] Fix UserCheckerListener registration with custom user checker (wouterj)

This PR was merged into the 5.1 branch.

Discussion
----------

[SecurityBundle] Fix UserCheckerListener registration with custom user checker

| Q             | A
| ------------- | ---
| Branch?       | 5.1
| Bug fix?      | yes
| New feature?  | no
| Deprecations? | no
| Tickets       | Fix symfony#37365
| License       | MIT
| Doc PR        | -

The user checker listener was wrongly registered on the global event dispatcher, as it can be customized per firewall. This PR fixes that + correctly uses the configured user checker instead of always trying to use `UserCheckerInterface`.

Commits
-------

d63f590 Fix UserCheckerListener registration with custom user checkers

Author: chalasr

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

@@ -478,6 +478,12 @@ private function createFirewall(ContainerBuilder $container, string $id, array $
                 ->replaceArgument(0, new Reference($managerId))
             ;
 
+            // user checker listener
+            $container
+                ->setDefinition('security.listener.user_checker.'.$id, new ChildDefinition('security.listener.user_checker'))
+                ->replaceArgument(0, new Reference('security.user_checker.'.$id))
+                ->addTag('kernel.event_subscriber', ['dispatcher' => $firewallEventDispatcherId]);
+
             $listeners[] = new Reference('security.firewall.authenticator.'.$id);
         }
 

src/Symfony/Bundle/SecurityBundle/Resources/config/security_authenticator.xml

@@ -54,9 +54,8 @@
             <argument type="service" id="security.encoder_factory" />
         </service>
 
-        <service id="security.listener.user_checker" class="Symfony\Component\Security\Http\EventListener\UserCheckerListener">
-            <tag name="kernel.event_subscriber" />
-            <argument type="service" id="Symfony\Component\Security\Core\User\UserCheckerInterface" />
+        <service id="security.listener.user_checker" class="Symfony\Component\Security\Http\EventListener\UserCheckerListener" abstract="true">
+            <argument type="abstract">user checker</argument>
         </service>
 
         <service id="security.listener.session"

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

@@ -27,6 +27,8 @@
 use Symfony\Component\HttpFoundation\Response;
 use Symfony\Component\Security\Core\Authentication\Token\TokenInterface;
 use Symfony\Component\Security\Core\Exception\AuthenticationException;
+use Symfony\Component\Security\Core\User\UserChecker;
+use Symfony\Component\Security\Core\User\UserCheckerInterface;
 use Symfony\Component\Security\Core\User\UserInterface;
 use Symfony\Component\Security\Core\User\UserProviderInterface;
 use Symfony\Component\Security\Guard\AuthenticatorInterface as GuardAuthenticatorInterface;
@@ -509,7 +511,7 @@ public function provideEntryPointRequiredData()
         ];
     }
 
-    public function testAlwaysAuthenticateBeforeGrantingCannotBeTrueWithAuthenticationManager()
+    public function testAlwaysAuthenticateBeforeGrantingCannotBeTrueWithAuthenticatorManager()
     {
         $this->expectException(InvalidConfigurationException::class);
         $this->expectExceptionMessage('The security option "always_authenticate_before_granting" cannot be used when "enable_authenticator_manager" is set to true. If you rely on this behavior, set it to false.');
@@ -559,7 +561,7 @@ public function provideConfigureCustomAuthenticatorData()
         ];
     }
 
-    public function testCompilesWithoutSessionListenerWithStatelessFirewallWithAuthenticationManager()
+    public function testCompilesWithoutSessionListenerWithStatelessFirewallWithAuthenticatorManager()
     {
         $container = $this->getRawContainer();
 
@@ -580,7 +582,7 @@ public function testCompilesWithoutSessionListenerWithStatelessFirewallWithAuthe
         $this->assertFalse($container->has('security.listener.session.'.$firewallId));
     }
 
-    public function testCompilesWithSessionListenerWithStatefulllFirewallWithAuthenticationManager()
+    public function testCompilesWithSessionListenerWithStatefulllFirewallWithAuthenticatorManager()
     {
         $container = $this->getRawContainer();
 
@@ -601,6 +603,37 @@ public function testCompilesWithSessionListenerWithStatefulllFirewallWithAuthent
         $this->assertTrue($container->has('security.listener.session.'.$firewallId));
     }
 
+    /**
+     * @dataProvider provideUserCheckerConfig
+     */
+    public function testUserCheckerWithAuthenticatorManager(array $config, string $expectedUserCheckerClass)
+    {
+        $container = $this->getRawContainer();
+        $container->register(TestUserChecker::class);
+
+        $container->loadFromExtension('security', [
+            'enable_authenticator_manager' => true,
+            'firewalls' => [
+                'main' => array_merge([
+                    'pattern' => '/.*',
+                    'http_basic' => true,
+                ], $config),
+            ],
+        ]);
+
+        $container->compile();
+
+        $userCheckerId = (string) $container->getDefinition('security.listener.user_checker.main')->getArgument(0);
+        $this->assertTrue($container->has($userCheckerId));
+        $this->assertEquals($expectedUserCheckerClass, $container->findDefinition($userCheckerId)->getClass());
+    }
+
+    public function provideUserCheckerConfig()
+    {
+        yield [[], UserChecker::class];
+        yield [['user_checker' => TestUserChecker::class], TestUserChecker::class];
+    }
+
     protected function getRawContainer()
     {
         $container = new ContainerBuilder();
@@ -689,3 +722,14 @@ public function supportsRememberMe()
     {
     }
 }
+
+class TestUserChecker implements UserCheckerInterface
+{
+    public function checkPreAuth(UserInterface $user)
+    {
+    }
+
+    public function checkPostAuth(UserInterface $user)
+    {
+    }
+}