[Security] Deprecate callable firewall listeners

Author: MatTheCat

CHANGELOG.md

@@ -22,6 +22,7 @@ CHANGELOG
     ) {
     }
     ```
+ * Deprecate `LazyFirewallContext::__invoke()`
 
 7.3
 ---

Debug/TraceableFirewallListener.php

@@ -16,6 +16,7 @@
 use Symfony\Bundle\SecurityBundle\Security\LazyFirewallContext;
 use Symfony\Component\HttpKernel\Event\RequestEvent;
 use Symfony\Component\Security\Http\Authenticator\Debug\TraceableAuthenticatorManagerListener;
+use Symfony\Component\Security\Http\Firewall\AbstractListener;
 use Symfony\Component\Security\Http\Firewall\FirewallListenerInterface;
 use Symfony\Contracts\Service\ResetInterface;
 
@@ -88,7 +89,11 @@ protected function callListeners(RequestEvent $event, iterable $listeners): void
         }
 
         foreach ($requestListeners as $listener) {
-            $listener($event);
+            if (!$listener instanceof FirewallListenerInterface) {
+                $listener($event);
+            } elseif (false !== $listener->supports($event->getRequest())) {
+                $listener->authenticate($event);
+            }
 
             if ($event->hasResponse()) {
                 break;

Security/FirewallContext.php

@@ -12,6 +12,7 @@
 namespace Symfony\Bundle\SecurityBundle\Security;
 
 use Symfony\Component\Security\Http\Firewall\ExceptionListener;
+use Symfony\Component\Security\Http\Firewall\FirewallListenerInterface;
 use Symfony\Component\Security\Http\Firewall\LogoutListener;
 
 /**
@@ -39,7 +40,7 @@ public function getConfig(): ?FirewallConfig
     }
 
     /**
-     * @return iterable<mixed, callable>
+     * @return iterable<mixed, FirewallListenerInterface|callable>
      */
     public function getListeners(): iterable
     {

Security/LazyFirewallContext.php

@@ -11,9 +11,11 @@
 
 namespace Symfony\Bundle\SecurityBundle\Security;
 
+use Symfony\Component\HttpFoundation\Request;
 use Symfony\Component\HttpKernel\Event\RequestEvent;
 use Symfony\Component\Security\Core\Authentication\Token\Storage\TokenStorage;
 use Symfony\Component\Security\Http\Event\LazyResponseEvent;
+use Symfony\Component\Security\Http\Firewall\AbstractListener;
 use Symfony\Component\Security\Http\Firewall\ExceptionListener;
 use Symfony\Component\Security\Http\Firewall\FirewallListenerInterface;
 use Symfony\Component\Security\Http\Firewall\LogoutListener;
@@ -23,7 +25,7 @@
  *
  * @author Nicolas Grekas <p@tchwork.com>
  */
-class LazyFirewallContext extends FirewallContext
+class LazyFirewallContext extends FirewallContext implements FirewallListenerInterface
 {
     public function __construct(
         iterable $listeners,
@@ -40,19 +42,26 @@ public function getListeners(): iterable
         return [$this];
     }
 
-    public function __invoke(RequestEvent $event): void
+    public function supports(Request $request): ?bool
+    {
+        return true;
+    }
+
+    public function authenticate(RequestEvent $event): void
     {
         $listeners = [];
         $request = $event->getRequest();
         $lazy = $request->isMethodCacheable();
 
         foreach (parent::getListeners() as $listener) {
-            if (!$lazy || !$listener instanceof FirewallListenerInterface) {
+            if (!$listener instanceof FirewallListenerInterface) {
+                trigger_deprecation('symfony/security-http', '7.4', 'Using a callable as firewall listener is deprecated, extend "%s" or implement "%s" instead.', AbstractListener::class, FirewallListenerInterface::class);
+
                 $listeners[] = $listener;
-                $lazy = $lazy && $listener instanceof FirewallListenerInterface;
+                $lazy = false;
             } elseif (false !== $supports = $listener->supports($request)) {
                 $listeners[] = [$listener, 'authenticate'];
-                $lazy = null === $supports;
+                $lazy = $lazy && null === $supports;
             }
         }
 
@@ -75,4 +84,19 @@ public function __invoke(RequestEvent $event): void
             }
         });
     }
+
+    public static function getPriority(): int
+    {
+        return 0;
+    }
+
+    /**
+     * @deprecated since Symfony 7.4, to be removed in 8.0
+     */
+    public function __invoke(RequestEvent $event): void
+    {
+        trigger_deprecation('symfony/security-bundle', '7.4', 'The "%s()" method is deprecated since Symfony 7.4 and will be removed in 8.0.', __METHOD__);
+
+        $this->authenticate($event);
+    }
 }

Tests/DataCollector/SecurityDataCollectorTest.php

@@ -32,6 +32,8 @@
 use Symfony\Component\Security\Core\Authorization\Voter\VoterInterface;
 use Symfony\Component\Security\Core\Role\RoleHierarchy;
 use Symfony\Component\Security\Core\User\InMemoryUser;
+use Symfony\Component\Security\Http\Firewall\AbstractListener;
+use Symfony\Component\Security\Http\Firewall\FirewallListenerInterface;
 use Symfony\Component\Security\Http\FirewallMapInterface;
 use Symfony\Component\Security\Http\Logout\LogoutUrlGenerator;
 use Symfony\Component\VarDumper\Caster\ClassStub;
@@ -193,8 +195,18 @@ public function testGetListeners()
         $request = new Request();
         $event = new RequestEvent($this->createMock(HttpKernelInterface::class), $request, HttpKernelInterface::MAIN_REQUEST);
         $event->setResponse($response = new Response());
-        $listener = function ($e) use ($event, &$listenerCalled) {
-            $listenerCalled += $e === $event;
+        $listener = new class extends AbstractListener {
+            public int $callCount = 0;
+
+            public function supports(Request $request): ?bool
+            {
+                return true;
+            }
+
+            public function authenticate(RequestEvent $event): void
+            {
+                ++$this->callCount;
+            }
         };
         $firewallMap = $this
             ->getMockBuilder(FirewallMap::class)
@@ -217,9 +229,9 @@ public function testGetListeners()
         $collector = new SecurityDataCollector(null, null, null, null, $firewallMap, $firewall, true);
         $collector->collect($request, $response);
 
-        $this->assertNotEmpty($collected = $collector->getListeners()[0]);
+        $this->assertCount(1, $collector->getListeners());
         $collector->lateCollect();
-        $this->assertSame(1, $listenerCalled);
+        $this->assertSame(1, $listener->callCount);
     }
 
     public function testCollectCollectsDecisionLogWhenStrategyIsAffirmative()

Tests/Debug/TraceableFirewallListenerTest.php

@@ -29,7 +29,9 @@
 use Symfony\Component\Security\Http\Authenticator\Passport\Badge\UserBadge;
 use Symfony\Component\Security\Http\Authenticator\Passport\Passport;
 use Symfony\Component\Security\Http\Authenticator\Passport\SelfValidatingPassport;
+use Symfony\Component\Security\Http\Firewall\AbstractListener;
 use Symfony\Component\Security\Http\Firewall\AuthenticatorManagerListener;
+use Symfony\Component\Security\Http\Firewall\FirewallListenerInterface;
 use Symfony\Component\Security\Http\Logout\LogoutUrlGenerator;
 
 /**
@@ -41,9 +43,19 @@ public function testOnKernelRequestRecordsListeners()
     {
         $request = new Request();
         $event = new RequestEvent($this->createMock(HttpKernelInterface::class), $request, HttpKernelInterface::MAIN_REQUEST);
-        $event->setResponse($response = new Response());
-        $listener = function ($e) use ($event, &$listenerCalled) {
-            $listenerCalled += $e === $event;
+        $event->setResponse(new Response());
+        $listener = new class extends AbstractListener {
+            public int $callCount = 0;
+
+            public function supports(Request $request): ?bool
+            {
+                return true;
+            }
+
+            public function authenticate(RequestEvent $event): void
+            {
+                ++$this->callCount;
+            }
         };
         $firewallMap = $this->createMock(FirewallMap::class);
         $firewallMap

composer.json

@@ -22,6 +22,7 @@
         "symfony/clock": "^6.4|^7.0|^8.0",
         "symfony/config": "^7.3|^8.0",
         "symfony/dependency-injection": "^6.4.11|^7.1.4|^8.0",
+        "symfony/deprecation-contracts": "^2.5|^3",
         "symfony/event-dispatcher": "^6.4|^7.0|^8.0",
         "symfony/http-kernel": "^6.4|^7.0|^8.0",
         "symfony/http-foundation": "^6.4|^7.0|^8.0",