[Security] made code easier to understand, added some missing unit tests

2 files changed, 248 insertions, 75 deletions

diff --git a/Http/Firewall/ExceptionListener.php b/Http/Firewall/ExceptionListener.php
index b2aaa04..e7e2989 100644
--- a/Http/Firewall/ExceptionListener.php
+++ b/Http/Firewall/ExceptionListener.php
@@ -81,100 +81,83 @@ class ExceptionListener
         $event->getDispatcher()->removeListener(KernelEvents::EXCEPTION, array($this, 'onKernelException'));
 
         $exception = $event->getException();
-        $request = $event->getRequest();
-
-        while (null !== $exception) {
+        do {
             if ($exception instanceof AuthenticationException) {
-                if (null !== $this->logger) {
-                    $this->logger->info(sprintf('Authentication exception occurred; redirecting to authentication entry point (%s)', $exception->getMessage()));
-                }
-
-                try {
-                    $response = $this->startAuthentication($request, $exception);
-
-                    break;
-                } catch (\Exception $e) {
-                    $event->setException($e);
-
-                    return;
-                }
+                return $this->handleAuthenticationException($event, $exception);
+            } elseif ($exception instanceof AccessDeniedException) {
+                return $this->handleAccessDeniedException($event, $exception);
+            } elseif ($exception instanceof LogoutException) {
+                return $this->handleLogoutException($event, $exception);
             }
+        } while (null !== $exception = $exception->getPrevious());
+    }
 
