bug #15925 Updating behavior to not continue after an authenticator has set the response (weaverryan)

This PR was merged into the 2.8 branch.

Discussion
----------

Updating behavior to not continue after an authenticator has set the response

| Q             | A
| ------------- | ---
| Bug fix?      | yes
| New feature?  | no
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets | https://github.com/symfony/symfony/pull/14673/files#r40492765
| License       | MIT
| Doc PR        | n/a

This mirrors the behavior in core: *if* a listener sets a response (on success or failure),
then the other listeners are not called. But if a response is *not* set
(which is sometimes the case for success, like in BasicAuthenticationListener),
then the other listeners are called, and can even fail.

It's all a bit of an edge-case, as only one authenticator (like authentication listener) would normally be doing any work on a request, but I think matching the other listeners (since I'm not aware of anyone having issues with its behavior) is best.

Commits
-------

5fa2684 Making all "debug" messages use the debug router
f403444 Updating behavior to not continue after an authenticator has set the response

2 files changed, 47 insertions, 8 deletions

diff --git a/Guard/Firewall/GuardAuthenticationListener.php b/Guard/Firewall/GuardAuthenticationListener.php
index 6140be0..0ac7c12 100644
--- a/Guard/Firewall/GuardAuthenticationListener.php
+++ b/Guard/Firewall/GuardAuthenticationListener.php
@@ -66,7 +66,7 @@ class GuardAuthenticationListener implements ListenerInterface
     public function handle(GetResponseEvent $event)
     {
         if (null !== $this->logger) {
-            $this->logger->info('Checking for guard authentication credentials.', array('firewall_key' => $this->providerKey, 'authenticators' => count($this->guardAuthenticators)));
+            $this->logger->debug('Checking for guard authentication credentials.', array('firewall_key' => $this->providerKey, 'authenticators' => count($this->guardAuthenticators)));
         }
 
         foreach ($this->guardAuthenticators as $key => $guardAuthenticator) {
@@ -75,6 +75,12 @@ class GuardAuthenticationListener implements ListenerInterface
             $uniqueGuardKey = $this->providerKey.'_'.$key;
 
             $this->executeGuardAuthenticator($uniqueGuardKey, $guardAuthenticator, $event);
+
+            if ($event->hasResponse()) {
+                $this->logger->debug(sprintf('The "%s" authenticator set the response. Any later authenticator will not be called', get_class($guardAuthenticator)));
+
+                break;
+            }
         }
     }
 
