Skip to content

Commit

Permalink
Merge branch '4.4' into 5.2
Browse files Browse the repository at this point in the history
* 4.4:
  [CI][Psalm] Install stable/released PHPUnit
  [Security] Add missing Finnish translations
  [Security][Guard] Prevent user enumeration via response content
  • Loading branch information
nicolas-grekas committed May 12, 2021
2 parents 5c56326 + 2b2e821 commit c55a8f7
Show file tree
Hide file tree
Showing 2 changed files with 34 additions and 1 deletion.
14 changes: 13 additions & 1 deletion Authentication/AuthenticatorManager.php
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,11 @@
use Symfony\Component\Security\Core\Authentication\Token\TokenInterface;
use Symfony\Component\Security\Core\AuthenticationEvents;
use Symfony\Component\Security\Core\Event\AuthenticationSuccessEvent;
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\CustomUserMessageAccountStatusException;
use Symfony\Component\Security\Core\Exception\UsernameNotFoundException;
use Symfony\Component\Security\Core\User\UserInterface;
use Symfony\Component\Security\Http\Authenticator\AuthenticatorInterface;
use Symfony\Component\Security\Http\Authenticator\InteractiveAuthenticatorInterface;
Expand Down Expand Up @@ -49,18 +53,20 @@ class AuthenticatorManager implements AuthenticatorManagerInterface, UserAuthent
private $eraseCredentials;
private $logger;
private $firewallName;
private $hideUserNotFoundExceptions;

/**
* @param AuthenticatorInterface[] $authenticators
*/
public function __construct(iterable $authenticators, TokenStorageInterface $tokenStorage, EventDispatcherInterface $eventDispatcher, string $firewallName, ?LoggerInterface $logger = null, bool $eraseCredentials = true)
public function __construct(iterable $authenticators, TokenStorageInterface $tokenStorage, EventDispatcherInterface $eventDispatcher, string $firewallName, ?LoggerInterface $logger = null, bool $eraseCredentials = true, bool $hideUserNotFoundExceptions = true)
{
$this->authenticators = $authenticators;
$this->tokenStorage = $tokenStorage;
$this->eventDispatcher = $eventDispatcher;
$this->firewallName = $firewallName;
$this->logger = $logger;
$this->eraseCredentials = $eraseCredentials;
$this->hideUserNotFoundExceptions = $hideUserNotFoundExceptions;
}

/**
Expand Down Expand Up @@ -232,6 +238,12 @@ private function handleAuthenticationFailure(AuthenticationException $authentica
$this->logger->info('Authenticator failed.', ['exception' => $authenticationException, 'authenticator' => \get_class($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 comparison
if ($this->hideUserNotFoundExceptions && ($authenticationException instanceof UsernameNotFoundException || ($authenticationException instanceof AccountStatusException && !$authenticationException instanceof CustomUserMessageAccountStatusException))) {
$authenticationException = new BadCredentialsException('Bad credentials.', 0, $authenticationException);
}

$response = $authenticator->onAuthenticationFailure($request, $authenticationException);
if (null !== $response && null !== $this->logger) {
$this->logger->debug('The "{authenticator}" authenticator set the failure response.', ['authenticator' => \get_class($authenticator)]);
Expand Down
21 changes: 21 additions & 0 deletions Tests/Authentication/AuthenticatorManagerTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
use Symfony\Component\Security\Core\Authentication\Token\Storage\TokenStorageInterface;
use Symfony\Component\Security\Core\Authentication\Token\TokenInterface;
use Symfony\Component\Security\Core\Exception\BadCredentialsException;
use Symfony\Component\Security\Core\Exception\UsernameNotFoundException;
use Symfony\Component\Security\Core\User\User;
use Symfony\Component\Security\Http\Authentication\AuthenticatorManager;
use Symfony\Component\Security\Http\Authenticator\InteractiveAuthenticatorInterface;
Expand Down Expand Up @@ -232,6 +233,26 @@ public function testInteractiveAuthenticator()
$this->assertSame($this->response, $response);
}

public function testAuthenticateRequestHidesInvalidUserExceptions()
{
$invalidUserException = new UsernameNotFoundException();
$authenticator = $this->createMock(InteractiveAuthenticatorInterface::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(function ($e) use ($invalidUserException) {
return $e instanceof BadCredentialsException && $invalidUserException === $e->getPrevious();
}))
->willReturn($this->response);

$manager = $this->createManager([$authenticator]);
$response = $manager->authenticateRequest($this->request);
$this->assertSame($this->response, $response);
}

private function createAuthenticator($supports = true)
{
$authenticator = $this->createMock(InteractiveAuthenticatorInterface::class);
Expand Down

0 comments on commit c55a8f7

Please sign in to comment.