bug #27016 [Security][Guard] GuardAuthenticationProvider::authenticate cannot return null (biomedia-thomas)

This PR was merged into the 2.8 branch.

Discussion
----------

[Security][Guard] GuardAuthenticationProvider::authenticate cannot return null

| Q             | A
| ------------- | ---
| Branch?       | 2.8
| Bug fix?      | yes
| New feature?  | no
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets | #26942
| License       | MIT

Authenticate method in GuardAuthenticationProvider returned null when the token does not originate from any of the guard authenticators. This check was not done in the supports method. According to the interface authenticate cannot return null. This patch copies theguard authenticator checks to the supports method.

Commits
-------

9dff22c [Security] guardAuthenticationProvider::authenticate cannot return null according to interface specification

src/Symfony/Component/Security/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\AuthenticationException;
 use Symfony\Component\Security\Core\Exception\BadCredentialsException;
 use Symfony\Component\Security\Core\Exception\UsernameNotFoundException;
 use Symfony\Component\Security\Guard\GuardAuthenticatorInterface;
@@ -63,7 +64,7 @@ class GuardAuthenticationProvider implements AuthenticationProviderInterface
      */
     public function authenticate(TokenInterface $token)
     {
-        if (!$this->supports($token)) {
+        if (!$token instanceof GuardTokenInterface) {
             throw new \InvalidArgumentException('GuardAuthenticationProvider only supports GuardTokenInterface.');
         }
 
@@ -87,19 +88,13 @@ class GuardAuthenticationProvider implements AuthenticationProviderInterface
             throw new AuthenticationExpiredException();
         }
 
-        // find the *one* GuardAuthenticator that this token originated from
+        $guardAuthenticator = $this->findOriginatingAuthenticator($token);
-        foreach ($this->guardAuthenticators as $key => $guardAuthenticator) {
-            // get a key that's unique to *this* guard authenticator
-            // this MUST be the same as GuardAuthenticationListener
-            $uniqueGuardKey = $this->providerKey.'_'.$key;
 
-            if ($uniqueGuardKey == $token->getGuardProviderKey()) {
+        if (null === $guardAuthenticator) {
-                return $this->authenticateViaGuard($guardAuthenticator, $token);
+            throw new AuthenticationException(sprintf('Token with provider key "%s" did not originate from any of the guard authenticators of provider "%s".', $token->getGuardProviderKey(), $this->providerKey));
-            }
         }
 
-        // no matching authenticator found - but there will be multiple GuardAuthenticationProvider
+        return $this->authenticateViaGuard($guardAuthenticator, $token);
-        // instances that will be checked if you have multiple firewalls.
     }
 
     private function authenticateViaGuard(GuardAuthenticatorInterface $guardAuthenticator, PreAuthenticationGuardToken $token)
@@ -108,18 +103,11 @@ class GuardAuthenticationProvider implements AuthenticationProviderInterface
         $user = $guardAuthenticator->getUser($token->getCredentials(), $this->userProvider);
 
         if (null === $user) {
-            throw new UsernameNotFoundException(sprintf(
+            throw new UsernameNotFoundException(sprintf('Null returned from %s::getUser()', get_class($guardAuthenticator)));
-                'Null returned from %s::getUser()',
-                get_class($guardAuthenticator)
-            ));
         }
 
         if (!$user instanceof UserInterface) {
-            throw new \UnexpectedValueException(sprintf(
+            throw new \UnexpectedValueException(sprintf('The %s::getUser() method must return a UserInterface. You returned %s.', get_class($guardAuthenticator), is_object($user) ? get_class($user) : gettype($user)));
-                'The %s::getUser() method must return a UserInterface. You returned %s.',
-                get_class($guardAuthenticator),
-                is_object($user) ? get_class($user) : gettype($user)
-            ));
         }
 
         $this->userChecker->checkPreAuth($user);
@@ -131,18 +119,37 @@ class GuardAuthenticationProvider implements AuthenticationProviderInterface
         // turn the UserInterface into a TokenInterface
         $authenticatedToken = $guardAuthenticator->createAuthenticatedToken($user, $this->providerKey);
         if (!$authenticatedToken instanceof TokenInterface) {
-            throw new \UnexpectedValueException(sprintf(
+            throw new \UnexpectedValueException(sprintf('The %s::createAuthenticatedToken() method must return a TokenInterface. You returned %s.', get_class($guardAuthenticator), is_object($authenticatedToken) ? get_class($authenticatedToken) : gettype($authenticatedToken)));
-                'The %s::createAuthenticatedToken() method must return a TokenInterface. You returned %s.',
-                get_class($guardAuthenticator),
-                is_object($authenticatedToken) ? get_class($authenticatedToken) : gettype($authenticatedToken)
-            ));
         }
 
         return $authenticatedToken;
     }
 
