[Security/Http] call auth listeners/guards eagerly when they "support" the request

Author: nicolas-grekas

CHANGELOG.md

@@ -14,6 +14,7 @@ CHANGELOG
  * Deprecated returning a non-boolean value when implementing `Guard\AuthenticatorInterface::checkCredentials()`.
  * Deprecated passing more than one attribute to `AccessDecisionManager::decide()` and `AuthorizationChecker::isGranted()`
  * Added new `argon2id` encoder, undeprecated the `bcrypt` and `argon2i` ones (using `auto` is still recommended by default.)
+ * Added `AbstractListener` which replaces the deprecated `ListenerInterface`
 
 4.3.0
 -----

Guard/Firewall/GuardAuthenticationListener.php

@@ -21,6 +21,7 @@
 use Symfony\Component\Security\Guard\AuthenticatorInterface;
 use Symfony\Component\Security\Guard\GuardAuthenticatorHandler;
 use Symfony\Component\Security\Guard\Token\PreAuthenticationGuardToken;
+use Symfony\Component\Security\Http\Firewall\AbstractListener;
 use Symfony\Component\Security\Http\Firewall\LegacyListenerTrait;
 use Symfony\Component\Security\Http\Firewall\ListenerInterface;
 use Symfony\Component\Security\Http\RememberMe\RememberMeServicesInterface;
@@ -33,7 +34,7 @@
  *
  * @final since Symfony 4.3
  */
