Merge branch '3.4' into 4.4

* 3.4:
  [Security][Guard] Prevent user enumeration via response content
This commit is contained in:
Nicolas Grekas 2021-05-12 14:42:28 +02:00
commit d5c0fbac85
5 changed files with 70 additions and 6 deletions

View File

@ -17,7 +17,7 @@
<argument type="service" id="security.authentication.session_strategy" /> <argument type="service" id="security.authentication.session_strategy" />
</call> </call>
</service> </service>
<service id="Symfony\Component\Security\Guard\GuardAuthenticatorHandler" alias="security.authentication.guard_handler" /> <service id="Symfony\Component\Security\Guard\GuardAuthenticatorHandler" alias="security.authentication.guard_handler" />
<!-- See GuardAuthenticationFactory --> <!-- See GuardAuthenticationFactory -->
@ -42,6 +42,7 @@
<argument /> <!-- Provider-shared Key --> <argument /> <!-- Provider-shared Key -->
<argument /> <!-- Authenticator --> <argument /> <!-- Authenticator -->
<argument type="service" id="logger" on-invalid="null" /> <argument type="service" id="logger" on-invalid="null" />
<argument>%security.authentication.hide_user_not_found%</argument>
</service> </service>
</services> </services>
</container> </container>

View File

@ -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\SwitchUserToken;
use Symfony\Component\Security\Core\Authentication\Token\TokenInterface; use Symfony\Component\Security\Core\Authentication\Token\TokenInterface;
use Symfony\Component\Security\Core\Authentication\Token\UsernamePasswordToken; 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\AuthenticationException;
use Symfony\Component\Security\Core\Exception\AuthenticationServiceException; use Symfony\Component\Security\Core\Exception\AuthenticationServiceException;
use Symfony\Component\Security\Core\Exception\BadCredentialsException; use Symfony\Component\Security\Core\Exception\BadCredentialsException;
@ -80,7 +81,7 @@ abstract class UserAuthenticationProvider implements AuthenticationProviderInter
$this->userChecker->checkPreAuth($user); $this->userChecker->checkPreAuth($user);
$this->checkAuthentication($user, $token); $this->checkAuthentication($user, $token);
$this->userChecker->checkPostAuth($user); $this->userChecker->checkPostAuth($user);
} catch (BadCredentialsException $e) { } catch (AccountStatusException $e) {
if ($this->hideUserNotFoundExceptions) { if ($this->hideUserNotFoundExceptions) {
throw new BadCredentialsException('Bad credentials.', 0, $e); throw new BadCredentialsException('Bad credentials.', 0, $e);
} }

View File

@ -83,7 +83,7 @@ class UserAuthenticationProviderTest extends TestCase
public function testAuthenticateWhenPreChecksFails() public function testAuthenticateWhenPreChecksFails()
{ {
$this->expectException(CredentialsExpiredException::class); $this->expectException(BadCredentialsException::class);
$userChecker = $this->createMock(UserCheckerInterface::class); $userChecker = $this->createMock(UserCheckerInterface::class);
$userChecker->expects($this->once()) $userChecker->expects($this->once())
->method('checkPreAuth') ->method('checkPreAuth')
@ -101,7 +101,7 @@ class UserAuthenticationProviderTest extends TestCase
public function testAuthenticateWhenPostChecksFails() public function testAuthenticateWhenPostChecksFails()
{ {
$this->expectException(AccountExpiredException::class); $this->expectException(BadCredentialsException::class);
$userChecker = $this->createMock(UserCheckerInterface::class); $userChecker = $this->createMock(UserCheckerInterface::class);
$userChecker->expects($this->once()) $userChecker->expects($this->once())
->method('checkPostAuth') ->method('checkPostAuth')
@ -128,7 +128,7 @@ class UserAuthenticationProviderTest extends TestCase
; ;
$provider->expects($this->once()) $provider->expects($this->once())
->method('checkAuthentication') ->method('checkAuthentication')
->willThrowException(new BadCredentialsException()) ->willThrowException(new CredentialsExpiredException())
; ;
$provider->authenticate($this->getSupportedToken()); $provider->authenticate($this->getSupportedToken());

View File

@ -17,7 +17,10 @@ use Symfony\Component\HttpFoundation\Response;
use Symfony\Component\HttpKernel\Event\RequestEvent; use Symfony\Component\HttpKernel\Event\RequestEvent;
use Symfony\Component\Security\Core\Authentication\AuthenticationManagerInterface; use Symfony\Component\Security\Core\Authentication\AuthenticationManagerInterface;
use Symfony\Component\Security\Core\Authentication\Token\TokenInterface; 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\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\AuthenticatorInterface;
use Symfony\Component\Security\Guard\GuardAuthenticatorHandler; use Symfony\Component\Security\Guard\GuardAuthenticatorHandler;
use Symfony\Component\Security\Guard\Token\PreAuthenticationGuardToken; use Symfony\Component\Security\Guard\Token\PreAuthenticationGuardToken;
@ -44,12 +47,13 @@ class GuardAuthenticationListener extends AbstractListener implements ListenerIn
private $guardAuthenticators; private $guardAuthenticators;
private $logger; private $logger;
private $rememberMeServices; private $rememberMeServices;
private $hideUserNotFoundExceptions;
/** /**
* @param string $providerKey The provider (i.e. firewall) key * @param string $providerKey The provider (i.e. firewall) key
* @param iterable|AuthenticatorInterface[] $guardAuthenticators The authenticators, with keys that match what's passed to GuardAuthenticationProvider * @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)) { if (empty($providerKey)) {
throw new \InvalidArgumentException('$providerKey must not be empty.'); throw new \InvalidArgumentException('$providerKey must not be empty.');
@ -60,6 +64,7 @@ class GuardAuthenticationListener extends AbstractListener implements ListenerIn
$this->providerKey = $providerKey; $this->providerKey = $providerKey;
$this->guardAuthenticators = $guardAuthenticators; $this->guardAuthenticators = $guardAuthenticators;
$this->logger = $logger; $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)]); $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); $response = $this->guardHandler->handleAuthenticationFailure($e, $request, $guardAuthenticator, $this->providerKey);
if ($response instanceof Response) { if ($response instanceof Response) {

View File

@ -19,6 +19,9 @@ use Symfony\Component\HttpKernel\Event\RequestEvent;
use Symfony\Component\Security\Core\Authentication\AuthenticationProviderManager; use Symfony\Component\Security\Core\Authentication\AuthenticationProviderManager;
use Symfony\Component\Security\Core\Authentication\Token\TokenInterface; use Symfony\Component\Security\Core\Authentication\Token\TokenInterface;
use Symfony\Component\Security\Core\Exception\AuthenticationException; 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\AuthenticatorInterface;
use Symfony\Component\Security\Guard\Firewall\GuardAuthenticationListener; use Symfony\Component\Security\Guard\Firewall\GuardAuthenticationListener;
use Symfony\Component\Security\Guard\GuardAuthenticatorHandler; use Symfony\Component\Security\Guard\GuardAuthenticatorHandler;
@ -211,6 +214,54 @@ class GuardAuthenticationListenerTest extends TestCase
$listener($this->event); $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() public function testSupportsReturnFalseSkipAuth()
{ {
$authenticator = $this->createMock(AuthenticatorInterface::class); $authenticator = $this->createMock(AuthenticatorInterface::class);