feature #16395 checkCredentials() force it to be an affirmative yes! (weaverryan)

This PR was squashed before being merged into the 2.8 branch (closes #16395).

Discussion
----------

checkCredentials() force it to be an affirmative yes!

| Q             | A
| ------------- | ---
| Bug fix?      | no
| New feature?  | no
| BC breaks?    | no (because 2.8 isn't released)
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets | n/a
| License       | MIT
| Doc PR        | n/a

This changes `GuardAuthenticatorInterface::checkCredentials()`: you now *must* return true in order for authentication to pass.

Before: You could do nothing (i.e. return null) and authentication would pass. You threw an AuthenticationException to cause a failure.

New: You *must* return `true` for authentication to pass. If you do nothing, we will throw a `BadCredentialsException` on your behalf. You can still throw your own exception.

This was a suggestion at symfony_live to make things more secure. I think it makes sense.

Commits
-------

14acadd checkCredentials() force it to be an affirmative yes!

3 files changed, 45 insertions, 3 deletions

diff --git a/Guard/GuardAuthenticatorInterface.php b/Guard/GuardAuthenticatorInterface.php
index 2db313c..6e62ae6 100644
--- a/Guard/GuardAuthenticatorInterface.php
+++ b/Guard/GuardAuthenticatorInterface.php
@@ -73,7 +73,11 @@ interface GuardAuthenticatorInterface extends AuthenticationEntryPointInterface
     public function getUser($credentials, UserProviderInterface $userProvider);
 
     /**
-     * Throw an AuthenticationException if the credentials are invalid.
+     * 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.
      *
      * The *credentials* are the return value from getCredentials()
      *
diff --git a/Guard/Provider/GuardAuthenticationProvider.php b/Guard/Provider/GuardAuthenticationProvider.php
index 2a58085..4347e02 100644
--- a/Guard/Provider/GuardAuthenticationProvider.php
+++ b/Guard/Provider/GuardAuthenticationProvider.php
@@ -13,6 +13,7 @@ namespace Symfony\Component\Security\Guard\Provider;
 
 use Symfony\Component\Security\Core\Authentication\Provider\AuthenticationProviderInterface;
 use Symfony\Component\Security\Core\Authentication\Token\AnonymousToken;
+use Symfony\Component\Security\Core\Exception\BadCredentialsException;
 use Symfony\Component\Security\Core\Exception\UsernameNotFoundException;
 use Symfony\Component\Security\Guard\GuardAuthenticatorInterface;
 use Symfony\Component\Security\Guard\Token\GuardTokenInterface;
@@ -122,7 +123,9 @@ class GuardAuthenticationProvider implements AuthenticationProviderInterface
         }
 
         $this->userChecker->checkPreAuth($user);
-        $guardAuthenticator->checkCredentials($token->getCredentials(), $user);
+        if (true !== $guardAuthenticator->checkCredentials($token->getCredentials(), $user)) {
+            throw new BadCredentialsException(sprintf('Authentication failed because %s::checkCredentials() did not return true.', get_class($guardAuthenticator)));
+        }
         $this->userChecker->checkPostAuth($user);
 
         // turn the UserInterface into a TokenInterface
diff --git a/Guard/Tests/Provider/GuardAuthenticationProviderTest.php b/Guard/Tests/Provider/GuardAuthenticationProviderTest.php
index 33c00e5..bfcf24b 100644
--- a/Guard/Tests/Provider/GuardAuthenticationProviderTest.php
+++ b/Guard/Tests/Provider/GuardAuthenticationProviderTest.php
@@ -60,7 +60,9 @@ class GuardAuthenticationProviderTest extends \PHPUnit_Framework_TestCase
         // checkCredentials is called
         $authenticatorB->expects($this->once())
             ->method('checkCredentials')
-            ->with($enteredCredentials, $mockedUser);
+            ->with($enteredCredentials, $mockedUser)
+            // authentication works!
+            ->will($this->returnValue(true));
         $authedToken = $this->getMock('Symfony\Component\Security\Core\Authentication\Token\TokenInterface');
         $authenticatorB->expects($this->once())
             ->method('createAuthenticatedToken')
@@ -81,6 +83,39 @@ class GuardAuthenticationProviderTest extends \PHPUnit_Framework_TestCase
     }
 
     /**
+     * @expectedException \Symfony\Component\Security\Core\Exception\BadCredentialsException
+     */
+    public function testCheckCredentialsReturningNonTrueFailsAuthentication()
+    {
+        $providerKey = 'my_uncool_firewall';
+
+        $authenticator = $this->getMock('Symfony\Component\Security\Guard\GuardAuthenticatorInterface');
+
+        // make sure the authenticator is used
+        $this->preAuthenticationToken->expects($this->any())
+            ->method('getGuardProviderKey')
+            // the 0 index, to match the only authenticator
+            ->will($this->returnValue('my_uncool_firewall_0'));
+
+        $this->preAuthenticationToken->expects($this->atLeastOnce())
+            ->method('getCredentials')
+            ->will($this->returnValue('non-null-value'));
+
+        $mockedUser = $this->getMock('Symfony\Component\Security\Core\User\UserInterface');
+        $authenticator->expects($this->once())
+            ->method('getUser')
+            ->will($this->returnValue($mockedUser));
+        // checkCredentials is called
+        $authenticator->expects($this->once())
+            ->method('checkCredentials')
+            // authentication fails :(
+            ->will($this->returnValue(null));
+
+        $provider = new GuardAuthenticationProvider(array($authenticator), $this->userProvider, $providerKey, $this->userChecker);
+        $provider->authenticate($this->preAuthenticationToken);
+    }
+
+    /**
      * @expectedException \Symfony\Component\Security\Core\Exception\AuthenticationExpiredException
      */
     public function testGuardWithNoLongerAuthenticatedTriggersLogout()