diff --git a/src/Symfony/Bundle/SecurityBundle/Resources/config/guard.xml b/src/Symfony/Bundle/SecurityBundle/Resources/config/guard.xml index 7b17aff868..2fae143899 100644 --- a/src/Symfony/Bundle/SecurityBundle/Resources/config/guard.xml +++ b/src/Symfony/Bundle/SecurityBundle/Resources/config/guard.xml @@ -17,7 +17,7 @@ - + @@ -42,6 +42,7 @@ + %security.authentication.hide_user_not_found% diff --git a/src/Symfony/Component/Security/Core/Authentication/Provider/UserAuthenticationProvider.php b/src/Symfony/Component/Security/Core/Authentication/Provider/UserAuthenticationProvider.php index 9912259b39..86ef627486 100644 --- a/src/Symfony/Component/Security/Core/Authentication/Provider/UserAuthenticationProvider.php +++ b/src/Symfony/Component/Security/Core/Authentication/Provider/UserAuthenticationProvider.php @@ -14,6 +14,7 @@ namespace Symfony\Component\Security\Core\Authentication\Provider; use Symfony\Component\Security\Core\Authentication\Token\SwitchUserToken; use Symfony\Component\Security\Core\Authentication\Token\TokenInterface; use Symfony\Component\Security\Core\Authentication\Token\UsernamePasswordToken; +use Symfony\Component\Security\Core\Exception\AccountStatusException; use Symfony\Component\Security\Core\Exception\AuthenticationException; use Symfony\Component\Security\Core\Exception\AuthenticationServiceException; use Symfony\Component\Security\Core\Exception\BadCredentialsException; @@ -80,7 +81,7 @@ abstract class UserAuthenticationProvider implements AuthenticationProviderInter $this->userChecker->checkPreAuth($user); $this->checkAuthentication($user, $token); $this->userChecker->checkPostAuth($user); - } catch (BadCredentialsException $e) { + } catch (AccountStatusException $e) { if ($this->hideUserNotFoundExceptions) { throw new BadCredentialsException('Bad credentials.', 0, $e); } diff --git a/src/Symfony/Component/Security/Core/Tests/Authentication/Provider/UserAuthenticationProviderTest.php b/src/Symfony/Component/Security/Core/Tests/Authentication/Provider/UserAuthenticationProviderTest.php index 1ce9b7914a..66c36971ea 100644 --- a/src/Symfony/Component/Security/Core/Tests/Authentication/Provider/UserAuthenticationProviderTest.php +++ b/src/Symfony/Component/Security/Core/Tests/Authentication/Provider/UserAuthenticationProviderTest.php @@ -83,7 +83,7 @@ class UserAuthenticationProviderTest extends TestCase public function testAuthenticateWhenPreChecksFails() { - $this->expectException(CredentialsExpiredException::class); + $this->expectException(BadCredentialsException::class); $userChecker = $this->createMock(UserCheckerInterface::class); $userChecker->expects($this->once()) ->method('checkPreAuth') @@ -101,7 +101,7 @@ class UserAuthenticationProviderTest extends TestCase public function testAuthenticateWhenPostChecksFails() { - $this->expectException(AccountExpiredException::class); + $this->expectException(BadCredentialsException::class); $userChecker = $this->createMock(UserCheckerInterface::class); $userChecker->expects($this->once()) ->method('checkPostAuth') @@ -128,7 +128,7 @@ class UserAuthenticationProviderTest extends TestCase ; $provider->expects($this->once()) ->method('checkAuthentication') - ->willThrowException(new BadCredentialsException()) + ->willThrowException(new CredentialsExpiredException()) ; $provider->authenticate($this->getSupportedToken()); diff --git a/src/Symfony/Component/Security/Guard/Firewall/GuardAuthenticationListener.php b/src/Symfony/Component/Security/Guard/Firewall/GuardAuthenticationListener.php index 45f20c3f9a..4375fac2ed 100644 --- a/src/Symfony/Component/Security/Guard/Firewall/GuardAuthenticationListener.php +++ b/src/Symfony/Component/Security/Guard/Firewall/GuardAuthenticationListener.php @@ -17,7 +17,10 @@ use Symfony\Component\HttpFoundation\Response; use Symfony\Component\HttpKernel\Event\RequestEvent; use Symfony\Component\Security\Core\Authentication\AuthenticationManagerInterface; use Symfony\Component\Security\Core\Authentication\Token\TokenInterface; +use Symfony\Component\Security\Core\Exception\AccountStatusException; use Symfony\Component\Security\Core\Exception\AuthenticationException; +use Symfony\Component\Security\Core\Exception\BadCredentialsException; +use Symfony\Component\Security\Core\Exception\UsernameNotFoundException; use Symfony\Component\Security\Guard\AuthenticatorInterface; use Symfony\Component\Security\Guard\GuardAuthenticatorHandler; use Symfony\Component\Security\Guard\Token\PreAuthenticationGuardToken; @@ -44,12 +47,13 @@ class GuardAuthenticationListener extends AbstractListener implements ListenerIn private $guardAuthenticators; private $logger; private $rememberMeServices; + private $hideUserNotFoundExceptions; /** * @param string $providerKey The provider (i.e. firewall) key * @param iterable|AuthenticatorInterface[] $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) + public function __construct(GuardAuthenticatorHandler $guardHandler, AuthenticationManagerInterface $authenticationManager, string $providerKey, iterable $guardAuthenticators, LoggerInterface $logger = null, bool $hideUserNotFoundExceptions = true) { if (empty($providerKey)) { throw new \InvalidArgumentException('$providerKey must not be empty.'); @@ -60,6 +64,7 @@ class GuardAuthenticationListener extends AbstractListener implements ListenerIn $this->providerKey = $providerKey; $this->guardAuthenticators = $guardAuthenticators; $this->logger = $logger; + $this->hideUserNotFoundExceptions = $hideUserNotFoundExceptions; } /** @@ -164,6 +169,12 @@ class GuardAuthenticationListener extends AbstractListener implements ListenerIn $this->logger->info('Guard authentication failed.', ['exception' => $e, 'authenticator' => \get_class($guardAuthenticator)]); } + // 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 = new BadCredentialsException('Bad credentials.', 0, $e); + } + $response = $this->guardHandler->handleAuthenticationFailure($e, $request, $guardAuthenticator, $this->providerKey); if ($response instanceof Response) { diff --git a/src/Symfony/Component/Security/Guard/Tests/Firewall/GuardAuthenticationListenerTest.php b/src/Symfony/Component/Security/Guard/Tests/Firewall/GuardAuthenticationListenerTest.php index 7d1c8e7246..512bf313f7 100644 --- a/src/Symfony/Component/Security/Guard/Tests/Firewall/GuardAuthenticationListenerTest.php +++ b/src/Symfony/Component/Security/Guard/Tests/Firewall/GuardAuthenticationListenerTest.php @@ -19,6 +19,9 @@ use Symfony\Component\HttpKernel\Event\RequestEvent; use Symfony\Component\Security\Core\Authentication\AuthenticationProviderManager; use Symfony\Component\Security\Core\Authentication\Token\TokenInterface; use Symfony\Component\Security\Core\Exception\AuthenticationException; +use Symfony\Component\Security\Core\Exception\BadCredentialsException; +use Symfony\Component\Security\Core\Exception\LockedException; +use Symfony\Component\Security\Core\Exception\UsernameNotFoundException; use Symfony\Component\Security\Guard\AuthenticatorInterface; use Symfony\Component\Security\Guard\Firewall\GuardAuthenticationListener; use Symfony\Component\Security\Guard\GuardAuthenticatorHandler; @@ -211,6 +214,54 @@ class GuardAuthenticationListenerTest extends TestCase $listener($this->event); } + /** + * @dataProvider exceptionsToHide + */ + public function testHandleHidesInvalidUserExceptions(AuthenticationException $exceptionToHide) + { + $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); + + $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 + ); + + $listener($this->event); + } + + public function exceptionsToHide() + { + return [ + [new UsernameNotFoundException()], + [new LockedException()], + ]; + } + public function testSupportsReturnFalseSkipAuth() { $authenticator = $this->createMock(AuthenticatorInterface::class);