-class GuardAuthenticationListener implements ListenerInterface
+class GuardAuthenticationListener extends AbstractListener implements ListenerInterface
 {
     use LegacyListenerTrait;
 
@@ -62,9 +63,9 @@ public function __construct(GuardAuthenticatorHandler $guardHandler, Authenticat
     }
 
     /**
-     * Iterates over each authenticator to see if each wants to authenticate the request.
+     * {@inheritdoc}
      */
-    public function __invoke(RequestEvent $event)
+    public function supports(Request $request): ?bool
     {
         if (null !== $this->logger) {
             $context = ['firewall_key' => $this->providerKey];
@@ -76,7 +77,39 @@ public function __invoke(RequestEvent $event)
             $this->logger->debug('Checking for guard authentication credentials.', $context);
         }
 
+        $guardAuthenticators = [];
+
         foreach ($this->guardAuthenticators as $key => $guardAuthenticator) {
+            if (null !== $this->logger) {
+                $this->logger->debug('Checking support on guard authenticator.', ['firewall_key' => $this->providerKey, 'authenticator' => \get_class($guardAuthenticator)]);
+            }
+
+            if ($guardAuthenticator->supports($request)) {
+                $guardAuthenticators[$key] = $guardAuthenticator;
+            } elseif (null !== $this->logger) {
+                $this->logger->debug('Guard authenticator does not support the request.', ['firewall_key' => $this->providerKey, 'authenticator' => \get_class($guardAuthenticator)]);
+            }
+        }
+
+        if (!$guardAuthenticators) {
+            return false;
+        }
+
+        $request->attributes->set('_guard_authenticators', $guardAuthenticators);
+
+        return true;
+    }
+
+    /**
+     * Iterates over each authenticator to see if each wants to authenticate the request.
+     */
+    public function authenticate(RequestEvent $event)
+    {
+        $request = $event->getRequest();
+        $guardAuthenticators = $request->attributes->get('_guard_authenticators');
+        $request->attributes->remove('_guard_authenticators');
+
+        foreach ($guardAuthenticators as $key => $guardAuthenticator) {
             // get a key that's unique to *this* guard authenticator
             // this MUST be the same as GuardAuthenticationProvider
             $uniqueGuardKey = $this->providerKey.'_'.$key;
@@ -97,19 +130,6 @@ private function executeGuardAuthenticator(string $uniqueGuardKey, Authenticator
     {
         $request = $event->getRequest();
         try {
-            if (null !== $this->logger) {
-                $this->logger->debug('Checking support on guard authenticator.', ['firewall_key' => $this->providerKey, 'authenticator' => \get_class($guardAuthenticator)]);
-            }
-
-            // abort the execution of the authenticator if it doesn't support the request
-            if (!$guardAuthenticator->supports($request)) {
-                if (null !== $this->logger) {
-                    $this->logger->debug('Guard authenticator does not support the request.', ['firewall_key' => $this->providerKey, 'authenticator' => \get_class($guardAuthenticator)]);
-                }
-
-                return;
-            }
-
             if (null !== $this->logger) {
                 $this->logger->debug('Calling getCredentials() on guard authenticator.', ['firewall_key' => $this->providerKey, 'authenticator' => \get_class($guardAuthenticator)]);
             }

Guard/composer.json

@@ -18,7 +18,7 @@
     "require": {
         "php": "^7.1.3",
         "symfony/security-core": "^3.4.22|^4.2.3|^5.0",
-        "symfony/security-http": "^4.3"
+        "symfony/security-http": "^4.4.1"
     },
     "require-dev": {
         "psr/log": "~1.0"

Http/Firewall.php

@@ -138,7 +138,7 @@ protected function handleRequest(GetResponseEvent $event, $listeners)
             if (\is_callable($listener)) {
                 $listener($event);
             } else {
-                @trigger_error(sprintf('Calling the "%s::handle()" method from the firewall is deprecated since Symfony 4.3, implement "__invoke()" instead.', \get_class($listener)), E_USER_DEPRECATED);
+                @trigger_error(sprintf('Calling the "%s::handle()" method from the firewall is deprecated since Symfony 4.3, extend "%s" instead.', \get_class($listener), AbstractListener::class), E_USER_DEPRECATED);
                 $listener->handle($event);
             }
 

Http/Firewall/AbstractAuthenticationListener.php

@@ -49,7 +49,7 @@
  * @author Fabien Potencier <fabien@symfony.com>
  * @author Johannes M. Schmitt <schmittjoh@gmail.com>
  */
-abstract class AbstractAuthenticationListener implements ListenerInterface
+abstract class AbstractAuthenticationListener extends AbstractListener implements ListenerInterface
 {
     use LegacyListenerTrait;
 
@@ -105,20 +105,24 @@ public function setRememberMeServices(RememberMeServicesInterface $rememberMeSer
         $this->rememberMeServices = $rememberMeServices;
     }
 
+    /**
+     * {@inheritdoc}
+     */
+    public function supports(Request $request): ?bool
+    {
+        return $this->requiresAuthentication($request);
+    }
+
     /**
      * Handles form based authentication.
      *
      * @throws \RuntimeException
      * @throws SessionUnavailableException
      */
-    public function __invoke(RequestEvent $event)
+    public function authenticate(RequestEvent $event)
     {
         $request = $event->getRequest();
 
-        if (!$this->requiresAuthentication($request)) {
-            return;
-        }
-
         if (!$request->hasSession()) {
             throw new \RuntimeException('This authentication method requires a session.');
         }

Http/Firewall/AbstractListener.php

@@ -0,0 +1,42 @@
+<?php
+
+/*
+ * This file is part of the Symfony package.
+ *
+ * (c) Fabien Potencier <fabien@symfony.com>
+ *
+ * For the full copyright and license information, please view the LICENSE
+ * file that was distributed with this source code.
+ */
+
+namespace Symfony\Component\Security\Http\Firewall;
+
+use Symfony\Component\HttpFoundation\Request;
+use Symfony\Component\HttpKernel\Event\RequestEvent;
+
+/**
+ * A base class for listeners that can tell whether they should authenticate incoming requests.
+ *
+ * @author Nicolas Grekas <p@tchwork.com>
+ */
+abstract class AbstractListener
+{
+    final public function __invoke(RequestEvent $event)
+    {
+        if (false !== $this->supports($event->getRequest())) {
+            $this->authenticate($event);
+        }
+    }
+
+    /**
+     * 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);
+}

Http/Firewall/AbstractPreAuthenticatedListener.php

@@ -35,7 +35,7 @@
  *
  * @internal since Symfony 4.3
  */
-abstract class AbstractPreAuthenticatedListener implements ListenerInterface
+abstract class AbstractPreAuthenticatedListener extends AbstractListener implements ListenerInterface
 {
     use LegacyListenerTrait;
 
@@ -56,20 +56,31 @@ public function __construct(TokenStorageInterface $tokenStorage, AuthenticationM
     }
 
     /**
-     * Handles pre-authentication.
+     * {@inheritdoc}
      */
-    public function __invoke(RequestEvent $event)
+    public function supports(Request $request): ?bool
     {
-        $request = $event->getRequest();
-
         try {
-            list($user, $credentials) = $this->getPreAuthenticatedData($request);
+            $request->attributes->set('_pre_authenticated_data', $this->getPreAuthenticatedData($request));
         } catch (BadCredentialsException $e) {
             $this->clearToken($e);
 
-            return;
+            return false;
         }
 
+        return true;
+    }
+
+    /**
+     * Handles pre-authentication.
+     */
+    public function authenticate(RequestEvent $event)
+    {
+        $request = $event->getRequest();
+
+        [$user, $credentials] = $request->attributes->get('_pre_authenticated_data');
+        $request->attributes->remove('_pre_authenticated_data');
+
         if (null !== $this->logger) {
             $this->logger->debug('Checking current security token.', ['token' => (string) $this->tokenStorage->getToken()]);
         }

Http/Firewall/AccessListener.php

@@ -11,10 +11,12 @@
 
 namespace Symfony\Component\Security\Http\Firewall;
 
+use Symfony\Component\HttpFoundation\Request;
 use Symfony\Component\HttpKernel\Event\RequestEvent;
 use Symfony\Component\Security\Core\Authentication\AuthenticationManagerInterface;
 use Symfony\Component\Security\Core\Authentication\Token\Storage\TokenStorageInterface;
 use Symfony\Component\Security\Core\Authorization\AccessDecisionManagerInterface;
+use Symfony\Component\Security\Core\Authorization\Voter\AuthenticatedVoter;
 use Symfony\Component\Security\Core\Exception\AccessDeniedException;
 use Symfony\Component\Security\Core\Exception\AuthenticationCredentialsNotFoundException;
 use Symfony\Component\Security\Http\AccessMapInterface;
@@ -27,7 +29,7 @@
  *
  * @final since Symfony 4.3
  */
-class AccessListener implements ListenerInterface
+class AccessListener extends AbstractListener implements ListenerInterface
 {
     use LegacyListenerTrait;
 
@@ -44,23 +46,35 @@ public function __construct(TokenStorageInterface $tokenStorage, AccessDecisionM
         $this->authManager = $authManager;
     }
 
+    /**
+     * {@inheritdoc}
+     */
+    public function supports(Request $request): ?bool
+    {
+        [$attributes] = $this->map->getPatterns($request);
+        $request->attributes->set('_access_control_attributes', $attributes);
+
+        return $attributes && [AuthenticatedVoter::IS_AUTHENTICATED_ANONYMOUSLY] !== $attributes ? true : null;
+    }
+
     /**
      * Handles access authorization.
      *
      * @throws AccessDeniedException
      * @throws AuthenticationCredentialsNotFoundException
      */
-    public function __invoke(RequestEvent $event)
+    public function authenticate(RequestEvent $event)
     {
         if (!$event instanceof LazyResponseEvent && null === $token = $this->tokenStorage->getToken()) {
             throw new AuthenticationCredentialsNotFoundException('A Token was not found in the TokenStorage.');
         }
 
         $request = $event->getRequest();
 
-        list($attributes) = $this->map->getPatterns($request);
+        $attributes = $request->attributes->get('_access_control_attributes');
+        $request->attributes->remove('_access_control_attributes');
 
-        if (!$attributes) {
+        if (!$attributes || ([AuthenticatedVoter::IS_AUTHENTICATED_ANONYMOUSLY] === $attributes && $event instanceof LazyResponseEvent)) {
             return;
         }
 

Http/Firewall/AnonymousAuthenticationListener.php

@@ -12,6 +12,7 @@
 namespace Symfony\Component\Security\Http\Firewall;
 
 use Psr\Log\LoggerInterface;
+use Symfony\Component\HttpFoundation\Request;
 use Symfony\Component\HttpKernel\Event\RequestEvent;
 use Symfony\Component\Security\Core\Authentication\AuthenticationManagerInterface;
 use Symfony\Component\Security\Core\Authentication\Token\AnonymousToken;
@@ -26,7 +27,7 @@
  *
  * @final since Symfony 4.3
  */
-class AnonymousAuthenticationListener implements ListenerInterface
+class AnonymousAuthenticationListener extends AbstractListener implements ListenerInterface
 {
     use LegacyListenerTrait;
 
@@ -43,10 +44,18 @@ public function __construct(TokenStorageInterface $tokenStorage, string $secret,
         $this->logger = $logger;
     }
 
+    /**
+     * {@inheritdoc}
+     */
+    public function supports(Request $request): ?bool
+    {
+        return null; // always run authenticate() lazily with lazy firewalls
+    }
+
     /**
      * Handles anonymous authentication.
      */
-    public function __invoke(RequestEvent $event)
+    public function authenticate(RequestEvent $event)
     {
         if (null !== $this->tokenStorage->getToken()) {
             return;

Http/Firewall/BasicAuthenticationListener.php

@@ -29,7 +29,7 @@
  *
  * @final since Symfony 4.3
  */
-class BasicAuthenticationListener implements ListenerInterface
+class BasicAuthenticationListener extends AbstractListener implements ListenerInterface
 {
     use LegacyListenerTrait;
 
@@ -55,10 +55,18 @@ public function __construct(TokenStorageInterface $tokenStorage, AuthenticationM
         $this->ignoreFailure = false;
     }
 
+    /**
+     * {@inheritdoc}
+     */
+    public function supports(Request $request): ?bool
+    {
+        return null !== $request->headers->get('PHP_AUTH_USER');
+    }
+
     /**
      * Handles basic authentication.
      */
-    public function __invoke(RequestEvent $event)
+    public function authenticate(RequestEvent $event)
     {
         $request = $event->getRequest();