diff --git a/src/Symfony/Bundle/SecurityBundle/DependencyInjection/MainConfiguration.php b/src/Symfony/Bundle/SecurityBundle/DependencyInjection/MainConfiguration.php index 20e6832de69f0..f0496c495cdd8 100644 --- a/src/Symfony/Bundle/SecurityBundle/DependencyInjection/MainConfiguration.php +++ b/src/Symfony/Bundle/SecurityBundle/DependencyInjection/MainConfiguration.php @@ -91,6 +91,7 @@ public function getConfigTreeBuilder() ->defaultValue(SessionAuthenticationStrategy::MIGRATE) ->end() ->booleanNode('hide_user_not_found')->defaultTrue()->end() + ->booleanNode('show_account_status_exceptions')->defaultFalse()->end() ->booleanNode('always_authenticate_before_granting') ->defaultFalse() ->setDeprecated('symfony/security-bundle', '5.4') diff --git a/src/Symfony/Bundle/SecurityBundle/DependencyInjection/SecurityExtension.php b/src/Symfony/Bundle/SecurityBundle/DependencyInjection/SecurityExtension.php index 41740edf2d4db..e04025d23bb54 100644 --- a/src/Symfony/Bundle/SecurityBundle/DependencyInjection/SecurityExtension.php +++ b/src/Symfony/Bundle/SecurityBundle/DependencyInjection/SecurityExtension.php @@ -170,6 +170,7 @@ public function load(array $configs, ContainerBuilder $container) $container->setParameter('security.access.always_authenticate_before_granting', $config['always_authenticate_before_granting']); $container->setParameter('security.authentication.hide_user_not_found', $config['hide_user_not_found']); + $container->setParameter('security.authentication.show_account_status_exceptions', $config['show_account_status_exceptions']); if (class_exists(Application::class)) { $loader->load('debug_console.php'); diff --git a/src/Symfony/Bundle/SecurityBundle/Resources/config/guard.php b/src/Symfony/Bundle/SecurityBundle/Resources/config/guard.php index 4697e135d80c2..bbaec4c047a15 100644 --- a/src/Symfony/Bundle/SecurityBundle/Resources/config/guard.php +++ b/src/Symfony/Bundle/SecurityBundle/Resources/config/guard.php @@ -50,6 +50,7 @@ service('logger')->nullOnInvalid(), param('security.authentication.hide_user_not_found'), service('security.token_storage'), + param('security.authentication.show_account_status_exceptions'), ]) ->tag('monolog.logger', ['channel' => 'security']) ->deprecate('symfony/security-bundle', '5.3', 'The "%service_id%" service is deprecated, use the new authenticator system instead.') diff --git a/src/Symfony/Bundle/SecurityBundle/Resources/config/security_authenticator.php b/src/Symfony/Bundle/SecurityBundle/Resources/config/security_authenticator.php index fd83cd3b96108..e624365e82be0 100644 --- a/src/Symfony/Bundle/SecurityBundle/Resources/config/security_authenticator.php +++ b/src/Symfony/Bundle/SecurityBundle/Resources/config/security_authenticator.php @@ -46,6 +46,7 @@ param('security.authentication.manager.erase_credentials'), param('security.authentication.hide_user_not_found'), abstract_arg('required badges'), + param('security.authentication.show_account_status_exceptions'), ]) ->tag('monolog.logger', ['channel' => 'security']) diff --git a/src/Symfony/Bundle/SecurityBundle/Resources/config/security_legacy.php b/src/Symfony/Bundle/SecurityBundle/Resources/config/security_legacy.php index ec829ea1cbf85..65d1d4f9921ea 100644 --- a/src/Symfony/Bundle/SecurityBundle/Resources/config/security_legacy.php +++ b/src/Symfony/Bundle/SecurityBundle/Resources/config/security_legacy.php @@ -122,6 +122,7 @@ abstract_arg('Provider-shared Key'), service('security.password_hasher_factory'), param('security.authentication.hide_user_not_found'), + param('security.authentication.show_account_status_exceptions'), ]) ->deprecate('symfony/security-bundle', '5.3', 'The "%service_id%" service is deprecated, use the new authenticator system instead.') @@ -136,6 +137,7 @@ param('security.authentication.hide_user_not_found'), abstract_arg('search dn'), abstract_arg('search password'), + param('security.authentication.show_account_status_exceptions'), ]) ->deprecate('symfony/security-bundle', '5.3', 'The "%service_id%" service is deprecated, use the new authenticator system instead.') diff --git a/src/Symfony/Component/Security/Core/Authentication/Provider/DaoAuthenticationProvider.php b/src/Symfony/Component/Security/Core/Authentication/Provider/DaoAuthenticationProvider.php index a0e01da4e1a28..6636d827ad460 100644 --- a/src/Symfony/Component/Security/Core/Authentication/Provider/DaoAuthenticationProvider.php +++ b/src/Symfony/Component/Security/Core/Authentication/Provider/DaoAuthenticationProvider.php @@ -42,9 +42,9 @@ class DaoAuthenticationProvider extends UserAuthenticationProvider /** * @param PasswordHasherFactoryInterface $hasherFactory */ - public function __construct(UserProviderInterface $userProvider, UserCheckerInterface $userChecker, string $providerKey, $hasherFactory, bool $hideUserNotFoundExceptions = true) + public function __construct(UserProviderInterface $userProvider, UserCheckerInterface $userChecker, string $providerKey, $hasherFactory, bool $hideUserNotFoundExceptions = true, bool $showAccountStatusExceptions = false) { - parent::__construct($userChecker, $providerKey, $hideUserNotFoundExceptions); + parent::__construct($userChecker, $providerKey, $hideUserNotFoundExceptions, $showAccountStatusExceptions); if ($hasherFactory instanceof EncoderFactoryInterface) { trigger_deprecation('symfony/security-core', '5.3', 'Passing a "%s" instance to the "%s" constructor is deprecated, use "%s" instead.', EncoderFactoryInterface::class, __CLASS__, PasswordHasherFactoryInterface::class); diff --git a/src/Symfony/Component/Security/Core/Authentication/Provider/LdapBindAuthenticationProvider.php b/src/Symfony/Component/Security/Core/Authentication/Provider/LdapBindAuthenticationProvider.php index ec2bc569cf9b3..2277de2a49554 100644 --- a/src/Symfony/Component/Security/Core/Authentication/Provider/LdapBindAuthenticationProvider.php +++ b/src/Symfony/Component/Security/Core/Authentication/Provider/LdapBindAuthenticationProvider.php @@ -42,9 +42,9 @@ class LdapBindAuthenticationProvider extends UserAuthenticationProvider private $searchDn; private $searchPassword; - public function __construct(UserProviderInterface $userProvider, UserCheckerInterface $userChecker, string $providerKey, LdapInterface $ldap, string $dnString = '{user_identifier}', bool $hideUserNotFoundExceptions = true, string $searchDn = '', string $searchPassword = '') + public function __construct(UserProviderInterface $userProvider, UserCheckerInterface $userChecker, string $providerKey, LdapInterface $ldap, string $dnString = '{user_identifier}', bool $hideUserNotFoundExceptions = true, string $searchDn = '', string $searchPassword = '', bool $showAccountStatusExceptions = false) { - parent::__construct($userChecker, $providerKey, $hideUserNotFoundExceptions); + parent::__construct($userChecker, $providerKey, $hideUserNotFoundExceptions, $showAccountStatusExceptions); $this->userProvider = $userProvider; $this->ldap = $ldap; diff --git a/src/Symfony/Component/Security/Core/Authentication/Provider/UserAuthenticationProvider.php b/src/Symfony/Component/Security/Core/Authentication/Provider/UserAuthenticationProvider.php index 81731e8dd0b67..06fe6841b2105 100644 --- a/src/Symfony/Component/Security/Core/Authentication/Provider/UserAuthenticationProvider.php +++ b/src/Symfony/Component/Security/Core/Authentication/Provider/UserAuthenticationProvider.php @@ -11,6 +11,7 @@ namespace Symfony\Component\Security\Core\Authentication\Provider; +use Exception; use Symfony\Component\Security\Core\Authentication\Token\SwitchUserToken; use Symfony\Component\Security\Core\Authentication\Token\TokenInterface; use Symfony\Component\Security\Core\Authentication\Token\UsernamePasswordToken; @@ -37,11 +38,12 @@ abstract class UserAuthenticationProvider implements AuthenticationProviderInter private $hideUserNotFoundExceptions; private $userChecker; private $providerKey; + private $showAccountStatusExceptions; /** * @throws \InvalidArgumentException */ - public function __construct(UserCheckerInterface $userChecker, string $providerKey, bool $hideUserNotFoundExceptions = true) + public function __construct(UserCheckerInterface $userChecker, string $providerKey, bool $hideUserNotFoundExceptions = true, bool $showAccountStatusExceptions = false) { if (empty($providerKey)) { throw new \InvalidArgumentException('$providerKey must not be empty.'); @@ -50,6 +52,7 @@ public function __construct(UserCheckerInterface $userChecker, string $providerK $this->userChecker = $userChecker; $this->providerKey = $providerKey; $this->hideUserNotFoundExceptions = $hideUserNotFoundExceptions; + $this->showAccountStatusExceptions = $showAccountStatusExceptions; } /** @@ -86,7 +89,7 @@ public function authenticate(TokenInterface $token) $this->checkAuthentication($user, $token); $this->userChecker->checkPostAuth($user); } catch (AccountStatusException|BadCredentialsException $e) { - if ($this->hideUserNotFoundExceptions && !$e instanceof CustomUserMessageAccountStatusException) { + if ($this->isFilteredException($e)) { throw new BadCredentialsException('Bad credentials.', 0, $e); } @@ -131,4 +134,17 @@ abstract protected function retrieveUser(string $username, UsernamePasswordToken * @throws AuthenticationException if the credentials could not be validated */ abstract protected function checkAuthentication(UserInterface $user, UsernamePasswordToken $token); + + private function isFilteredException(Exception $exception): bool + { + if (!$this->hideUserNotFoundExceptions) { + return false; + } + + if ($this->showAccountStatusExceptions) { + return false; + } + + return !$exception instanceof CustomUserMessageAccountStatusException; + } } diff --git a/src/Symfony/Component/Security/Guard/Firewall/GuardAuthenticationListener.php b/src/Symfony/Component/Security/Guard/Firewall/GuardAuthenticationListener.php index cf6cb80e569e2..d3172e450c625 100644 --- a/src/Symfony/Component/Security/Guard/Firewall/GuardAuthenticationListener.php +++ b/src/Symfony/Component/Security/Guard/Firewall/GuardAuthenticationListener.php @@ -11,6 +11,7 @@ namespace Symfony\Component\Security\Guard\Firewall; +use Exception; use Psr\Log\LoggerInterface; use Symfony\Component\HttpFoundation\Request; use Symfony\Component\HttpFoundation\Response; @@ -51,12 +52,13 @@ class GuardAuthenticationListener extends AbstractListener private $rememberMeServices; private $hideUserNotFoundExceptions; private $tokenStorage; + private $showAccountStatusExceptions; /** * @param string $providerKey The provider (i.e. firewall) key * @param iterable $guardAuthenticators The authenticators, with keys that match what's passed to GuardAuthenticationProvider */ - public function __construct(GuardAuthenticatorHandler $guardHandler, AuthenticationManagerInterface $authenticationManager, string $providerKey, iterable $guardAuthenticators, ?LoggerInterface $logger = null, bool $hideUserNotFoundExceptions = true, ?TokenStorageInterface $tokenStorage = null) + public function __construct(GuardAuthenticatorHandler $guardHandler, AuthenticationManagerInterface $authenticationManager, string $providerKey, iterable $guardAuthenticators, ?LoggerInterface $logger = null, bool $hideUserNotFoundExceptions = true, ?TokenStorageInterface $tokenStorage = null, bool $showAccountStatusExceptions = false) { if (empty($providerKey)) { throw new \InvalidArgumentException('$providerKey must not be empty.'); @@ -69,6 +71,7 @@ public function __construct(GuardAuthenticatorHandler $guardHandler, Authenticat $this->logger = $logger; $this->hideUserNotFoundExceptions = $hideUserNotFoundExceptions; $this->tokenStorage = $tokenStorage; + $this->showAccountStatusExceptions = $showAccountStatusExceptions; } /** @@ -180,7 +183,7 @@ private function executeGuardAuthenticator(string $uniqueGuardKey, Authenticator // 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 - if ($this->hideUserNotFoundExceptions && ($e instanceof UsernameNotFoundException || ($e instanceof AccountStatusException && !$e instanceof CustomUserMessageAccountStatusException))) { + if ($this->isFilteredException($e)) { $e = new BadCredentialsException('Bad credentials.', 0, $e); } @@ -247,4 +250,21 @@ private function triggerRememberMe(AuthenticatorInterface $guardAuthenticator, R $this->rememberMeServices->loginSuccess($request, $response, $token); } + + private function isFilteredException(Exception $exception): bool + { + if (!$this->hideUserNotFoundExceptions) { + return false; + } + + if ($exception instanceof UsernameNotFoundException) { + return true; + } + + if ($this->showAccountStatusExceptions) { + return false; + } + + return $exception instanceof AccountStatusException && !$exception instanceof CustomUserMessageAccountStatusException; + } } diff --git a/src/Symfony/Component/Security/Guard/Tests/Firewall/GuardAuthenticationListenerTest.php b/src/Symfony/Component/Security/Guard/Tests/Firewall/GuardAuthenticationListenerTest.php index f1f9cec886a47..a8c03cc11f6eb 100644 --- a/src/Symfony/Component/Security/Guard/Tests/Firewall/GuardAuthenticationListenerTest.php +++ b/src/Symfony/Component/Security/Guard/Tests/Firewall/GuardAuthenticationListenerTest.php @@ -257,11 +257,63 @@ public function testHandleHidesInvalidUserExceptions(AuthenticationException $ex $listener($this->event); } + /** + * @dataProvider exceptionsToHide + */ + public function testHandleHidesInvalidUserExceptionsShowAccountStatusException(AuthenticationException $exceptionToHide, bool $accountStatusException) + { + $authenticator = $this->createMock(AuthenticatorInterface::class); + $providerKey = 'my_firewall2'; + + $authenticator + ->expects($this->once()) + ->method('supports') + ->willReturn(true); + $authenticator + ->expects($this->once()) + ->method('getCredentials') + ->willReturn(['username' => 'robin', 'password' => 'hood']); + + $this->authenticationManager + ->expects($this->once()) + ->method('authenticate') + ->willThrowException($exceptionToHide); + + if ($accountStatusException) { + $this->guardAuthenticatorHandler + ->expects($this->once()) + ->method('handleAuthenticationFailure') + ->with($this->callback(function ($e) use ($exceptionToHide) { + return $e === $exceptionToHide; + }), $this->request, $authenticator, $providerKey); + } else { + $this->guardAuthenticatorHandler + ->expects($this->once()) + ->method('handleAuthenticationFailure') + ->with($this->callback(function ($e) use ($exceptionToHide) { + return $e instanceof BadCredentialsException && $exceptionToHide === $e->getPrevious(); + }), $this->request, $authenticator, $providerKey); + } + + $listener = new GuardAuthenticationListener( + $this->guardAuthenticatorHandler, + $this->authenticationManager, + $providerKey, + [$authenticator], + $this->logger, + true, + null, + true + ); + + $listener($this->event); + } + public static function exceptionsToHide() { return [ - [new UserNotFoundException()], - [new LockedException()], + [new UserNotFoundException(), false], + [new LockedException(), true], ]; } diff --git a/src/Symfony/Component/Security/Http/Authentication/AuthenticatorManager.php b/src/Symfony/Component/Security/Http/Authentication/AuthenticatorManager.php index 7fb99b87ab2fd..18a1bf447bfcd 100644 --- a/src/Symfony/Component/Security/Http/Authentication/AuthenticatorManager.php +++ b/src/Symfony/Component/Security/Http/Authentication/AuthenticatorManager.php @@ -54,11 +54,12 @@ class AuthenticatorManager implements AuthenticatorManagerInterface, UserAuthent private $firewallName; private $hideUserNotFoundExceptions; private $requiredBadges; + private $showAccountStatusExceptions; /** * @param iterable $authenticators */ - public function __construct(iterable $authenticators, TokenStorageInterface $tokenStorage, EventDispatcherInterface $eventDispatcher, string $firewallName, ?LoggerInterface $logger = null, bool $eraseCredentials = true, bool $hideUserNotFoundExceptions = true, array $requiredBadges = []) + public function __construct(iterable $authenticators, TokenStorageInterface $tokenStorage, EventDispatcherInterface $eventDispatcher, string $firewallName, ?LoggerInterface $logger = null, bool $eraseCredentials = true, bool $hideUserNotFoundExceptions = true, array $requiredBadges = [], bool $showAccountStatusExceptions = false) { $this->authenticators = $authenticators; $this->tokenStorage = $tokenStorage; @@ -68,6 +69,7 @@ public function __construct(iterable $authenticators, TokenStorageInterface $tok $this->eraseCredentials = $eraseCredentials; $this->hideUserNotFoundExceptions = $hideUserNotFoundExceptions; $this->requiredBadges = $requiredBadges; + $this->showAccountStatusExceptions = $showAccountStatusExceptions; } /** @@ -269,7 +271,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->isFilteredException($authenticationException)) { $authenticationException = new BadCredentialsException('Bad credentials.', 0, $authenticationException); } @@ -283,4 +285,21 @@ private function handleAuthenticationFailure(AuthenticationException $authentica // returning null is ok, it means they want the request to continue return $loginFailureEvent->getResponse(); } + + private function isFilteredException(AuthenticationException $exception): bool + { + if (!$this->hideUserNotFoundExceptions) { + return false; + } + + if ($exception instanceof UserNotFoundException) { + return true; + } + + if ($this->showAccountStatusExceptions) { + return false; + } + + return $exception instanceof AccountStatusException && !$exception instanceof CustomUserMessageAccountStatusException; + } }