Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
bug #9879 [Security] Fix ExceptionListener to catch correctly AccessD…
…eniedException if is not first exception (fabpot)

This PR was merged into the 2.3 branch.

Discussion
----------

[Security] Fix ExceptionListener to catch correctly AccessDeniedException if is not first exception

| Q             | A
| ------------- | ---
| Bug fix?      | yes
| New feature?  | no
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets | #9544, #8467?, #9823
| License       | MIT
| Doc PR        |

Same as #9823 but with some refactoring of the code and with some unit tests.

When merging to 2.4, the unit tests can be simplified a bit.

Commits
-------

172fd63 [Security] made code easier to understand, added some missing unit tests
616b6c5 [Security] fixed error 500 instead of 403 if previous exception is provided to AccessDeniedException
  • Loading branch information
fabpot committed Dec 29, 2013
2 parents c63bbe9 + 172fd63 commit e9d12dd
Show file tree
Hide file tree
Showing 2 changed files with 246 additions and 57 deletions.
113 changes: 56 additions & 57 deletions src/Symfony/Component/Security/Http/Firewall/ExceptionListener.php
Expand Up @@ -81,84 +81,83 @@ public function onKernelException(GetResponseForExceptionEvent $event)
$event->getDispatcher()->removeListener(KernelEvents::EXCEPTION, array($this, 'onKernelException'));

$exception = $event->getException();
$request = $event->getRequest();
do {
if ($exception instanceof AuthenticationException) {
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());
}

// determine the actual cause for the exception
while (null !== $previous = $exception->getPrevious()) {
$exception = $previous;
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()));
}

if ($exception instanceof AuthenticationException) {
try {
$event->setResponse($this->startAuthentication($event->getRequest(), $exception));
} catch (\Exception $e) {
$event->setException($e);
}
}

private function handleAccessDeniedException(GetResponseForExceptionEvent $event, AccessDeniedException $exception)
{
$event->setException(new AccessDeniedHttpException($exception->getMessage(), $exception));

$token = $this->context->getToken();
if (!$this->authenticationTrustResolver->isFullFledged($token)) {
if (null !== $this->logger) {
$this->logger->info(sprintf('Authentication exception occurred; redirecting to authentication entry point (%s)', $exception->getMessage()));
$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 {
$response = $this->startAuthentication($request, $exception);
$insufficientAuthenticationException = new InsufficientAuthenticationException('Full authentication is required to access this resource.', 0, $exception);
$insufficientAuthenticationException->setToken($token);

$event->setResponse($this->startAuthentication($event->getRequest(), $insufficientAuthenticationException));
} catch (\Exception $e) {
$event->setException($e);

return;
}
} elseif ($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()));
}
return;
}

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 {
$insufficientAuthenticationException = new InsufficientAuthenticationException('Full authentication is required to access this resource.', 0, $exception);
$insufficientAuthenticationException->setToken($token);
$response = $this->startAuthentication($request, $insufficientAuthenticationException);
} catch (\Exception $e) {
$event->setException($e);
try {
if (null !== $this->accessDeniedHandler) {
$response = $this->accessDeniedHandler->handle($event->getRequest(), $exception);

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()));
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);

try {
if (null !== $this->accessDeniedHandler) {
$response = $this->accessDeniedHandler->handle($request, $exception);

if (!$response instanceof Response) {
return;
}
} elseif (null !== $this->errorPage) {
$subRequest = $this->httpUtils->createRequest($request, $this->errorPage);
$subRequest->attributes->set(SecurityContextInterface::ACCESS_DENIED_ERROR, $exception);

$response = $event->getKernel()->handle($subRequest, HttpKernelInterface::SUB_REQUEST, true);
} else {
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()));
}

$event->setException(new \RuntimeException('Exception thrown when handling an exception.', 0, $e));

return;
}
$event->setResponse($event->getKernel()->handle($subRequest, HttpKernelInterface::SUB_REQUEST, true));
}
} elseif ($exception instanceof LogoutException) {
} catch (\Exception $e) {
if (null !== $this->logger) {
$this->logger->info(sprintf('Logout exception occurred; wrapping with AccessDeniedHttpException (%s)', $exception->getMessage()));
$this->logger->error(sprintf('Exception thrown when handling an exception (%s: %s)', get_class($e), $e->getMessage()));
}

return;
} else {
return;
$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()));
}
}

/**
Expand Down
@@ -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
);
}
}

0 comments on commit e9d12dd

Please sign in to comment.