bug #38751 [Security] Move AbstractListener abstract methods to the new FirewallListenerInterface (chalasr)

This PR was merged into the 5.x branch.

Discussion
----------

[Security] Move AbstractListener abstract methods to the new FirewallListenerInterface

| Q             | A
| ------------- | ---
| Branch?       | 5.x
| Bug fix?      | no
| New feature?  | -
| Deprecations? | no
| Tickets       | -
| License       | MIT
| Doc PR        | -

We added a FirewallListenerInterface in 5.2, let's make it complete to allow for cleaner firewall listener implementations.

Commits
-------

5dd70bd62e [Security] Move AbstractListener abstract methods to the new FirewallListenerInterface

src/Symfony/Bundle/SecurityBundle/Debug/TraceableFirewallListener.php

@@ -15,7 +15,7 @@ use Symfony\Bundle\SecurityBundle\EventListener\FirewallListener;
 use Symfony\Bundle\SecurityBundle\Security\FirewallContext;
 use Symfony\Bundle\SecurityBundle\Security\LazyFirewallContext;
 use Symfony\Component\HttpKernel\Event\RequestEvent;
-use Symfony\Component\Security\Http\Firewall\AbstractListener;
+use Symfony\Component\Security\Http\Firewall\FirewallListenerInterface;
 
 /**
  * Firewall collecting called listeners.
@@ -41,7 +41,7 @@ final class TraceableFirewallListener extends FirewallListener
                 \Closure::bind(function () use (&$wrappedLazyListeners, &$wrappedListeners) {
                     $listeners = [];
                     foreach ($this->listeners as $listener) {
-                        if ($listener instanceof AbstractListener) {
+                        if ($listener instanceof FirewallListenerInterface) {
                             $listener = new WrappedLazyListener($listener);
                             $listeners[] = $listener;
                             $wrappedLazyListeners[] = $listener;
@@ -58,7 +58,7 @@ final class TraceableFirewallListener extends FirewallListener
 
                 $listener($event);
             } else {
-                $wrappedListener = $listener instanceof AbstractListener ? new WrappedLazyListener($listener) : new WrappedListener($listener);
+                $wrappedListener = $listener instanceof FirewallListenerInterface ? new WrappedLazyListener($listener) : new WrappedListener($listener);
                 $wrappedListener($event);
                 $wrappedListeners[] = $wrappedListener->getInfo();
             }

src/Symfony/Bundle/SecurityBundle/Debug/WrappedLazyListener.php

@@ -15,6 +15,7 @@ use Symfony\Component\HttpFoundation\Request;
 use Symfony\Component\HttpKernel\Event\RequestEvent;
 use Symfony\Component\Security\Core\Exception\LazyResponseException;
 use Symfony\Component\Security\Http\Firewall\AbstractListener;
+use Symfony\Component\Security\Http\Firewall\FirewallListenerInterface;
 
 /**
  * Wraps a lazy security listener.
@@ -27,7 +28,7 @@ final class WrappedLazyListener extends AbstractListener
 {
     use TraceableListenerTrait;
 
-    public function __construct(AbstractListener $listener)
+    public function __construct(FirewallListenerInterface $listener)
     {
         $this->listener = $listener;
     }

src/Symfony/Bundle/SecurityBundle/Security/LazyFirewallContext.php

@@ -14,8 +14,8 @@ namespace Symfony\Bundle\SecurityBundle\Security;
 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;
 
 /**
@@ -46,9 +46,9 @@ class LazyFirewallContext extends FirewallContext
         $lazy = $request->isMethodCacheable();
 
         foreach (parent::getListeners() as $listener) {
-            if (!$lazy || !$listener instanceof AbstractListener) {
+            if (!$lazy || !$listener instanceof FirewallListenerInterface) {
                 $listeners[] = $listener;
-                $lazy = $lazy && $listener instanceof AbstractListener;
+                $lazy = $lazy && $listener instanceof FirewallListenerInterface;
             } elseif (false !== $supports = $listener->supports($request)) {
                 $listeners[] = [$listener, 'authenticate'];
                 $lazy = null === $supports;

src/Symfony/Bundle/SecurityBundle/Tests/DependencyInjection/Compiler/SortFirewallListenersPassTest.php

@@ -17,6 +17,8 @@ use Symfony\Bundle\SecurityBundle\Security\FirewallContext;
 use Symfony\Component\DependencyInjection\Argument\IteratorArgument;
 use Symfony\Component\DependencyInjection\ContainerBuilder;
 use Symfony\Component\DependencyInjection\Reference;
+use Symfony\Component\HttpFoundation\Request;
+use Symfony\Component\HttpKernel\Event\RequestEvent;
 use Symfony\Component\Security\Http\Firewall\FirewallListenerInterface;
 
 class SortFirewallListenersPassTest extends TestCase
@@ -59,6 +61,14 @@ class SortFirewallListenersPassTest extends TestCase
 
 class FirewallListenerPriorityMinus1 implements FirewallListenerInterface
 {
+    public function supports(Request $request): ?bool
+    {
+    }
+
+    public function authenticate(RequestEvent $event)
+    {
+    }
+
     public static function getPriority(): int
     {
         return -1;
@@ -67,6 +77,14 @@ class FirewallListenerPriorityMinus1 implements FirewallListenerInterface
 
 class FirewallListenerPriority1 implements FirewallListenerInterface
 {
+    public function supports(Request $request): ?bool
+    {
+    }
+
+    public function authenticate(RequestEvent $event)
+    {
+    }
+
     public static function getPriority(): int
     {
         return 1;
@@ -75,6 +93,14 @@ class FirewallListenerPriority1 implements FirewallListenerInterface
 
 class FirewallListenerPriority2 implements FirewallListenerInterface
 {
+    public function supports(Request $request): ?bool
+    {
+    }
+
+    public function authenticate(RequestEvent $event)
+    {
+    }
+
     public static function getPriority(): int
     {
         return 2;

src/Symfony/Component/Security/CHANGELOG.md

@@ -14,6 +14,7 @@ CHANGELOG
  * Added a CurrentUser attribute to force the UserValueResolver to resolve an argument to the current user.
  * Added `LoginThrottlingListener`.
  * Added `LoginLinkAuthenticator`.
+ * Moved methods `supports()` and `authenticate()` from `AbstractListener` to `FirewallListenerInterface`.
 
 5.1.0
 -----

src/Symfony/Component/Security/Http/Authentication/AuthenticatorManagerInterface.php

@@ -13,7 +13,7 @@ namespace Symfony\Component\Security\Http\Authentication;
 
 use Symfony\Component\HttpFoundation\Request;
 use Symfony\Component\HttpFoundation\Response;
-use Symfony\Component\Security\Http\Firewall\AbstractListener;
+use Symfony\Component\Security\Http\Firewall\FirewallListenerInterface;
 
 /**
  * @author Wouter de Jong <wouter@wouterj.nl>
@@ -26,7 +26,7 @@ interface AuthenticatorManagerInterface
     /**
      * Called to see if authentication should be attempted on this request.
      *
-     * @see AbstractListener::supports()
+     * @see FirewallListenerInterface::supports()
      */
     public function supports(Request $request): ?bool;
 

