bug #27016 [Security][Guard] GuardAuthenticationProvider::authenticat…

…e 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 @@
 
 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 @@ public function __construct(array $guardAuthenticators, UserProviderInterface $u
      */
     public function authenticate(TokenInterface $token)
     {
-        if (!$this->supports($token)) {
+        if (!$token instanceof GuardTokenInterface) {
             throw new \InvalidArgumentException('GuardAuthenticationProvider only supports GuardTokenInterface.');
         }
 
@@ -87,19 +88,13 @@ public function authenticate(TokenInterface $token)
             throw new AuthenticationExpiredException();
         }
 
-        // 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;
+        $guardAuthenticator = $this->findOriginatingAuthenticator($token);
 
-            if ($uniqueGuardKey == $token->getGuardProviderKey()) {
-                return $this->authenticateViaGuard($guardAuthenticator, $token);
-            }
+        if (null === $guardAuthenticator) {
+            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
-        // instances that will be checked if you have multiple firewalls.
+        return $this->authenticateViaGuard($guardAuthenticator, $token);
     }
 
     private function authenticateViaGuard(GuardAuthenticatorInterface $guardAuthenticator, PreAuthenticationGuardToken $token)
@@ -108,18 +103,11 @@ private function authenticateViaGuard(GuardAuthenticatorInterface $guardAuthenti
         $user = $guardAuthenticator->getUser($token->getCredentials(), $this->userProvider);
 
         if (null === $user) {
-            throw new UsernameNotFoundException(sprintf(
-                'Null returned from %s::getUser()',
-                get_class($guardAuthenticator)
-            ));
+            throw new UsernameNotFoundException(sprintf('Null returned from %s::getUser()', get_class($guardAuthenticator)));
         }
 
         if (!$user instanceof UserInterface) {
-            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)
-            ));
+            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)));
         }
 
         $this->userChecker->checkPreAuth($user);
@@ -131,18 +119,37 @@ private function authenticateViaGuard(GuardAuthenticatorInterface $guardAuthenti
         // turn the UserInterface into a TokenInterface
         $authenticatedToken = $guardAuthenticator->createAuthenticatedToken($user, $this->providerKey);
         if (!$authenticatedToken instanceof TokenInterface) {
-            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)
-            ));
+            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)));
         }
 
         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 @@
 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 @@ public function testGuardWithNoLongerAuthenticatedTriggersLogout()
         $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();