+    private function findOriginatingAuthenticator(PreAuthenticationGuardToken $token)
+    {
+        // find the *one* GuardAuthenticator that this token originated from
+        foreach ($this->guardAuthenticators as $key => $guardAuthenticator) {
+            // get a key that's unique to *this* guard authenticator
+            // this MUST be the same as GuardAuthenticationListener
+            $uniqueGuardKey = $this->providerKey.'_'.$key;
+
+            if ($uniqueGuardKey === $token->getGuardProviderKey()) {
+                return $guardAuthenticator;
+            }
+        }
+
+        // no matching authenticator found - but there will be multiple GuardAuthenticationProvider
+        // instances that will be checked if you have multiple firewalls.
+
+        return null;
+    }
+
     public function supports(TokenInterface $token)
     {
+        if ($token instanceof PreAuthenticationGuardToken) {
+            return null !== $this->findOriginatingAuthenticator($token);
+        }
+
         return $token instanceof GuardTokenInterface;
     }
 }

src/Symfony/Component/Security/Guard/Tests/Provider/GuardAuthenticationProviderTest.php

@@ -14,6 +14,7 @@ namespace Symfony\Component\Security\Guard\Tests\Provider;
 use PHPUnit\Framework\TestCase;
 use Symfony\Component\Security\Guard\Provider\GuardAuthenticationProvider;
 use Symfony\Component\Security\Guard\Token\PostAuthenticationGuardToken;
+use Symfony\Component\Security\Guard\Token\PreAuthenticationGuardToken;
 
 /**
  * @author Ryan Weaver <weaverryan@gmail.com>
@@ -133,6 +134,40 @@ class GuardAuthenticationProviderTest extends TestCase
         $actualToken = $provider->authenticate($token);
     }
 
+    public function testSupportsChecksGuardAuthenticatorsTokenOrigin()
+    {
+        $authenticatorA = $this->getMockBuilder('Symfony\Component\Security\Guard\GuardAuthenticatorInterface')->getMock();
+        $authenticatorB = $this->getMockBuilder('Symfony\Component\Security\Guard\GuardAuthenticatorInterface')->getMock();
+        $authenticators = array($authenticatorA, $authenticatorB);
+
+        $mockedUser = $this->getMockBuilder('Symfony\Component\Security\Core\User\UserInterface')->getMock();
+        $provider = new GuardAuthenticationProvider($authenticators, $this->userProvider, 'first_firewall', $this->userChecker);
+
+        $token = new PreAuthenticationGuardToken($mockedUser, 'first_firewall_1');
+        $supports = $provider->supports($token);
+        $this->assertTrue($supports);
+
+        $token = new PreAuthenticationGuardToken($mockedUser, 'second_firewall_0');
+        $supports = $provider->supports($token);
+        $this->assertFalse($supports);
+    }
+
+    /**
+     * @expectedException \Symfony\Component\Security\Core\Exception\AuthenticationException
+     * @expectedExceptionMessageRegExp /second_firewall_0/
+     */
+    public function testAuthenticateFailsOnNonOriginatingToken()
+    {
+        $authenticatorA = $this->getMockBuilder('Symfony\Component\Security\Guard\GuardAuthenticatorInterface')->getMock();
+        $authenticators = array($authenticatorA);
+
+        $mockedUser = $this->getMockBuilder('Symfony\Component\Security\Core\User\UserInterface')->getMock();
+        $provider = new GuardAuthenticationProvider($authenticators, $this->userProvider, 'first_firewall', $this->userChecker);
+
+        $token = new PreAuthenticationGuardToken($mockedUser, 'second_firewall_0');
+        $provider->authenticate($token);
+    }
+
     protected function setUp()
     {
         $this->userProvider = $this->getMockBuilder('Symfony\Component\Security\Core\User\UserProviderInterface')->getMock();