@@ -83,7 +89,7 @@ class GuardAuthenticationListener implements ListenerInterface
         $request = $event->getRequest();
         try {
             if (null !== $this->logger) {
-                $this->logger->info('Calling getCredentials on guard configurator.', array('firewall_key' => $this->providerKey, 'authenticator' => get_class($guardAuthenticator)));
+                $this->logger->debug('Calling getCredentials() on guard configurator.', array('firewall_key' => $this->providerKey, 'authenticator' => get_class($guardAuthenticator)));
             }
 
             // allow the authenticator to fetch authentication info from the request
@@ -98,7 +104,7 @@ class GuardAuthenticationListener implements ListenerInterface
             $token = new PreAuthenticationGuardToken($credentials, $uniqueGuardKey);
 
             if (null !== $this->logger) {
-                $this->logger->info('Passing guard token information to the GuardAuthenticationProvider', array('firewall_key' => $this->providerKey, 'authenticator' => get_class($guardAuthenticator)));
+                $this->logger->debug('Passing guard token information to the GuardAuthenticationProvider', array('firewall_key' => $this->providerKey, 'authenticator' => get_class($guardAuthenticator)));
             }
             // pass the token into the AuthenticationManager system
             // this indirectly calls GuardAuthenticationProvider::authenticate()
@@ -130,13 +136,13 @@ class GuardAuthenticationListener implements ListenerInterface
         $response = $this->guardHandler->handleAuthenticationSuccess($token, $request, $guardAuthenticator, $this->providerKey);
         if ($response instanceof Response) {
             if (null !== $this->logger) {
-                $this->logger->info('Guard authenticator set success response.', array('response' => $response, 'authenticator' => get_class($guardAuthenticator)));
+                $this->logger->debug('Guard authenticator set success response.', array('response' => $response, 'authenticator' => get_class($guardAuthenticator)));
             }
 
             $event->setResponse($response);
         } else {
             if (null !== $this->logger) {
-                $this->logger->info('Guard authenticator set no success response: request continues.', array('authenticator' => get_class($guardAuthenticator)));
+                $this->logger->debug('Guard authenticator set no success response: request continues.', array('authenticator' => get_class($guardAuthenticator)));
             }
         }
 
@@ -167,7 +173,7 @@ class GuardAuthenticationListener implements ListenerInterface
     {
         if (null === $this->rememberMeServices) {
             if (null !== $this->logger) {
-                $this->logger->info('Remember me skipped: it is not configured for the firewall.', array('authenticator' => get_class($guardAuthenticator)));
+                $this->logger->debug('Remember me skipped: it is not configured for the firewall.', array('authenticator' => get_class($guardAuthenticator)));
             }
 
             return;
@@ -175,7 +181,7 @@ class GuardAuthenticationListener implements ListenerInterface
 
         if (!$guardAuthenticator->supportsRememberMe()) {
             if (null !== $this->logger) {
-                $this->logger->info('Remember me skipped: your authenticator does not support it.', array('authenticator' => get_class($guardAuthenticator)));
+                $this->logger->debug('Remember me skipped: your authenticator does not support it.', array('authenticator' => get_class($guardAuthenticator)));
             }
 
             return;
diff --git a/Guard/Tests/Firewall/GuardAuthenticationListenerTest.php b/Guard/Tests/Firewall/GuardAuthenticationListenerTest.php
index 26d17dd..3224fee 100644
--- a/Guard/Tests/Firewall/GuardAuthenticationListenerTest.php
+++ b/Guard/Tests/Firewall/GuardAuthenticationListenerTest.php
@@ -79,6 +79,36 @@ class GuardAuthenticationListenerTest extends \PHPUnit_Framework_TestCase
         $listener->handle($this->event);
     }
 
+    public function testHandleSuccessStopsAfterResponseIsSet()
+    {
+        $authenticator1 = $this->getMock('Symfony\Component\Security\Guard\GuardAuthenticatorInterface');
+        $authenticator2 = $this->getMock('Symfony\Component\Security\Guard\GuardAuthenticatorInterface');
+
+        // mock the first authenticator to fail, and set a Response
+        $authenticator1
+            ->expects($this->once())
+            ->method('getCredentials')
+            ->willThrowException(new AuthenticationException());
+        $this->guardAuthenticatorHandler
+            ->expects($this->once())
+            ->method('handleAuthenticationFailure')
+            ->willReturn(new Response());
+        // the second authenticator should *never* be called
+        $authenticator2
+            ->expects($this->never())
+            ->method('getCredentials');
+
+        $listener = new GuardAuthenticationListener(
+            $this->guardAuthenticatorHandler,
+            $this->authenticationManager,
+            'my_firewall',
+            array($authenticator1, $authenticator2),
+            $this->logger
+        );
+
+        $listener->handle($this->event);
+    }
+
     public function testHandleSuccessWithRememberMe()
     {
         $authenticator = $this->getMock('Symfony\Component\Security\Guard\GuardAuthenticatorInterface');
@@ -201,7 +231,10 @@ class GuardAuthenticationListenerTest extends \PHPUnit_Framework_TestCase
 
         $this->request = new Request(array(), array(), array(), array(), array(), array());
 
-        $this->event = $this->getMock('Symfony\Component\HttpKernel\Event\GetResponseEvent', array(), array(), '', false);
+        $this->event = $this->getMockBuilder('Symfony\Component\HttpKernel\Event\GetResponseEvent')
+            ->disableOriginalConstructor()
+            ->setMethods(array('getRequest'))
+            ->getMock();
         $this->event
             ->expects($this->any())
             ->method('getRequest')