diff --git a/UPGRADE-7.3.md b/UPGRADE-7.3.md index 0a11e920cca18..95dd40d8bf835 100644 --- a/UPGRADE-7.3.md +++ b/UPGRADE-7.3.md @@ -39,6 +39,12 @@ FrameworkBundle because its default value will change in version 8.0 * Deprecate the `--show-arguments` option of the `container:debug` command, as arguments are now always shown + +SecurityBundle +-------------- + + * Deprecate the `security.hide_user_not_found` config option in favor of `security.expose_security_errors` + Serializer ---------- diff --git a/src/Symfony/Bundle/SecurityBundle/CHANGELOG.md b/src/Symfony/Bundle/SecurityBundle/CHANGELOG.md index ae199536724f0..d9339926d61df 100644 --- a/src/Symfony/Bundle/SecurityBundle/CHANGELOG.md +++ b/src/Symfony/Bundle/SecurityBundle/CHANGELOG.md @@ -6,6 +6,8 @@ CHANGELOG * Add `Security::isGrantedForUser()` to test user authorization without relying on the session. For example, users not currently logged in, or while processing a message from a message queue * Add encryption support to `OidcTokenHandler` (JWE) + * Add `expose_security_errors` config option to display `AccountStatusException` + * Deprecate the `security.hide_user_not_found` config option in favor of `security.expose_security_errors` 7.2 --- diff --git a/src/Symfony/Bundle/SecurityBundle/DependencyInjection/MainConfiguration.php b/src/Symfony/Bundle/SecurityBundle/DependencyInjection/MainConfiguration.php index a45276066484c..9854a1f047a7a 100644 --- a/src/Symfony/Bundle/SecurityBundle/DependencyInjection/MainConfiguration.php +++ b/src/Symfony/Bundle/SecurityBundle/DependencyInjection/MainConfiguration.php @@ -17,6 +17,7 @@ use Symfony\Component\Config\Definition\Builder\TreeBuilder; use Symfony\Component\Config\Definition\ConfigurationInterface; use Symfony\Component\Config\Definition\Exception\InvalidConfigurationException; +use Symfony\Component\Security\Http\Authentication\ExposeSecurityLevel; use Symfony\Component\Security\Http\EntryPoint\AuthenticationEntryPointInterface; use Symfony\Component\Security\Http\Session\SessionAuthenticationStrategy; @@ -54,13 +55,30 @@ public function getConfigTreeBuilder(): TreeBuilder $rootNode = $tb->getRootNode(); $rootNode + ->beforeNormalization() + ->always() + ->then(function ($v) { + if (isset($v['hide_user_not_found']) && !isset($v['expose_security_errors'])) { + $v['expose_security_errors'] = $v['hide_user_not_found'] ? ExposeSecurityLevel::None : ExposeSecurityLevel::All; + } + + return $v; + }) + ->end() ->children() ->scalarNode('access_denied_url')->defaultNull()->example('/foo/error403')->end() ->enumNode('session_fixation_strategy') ->values([SessionAuthenticationStrategy::NONE, SessionAuthenticationStrategy::MIGRATE, SessionAuthenticationStrategy::INVALIDATE]) ->defaultValue(SessionAuthenticationStrategy::MIGRATE) ->end() - ->booleanNode('hide_user_not_found')->defaultTrue()->end() + ->booleanNode('hide_user_not_found') + ->setDeprecated('symfony/security-bundle', '7.3', 'The "%node%" option is deprecated and will be removed in 8.0. Use the "expose_security_errors" option instead.') + ->end() + ->enumNode('expose_security_errors') + ->beforeNormalization()->ifString()->then(fn ($v) => ['value' => ExposeSecurityLevel::tryFrom($v)])->end() + ->values(ExposeSecurityLevel::cases()) + ->defaultValue(ExposeSecurityLevel::None) + ->end() ->booleanNode('erase_credentials')->defaultTrue()->end() ->arrayNode('access_decision_manager') ->addDefaultsIfNotSet() diff --git a/src/Symfony/Bundle/SecurityBundle/DependencyInjection/SecurityExtension.php b/src/Symfony/Bundle/SecurityBundle/DependencyInjection/SecurityExtension.php index f454b9318c183..dd1b8cdb490cc 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\Authentication\ExposeSecurityLevel; use Symfony\Component\Security\Http\Authenticator\Debug\TraceableAuthenticator; use Symfony\Component\Security\Http\Authenticator\Debug\TraceableAuthenticatorManagerListener; use Symfony\Component\Security\Http\Event\CheckPassportEvent; @@ -154,7 +155,8 @@ public function load(array $configs, ContainerBuilder $container): void )); } - $container->setParameter('security.authentication.hide_user_not_found', $config['hide_user_not_found']); + $container->setParameter('security.authentication.hide_user_not_found', ExposeSecurityLevel::All !== $config['expose_security_errors']); + $container->setParameter('.security.authentication.expose_security_errors', $config['expose_security_errors']); if (class_exists(Application::class)) { $loader->load('debug_console.php'); diff --git a/src/Symfony/Bundle/SecurityBundle/Resources/config/schema/security-1.0.xsd b/src/Symfony/Bundle/SecurityBundle/Resources/config/schema/security-1.0.xsd index d7d736e91af9d..692321a4907da 100644 --- a/src/Symfony/Bundle/SecurityBundle/Resources/config/schema/security-1.0.xsd +++ b/src/Symfony/Bundle/SecurityBundle/Resources/config/schema/security-1.0.xsd @@ -55,6 +55,7 @@ + @@ -68,6 +69,14 @@ + + + + + + + + diff --git a/src/Symfony/Bundle/SecurityBundle/Resources/config/security_authenticator.php b/src/Symfony/Bundle/SecurityBundle/Resources/config/security_authenticator.php index 1ea4ef5568fd3..babcdb8206df7 100644 --- a/src/Symfony/Bundle/SecurityBundle/Resources/config/security_authenticator.php +++ b/src/Symfony/Bundle/SecurityBundle/Resources/config/security_authenticator.php @@ -42,7 +42,7 @@ abstract_arg('provider key'), service('logger')->nullOnInvalid(), param('security.authentication.manager.erase_credentials'), - param('security.authentication.hide_user_not_found'), + param('.security.authentication.expose_security_errors'), abstract_arg('required badges'), ]) ->tag('monolog.logger', ['channel' => 'security']) diff --git a/src/Symfony/Bundle/SecurityBundle/Tests/DependencyInjection/MainConfigurationTest.php b/src/Symfony/Bundle/SecurityBundle/Tests/DependencyInjection/MainConfigurationTest.php index 8d3fed44695d2..6479e56a668e7 100644 --- a/src/Symfony/Bundle/SecurityBundle/Tests/DependencyInjection/MainConfigurationTest.php +++ b/src/Symfony/Bundle/SecurityBundle/Tests/DependencyInjection/MainConfigurationTest.php @@ -12,13 +12,17 @@ namespace Symfony\Bundle\SecurityBundle\Tests\DependencyInjection; use PHPUnit\Framework\TestCase; +use Symfony\Bridge\PhpUnit\ExpectUserDeprecationMessageTrait; use Symfony\Bundle\SecurityBundle\DependencyInjection\MainConfiguration; use Symfony\Bundle\SecurityBundle\DependencyInjection\Security\Factory\AuthenticatorFactoryInterface; use Symfony\Component\Config\Definition\Exception\InvalidConfigurationException; use Symfony\Component\Config\Definition\Processor; +use Symfony\Component\Security\Http\Authentication\ExposeSecurityLevel; class MainConfigurationTest extends TestCase { + use ExpectUserDeprecationMessageTrait; + /** * The minimal, required config needed to not have any required validation * issues. @@ -228,4 +232,52 @@ public function testFirewalls() $configuration = new MainConfiguration(['stub' => $factory], []); $configuration->getConfigTreeBuilder(); } + + /** + * @dataProvider provideHideUserNotFoundData + */ + public function testExposeSecurityErrors(array $config, ExposeSecurityLevel $expectedExposeSecurityErrors) + { + $config = array_merge(static::$minimalConfig, $config); + + $processor = new Processor(); + $configuration = new MainConfiguration([], []); + $processedConfig = $processor->processConfiguration($configuration, [$config]); + + $this->assertEquals($expectedExposeSecurityErrors, $processedConfig['expose_security_errors']); + $this->assertArrayNotHasKey('hide_user_not_found', $processedConfig); + } + + public static function provideHideUserNotFoundData(): iterable + { + yield [[], ExposeSecurityLevel::None]; + yield [['expose_security_errors' => ExposeSecurityLevel::None], ExposeSecurityLevel::None]; + yield [['expose_security_errors' => ExposeSecurityLevel::AccountStatus], ExposeSecurityLevel::AccountStatus]; + yield [['expose_security_errors' => ExposeSecurityLevel::All], ExposeSecurityLevel::All]; + } + + /** + * @dataProvider provideHideUserNotFoundLegacyData + * + * @group legacy + */ + public function testExposeSecurityErrorsWithLegacyConfig(array $config, ExposeSecurityLevel $expectedExposeSecurityErrors, ?bool $expectedHideUserNotFound) + { + $this->expectUserDeprecationMessage('Since symfony/security-bundle 7.3: The "hide_user_not_found" option is deprecated and will be removed in 8.0. Use the "expose_security_errors" option instead.'); + + $config = array_merge(static::$minimalConfig, $config); + + $processor = new Processor(); + $configuration = new MainConfiguration([], []); + $processedConfig = $processor->processConfiguration($configuration, [$config]); + + $this->assertEquals($expectedExposeSecurityErrors, $processedConfig['expose_security_errors']); + $this->assertEquals($expectedHideUserNotFound, $processedConfig['hide_user_not_found']); + } + + public static function provideHideUserNotFoundLegacyData(): iterable + { + yield [['hide_user_not_found' => true], ExposeSecurityLevel::None, true]; + yield [['hide_user_not_found' => false], ExposeSecurityLevel::All, false]; + } } diff --git a/src/Symfony/Component/Security/Http/Authentication/AuthenticatorManager.php b/src/Symfony/Component/Security/Http/Authentication/AuthenticatorManager.php index 6db5a63f8be5f..6a5f365e58331 100644 --- a/src/Symfony/Component/Security/Http/Authentication/AuthenticatorManager.php +++ b/src/Symfony/Component/Security/Http/Authentication/AuthenticatorManager.php @@ -46,6 +46,8 @@ */ class AuthenticatorManager implements AuthenticatorManagerInterface, UserAuthenticatorInterface { + private ExposeSecurityLevel $exposeSecurityErrors; + /** * @param iterable $authenticators */ @@ -56,9 +58,17 @@ public function __construct( private string $firewallName, private ?LoggerInterface $logger = null, private bool $eraseCredentials = true, - private bool $hideUserNotFoundExceptions = true, + ExposeSecurityLevel|bool $exposeSecurityErrors = ExposeSecurityLevel::None, private array $requiredBadges = [], ) { + if (\is_bool($exposeSecurityErrors)) { + trigger_deprecation('symfony/security-http', '7.3', 'Passing a boolean as "exposeSecurityErrors" parameter is deprecated, use %s value instead.', ExposeSecurityLevel::class); + + // The old parameter had an inverted meaning ($hideUserNotFoundExceptions), for that reason the current name does not reflect the behavior + $exposeSecurityErrors = $exposeSecurityErrors ? ExposeSecurityLevel::None : ExposeSecurityLevel::All; + } + + $this->exposeSecurityErrors = $exposeSecurityErrors; } /** @@ -250,7 +260,7 @@ private function handleAuthenticationFailure(AuthenticationException $authentica // Avoid leaking error details in case of invalid user (e.g. user not found or invalid account status) // to prevent user enumeration via response content comparison - if ($this->hideUserNotFoundExceptions && ($authenticationException instanceof UserNotFoundException || ($authenticationException instanceof AccountStatusException && !$authenticationException instanceof CustomUserMessageAccountStatusException))) { + if ($this->isSensitiveException($authenticationException)) { $authenticationException = new BadCredentialsException('Bad credentials.', 0, $authenticationException); } @@ -264,4 +274,17 @@ private function handleAuthenticationFailure(AuthenticationException $authentica // returning null is ok, it means they want the request to continue return $loginFailureEvent->getResponse(); } + + private function isSensitiveException(AuthenticationException $exception): bool + { + if (ExposeSecurityLevel::All !== $this->exposeSecurityErrors && $exception instanceof UserNotFoundException) { + return true; + } + + if (ExposeSecurityLevel::None === $this->exposeSecurityErrors && $exception instanceof AccountStatusException && !$exception instanceof CustomUserMessageAccountStatusException) { + return true; + } + + return false; + } } diff --git a/src/Symfony/Component/Security/Http/Authentication/ExposeSecurityLevel.php b/src/Symfony/Component/Security/Http/Authentication/ExposeSecurityLevel.php new file mode 100644 index 0000000000000..c80fd49607f8e --- /dev/null +++ b/src/Symfony/Component/Security/Http/Authentication/ExposeSecurityLevel.php @@ -0,0 +1,22 @@ + + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ + +namespace Symfony\Component\Security\Http\Authentication; + +/** + * @author Christian Gripp + */ +enum ExposeSecurityLevel: string +{ + case None = 'none'; + case AccountStatus = 'account_status'; + case All = 'all'; +} diff --git a/src/Symfony/Component/Security/Http/CHANGELOG.md b/src/Symfony/Component/Security/Http/CHANGELOG.md index 8f6902f29c0e0..8dde7c241d731 100644 --- a/src/Symfony/Component/Security/Http/CHANGELOG.md +++ b/src/Symfony/Component/Security/Http/CHANGELOG.md @@ -5,6 +5,7 @@ CHANGELOG --- * Add encryption support to `OidcTokenHandler` (JWE) + * Replace `$hideAccountStatusExceptions` argument with `$exposeSecurityErrors` in `AuthenticatorManager` constructor 7.2 --- diff --git a/src/Symfony/Component/Security/Http/Tests/Authentication/AuthenticatorManagerBCTest.php b/src/Symfony/Component/Security/Http/Tests/Authentication/AuthenticatorManagerBCTest.php new file mode 100644 index 0000000000000..9775e5a15285a --- /dev/null +++ b/src/Symfony/Component/Security/Http/Tests/Authentication/AuthenticatorManagerBCTest.php @@ -0,0 +1,488 @@ + + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ + +namespace Symfony\Component\Security\Http\Tests\Authentication; + +use PHPUnit\Framework\MockObject\MockObject; +use PHPUnit\Framework\TestCase; +use Psr\Log\AbstractLogger; +use Psr\Log\LoggerInterface; +use Symfony\Bridge\PhpUnit\ExpectUserDeprecationMessageTrait; +use Symfony\Component\EventDispatcher\EventDispatcher; +use Symfony\Component\HttpFoundation\Request; +use Symfony\Component\HttpFoundation\Response; +use Symfony\Component\Security\Core\Authentication\Token\Storage\TokenStorageInterface; +use Symfony\Component\Security\Core\Authentication\Token\TokenInterface; +use Symfony\Component\Security\Core\Authentication\Token\UsernamePasswordToken; +use Symfony\Component\Security\Core\Exception\BadCredentialsException; +use Symfony\Component\Security\Core\Exception\LockedException; +use Symfony\Component\Security\Core\Exception\UserNotFoundException; +use Symfony\Component\Security\Core\User\InMemoryUser; +use Symfony\Component\Security\Http\Authentication\AuthenticatorManager; +use Symfony\Component\Security\Http\Authenticator\Debug\TraceableAuthenticator; +use Symfony\Component\Security\Http\Authenticator\InteractiveAuthenticatorInterface; +use Symfony\Component\Security\Http\Authenticator\Passport\Badge\CsrfTokenBadge; +use Symfony\Component\Security\Http\Authenticator\Passport\Badge\UserBadge; +use Symfony\Component\Security\Http\Authenticator\Passport\Credentials\PasswordCredentials; +use Symfony\Component\Security\Http\Authenticator\Passport\Passport; +use Symfony\Component\Security\Http\Authenticator\Passport\SelfValidatingPassport; +use Symfony\Component\Security\Http\Event\AuthenticationTokenCreatedEvent; +use Symfony\Component\Security\Http\Event\CheckPassportEvent; +use Symfony\Component\Security\Http\Tests\Fixtures\DummySupportsAuthenticator; + +class AuthenticatorManagerBCTest extends TestCase +{ + use ExpectUserDeprecationMessageTrait; + + private MockObject&TokenStorageInterface $tokenStorage; + private EventDispatcher $eventDispatcher; + private Request $request; + private InMemoryUser $user; + private MockObject&TokenInterface $token; + private Response $response; + + protected function setUp(): void + { + $this->tokenStorage = $this->createMock(TokenStorageInterface::class); + $this->eventDispatcher = new EventDispatcher(); + $this->request = new Request(); + $this->user = new InMemoryUser('wouter', null); + $this->token = $this->createMock(TokenInterface::class); + $this->token->expects($this->any())->method('getUser')->willReturn($this->user); + $this->response = $this->createMock(Response::class); + } + + /** + * @dataProvider provideSupportsData + * + * @group legacy + */ + public function testSupports($authenticators, $result) + { + $manager = $this->createManager($authenticators, hideUserNotFoundExceptions: true); + + $this->assertEquals($result, $manager->supports($this->request)); + } + + public static function provideSupportsData() + { + yield [[self::createDummySupportsAuthenticator(null), self::createDummySupportsAuthenticator(null)], null]; + yield [[self::createDummySupportsAuthenticator(null), self::createDummySupportsAuthenticator(false)], null]; + + yield [[self::createDummySupportsAuthenticator(null), self::createDummySupportsAuthenticator(true)], true]; + yield [[self::createDummySupportsAuthenticator(true), self::createDummySupportsAuthenticator(false)], true]; + + yield [[self::createDummySupportsAuthenticator(false), self::createDummySupportsAuthenticator(false)], false]; + yield [[], false]; + } + + /** + * @group legacy + */ + public function testSupportsInvalidAuthenticator() + { + $manager = $this->createManager([new \stdClass()], hideUserNotFoundExceptions: true); + + $this->expectExceptionObject( + new \InvalidArgumentException('Authenticator "stdClass" must implement "Symfony\Component\Security\Http\Authenticator\AuthenticatorInterface".') + ); + + $manager->supports($this->request); + } + + /** + * @group legacy + */ + public function testSupportCheckedUponRequestAuthentication() + { + // the attribute stores the supported authenticators, returning false now + // means support changed between calling supports() and authenticateRequest() + // (which is the case with lazy firewalls) + $authenticator = $this->createAuthenticator(false); + $this->request->attributes->set('_security_authenticators', [$authenticator]); + + $authenticator->expects($this->never())->method('authenticate'); + + $manager = $this->createManager([$authenticator], hideUserNotFoundExceptions: true); + $manager->authenticateRequest($this->request); + } + + /** + * @dataProvider provideMatchingAuthenticatorIndex + * + * @group legacy + */ + public function testAuthenticateRequest($matchingAuthenticatorIndex) + { + $authenticators = [$this->createAuthenticator(0 === $matchingAuthenticatorIndex), $this->createAuthenticator(1 === $matchingAuthenticatorIndex)]; + $this->request->attributes->set('_security_authenticators', $authenticators); + $matchingAuthenticator = $authenticators[$matchingAuthenticatorIndex]; + + $authenticators[($matchingAuthenticatorIndex + 1) % 2]->expects($this->never())->method('authenticate'); + + $matchingAuthenticator->expects($this->any())->method('authenticate')->willReturn(new SelfValidatingPassport(new UserBadge('wouter', fn () => $this->user))); + + $listenerCalled = false; + $this->eventDispatcher->addListener(CheckPassportEvent::class, function (CheckPassportEvent $event) use (&$listenerCalled, $matchingAuthenticator) { + if ($event->getAuthenticator() === $matchingAuthenticator && $event->getPassport()->getUser() === $this->user) { + $listenerCalled = true; + } + }); + $matchingAuthenticator->expects($this->any())->method('createToken')->willReturn($this->token); + + $this->tokenStorage->expects($this->once())->method('setToken')->with($this->token); + + $manager = $this->createManager($authenticators, hideUserNotFoundExceptions: true); + $this->assertNull($manager->authenticateRequest($this->request)); + $this->assertTrue($listenerCalled, 'The CheckPassportEvent listener is not called'); + } + + public static function provideMatchingAuthenticatorIndex() + { + yield [0]; + yield [1]; + } + + /** + * @group legacy + */ + public function testNoCredentialsValidated() + { + $authenticator = $this->createAuthenticator(); + $this->request->attributes->set('_security_authenticators', [$authenticator]); + + $authenticator->expects($this->any())->method('authenticate')->willReturn(new Passport(new UserBadge('wouter', fn () => $this->user), new PasswordCredentials('pass'))); + + $authenticator->expects($this->once()) + ->method('onAuthenticationFailure') + ->with($this->request, $this->isInstanceOf(BadCredentialsException::class)); + + $manager = $this->createManager([$authenticator], hideUserNotFoundExceptions: true); + $manager->authenticateRequest($this->request); + } + + /** + * @group legacy + */ + public function testRequiredBadgeMissing() + { + $authenticator = $this->createAuthenticator(); + $this->request->attributes->set('_security_authenticators', [$authenticator]); + + $authenticator->expects($this->any())->method('authenticate')->willReturn(new SelfValidatingPassport(new UserBadge('wouter'))); + + $authenticator->expects($this->once())->method('onAuthenticationFailure')->with($this->anything(), $this->callback(fn ($exception) => 'Authentication failed; Some badges marked as required by the firewall config are not available on the passport: "'.CsrfTokenBadge::class.'".' === $exception->getMessage())); + + $manager = $this->createManager([$authenticator], 'main', true, [CsrfTokenBadge::class], hideUserNotFoundExceptions: true); + $manager->authenticateRequest($this->request); + } + + /** + * @group legacy + */ + public function testAllRequiredBadgesPresent() + { + $authenticator = $this->createAuthenticator(); + $this->request->attributes->set('_security_authenticators', [$authenticator]); + + $csrfBadge = new CsrfTokenBadge('csrfid', 'csrftoken'); + $csrfBadge->markResolved(); + $authenticator->expects($this->any())->method('authenticate')->willReturn(new SelfValidatingPassport(new UserBadge('wouter'), [$csrfBadge])); + $authenticator->expects($this->any())->method('createToken')->willReturn(new UsernamePasswordToken($this->user, 'main')); + + $authenticator->expects($this->once())->method('onAuthenticationSuccess'); + + $manager = $this->createManager([$authenticator], 'main', true, [CsrfTokenBadge::class], hideUserNotFoundExceptions: true); + $manager->authenticateRequest($this->request); + } + + /** + * @dataProvider provideEraseCredentialsData + * + * @group legacy + */ + public function testEraseCredentials($eraseCredentials) + { + $authenticator = $this->createAuthenticator(); + $this->request->attributes->set('_security_authenticators', [$authenticator]); + + $authenticator->expects($this->any())->method('authenticate')->willReturn(new SelfValidatingPassport(new UserBadge('wouter', fn () => $this->user))); + + $authenticator->expects($this->any())->method('createToken')->willReturn($this->token); + + $this->token->expects($eraseCredentials ? $this->once() : $this->never())->method('eraseCredentials'); + + $manager = $this->createManager([$authenticator], 'main', $eraseCredentials, hideUserNotFoundExceptions: true); + $manager->authenticateRequest($this->request); + } + + public static function provideEraseCredentialsData() + { + yield [true]; + yield [false]; + } + + /** + * @group legacy + */ + public function testAuthenticateRequestCanModifyTokenFromEvent() + { + $authenticator = $this->createAuthenticator(); + $this->request->attributes->set('_security_authenticators', [$authenticator]); + + $authenticator->expects($this->any())->method('authenticate')->willReturn(new SelfValidatingPassport(new UserBadge('wouter', fn () => $this->user))); + + $authenticator->expects($this->any())->method('createToken')->willReturn($this->token); + + $modifiedToken = $this->createMock(TokenInterface::class); + $modifiedToken->expects($this->any())->method('getUser')->willReturn($this->user); + $listenerCalled = false; + $this->eventDispatcher->addListener(AuthenticationTokenCreatedEvent::class, function (AuthenticationTokenCreatedEvent $event) use (&$listenerCalled, $modifiedToken) { + $event->setAuthenticatedToken($modifiedToken); + $listenerCalled = true; + }); + + $this->tokenStorage->expects($this->once())->method('setToken')->with($this->identicalTo($modifiedToken)); + + $manager = $this->createManager([$authenticator], hideUserNotFoundExceptions: true); + $this->assertNull($manager->authenticateRequest($this->request)); + $this->assertTrue($listenerCalled, 'The AuthenticationTokenCreatedEvent listener is not called'); + } + + /** + * @group legacy + */ + public function testAuthenticateUser() + { + $authenticator = $this->createAuthenticator(); + $authenticator->expects($this->any())->method('onAuthenticationSuccess')->willReturn($this->response); + + $badge = new UserBadge('alex'); + + $authenticator + ->expects($this->any()) + ->method('createToken') + ->willReturnCallback(function (Passport $passport) use ($badge) { + $this->assertSame(['attr' => 'foo', 'attr2' => 'bar'], $passport->getAttributes()); + $this->assertSame([UserBadge::class => $badge], $passport->getBadges()); + + return $this->token; + }); + + $this->tokenStorage->expects($this->once())->method('setToken')->with($this->token); + + $manager = $this->createManager([$authenticator], hideUserNotFoundExceptions: true); + $manager->authenticateUser($this->user, $authenticator, $this->request, [$badge], ['attr' => 'foo', 'attr2' => 'bar']); + } + + /** + * @group legacy + */ + public function testAuthenticateUserCanModifyTokenFromEvent() + { + $authenticator = $this->createAuthenticator(); + $authenticator->expects($this->any())->method('createToken')->willReturn($this->token); + $authenticator->expects($this->any())->method('onAuthenticationSuccess')->willReturn($this->response); + + $modifiedToken = $this->createMock(TokenInterface::class); + $modifiedToken->expects($this->any())->method('getUser')->willReturn($this->user); + $listenerCalled = false; + $this->eventDispatcher->addListener(AuthenticationTokenCreatedEvent::class, function (AuthenticationTokenCreatedEvent $event) use (&$listenerCalled, $modifiedToken) { + $event->setAuthenticatedToken($modifiedToken); + $listenerCalled = true; + }); + + $this->tokenStorage->expects($this->once())->method('setToken')->with($this->identicalTo($modifiedToken)); + + $manager = $this->createManager([$authenticator], hideUserNotFoundExceptions: true); + $manager->authenticateUser($this->user, $authenticator, $this->request); + $this->assertTrue($listenerCalled, 'The AuthenticationTokenCreatedEvent listener is not called'); + } + + /** + * @group legacy + */ + public function testInteractiveAuthenticator() + { + $authenticator = $this->createMock(TestInteractiveBCAuthenticator::class); + $authenticator->expects($this->any())->method('isInteractive')->willReturn(true); + $this->request->attributes->set('_security_authenticators', [$authenticator]); + + $authenticator->expects($this->any())->method('authenticate')->willReturn(new SelfValidatingPassport(new UserBadge('wouter', fn () => $this->user))); + $authenticator->expects($this->any())->method('createToken')->willReturn($this->token); + + $this->tokenStorage->expects($this->once())->method('setToken')->with($this->token); + + $authenticator->expects($this->any()) + ->method('onAuthenticationSuccess') + ->with($this->anything(), $this->token, 'main') + ->willReturn($this->response); + + $manager = $this->createManager([$authenticator], hideUserNotFoundExceptions: true); + $response = $manager->authenticateRequest($this->request); + $this->assertSame($this->response, $response); + } + + /** + * @group legacy + */ + public function testLegacyInteractiveAuthenticator() + { + $authenticator = $this->createMock(InteractiveAuthenticatorInterface::class); + $authenticator->expects($this->any())->method('isInteractive')->willReturn(true); + $this->request->attributes->set('_security_authenticators', [$authenticator]); + + $authenticator->expects($this->any())->method('authenticate')->willReturn(new SelfValidatingPassport(new UserBadge('wouter', fn () => $this->user))); + $authenticator->expects($this->any())->method('createToken')->willReturn($this->token); + + $this->tokenStorage->expects($this->once())->method('setToken')->with($this->token); + + $authenticator->expects($this->any()) + ->method('onAuthenticationSuccess') + ->with($this->anything(), $this->token, 'main') + ->willReturn($this->response); + + $manager = $this->createManager([$authenticator], hideUserNotFoundExceptions: true); + $response = $manager->authenticateRequest($this->request); + $this->assertSame($this->response, $response); + } + + /** + * @group legacy + */ + public function testAuthenticateRequestHidesInvalidUserExceptions() + { + $invalidUserException = new UserNotFoundException(); + $authenticator = $this->createMock(TestInteractiveBCAuthenticator::class); + $this->request->attributes->set('_security_authenticators', [$authenticator]); + + $authenticator->expects($this->any())->method('authenticate')->willThrowException($invalidUserException); + + $authenticator->expects($this->any()) + ->method('onAuthenticationFailure') + ->with($this->equalTo($this->request), $this->callback(fn ($e) => $e instanceof BadCredentialsException && $invalidUserException === $e->getPrevious())) + ->willReturn($this->response); + + $manager = $this->createManager([$authenticator], hideUserNotFoundExceptions: true); + $response = $manager->authenticateRequest($this->request); + $this->assertSame($this->response, $response); + } + + /** + * @group legacy + */ + public function testAuthenticateRequestShowsAccountStatusException() + { + $invalidUserException = new LockedException(); + $authenticator = $this->createMock(TestInteractiveBCAuthenticator::class); + $this->request->attributes->set('_security_authenticators', [$authenticator]); + + $authenticator->expects($this->any())->method('authenticate')->willThrowException($invalidUserException); + + $authenticator->expects($this->any()) + ->method('onAuthenticationFailure') + ->with($this->equalTo($this->request), $this->callback(fn ($e) => $e === $invalidUserException)) + ->willReturn($this->response); + + $manager = $this->createManager([$authenticator], hideUserNotFoundExceptions: false); + $response = $manager->authenticateRequest($this->request); + $this->assertSame($this->response, $response); + } + + /** + * @group legacy + */ + public function testAuthenticateRequestHidesInvalidAccountStatusException() + { + $invalidUserException = new LockedException(); + $authenticator = $this->createMock(TestInteractiveBCAuthenticator::class); + $this->request->attributes->set('_security_authenticators', [$authenticator]); + + $authenticator->expects($this->any())->method('authenticate')->willThrowException($invalidUserException); + + $authenticator->expects($this->any()) + ->method('onAuthenticationFailure') + ->with($this->equalTo($this->request), $this->callback(fn ($e) => $e instanceof BadCredentialsException && $invalidUserException === $e->getPrevious())) + ->willReturn($this->response); + + $manager = $this->createManager([$authenticator], hideUserNotFoundExceptions: true); + $response = $manager->authenticateRequest($this->request); + $this->assertSame($this->response, $response); + } + + /** + * @group legacy + */ + public function testLogsUseTheDecoratedAuthenticatorWhenItIsTraceable() + { + $authenticator = $this->createMock(TestInteractiveBCAuthenticator::class); + $authenticator->expects($this->any())->method('isInteractive')->willReturn(true); + $this->request->attributes->set('_security_authenticators', [new TraceableAuthenticator($authenticator)]); + + $authenticator->expects($this->any())->method('authenticate')->willReturn(new SelfValidatingPassport(new UserBadge('wouter', fn () => $this->user))); + $authenticator->expects($this->any())->method('createToken')->willReturn($this->token); + + $this->tokenStorage->expects($this->once())->method('setToken')->with($this->token); + + $authenticator->expects($this->any()) + ->method('onAuthenticationSuccess') + ->with($this->anything(), $this->token, 'main') + ->willReturn($this->response); + + $authenticator->expects($this->any()) + ->method('onAuthenticationSuccess') + ->with($this->anything(), $this->token, 'main') + ->willReturn($this->response); + + $logger = new class extends AbstractLogger { + public array $logContexts = []; + + public function log($level, $message, array $context = []): void + { + if ($context['authenticator'] ?? false) { + $this->logContexts[] = $context; + } + } + }; + + $manager = $this->createManager([$authenticator], 'main', true, [], $logger, hideUserNotFoundExceptions: true); + $response = $manager->authenticateRequest($this->request); + $this->assertSame($this->response, $response); + $this->assertStringContainsString($authenticator::class, $logger->logContexts[0]['authenticator']); + } + + private function createAuthenticator(?bool $supports = true) + { + $authenticator = $this->createMock(TestInteractiveBCAuthenticator::class); + $authenticator->expects($this->any())->method('supports')->willReturn($supports); + + return $authenticator; + } + + private static function createDummySupportsAuthenticator(?bool $supports = true) + { + return new DummySupportsAuthenticator($supports); + } + + private function createManager($authenticators, $firewallName = 'main', $eraseCredentials = true, array $requiredBadges = [], ?LoggerInterface $logger = null, bool $hideUserNotFoundExceptions = true) + { + $this->expectUserDeprecationMessage('Since symfony/security-http 7.3: Passing a boolean as "exposeSecurityErrors" parameter is deprecated, use Symfony\Component\Security\Http\Authentication\ExposeSecurityLevel value instead.'); + + return new AuthenticatorManager($authenticators, $this->tokenStorage, $this->eventDispatcher, $firewallName, $logger, $eraseCredentials, $hideUserNotFoundExceptions, $requiredBadges); + } +} + +abstract class TestInteractiveBCAuthenticator implements InteractiveAuthenticatorInterface +{ + public function createToken(Passport $passport, string $firewallName): TokenInterface + { + } +} diff --git a/src/Symfony/Component/Security/Http/Tests/Authentication/AuthenticatorManagerTest.php b/src/Symfony/Component/Security/Http/Tests/Authentication/AuthenticatorManagerTest.php index a6d090768ceff..395a43b8abb4c 100644 --- a/src/Symfony/Component/Security/Http/Tests/Authentication/AuthenticatorManagerTest.php +++ b/src/Symfony/Component/Security/Http/Tests/Authentication/AuthenticatorManagerTest.php @@ -22,9 +22,11 @@ use Symfony\Component\Security\Core\Authentication\Token\TokenInterface; use Symfony\Component\Security\Core\Authentication\Token\UsernamePasswordToken; use Symfony\Component\Security\Core\Exception\BadCredentialsException; +use Symfony\Component\Security\Core\Exception\LockedException; use Symfony\Component\Security\Core\Exception\UserNotFoundException; use Symfony\Component\Security\Core\User\InMemoryUser; use Symfony\Component\Security\Http\Authentication\AuthenticatorManager; +use Symfony\Component\Security\Http\Authentication\ExposeSecurityLevel; use Symfony\Component\Security\Http\Authenticator\Debug\TraceableAuthenticator; use Symfony\Component\Security\Http\Authenticator\InteractiveAuthenticatorInterface; use Symfony\Component\Security\Http\Authenticator\Passport\Badge\CsrfTokenBadge; @@ -61,7 +63,7 @@ protected function setUp(): void */ public function testSupports($authenticators, $result) { - $manager = $this->createManager($authenticators); + $manager = $this->createManager($authenticators, exposeSecurityErrors: ExposeSecurityLevel::None); $this->assertEquals($result, $manager->supports($this->request)); } @@ -80,7 +82,7 @@ public static function provideSupportsData() public function testSupportsInvalidAuthenticator() { - $manager = $this->createManager([new \stdClass()]); + $manager = $this->createManager([new \stdClass()], exposeSecurityErrors: ExposeSecurityLevel::None); $this->expectExceptionObject( new \InvalidArgumentException('Authenticator "stdClass" must implement "Symfony\Component\Security\Http\Authenticator\AuthenticatorInterface".') @@ -99,7 +101,7 @@ public function testSupportCheckedUponRequestAuthentication() $authenticator->expects($this->never())->method('authenticate'); - $manager = $this->createManager([$authenticator]); + $manager = $this->createManager([$authenticator], exposeSecurityErrors: ExposeSecurityLevel::None); $manager->authenticateRequest($this->request); } @@ -126,7 +128,7 @@ public function testAuthenticateRequest($matchingAuthenticatorIndex) $this->tokenStorage->expects($this->once())->method('setToken')->with($this->token); - $manager = $this->createManager($authenticators); + $manager = $this->createManager($authenticators, exposeSecurityErrors: ExposeSecurityLevel::None); $this->assertNull($manager->authenticateRequest($this->request)); $this->assertTrue($listenerCalled, 'The CheckPassportEvent listener is not called'); } @@ -148,7 +150,7 @@ public function testNoCredentialsValidated() ->method('onAuthenticationFailure') ->with($this->request, $this->isInstanceOf(BadCredentialsException::class)); - $manager = $this->createManager([$authenticator]); + $manager = $this->createManager([$authenticator], exposeSecurityErrors: ExposeSecurityLevel::None); $manager->authenticateRequest($this->request); } @@ -161,7 +163,7 @@ public function testRequiredBadgeMissing() $authenticator->expects($this->once())->method('onAuthenticationFailure')->with($this->anything(), $this->callback(fn ($exception) => 'Authentication failed; Some badges marked as required by the firewall config are not available on the passport: "'.CsrfTokenBadge::class.'".' === $exception->getMessage())); - $manager = $this->createManager([$authenticator], 'main', true, [CsrfTokenBadge::class]); + $manager = $this->createManager([$authenticator], 'main', true, [CsrfTokenBadge::class], exposeSecurityErrors: ExposeSecurityLevel::None); $manager->authenticateRequest($this->request); } @@ -177,7 +179,7 @@ public function testAllRequiredBadgesPresent() $authenticator->expects($this->once())->method('onAuthenticationSuccess'); - $manager = $this->createManager([$authenticator], 'main', true, [CsrfTokenBadge::class]); + $manager = $this->createManager([$authenticator], 'main', true, [CsrfTokenBadge::class], exposeSecurityErrors: ExposeSecurityLevel::None); $manager->authenticateRequest($this->request); } @@ -195,7 +197,7 @@ public function testEraseCredentials($eraseCredentials) $this->token->expects($eraseCredentials ? $this->once() : $this->never())->method('eraseCredentials'); - $manager = $this->createManager([$authenticator], 'main', $eraseCredentials); + $manager = $this->createManager([$authenticator], 'main', $eraseCredentials, exposeSecurityErrors: ExposeSecurityLevel::None); $manager->authenticateRequest($this->request); } @@ -224,7 +226,7 @@ public function testAuthenticateRequestCanModifyTokenFromEvent() $this->tokenStorage->expects($this->once())->method('setToken')->with($this->identicalTo($modifiedToken)); - $manager = $this->createManager([$authenticator]); + $manager = $this->createManager([$authenticator], exposeSecurityErrors: ExposeSecurityLevel::None); $this->assertNull($manager->authenticateRequest($this->request)); $this->assertTrue($listenerCalled, 'The AuthenticationTokenCreatedEvent listener is not called'); } @@ -248,7 +250,7 @@ public function testAuthenticateUser() $this->tokenStorage->expects($this->once())->method('setToken')->with($this->token); - $manager = $this->createManager([$authenticator]); + $manager = $this->createManager([$authenticator], exposeSecurityErrors: ExposeSecurityLevel::None); $manager->authenticateUser($this->user, $authenticator, $this->request, [$badge], ['attr' => 'foo', 'attr2' => 'bar']); } @@ -268,7 +270,7 @@ public function testAuthenticateUserCanModifyTokenFromEvent() $this->tokenStorage->expects($this->once())->method('setToken')->with($this->identicalTo($modifiedToken)); - $manager = $this->createManager([$authenticator]); + $manager = $this->createManager([$authenticator], exposeSecurityErrors: ExposeSecurityLevel::None); $manager->authenticateUser($this->user, $authenticator, $this->request); $this->assertTrue($listenerCalled, 'The AuthenticationTokenCreatedEvent listener is not called'); } @@ -289,7 +291,7 @@ public function testInteractiveAuthenticator() ->with($this->anything(), $this->token, 'main') ->willReturn($this->response); - $manager = $this->createManager([$authenticator]); + $manager = $this->createManager([$authenticator], exposeSecurityErrors: ExposeSecurityLevel::None); $response = $manager->authenticateRequest($this->request); $this->assertSame($this->response, $response); } @@ -310,7 +312,7 @@ public function testLegacyInteractiveAuthenticator() ->with($this->anything(), $this->token, 'main') ->willReturn($this->response); - $manager = $this->createManager([$authenticator]); + $manager = $this->createManager([$authenticator], exposeSecurityErrors: ExposeSecurityLevel::None); $response = $manager->authenticateRequest($this->request); $this->assertSame($this->response, $response); } @@ -328,7 +330,43 @@ public function testAuthenticateRequestHidesInvalidUserExceptions() ->with($this->equalTo($this->request), $this->callback(fn ($e) => $e instanceof BadCredentialsException && $invalidUserException === $e->getPrevious())) ->willReturn($this->response); - $manager = $this->createManager([$authenticator]); + $manager = $this->createManager([$authenticator], exposeSecurityErrors: ExposeSecurityLevel::None); + $response = $manager->authenticateRequest($this->request); + $this->assertSame($this->response, $response); + } + + public function testAuthenticateRequestShowsAccountStatusException() + { + $invalidUserException = new LockedException(); + $authenticator = $this->createMock(TestInteractiveAuthenticator::class); + $this->request->attributes->set('_security_authenticators', [$authenticator]); + + $authenticator->expects($this->any())->method('authenticate')->willThrowException($invalidUserException); + + $authenticator->expects($this->any()) + ->method('onAuthenticationFailure') + ->with($this->equalTo($this->request), $this->callback(fn ($e) => $e === $invalidUserException)) + ->willReturn($this->response); + + $manager = $this->createManager([$authenticator], exposeSecurityErrors: ExposeSecurityLevel::AccountStatus); + $response = $manager->authenticateRequest($this->request); + $this->assertSame($this->response, $response); + } + + public function testAuthenticateRequestHidesInvalidAccountStatusException() + { + $invalidUserException = new LockedException(); + $authenticator = $this->createMock(TestInteractiveAuthenticator::class); + $this->request->attributes->set('_security_authenticators', [$authenticator]); + + $authenticator->expects($this->any())->method('authenticate')->willThrowException($invalidUserException); + + $authenticator->expects($this->any()) + ->method('onAuthenticationFailure') + ->with($this->equalTo($this->request), $this->callback(fn ($e) => $e instanceof BadCredentialsException && $invalidUserException === $e->getPrevious())) + ->willReturn($this->response); + + $manager = $this->createManager([$authenticator], exposeSecurityErrors: ExposeSecurityLevel::None); $response = $manager->authenticateRequest($this->request); $this->assertSame($this->response, $response); } @@ -365,7 +403,7 @@ public function log($level, $message, array $context = []): void } }; - $manager = $this->createManager([$authenticator], 'main', true, [], $logger); + $manager = $this->createManager([$authenticator], 'main', true, [], $logger, exposeSecurityErrors: ExposeSecurityLevel::None); $response = $manager->authenticateRequest($this->request); $this->assertSame($this->response, $response); $this->assertStringContainsString($authenticator::class, $logger->logContexts[0]['authenticator']); @@ -384,9 +422,9 @@ private static function createDummySupportsAuthenticator(?bool $supports = true) return new DummySupportsAuthenticator($supports); } - private function createManager($authenticators, $firewallName = 'main', $eraseCredentials = true, array $requiredBadges = [], ?LoggerInterface $logger = null) + private function createManager($authenticators, $firewallName = 'main', $eraseCredentials = true, array $requiredBadges = [], ?LoggerInterface $logger = null, ExposeSecurityLevel $exposeSecurityErrors = ExposeSecurityLevel::AccountStatus) { - return new AuthenticatorManager($authenticators, $this->tokenStorage, $this->eventDispatcher, $firewallName, $logger, $eraseCredentials, true, $requiredBadges); + return new AuthenticatorManager($authenticators, $this->tokenStorage, $this->eventDispatcher, $firewallName, $logger, $eraseCredentials, $exposeSecurityErrors, $requiredBadges); } }