From 06f78764b116fa8fa1b56c14171e278cd1808ce5 Mon Sep 17 00:00:00 2001 From: MatTheCat Date: Wed, 19 Jun 2024 18:59:55 +0200 Subject: [PATCH] [Security] Display authenticators in the profiler even if they are all skipped --- .../Command/DebugFirewallCommand.php | 3 +- .../DependencyInjection/SecurityExtension.php | 10 ++++ .../views/Collector/security.html.twig | 2 +- .../Debug/TraceableFirewallListenerTest.php | 3 +- .../Authentication/AuthenticatorManager.php | 6 +- .../Debug/TraceableAuthenticator.php | 12 ++-- .../TraceableAuthenticatorManagerListener.php | 60 ++++++++----------- .../Debug/TraceableAuthenticatorTest.php | 13 ++++ 8 files changed, 64 insertions(+), 45 deletions(-) diff --git a/src/Symfony/Bundle/SecurityBundle/Command/DebugFirewallCommand.php b/src/Symfony/Bundle/SecurityBundle/Command/DebugFirewallCommand.php index 3f8b088f9c776..e5994510da126 100644 --- 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 @@ -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 index 2fd979872ed07..d6f658391d68c 100644 --- 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 index 4fe1140eadfda..2715ed6a85d11 100644 --- 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 @@ {{ profiler_dump(authenticator.stub) }} - {{ source('@WebProfiler/Icon/' ~ (authenticator.supports ? 'yes' : 'no') ~ '.svg') }} + {{ source('@WebProfiler/Icon/' ~ (authenticator.supports is same as (false) ? 'no' : 'yes') ~ '.svg') }} {{ authenticator.authenticated is not null ? source('@WebProfiler/Icon/' ~ (authenticator.authenticated ? 'yes' : 'no') ~ '.svg') : '' }} {{ authenticator.duration is null ? '(none)' : '%0.2f ms'|format(authenticator.duration * 1000) }} {{ authenticator.passport ? profiler_dump(authenticator.passport) : '(none)' }} diff --git a/src/Symfony/Bundle/SecurityBundle/Tests/Debug/TraceableFirewallListenerTest.php b/src/Symfony/Bundle/SecurityBundle/Tests/Debug/TraceableFirewallListenerTest.php index 6dad1f3a72913..cdf53c2007756 100644 --- 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 index 791cece0ca08a..aafacafc23f47 100644 --- 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 index 34c3c62f68848..7d1f105ce07a8 100644 --- 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/src/Symfony/Component/Security/Http/Authenticator/Debug/TraceableAuthenticatorManagerListener.php b/src/Symfony/Component/Security/Http/Authenticator/Debug/TraceableAuthenticatorManagerListener.php index 73ff7347cb58a..6960d13373479 100644 --- a/src/Symfony/Component/Security/Http/Authenticator/Debug/TraceableAuthenticatorManagerListener.php +++ b/src/Symfony/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 index 67afed44aecfc..7b3c4c09b8d82 100644 --- a/src/Symfony/Component/Security/Http/Tests/Authenticator/Debug/TraceableAuthenticatorTest.php +++ b/src/Symfony/Component/Security/Http/Tests/Authenticator/Debug/TraceableAuthenticatorTest.php @@ -26,6 +26,11 @@ 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') @@ -33,15 +38,23 @@ public function testGetInfo() ->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']);