-            if ($exception instanceof AccessDeniedException) {
-                $event->setException(new AccessDeniedHttpException($exception->getMessage(), $exception));
-
-                $token = $this->context->getToken();
-                if (!$this->authenticationTrustResolver->isFullFledged($token)) {
-                    if (null !== $this->logger) {
-                        $this->logger->debug(sprintf('Access is denied (user is not fully authenticated) by "%s" at line %s; redirecting to authentication entry point', $exception->getFile(), $exception->getLine()));
-                    }
-
-                    try {
-                        $insufficientAuthenticationException = new InsufficientAuthenticationException('Full authentication is required to access this resource.', 0, $exception);
-                        $insufficientAuthenticationException->setToken($token);
-                        $response = $this->startAuthentication($request, $insufficientAuthenticationException);
-
-                        break;
-                    } catch (\Exception $e) {
-                        $event->setException($e);
-
-                        return;
-                    }
-                } else {
-                    if (null !== $this->logger) {
-                        $this->logger->debug(sprintf('Access is denied (and user is neither anonymous, nor remember-me) by "%s" at line %s', $exception->getFile(), $exception->getLine()));
-                    }
-
-                    try {
-                        if (null !== $this->accessDeniedHandler) {
-                            $response = $this->accessDeniedHandler->handle($request, $exception);
-
-                            if (!$response instanceof Response) {
-                                return;
-                            }
+    private function handleAuthenticationException(GetResponseForExceptionEvent $event, AuthenticationException $exception)
+    {
+        if (null !== $this->logger) {
+            $this->logger->info(sprintf('Authentication exception occurred; redirecting to authentication entry point (%s)', $exception->getMessage()));
+        }
 
-                            break;
-                        }
+        try {
+            $event->setResponse($this->startAuthentication($event->getRequest(), $exception));
+        } catch (\Exception $e) {
+            $event->setException($e);
+        }
+    }
 
-                       if (null !== $this->errorPage) {
-                            $subRequest = $this->httpUtils->createRequest($request, $this->errorPage);
-                            $subRequest->attributes->set(SecurityContextInterface::ACCESS_DENIED_ERROR, $exception);
+    private function handleAccessDeniedException(GetResponseForExceptionEvent $event, AccessDeniedException $exception)
+    {
+        $event->setException(new AccessDeniedHttpException($exception->getMessage(), $exception));
 
-                            $response = $event->getKernel()->handle($subRequest, HttpKernelInterface::SUB_REQUEST, true);
+        $token = $this->context->getToken();
+        if (!$this->authenticationTrustResolver->isFullFledged($token)) {
+            if (null !== $this->logger) {
+                $this->logger->debug(sprintf('Access is denied (user is not fully authenticated) by "%s" at line %s; redirecting to authentication entry point', $exception->getFile(), $exception->getLine()));
+            }
 
-                            break;
-                       }
+            try {
+                $insufficientAuthenticationException = new InsufficientAuthenticationException('Full authentication is required to access this resource.', 0, $exception);
+                $insufficientAuthenticationException->setToken($token);
 
-                       return;
+                $event->setResponse($this->startAuthentication($event->getRequest(), $insufficientAuthenticationException));
+            } catch (\Exception $e) {
+                $event->setException($e);
+            }
 
-                    } catch (\Exception $e) {
-                        if (null !== $this->logger) {
-                            $this->logger->error(sprintf('Exception thrown when handling an exception (%s: %s)', get_class($e), $e->getMessage()));
-                        }
+            return;
+        }
 
-                        $event->setException(new \RuntimeException('Exception thrown when handling an exception.', 0, $e));
+        if (null !== $this->logger) {
+            $this->logger->debug(sprintf('Access is denied (and user is neither anonymous, nor remember-me) by "%s" at line %s', $exception->getFile(), $exception->getLine()));
+        }
 
-                        return;
-                    }
-                }
-            }
+        try {
+            if (null !== $this->accessDeniedHandler) {
+                $response = $this->accessDeniedHandler->handle($event->getRequest(), $exception);
 
-            if ($exception instanceof LogoutException) {
-                if (null !== $this->logger) {
-                    $this->logger->info(sprintf('Logout exception occurred; wrapping with AccessDeniedHttpException (%s)', $exception->getMessage()));
+                if ($response instanceof Response) {
+                    $event->setResponse($response);
                 }
+            } elseif (null !== $this->errorPage) {
+                $subRequest = $this->httpUtils->createRequest($event->getRequest(), $this->errorPage);
+                $subRequest->attributes->set(SecurityContextInterface::ACCESS_DENIED_ERROR, $exception);
 
-                return;
+                $event->setResponse($event->getKernel()->handle($subRequest, HttpKernelInterface::SUB_REQUEST, true));
             }
-
-            if (null === $exception->getPrevious()) {
-                return;
+        } catch (\Exception $e) {
+            if (null !== $this->logger) {
+                $this->logger->error(sprintf('Exception thrown when handling an exception (%s: %s)', get_class($e), $e->getMessage()));
             }
 
-            $exception = $exception->getPrevious();
+            $event->setException(new \RuntimeException('Exception thrown when handling an exception.', 0, $e));
         }
+    }
 
-        $event->setResponse($response);
+    private function handleLogoutException(GetResponseForExceptionEvent $event, LogoutException $exception)
+    {
+        if (null !== $this->logger) {
+            $this->logger->info(sprintf('Logout exception occurred; wrapping with AccessDeniedHttpException (%s)', $exception->getMessage()));
+        }
     }
 
     /**
diff --git a/Tests/Http/Firewall/ExceptionListenerTest.php b/Tests/Http/Firewall/ExceptionListenerTest.php
new file mode 100644
index 0000000..6e996cd
--- /dev/null
+++ b/Tests/Http/Firewall/ExceptionListenerTest.php
@@ -0,0 +1,190 @@
+<?php
+
+/*
+ * This file is part of the Symfony package.
+ *
+ * (c) Fabien Potencier <fabien@symfony.com>
+ *
+ * For the full copyright and license information, please view the LICENSE
+ * file that was distributed with this source code.
+ */
+
+namespace Symfony\Component\Security\Tests\Http\Firewall;
+
+use Symfony\Component\HttpFoundation\Request;
+use Symfony\Component\HttpFoundation\Response;
+use Symfony\Component\HttpKernel\Event\GetResponseForExceptionEvent;
+use Symfony\Component\HttpKernel\HttpKernelInterface;
+use Symfony\Component\Security\Core\Authentication\AuthenticationTrustResolverInterface;
+use Symfony\Component\Security\Core\Exception\AccessDeniedException;
+use Symfony\Component\Security\Core\Exception\AuthenticationException;
+use Symfony\Component\Security\Core\SecurityContextInterface;
+use Symfony\Component\Security\Http\Authorization\AccessDeniedHandlerInterface;
+use Symfony\Component\Security\Http\EntryPoint\AuthenticationEntryPointInterface;
+use Symfony\Component\Security\Http\Firewall\ExceptionListener;
+use Symfony\Component\Security\Http\HttpUtils;
+
+class ExceptionListenerTest extends \PHPUnit_Framework_TestCase
+{
+    /**
+     * @dataProvider getAuthenticationExceptionProvider
+     */
+    public function testAuthenticationExceptionWithoutEntryPoint(\Exception $exception, \Exception $eventException = null)
+    {
+        $event = $this->createEvent($exception);
+
+        $listener = $this->createExceptionListener();
+        $listener->onKernelException($event);
+
+        $this->assertNull($event->getResponse());
+        $this->assertSame(null === $eventException ? $exception : $eventException, $event->getException());
+    }
+
+    /**
+     * @dataProvider getAuthenticationExceptionProvider
+     */
+    public function testAuthenticationExceptionWithEntryPoint(\Exception $exception, \Exception $eventException = null)
+    {
+        $event = $this->createEvent($exception = new AuthenticationException());
+
+        $listener = $this->createExceptionListener(null, null, null, $this->createEntryPoint());
+        $listener->onKernelException($event);
+
+        $this->assertEquals('OK', $event->getResponse()->getContent());
+        $this->assertSame($exception, $event->getException());
+    }
+
+    public function getAuthenticationExceptionProvider()
+    {
+        return array(
+            array(new AuthenticationException()),
+            array(new \LogicException('random', 0, $e = new AuthenticationException()), $e),
+            array(new \LogicException('random', 0, $e = new AuthenticationException('embed', 0, new AuthenticationException())), $e),
+            array(new \LogicException('random', 0, $e = new AuthenticationException('embed', 0, new AccessDeniedException())), $e),
+            array(new AuthenticationException('random', 0, new \LogicException())),
+        );
+    }
+
+    /**
+     * @dataProvider getAccessDeniedExceptionProvider
+     */
+    public function testAccessDeniedExceptionFullFledgedAndWithoutAccessDeniedHandlerAndWithoutErrorPage(\Exception $exception, \Exception $eventException = null)
+    {
+        $event = $this->createEvent($exception);
+
+        $listener = $this->createExceptionListener(null, $this->createTrustResolver(true));
+        $listener->onKernelException($event);
+
+        $this->assertNull($event->getResponse());
+        $this->assertSame(null === $eventException ? $exception : $eventException, $event->getException()->getPrevious());
+    }
+
+    /**
+     * @dataProvider getAccessDeniedExceptionProvider
+     */
+    public function testAccessDeniedExceptionFullFledgedAndWithoutAccessDeniedHandlerAndWithErrorPage(\Exception $exception, \Exception $eventException = null)
+    {
+        $kernel = $this->getMock('Symfony\Component\HttpKernel\HttpKernelInterface');
+        $kernel->expects($this->once())->method('handle')->will($this->returnValue(new Response('error')));
+
+        $event = $this->createEvent($exception, $kernel);
+
+        $httpUtils = $this->getMock('Symfony\Component\Security\Http\HttpUtils');
+        $httpUtils->expects($this->once())->method('createRequest')->will($this->returnValue(Request::create('/error')));
+
+        $listener = $this->createExceptionListener(null, $this->createTrustResolver(true), $httpUtils, null, '/error');
+        $listener->onKernelException($event);
+
+        $this->assertEquals('error', $event->getResponse()->getContent());
+        $this->assertSame(null === $eventException ? $exception : $eventException, $event->getException()->getPrevious());
+    }
+
+    /**
+     * @dataProvider getAccessDeniedExceptionProvider
+     */
+    public function testAccessDeniedExceptionFullFledgedAndWithAccessDeniedHandlerAndWithoutErrorPage(\Exception $exception, \Exception $eventException = null)
+    {
+        $event = $this->createEvent($exception);
+
+        $accessDeniedHandler = $this->getMock('Symfony\Component\Security\Http\Authorization\AccessDeniedHandlerInterface');
+        $accessDeniedHandler->expects($this->once())->method('handle')->will($this->returnValue(new Response('error')));
+
+        $listener = $this->createExceptionListener(null, $this->createTrustResolver(true), null, null, null, $accessDeniedHandler);
+        $listener->onKernelException($event);
+
+        $this->assertEquals('error', $event->getResponse()->getContent());
+        $this->assertSame(null === $eventException ? $exception : $eventException, $event->getException()->getPrevious());
+    }
+
+    /**
+     * @dataProvider getAccessDeniedExceptionProvider
+     */
+    public function testAccessDeniedExceptionNotFullFledged(\Exception $exception, \Exception $eventException = null)
+    {
+        $event = $this->createEvent($exception);
+
+        $context = $this->getMock('Symfony\Component\Security\Core\SecurityContextInterface');
+        $context->expects($this->once())->method('getToken')->will($this->returnValue($this->getMock('Symfony\Component\Security\Core\Authentication\Token\TokenInterface')));
+
+        $listener = $this->createExceptionListener($context, $this->createTrustResolver(false), null, $this->createEntryPoint());
+        $listener->onKernelException($event);
+
+        $this->assertEquals('OK', $event->getResponse()->getContent());
+        $this->assertSame(null === $eventException ? $exception : $eventException, $event->getException()->getPrevious());
+    }
+
+    public function getAccessDeniedExceptionProvider()
+    {
+        return array(
+            array(new AccessDeniedException()),
+            array(new \LogicException('random', 0, $e = new AccessDeniedException()), $e),
+            array(new \LogicException('random', 0, $e = new AccessDeniedException('embed', new AccessDeniedException())), $e),
+            array(new \LogicException('random', 0, $e = new AccessDeniedException('embed', new AuthenticationException())), $e),
+            array(new AccessDeniedException('random', new \LogicException())),
+        );
+    }
+
+    private function createEntryPoint()
+    {
+        $entryPoint = $this->getMock('Symfony\Component\Security\Http\EntryPoint\AuthenticationEntryPointInterface');
+        $entryPoint->expects($this->once())->method('start')->will($this->returnValue(new Response('OK')));
+
+        return $entryPoint;
+    }
+
+    private function createTrustResolver($fullFledged)
+    {
+        $trustResolver = $this->getMock('Symfony\Component\Security\Core\Authentication\AuthenticationTrustResolverInterface');
+        $trustResolver->expects($this->once())->method('isFullFledged')->will($this->returnValue($fullFledged));
+
+        return $trustResolver;
+    }
+
+    private function createEvent(\Exception $exception, $kernel = null)
+    {
+        if (null === $kernel) {
+            $kernel = $this->getMock('Symfony\Component\HttpKernel\HttpKernelInterface');
+        }
+
+        $event = new GetResponseForExceptionEvent($kernel, Request::create('/'), HttpKernelInterface::MASTER_REQUEST, $exception);
+
+        // FIXME: to be removed in 2.4
+        $dispatcher = $this->getMock('Symfony\Component\EventDispatcher\EventDispatcherInterface');
+        $event->setDispatcher($dispatcher);
+
+        return $event;
+    }
+
+    private function createExceptionListener(SecurityContextInterface $context = null, AuthenticationTrustResolverInterface $trustResolver = null, HttpUtils $httpUtils = null, AuthenticationEntryPointInterface $authenticationEntryPoint = null, $errorPage = null, AccessDeniedHandlerInterface $accessDeniedHandler = null)
+    {
+        return new ExceptionListener(
+            $context ? $context : $this->getMock('Symfony\Component\Security\Core\SecurityContextInterface'),
+            $trustResolver ? $trustResolver : $this->getMock('Symfony\Component\Security\Core\Authentication\AuthenticationTrustResolverInterface'),
+            $httpUtils ? $httpUtils : $this->getMock('Symfony\Component\Security\Http\HttpUtils'),
+            'key',
+            $authenticationEntryPoint,
+            $errorPage,
+            $accessDeniedHandler
+        );
+    }
+}