Multi step authentication

src/Symfony/Bundle/SecurityBundle/DependencyInjection/Security/Factory/FirewallListenerFactoryInterface.php

@@ -0,0 +1,15 @@
+<?php
+
+namespace Symfony\Bundle\SecurityBundle\DependencyInjection\Security\Factory;
+
+use Symfony\Component\DependencyInjection\ContainerBuilder;
+
+interface FirewallListenerFactoryInterface
+{
+    /**
+     * Creates the firewall listener service(s) for the provided configuration.
+     *
+     * @return string|string[] The listener service ID(s) to be used by the firewall
+     */
+    public function createListeners(ContainerBuilder $container, string $firewallName, array $config);
+}

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

@@ -13,6 +13,7 @@
 
 use Symfony\Bundle\SecurityBundle\DependencyInjection\Security\Factory\AuthenticatorFactoryInterface;
 use Symfony\Bundle\SecurityBundle\DependencyInjection\Security\Factory\EntryPointFactoryInterface;
+use Symfony\Bundle\SecurityBundle\DependencyInjection\Security\Factory\FirewallListenerFactoryInterface;
 use Symfony\Bundle\SecurityBundle\DependencyInjection\Security\Factory\RememberMeFactory;
 use Symfony\Bundle\SecurityBundle\DependencyInjection\Security\Factory\SecurityFactoryInterface;
 use Symfony\Bundle\SecurityBundle\DependencyInjection\Security\UserProvider\UserProviderFactoryInterface;
@@ -568,6 +569,18 @@ private function createAuthenticationListeners(ContainerBuilder $container, stri
                         $listeners[] = new Reference($listenerId);
                         $authenticationProviders[] = $provider;
                     }
+
+                    if ($factory instanceof FirewallListenerFactoryInterface) {
+                        $firewallListener = $factory->createListeners($container, $id, $firewall[$key]);
+                        if (\is_array($firewallListener)) {
+                            foreach ($firewallListener as $listenerId) {
+                                $listeners[] = new Reference($listenerId);
+                            }
+                        } else {
+                            $listeners[] = new Reference($firewallListener);
+                        }
+                    }
+
                     $hasListeners = true;
                 }
             }

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

@@ -171,6 +171,22 @@ private function executeAuthenticator(AuthenticatorInterface $authenticator, Req
                 $authenticatedToken->eraseCredentials();
             }
 
+            // Since the security token is carrying the state of authentication around between request, for 2fa I'm
+            // making the system to carry a TwoFactorToken around, which has some specific characteristics.
+            // The main one are, it's signalizes to the system:
+            // 1) The authentication is not complete yes IS_AUTHENTICATED_FULLY == false
+            // 2) It actives the 2fa-specific logic, like redirecting to the 2fa form
+
+            // $authenticatedToken is coming from the authenticator, which initially identifies the user (e.g. username+
+            // password). Here at this point I have to hook-in to make multi-step authentication work.
+            // I need to exchange $authenticatedToken (which already exposes all roles/privileges) with a TwoFactorToken
+            // to put authentication into that intermediate state, which is neither anonymous, nor fully authenticated.
+            // I'm doing this by decorating all authenticators, so that I can "magically" exchange $authenticatedToken
+            // before it reaches these lines of code here and becomes visible to the system in the dispatched event
+            // below. But this is really hacky. I'd like to have a better way to hook-in after the authenticator,
+            // "raise a veto" and change the token before it becomes effective in the system.
+
+            // Btw. this if statement is unnecessary, $this->eventDispatcher is always !== null ;)
             if (null !== $this->eventDispatcher) {
                 $this->eventDispatcher->dispatch(new AuthenticationSuccessEvent($authenticatedToken), AuthenticationEvents::AUTHENTICATION_SUCCESS);
             }

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

@@ -15,7 +15,7 @@
 use Symfony\Component\HttpKernel\Event\FinishRequestEvent;
 use Symfony\Component\HttpKernel\Event\RequestEvent;
 use Symfony\Component\HttpKernel\KernelEvents;
