symfony · fabpot · Aug 25, 2019 · Aug 23, 2019
diff --git a/UPGRADE-4.4.md b/UPGRADE-4.4.md
@@ -194,6 +194,7 @@ Security
 
  * The `LdapUserProvider` class has been deprecated, use `Symfony\Component\Ldap\Security\LdapUserProvider` instead.
  * Implementations of `PasswordEncoderInterface` and `UserPasswordEncoderInterface` should add a new `needsRehash()` method
+ * Deprecated returning a non-boolean value when implementing `Guard\AuthenticatorInterface::checkCredentials()`. Please explicitly return `false` to indicate invalid credentials.
 
 Stopwatch
 ---------

diff --git a/UPGRADE-5.0.md b/UPGRADE-5.0.md
@@ -467,6 +467,7 @@ Security
  * The `BCryptPasswordEncoder` class has been removed, use `NativePasswordEncoder` instead.
  * Classes implementing the `TokenInterface` must implement the two new methods
    `__serialize` and `__unserialize`
+ * Implementations of `Guard\AuthenticatorInterface::checkCredentials()` must return a boolean value now. Please explicitly return `false` to indicate invalid credentials.
 
 SecurityBundle
 --------------

@@ -11,6 +11,7 @@ CHANGELOG
  * Added `Guard\PasswordAuthenticatedInterface`, an optional interface
    for "guard" authenticators that deal with user passwords
  * Marked all dispatched event classes as `@final`
+ * Deprecated returning a non-boolean value when implementing `Guard\AuthenticatorInterface::checkCredentials()`.
 
 4.3.0
 -----

@@ -83,9 +83,8 @@ public function getUser($credentials, UserProviderInterface $userProvider);
     /**
      * Returns true if the credentials are valid.
      *
-     * If any value other than true is returned, authentication will
-     * fail. You may also throw an AuthenticationException if you wish
-     * to cause authentication to fail.
+     * If false is returned, authentication will fail. You may also throw
+     * an AuthenticationException if you wish to cause authentication to fail.
      *
      * The *credentials* are the return value from getCredentials()
      *

@@ -113,7 +113,11 @@ private function authenticateViaGuard(AuthenticatorInterface $guardAuthenticator
         }
 
         $this->userChecker->checkPreAuth($user);
-        if (true !== $guardAuthenticator->checkCredentials($token->getCredentials(), $user)) {
+        if (true !== $checkCredentialsResult = $guardAuthenticator->checkCredentials($token->getCredentials(), $user)) {
+            if (false !== $checkCredentialsResult) {
+                @trigger_error(sprintf('%s::checkCredentials() must return a boolean value. You returned %s. This behavior is deprecated in Symfony 4.4 and will trigger a TypeError in Symfony 5.', \get_class($guardAuthenticator), \is_object($checkCredentialsResult) ? \get_class($checkCredentialsResult) : \gettype($checkCredentialsResult)), E_USER_DEPRECATED);
+            }
+
             throw new BadCredentialsException(sprintf('Authentication failed because %s::checkCredentials() did not return true.', \get_class($guardAuthenticator)));
         }
         if ($this->userProvider instanceof PasswordUpgraderInterface && $guardAuthenticator instanceof PasswordAuthenticatedInterface && null !== $this->passwordEncoder && (null !== $password = $guardAuthenticator->getPassword($token->getCredentials())) && method_exists($this->passwordEncoder, 'needsRehash') && $this->passwordEncoder->needsRehash($user)) {

@@ -13,6 +13,7 @@
 
 use PHPUnit\Framework\TestCase;
 use Symfony\Component\Security\Core\Authentication\Token\TokenInterface;
+use Symfony\Component\Security\Core\Exception\BadCredentialsException;
 use Symfony\Component\Security\Core\User\UserInterface;
 use Symfony\Component\Security\Guard\AuthenticatorInterface;
 use Symfony\Component\Security\Guard\Provider\GuardAuthenticationProvider;
@@ -87,6 +88,41 @@ public function testAuthenticate()
         $this->assertSame($authedToken, $actualAuthedToken);
     }
 
+    public function testCheckCredentialsReturningFalseFailsAuthentication()
+    {
+        $this->expectException(BadCredentialsException::class);
+        $providerKey = 'my_uncool_firewall';
+
+        $authenticator = $this->createMock(AuthenticatorInterface::class);
+
+        // make sure the authenticator is used
+        $this->preAuthenticationToken->expects($this->any())
+            ->method('getGuardProviderKey')
+            // the 0 index, to match the only authenticator
+            ->willReturn('my_uncool_firewall_0');
+
+        $this->preAuthenticationToken->expects($this->atLeastOnce())
+            ->method('getCredentials')
+            ->willReturn('non-null-value');
+
+        $mockedUser = $this->createMock(UserInterface::class);
+        $authenticator->expects($this->once())
+            ->method('getUser')
+            ->willReturn($mockedUser);
+        // checkCredentials is called
+        $authenticator->expects($this->once())
+            ->method('checkCredentials')
+            // authentication fails :(
+            ->willReturn(false);
+
+        $provider = new GuardAuthenticationProvider([$authenticator], $this->userProvider, $providerKey, $this->userChecker);
+        $provider->authenticate($this->preAuthenticationToken);
+    }
+
+    /**
+     * @group legacy
+     * @expectedDeprecation %s::checkCredentials() must return a boolean value. You returned NULL. This behavior is deprecated in Symfony 4.4 and will trigger a TypeError in Symfony 5.
+     */
     public function testCheckCredentialsReturningNonTrueFailsAuthentication()
     {
         $this->expectException('Symfony\Component\Security\Core\Exception\BadCredentialsException');