src/Symfony/Component/Security/Http/Firewall/AbstractListener.php

@@ -11,7 +11,6 @@
 
 namespace Symfony\Component\Security\Http\Firewall;
 
-use Symfony\Component\HttpFoundation\Request;
 use Symfony\Component\HttpKernel\Event\RequestEvent;
 
 /**
@@ -28,18 +27,6 @@ abstract class AbstractListener implements FirewallListenerInterface
         }
     }
 
-    /**
-     * Tells whether the authenticate() method should be called or not depending on the incoming request.
-     *
-     * Returning null means authenticate() can be called lazily when accessing the token storage.
-     */
-    abstract public function supports(Request $request): ?bool;
-
-    /**
-     * Does whatever is required to authenticate the request, typically calling $event->setResponse() internally.
-     */
-    abstract public function authenticate(RequestEvent $event);
-
     public static function getPriority(): int
     {
         return 0; // Default

src/Symfony/Component/Security/Http/Firewall/FirewallListenerInterface.php

@@ -11,13 +11,30 @@
 
 namespace Symfony\Component\Security\Http\Firewall;
 
+use Symfony\Component\HttpFoundation\Request;
+use Symfony\Component\HttpKernel\Event\RequestEvent;
+
 /**
- * Can be implemented by firewall listeners to define their priority in execution.
+ * Can be implemented by firewall listeners.
  *
  * @author Christian Scheb <me@christianscheb.de>
+ * @author Nicolas Grekas <p@tchwork.com>
+ * @author Robin Chalas <robin.chalas@gmail.com>
  */
 interface FirewallListenerInterface
 {
+    /**
+     * Tells whether the authenticate() method should be called or not depending on the incoming request.
+     *
+     * Returning null means authenticate() can be called lazily when accessing the token storage.
+     */
+    public function supports(Request $request): ?bool;
+
+    /**
+     * Does whatever is required to authenticate the request, typically calling $event->setResponse() internally.
+     */
+    public function authenticate(RequestEvent $event);
+
     /**
      * Defines the priority of the listener.
      * The higher the number, the earlier a listener is executed.