Merge branch '3.4' into 4.4

* 3.4: [Security][Guard] Prevent user enumeration via response content
symfony · May 12, 2021 · d0326e1 · d0326e1
2 parents 20f522a + ba995e9
commit d0326e1
Show file tree

Hide file tree

Showing 2 changed files with 63 additions and 1 deletion.
diff --git a/Firewall/GuardAuthenticationListener.php b/Firewall/GuardAuthenticationListener.php
@@ -17,7 +17,10 @@
 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 @@ public function __construct(GuardAuthenticatorHandler $guardHandler, Authenticat
         $this->providerKey = $providerKey;
         $this->guardAuthenticators = $guardAuthenticators;
         $this->logger = $logger;
+        $this->hideUserNotFoundExceptions = $hideUserNotFoundExceptions;
     }
 
     /**
@@ -164,6 +169,12 @@ private function executeGuardAuthenticator(string $uniqueGuardKey, Authenticator
                 $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/Tests/Firewall/GuardAuthenticationListenerTest.php b/Tests/Firewall/GuardAuthenticationListenerTest.php
@@ -19,6 +19,9 @@
 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 @@ public function testHandleCatchesAuthenticationException()
         $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);