feature symfony#57369 [Security] Display authenticators in the profil…

…er even if they are all skipped (MatTheCat) This PR was squashed before being merged into the 7.2 branch. Discussion ---------- [Security] Display authenticators in the profiler even if they are all skipped | Q | A | ------------- | --- | Branch? | 7.2 | Bug fix? | no | New feature? | yes | Deprecations? | no | Issues | N/A | License | MIT Currently if no authenticator supports the request, the `_security_skipped_authenticators` request attribute is not set which (among others) results in an empty list in the profiler’s security panel’s authenticators tab: ![](https://github.com/symfony/symfony/assets/1898254/37f63661-ad21-4945-b05a-396f4781b88f) It makes more sense to display them all as not supported: ![](https://github.com/symfony/symfony/assets/1898254/ca7241c8-92f1-47f7-bdea-96fce6daf910) Commits ------- 06f7876 [Security] Display authenticators in the profiler even if they are all skipped
fabpot · Jun 25, 2024 · cd255c3 · cd255c3
2 parents def7383 + 06f7876
commit cd255c3
Show file tree

Hide file tree

Showing 8 changed files with 64 additions and 45 deletions.
diff --git a/src/Symfony/Bundle/SecurityBundle/Command/DebugFirewallCommand.php b/src/Symfony/Bundle/SecurityBundle/Command/DebugFirewallCommand.php
@@ -25,6 +25,7 @@
 use Symfony\Component\Console\Style\SymfonyStyle;
 use Symfony\Component\EventDispatcher\EventDispatcherInterface;
 use Symfony\Component\Security\Http\Authenticator\AuthenticatorInterface;
+use Symfony\Component\Security\Http\Authenticator\Debug\TraceableAuthenticator;
 
 /**
  * @author Timo Bakx <[email protected]>
@@ -210,7 +211,7 @@ private function displayAuthenticators(string $name, SymfonyStyle $io): void
         $io->table(
             ['Classname'],
             array_map(
-                fn ($authenticator) => [$authenticator::class],
+                fn ($authenticator) => [($authenticator instanceof TraceableAuthenticator ? $authenticator->getAuthenticator() : $authenticator)::class],
                 $authenticators
             )
         );

diff --git a/src/Symfony/Bundle/SecurityBundle/DependencyInjection/SecurityExtension.php b/src/Symfony/Bundle/SecurityBundle/DependencyInjection/SecurityExtension.php
@@ -58,6 +58,7 @@
 use Symfony\Component\Security\Core\User\ChainUserProvider;
 use Symfony\Component\Security\Core\User\UserCheckerInterface;
 use Symfony\Component\Security\Core\User\UserProviderInterface;
+use Symfony\Component\Security\Http\Authenticator\Debug\TraceableAuthenticator;
 use Symfony\Component\Security\Http\Authenticator\Debug\TraceableAuthenticatorManagerListener;
 use Symfony\Component\Security\Http\Event\CheckPassportEvent;
 use Symfony\Flex\Command\InstallRecipesCommand;
@@ -638,6 +639,15 @@ private function createAuthenticationListeners(ContainerBuilder $container, stri
             }
         }
 
+        if ($container->hasDefinition('debug.security.firewall')) {
+            foreach ($authenticationProviders as $authenticatorId) {
+                $container->register('debug.'.$authenticatorId, TraceableAuthenticator::class)
+                    ->setDecoratedService($authenticatorId)
+                    ->setArguments([new Reference('debug.'.$authenticatorId.'.inner')])
+                ;
+            }
+        }
+
         // the actual entry point is configured by the RegisterEntryPointPass
         $container->setParameter('security.'.$id.'._indexed_authenticators', $entryPoints);
 

diff --git a/src/Symfony/Bundle/SecurityBundle/Resources/views/Collector/security.html.twig b/src/Symfony/Bundle/SecurityBundle/Resources/views/Collector/security.html.twig
@@ -360,7 +360,7 @@
 
                                 <tr>
                                     <td class="font-normal">{{ profiler_dump(authenticator.stub) }}</td>
-                                    <td class="no-wrap">{{ source('@WebProfiler/Icon/' ~ (authenticator.supports ? 'yes' : 'no') ~ '.svg') }}</td>
+                                    <td class="no-wrap">{{ source('@WebProfiler/Icon/' ~ (authenticator.supports is same as (false) ? 'no' : 'yes') ~ '.svg') }}</td>
                                     <td class="no-wrap">{{ authenticator.authenticated is not null ? source('@WebProfiler/Icon/' ~ (authenticator.authenticated ? 'yes' : 'no') ~ '.svg') : '' }}</td>
                                     <td class="no-wrap">{{ authenticator.duration is null ? '(none)' : '%0.2f ms'|format(authenticator.duration * 1000) }}</td>
                                     <td class="font-normal">{{ authenticator.passport ? profiler_dump(authenticator.passport) : '(none)' }}</td>

diff --git a/src/Symfony/Bundle/SecurityBundle/Tests/Debug/TraceableFirewallListenerTest.php b/src/Symfony/Bundle/SecurityBundle/Tests/Debug/TraceableFirewallListenerTest.php
@@ -22,6 +22,7 @@
 use Symfony\Component\Security\Core\Authentication\Token\Storage\TokenStorageInterface;
 use Symfony\Component\Security\Core\Authentication\Token\TokenInterface;
 use Symfony\Component\Security\Http\Authentication\AuthenticatorManager;
+use Symfony\Component\Security\Http\Authenticator\Debug\TraceableAuthenticator;
 use Symfony\Component\Security\Http\Authenticator\Debug\TraceableAuthenticatorManagerListener;
 use Symfony\Component\Security\Http\Authenticator\InteractiveAuthenticatorInterface;
 use Symfony\Component\Security\Http\Authenticator\Passport\Badge\UserBadge;
@@ -99,7 +100,7 @@ public function testOnKernelRequestRecordsAuthenticatorsInfo()
         $tokenStorage = $this->createMock(TokenStorageInterface::class);
         $dispatcher = new EventDispatcher();
         $authenticatorManager = new AuthenticatorManager(
-            [$notSupportingAuthenticator, $supportingAuthenticator],
+            [new TraceableAuthenticator($notSupportingAuthenticator), new TraceableAuthenticator($supportingAuthenticator)],
             $tokenStorage,
             $dispatcher,
             'main'

diff --git a/src/Symfony/Component/Security/Http/Authentication/AuthenticatorManager.php b/src/Symfony/Component/Security/Http/Authentication/AuthenticatorManager.php
@@ -108,13 +108,13 @@ public function supports(Request $request): ?bool
             }
         }
 
+        $request->attributes->set('_security_skipped_authenticators', $skippedAuthenticators);
+        $request->attributes->set('_security_authenticators', $authenticators);
+
         if (!$authenticators) {
             return false;
         }
 
-        $request->attributes->set('_security_authenticators', $authenticators);
-        $request->attributes->set('_security_skipped_authenticators', $skippedAuthenticators);
-
         return $lazy ? null : true;
     }
 

diff --git a/src/Symfony/Component/Security/Http/Authenticator/Debug/TraceableAuthenticator.php b/src/Symfony/Component/Security/Http/Authenticator/Debug/TraceableAuthenticator.php
@@ -30,6 +30,7 @@
  */
 final class TraceableAuthenticator implements AuthenticatorInterface, InteractiveAuthenticatorInterface, AuthenticationEntryPointInterface
 {
+    private ?bool $supports = false;
     private ?Passport $passport = null;
     private ?float $duration = null;
     private ClassStub|string $stub;
@@ -42,7 +43,7 @@ public function __construct(private AuthenticatorInterface $authenticator)
     public function getInfo(): array
     {
         return [
-            'supports' => true,
+            'supports' => $this->supports,
             'passport' => $this->passport,
             'duration' => $this->duration,
             'stub' => $this->stub ??= class_exists(ClassStub::class) ? new ClassStub($this->authenticator::class) : $this->authenticator::class,
@@ -61,14 +62,17 @@ static function (BadgeInterface $badge): array {
 
     public function supports(Request $request): ?bool
     {
-        return $this->authenticator->supports($request);
+        return $this->supports = $this->authenticator->supports($request);
     }
 
     public function authenticate(Request $request): Passport
     {
         $startTime = microtime(true);
-        $this->passport = $this->authenticator->authenticate($request);
-        $this->duration = microtime(true) - $startTime;
+        try {
+            $this->passport = $this->authenticator->authenticate($request);
+        } finally {
+            $this->duration = microtime(true) - $startTime;
+        }
 
         return $this->passport;
     }

diff --git a/...ony/Component/Security/Http/Authenticator/Debug/TraceableAuthenticatorManagerListener.php b/...ony/Component/Security/Http/Authenticator/Debug/TraceableAuthenticatorManagerListener.php
@@ -15,7 +15,6 @@
 use Symfony\Component\HttpKernel\Event\RequestEvent;
 use Symfony\Component\Security\Http\Firewall\AbstractListener;
 use Symfony\Component\Security\Http\Firewall\AuthenticatorManagerListener;
-use Symfony\Component\VarDumper\Caster\ClassStub;
 use Symfony\Contracts\Service\ResetInterface;
 
 /**
@@ -25,50 +24,38 @@
  */
 final class TraceableAuthenticatorManagerListener extends AbstractListener implements ResetInterface
 {
-    private array $authenticatorsInfo = [];
-    private bool $hasVardumper;
+    private array $authenticators = [];
 
-    public function __construct(
-        private AuthenticatorManagerListener $authenticationManagerListener,
-    ) {
-        $this->hasVardumper = class_exists(ClassStub::class);
-    }
-
-    public function supports(Request $request): ?bool
+    public function __construct(private AuthenticatorManagerListener $authenticationManagerListener)
     {
-        return $this->authenticationManagerListener->supports($request);
     }
 
-    public function authenticate(RequestEvent $event): void
+    public function supports(Request $request): ?bool
     {
-        $request = $event->getRequest();
-
-        if (!$authenticators = $request->attributes->get('_security_authenticators')) {
-            return;
-        }
+        $supports = $this->authenticationManagerListener->supports($request);
 
-        foreach ($request->attributes->get('_security_skipped_authenticators') as $skippedAuthenticator) {
-            $this->authenticatorsInfo[] = [
-                'supports' => false,
-                'stub' => $this->hasVardumper ? new ClassStub($skippedAuthenticator::class) : $skippedAuthenticator::class,
-                'passport' => null,
-                'duration' => 0,
-                'authenticated' => null,
-                'badges' => [],
-            ];
+        foreach($request->attributes->get('_security_skipped_authenticators') as $authenticator) {
+            $this->authenticators[] = $authenticator instanceof TraceableAuthenticator
+                ? $authenticator
+                : new TraceableAuthenticator($authenticator)
+            ;
         }
 
-        foreach ($authenticators as $key => $authenticator) {
-            $authenticators[$key] = new TraceableAuthenticator($authenticator);
+        $supportedAuthenticators = [];
+        foreach ($request->attributes->get('_security_authenticators') as $authenticator) {
+            $this->authenticators[] = $supportedAuthenticators[] = $authenticator instanceof TraceableAuthenticator
+                ? $authenticator :
+                new TraceableAuthenticator($authenticator)
+            ;
         }
+        $request->attributes->set('_security_authenticators', $supportedAuthenticators);
 
-        $request->attributes->set('_security_authenticators', $authenticators);
+        return $supports;
+    }
 
+    public function authenticate(RequestEvent $event): void
+    {
         $this->authenticationManagerListener->authenticate($event);
-
-        foreach ($authenticators as $authenticator) {
-            $this->authenticatorsInfo[] = $authenticator->getInfo();
-        }
     }
 
     public function getAuthenticatorManagerListener(): AuthenticatorManagerListener
@@ -78,11 +65,14 @@ public function getAuthenticatorManagerListener(): AuthenticatorManagerListener
 
     public function getAuthenticatorsInfo(): array
     {
-        return $this->authenticatorsInfo;
+        return array_map(
+            static fn (TraceableAuthenticator $authenticator) => $authenticator->getInfo(),
+            $this->authenticators
+        );
     }
 
     public function reset(): void
     {
-        $this->authenticatorsInfo = [];
+        $this->authenticators = [];
     }
 }
diff --git a/src/Symfony/Component/Security/Http/Tests/Authenticator/Debug/TraceableAuthenticatorTest.php b/src/Symfony/Component/Security/Http/Tests/Authenticator/Debug/TraceableAuthenticatorTest.php
@@ -26,22 +26,35 @@ public function testGetInfo()
         $passport = new SelfValidatingPassport(new UserBadge('robin', function () {}));
 
         $authenticator = $this->createMock(AuthenticatorInterface::class);
+        $authenticator->expects($this->once())
+            ->method('supports')
+            ->with($request)
+            ->willReturn(true);
+
         $authenticator
             ->expects($this->once())
             ->method('authenticate')
             ->with($request)
             ->willReturn($passport);
 
         $traceable = new TraceableAuthenticator($authenticator);
+        $this->assertTrue($traceable->supports($request));
         $this->assertSame($passport, $traceable->authenticate($request));
         $this->assertSame($passport, $traceable->getInfo()['passport']);
     }
 
     public function testGetInfoWithoutAuth()
     {
+        $request = new Request();
+
         $authenticator = $this->createMock(AuthenticatorInterface::class);
+        $authenticator->expects($this->once())
+            ->method('supports')
+            ->with($request)
+            ->willReturn(false);
 
         $traceable = new TraceableAuthenticator($authenticator);
+        $this->assertFalse($traceable->supports($request));
         $this->assertNull($traceable->getInfo()['passport']);
         $this->assertIsArray($traceable->getInfo()['badges']);
         $this->assertSame([], $traceable->getInfo()['badges']);