From ba995e9e011d1861b2cfb318057379eb726f052c Mon Sep 17 00:00:00 2001
From: Robin Chalas <robin.chalas@gmail.com>
Date: Sat, 2 Jan 2021 17:06:12 +0100
Subject: [PATCH] [Security][Guard] Prevent user enumeration via response
 content

---
 Firewall/GuardAuthenticationListener.php      | 13 ++++-
 .../GuardAuthenticationListenerTest.php       | 51 +++++++++++++++++++
 2 files changed, 63 insertions(+), 1 deletion(-)

diff --git a/Firewall/GuardAuthenticationListener.php b/Firewall/GuardAuthenticationListener.php
index 11bda3c..190290c 100644
--- a/Firewall/GuardAuthenticationListener.php
+++ b/Firewall/GuardAuthenticationListener.php
@@ -17,7 +17,10 @@
 use Symfony\Component\HttpKernel\Event\GetResponseEvent;
 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\AbstractGuardAuthenticator;
 use Symfony\Component\Security\Guard\AuthenticatorInterface;
 use Symfony\Component\Security\Guard\GuardAuthenticatorHandler;
@@ -40,6 +43,7 @@ class GuardAuthenticationListener implements ListenerInterface
     private $guardAuthenticators;
     private $logger;
     private $rememberMeServices;
+    private $hideUserNotFoundExceptions;
 
     /**
      * @param GuardAuthenticatorHandler         $guardHandler          The Guard handler
@@ -48,7 +52,7 @@ class GuardAuthenticationListener implements ListenerInterface
      * @param iterable|AuthenticatorInterface[] $guardAuthenticators   The authenticators, with keys that match what's passed to GuardAuthenticationProvider
      * @param LoggerInterface                   $logger                A LoggerInterface instance
      */
-    public function __construct(GuardAuthenticatorHandler $guardHandler, AuthenticationManagerInterface $authenticationManager, $providerKey, $guardAuthenticators, LoggerInterface $logger = null)
+    public function __construct(GuardAuthenticatorHandler $guardHandler, AuthenticationManagerInterface $authenticationManager, $providerKey, $guardAuthenticators, LoggerInterface $logger = null, $hideUserNotFoundExceptions = true)
     {
         if (empty($providerKey)) {
             throw new \InvalidArgumentException('$providerKey must not be empty.');
@@ -59,6 +63,7 @@ public function __construct(GuardAuthenticatorHandler $guardHandler, Authenticat
         $this->providerKey = $providerKey;
         $this->guardAuthenticators = $guardAuthenticators;
         $this->logger = $logger;
+        $this->hideUserNotFoundExceptions = $hideUserNotFoundExceptions;
     }
 
     /**
@@ -163,6 +168,12 @@ private function executeGuardAuthenticator($uniqueGuardKey, GuardAuthenticatorIn
                 $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
index 6572696..a2e4671 100644
--- a/Tests/Firewall/GuardAuthenticationListenerTest.php
+++ b/Tests/Firewall/GuardAuthenticationListenerTest.php
@@ -16,6 +16,9 @@
 use Symfony\Component\HttpFoundation\Response;
 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\AbstractGuardAuthenticator;
 use Symfony\Component\Security\Guard\AuthenticatorInterface;
 use Symfony\Component\Security\Guard\Firewall\GuardAuthenticationListener;
@@ -208,6 +211,54 @@ public function testHandleCatchesAuthenticationException()
         $listener->handle($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->handle($this->event);
+    }
+
+    public function exceptionsToHide()
+    {
+        return [
+            [new UsernameNotFoundException()],
+            [new LockedException()],
+        ];
+    }
+
     /**
      * @group legacy
      */