bug #9879 [Security] Fix ExceptionListener to catch correctly AccessDeniedException 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 tests616b6c5
[Security] fixed error 500 instead of 403 if previous exception is provided to AccessDeniedException
This commit is contained in:
commit
e9d12dd520
@ -81,84 +81,83 @@ class ExceptionListener
|
||||
$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()));
|
||||
}
|
||||
|
||||
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);
|
||||
|
||||
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;
|
||||
}
|
||||
} 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;
|
||||
}
|
||||
}
|
||||
} elseif ($exception instanceof LogoutException) {
|
||||
if (null !== $this->logger) {
|
||||
$this->logger->info(sprintf('Logout exception occurred; wrapping with AccessDeniedHttpException (%s)', $exception->getMessage()));
|
||||
}
|
||||
|
||||
return;
|
||||
} else {
|
||||
return;
|
||||
}
|
||||
|
||||
$event->setResponse($response);
|
||||
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($event->getRequest(), $exception);
|
||||
|
||||
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);
|
||||
|
||||
$event->setResponse($event->getKernel()->handle($subRequest, HttpKernelInterface::SUB_REQUEST, true));
|
||||
}
|
||||
} 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));
|
||||
}
|
||||
}
|
||||
|
||||
private function handleLogoutException(GetResponseForExceptionEvent $event, LogoutException $exception)
|
||||
{
|
||||
if (null !== $this->logger) {
|
||||
$this->logger->info(sprintf('Logout exception occurred; wrapping with AccessDeniedHttpException (%s)', $exception->getMessage()));
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
|
@ -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
|
||||
);
|
||||
}
|
||||
}
|
Reference in New Issue
Block a user