-use Symfony\Component\Security\Http\Firewall\AccessListener;
+use Symfony\Component\Security\Http\Firewall\FirewallListenerInterface;
 use Symfony\Contracts\EventDispatcher\EventDispatcherInterface;
 
 /**
@@ -59,29 +59,19 @@ public function onKernelRequest(RequestEvent $event)
             $exceptionListener->register($this->dispatcher);
         }
 
-        $authenticationListeners = function () use ($authenticationListeners, $logoutListener) {
-            $accessListener = null;
-
-            foreach ($authenticationListeners as $listener) {
-                if ($listener instanceof AccessListener) {
-                    $accessListener = $listener;
-
-                    continue;
-                }
-
-                yield $listener;
-            }
+        $firewallListeners = $authenticationListeners;
+        if (null !== $logoutListener) {
+            $firewallListeners[] = $logoutListener;
+        }
 
-            if (null !== $logoutListener) {
-                yield $logoutListener;
-            }
+        uasort($firewallListeners, function ($a, $b) {
+            $aPriority = $a instanceof FirewallListenerInterface ? $a->getPriority() : 0;
+            $bPriority = $b instanceof FirewallListenerInterface ? $b->getPriority() : 0;
 
-            if (null !== $accessListener) {
-                yield $accessListener;
-            }
-        };
+            return $aPriority <=> $bPriority;
+        });
 
-        $this->callListeners($event, $authenticationListeners());
+        $this->callListeners($event, $firewallListeners);
     }
 
     public function onKernelFinishRequest(FinishRequestEvent $event)

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

@@ -19,7 +19,7 @@
  *
  * @author Nicolas Grekas <p@tchwork.com>
  */
-abstract class AbstractListener
+abstract class AbstractListener implements FirewallListenerInterface
 {
     final public function __invoke(RequestEvent $event)
     {
@@ -39,4 +39,9 @@ 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 function getPriority(): int
+    {
+        return 0; // Default
+    }
 }

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

@@ -120,4 +120,9 @@ private function createAccessDeniedException(Request $request, array $attributes
 
         return $exception;
     }
+
+    public function getPriority(): int
+    {
+        return -255;
+    }
 }

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

@@ -0,0 +1,14 @@
+<?php
+
+namespace Symfony\Component\Security\Http\Firewall;
+
+interface FirewallListenerInterface
+{
+    /**
+     * Defines the priority of the listener.
+     * The higher the number, the earlier a listener is executed.
+     *
+     * @return int
+     */
+    public function getPriority(): int;
+}

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

@@ -142,4 +142,9 @@ protected function requiresLogout(Request $request): bool
     {
         return isset($this->options['logout_path']) && $this->httpUtils->checkRequestPath($request, $this->options['logout_path']);
     }
+
+    public function getPriority(): int
+    {
+        return -254;
+    }
 }

src/Symfony/Component/Security/Http/RememberMe/AbstractRememberMeServices.php

@@ -182,6 +182,12 @@ final public function loginSuccess(Request $request, Response $response, TokenIn
         // Make sure any old remember-me cookies are cancelled
         $this->cancelCookie($request);
 
+        // I'd suggest to extend the conditions under which the security system allows a remember-me cookie to be set.
+        // This class should check \Symfony\Component\Security\Core\Authentication\AuthenticationTrustResolverInterface::isFullFledged
+        // Why? 1st) because it makes sense to have that requirement
+        // 2nd) in a multi-step authentication process it would help to prevent the remember-me cookie from being set
+        // Right now I have to do "hacks" to prevent the cookie being set, because AbstractRememberMeServices implicitly
+        // assumes that when it's called it's secure to set a remember-me cookie.
         if (!$token->getUser() instanceof UserInterface) {
             if (null !== $this->logger) {
                 $this->logger->debug('Remember-me ignores token since it does not contain a UserInterface implementation.');
@@ -190,6 +196,12 @@ final public function loginSuccess(Request $request, Response $response, TokenIn
             return;
         }
 
+        // Having this here makes it unnecessary hard to set a remember-me cookie after 2fa, because this requires
+        // the parameter to be present in the request. There's no way to reach the logic for setting the cookie without
+        // fulfilling this requirement.
+        // Imo the logic for settings the cookie (everything that's implemented in the sub-classes) should be decoupled
+        // from the conditions under which the cookie can be set (everything here). Effectively making the sub-classes
+        // separate classes and a dependency, instead of inheritance.
         if (!$this->isRememberMeRequested($request)) {
             if (null !== $this->logger) {
                 $this->logger->debug('Remember-me was not requested.');