10000 bug #9879 [Security] Fix ExceptionListener to catch correctly AccessD… · symfony/symfony@e9d12dd · GitHub
[go: up one dir, main page]
Skip to content

Commit e9d12dd

Browse files
fabpotfabpot
committed
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 tests 616b6c5 [Security] fixed error 500 instead of 403 if previous exception is provided to AccessDeniedException
2 parents c63bbe9 + 172fd63 commit e9d12dd

File tree

2 files changed

+246
-57
lines changed

2 files changed

+246
-57
lines changed

‎src/Symfony/Component/Security/Http/Firewall/ExceptionListener.php

Lines changed: 56 additions & 57 deletions
Original file line numberDiff line numberDiff line change
@@ -81,84 +81,83 @@ public function onKernelException(GetResponseForExceptionEvent $event)
8181
$event->getDispatcher()->removeListener(KernelEvents::EXCEPTION, array($this, 'onKernelException'));
8282

8383
$exception = $event->getException();
84-
$request = $event->getRequest();
84+
do {
85+
if ($exception instanceof AuthenticationException) {
86+
return $this->handleAuthenticationException($event, $exception);
87+
} elseif ($exception instanceof AccessDeniedException) {
88+
return $this->handleAccessDeniedException($event, $exception);
89+
} elseif ($exception instanceof LogoutException) {
90+
return $this->handleLogoutException($event, $exception);
91+
}
92+
} while (null !== $exception = $exception->getPrevious());
93+
}
8594

86-
// determine the actual cause for the exception
87-
while (null !== $previous = $exception->getPrevious()) {
88-
$exception = $previous;
95+
private function handleAuthenticationException(GetResponseForExceptionEvent $event, AuthenticationExc 57A6 eption $exception)
96+
{
97+
if (null !== $this->logger) {
98+
$this->logger->info(sprintf('Authentication exception occurred; redirecting to authentication entry point (%s)', $exception->getMessage()));
8999
}
90100

91-
if ($exception instanceof AuthenticationException) {
101+
try {
102+
$event->setResponse($this->startAuthentication($event->getRequest(), $exception));
103+
} catch (\Exception $e) {
104+
$event->setException($e);
105+
}
106+
}
107+
108+
private function handleAccessDeniedException(GetResponseForExceptionEvent $event, AccessDeniedException $exception)
109+
{
110+
$event->setException(new AccessDeniedHttpException($exception->getMessage(), $exception));
111+
112+
$token = $this->context->getToken();
113+
if (!$this->authenticationTrustResolver->isFullFledged($token)) {
92114
if (null !== $this->logger) {
93-
$this->logger->info(sprintf('Authentication exception occurred; redirecting to authentication entry point (%s)', $exception->getMessage()));
115+
$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()));
94116
}
95117

96118
try {
97-
$response = $this->startAuthentication($request, $exception);
119+
$insufficientAuthenticationException = new InsufficientAuthenticationException('Full authentication is required to access this resource.', 0, $exception);
120+
$insufficientAuthenticationException->setToken($token);
121+
122+
$event->setResponse($this->startAuthentication($event->getRequest(), $insufficientAuthenticationException));
98123
} catch (\Exception $e) {
99124
$event->setException($e);
100-
101-
return;
102125
}
103-
} elseif ($exception instanceof AccessDeniedException) {
104-
$event->setException(new AccessDeniedHttpException($exception->getMessage(), $exception));
105126

106-
$token = $this->context->getToken();
107-
if (!$this->authenticationTrustResolver->isFullFledged($token)) {
108-
if (null !== $this->logger) {
109-
$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()));
110-
}
127+
return;
128+
}
129+
130+
if (null !== $this->logger) {
131+
$this->logger->debug(sprintf('Access is denied (and user is neither anonymous, nor remember-me) by "%s" at line %s', $exception->getFile(), $exception->getLine()));
132+
}
111133

112-
try {
113-
$insufficientAuthenticationException = new InsufficientAuthenticationException('Full authentication is required to access this resource.', 0, $exception);
114-
$insufficientAuthenticationException->setToken($token);
115-
$response = $this->startAuthentication($request, $insufficientAuthenticationException);
116-
} catch (\Exception $e) {
117-
$event->setException($e);
134+
try {
135+
if (null !== $this->accessDeniedHandler) {
136+
$response = $this->accessDeniedHandler->handle($event->getRequest(), $exception);
118137

119-
return;
120-
}
121-
} else {
122-
if (null !== $this->logger) {
123-
$this->logger->debug(sprintf('Access is denied (and user is neither anonymous, nor remember-me) by "%s" at line %s', $exception->getFile(), $exception->getLine()));
138+
if ($response instanceof Response) {
139+
$event->setResponse($response);
124140
}
141+
} elseif (null !== $this->errorPage) {
142+
$subRequest = $this->httpUtils->createRequest($event->getRequest(), $this->errorPage);
143+
$subRequest->attributes->set(SecurityContextInterface::ACCESS_DENIED_ERROR, $exception);
125144

126-
try {
127-
if (null !== $this->accessDeniedHandler) {
128-
$response = $this->accessDeniedHandler->handle($request, $exception);
129-
130-
if (!$response instanceof Response) {
131-
return;
132-
}
133-
} elseif (null !== $this->errorPage) {
134-
$subRequest = $this->httpUtils->createRequest($request, $this->errorPage);
135-
$subRequest->attributes->set(SecurityContextInterface::ACCESS_DENIED_ERROR, $exception);
136-
137-
$response = $event->getKernel()->handle($subRequest, HttpKernelInterface::SUB_REQUEST, true);
138-
} else {
139-
return;
140-
}
141-
} catch (\Exception $e) {
142-
if (null !== $this->logger) {
143-
$this->logger->error(sprintf('Exception thrown when handling an exception (%s: %s)', get_class($e), $e->getMessage()));
144-
}
145-
146-
$event->setException(new \RuntimeException('Exception thrown when handling an exception.', 0, $e));
147-
148-
return;
149-
}
145+
$event->setResponse($event->getKernel()->handle($subRequest, HttpKernelInterface::SUB_REQUEST, true));
150146
}
151-
} elseif ($exception instanceof LogoutException) {
147+
} catch (\Exception $e) {
152148
if (null !== $this->logger) {
153-
$this->logger->info(sprintf('Logout exception occurred; wrapping with AccessDeniedHttpException (%s)', $exception->getMessage()));
149+
$this->logger->error(sprintf('Exception thrown when handling an exception (%s: %s)', get_class($e), $e->getMessage()));
154150
}
155151

156-
return;
157-
} else {
158-
return;
152+
$event->setException(new \RuntimeException('Exception thrown when handling an exception.', 0, $e));
159153
}
154+
}
160155

161-
$event->setResponse($response);
156+
private function handleLogoutException(GetResponseForExceptionEvent $event, LogoutException $exception)
157+
{
158+
if (null !== $this->logger) {
159+
$this->logger->info(sprintf('Logout exception occurred; wrapping with AccessDeniedHttpException (%s)', $exception->getMessage()));
160+
}
162161
}
163162

164163
/**

‎src/Symfony/Component/Security/Tests/Http/Firewall/ExceptionListenerTest.php

Lines changed: 190 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,190 @@
1+
<?php
2+
3+
/*
4+
* This file is part of the Symfony package.
5+
*
6+
* (c) Fabien Potencier <fabien@symfony.com>
7+
*
8+
* For the full copyright and license information, please view the LICENSE
9+
* file that was distributed with this source code.
10+
*/
11+
12+
namespace Symfony\Component\Security\Tests\Http\Firewall;
13+
14+
use Symfony\Component\HttpFoundation\Request;
15+
use Symfony\Component\HttpFoundation\Response;
16+
use Symfony\Component\HttpKernel\Event\GetResponseForExceptionEvent;
17+
use Symfony\Component\HttpKernel\HttpKernelInterface;
18+
use Symfony\Component\Security\Core\Authentication\AuthenticationTrustResolverInterface;
19+
use Symfony\Component\Security\Core\Exception\AccessDeniedException;
20+
use Symfony\Component\Security\Core\Exception\AuthenticationException;
21+
use Symfony\Component\Security\Core\SecurityContextInterface;
22+
use Symfony\Component\Security\Http\Authorization\AccessDeniedHandlerInterface;
23+
use Symfony\Component\Security\Http\EntryPoint\AuthenticationEntryPointInterface;
24+
use Symfony\Component\Security\Http\Firewall\ExceptionListener;
25+
use Symfony\Component\Security\Http\HttpUtils;
26+
27+
class ExceptionListenerTest extends \PHPUnit_Framework_TestCase
28+
{
29+
/**
30+
* @dataProvider getAuthenticationExceptionProvider
31+
*/
32+
public function testAuthenticationExceptionWithoutEntryPoint(\Exception $exception, \Exception $eventException = null)
33+
{
34+
$event = $this->createEvent($exception);
35+
36+
$listener = $this->createExceptionListener();
37+
$listener->onKernelException($event);
38+
39+
$this->assertNull($event->getResponse());
40+
$this->assertSame(null === $eventException ? $exception : $eventException, $event->getException());
41+
}
42+
43+
/**
44+
* @dataProvider getAuthenticationExceptionProvider
45+
*/
46+
public function testAuthenticationExceptionWithEntryPoint(\Exception $exception, \Exception $eventException = null)
47+
{
48+
$event = $this->createEvent($exception = new AuthenticationException());
49+
50+
$listener = $this->createExceptionListener(null, null, null, $this->createEntryPoint());
51+
$listener->onKernelException($event);
52+
53+
$this->assertEquals('OK', $event->getResponse()->getContent());
54+
$this->assertSame($exception, $event->getException());
55+
}
56+
57+
public function getAuthenticationExceptionProvider()
58+
{
59+
return array(
60+
array(new AuthenticationException()),
61+
array(new \LogicException('random', 0, $e = new AuthenticationException()), $e),
62+
array(new \LogicException('random', 0, $e = new AuthenticationException('embed', 0, new AuthenticationException())), $e),
63+
array(new \LogicException('random', 0, $e = new AuthenticationException('embed', 0, new AccessDeniedException())), $e),
64+
array(new AuthenticationException('random', 0, new \LogicException())),
65+
);
66+
}
67+
68+
/**
69+
* @dataProvider getAccessDeniedExceptionProvider
70+
*/
71+
public function testAccessDeniedExceptionFullFledgedAndWithoutAccessDeniedHandlerAndWithoutErrorPage(\Exception $exception, \Exception $eventException = null)
72+
{
73+
$event = $this->createEvent($exception);
74+
75+
$listener = $this->createExceptionListener(null, $this->createTrustResolver(true));
76+
$listener->onKernelException($event);
77+
78+
$this->assertNull($event->getResponse());
79+
$this->assertSame(null === $eventException ? $exception : $eventException, $event->getException()->getPrevious());
80+
}
81+
82+
/**
83+
* @dataProvider getAccessDeniedExceptionProvider
84+
*/
85+
public function testAccessDeniedExceptionFullFledgedAndWithoutAccessDeniedHandlerAndWithErrorPage(\Exception $exception, \Exception $eventException = null)
86+
{
87+
$kernel = $this->getMock('Symfony\Component\HttpKernel\HttpKernelInterface');
88+
$kernel->expects($this->once())->method('handle')->will($this->returnValue(new Response('error')));
89+
90+
$event = $this->createEvent($exception, $kernel);
91+
92+
$httpUtils = $this->getMock('Symfony\Component\Security\Http\HttpUtils');
93+
$httpUtils->expects($this->once())->method('createRequest')->will($this->returnValue(Request::create('/error')));
94+
95+
$listener = $this->createExceptionListener(null, $this->createTrustResolver(true), $httpUtils, null, '/error');
96+
$listener->onKernelException($event);
97+
98+
$this->assertEquals('error', $event->getResponse()->getContent());
99+
$this->assertSame(null === $eventException ? $exception : $eventException, $event->getException()->getPrevious());
100+
}
101+
102+
/**
103+
* @dataProvider getAccessDeniedExceptionProvider
104+
*/
105+
public function testAccessDeniedExceptionFullFledgedAndWithAccessDeniedHandlerAndWithoutErrorPage(\Exception $exception, \Exception $eventException = null)
106+
{
107+
$event = $this->createEvent($exception);
108+
109+
$accessDeniedHandler = $this->getMock('Symfony\Component\Security\Http\Authorization\AccessDeniedHandlerInterface');
110+
$accessDeniedHandler->expects($this->once())->method('handle')->will($this->returnValue(new Response('error')));
111+
112+
$listener = $this->createExceptionListener(null, $this->createTrustResolver(true), null, null, null, $accessDeniedHandler);
113+
$listener->onKernelException($event);
114+
115+
$this->assertEquals('error', $event->getResponse()->getContent());
116+
$this->assertSame(null === $eventException ? $exception : $eventException, $event->getException()->getPrevious());
117+
}
118+
119+
/**
120+
* @dataProvider getAccessDeniedExceptionProvider
121+
*/
122+
public function testAccessDeniedExceptionNotFullFledged(\Exception $exception, \Exception $eventException = null)
123+
{
124+
$event = $this->createEvent($exception);
125+
126+
$context = $this->getMock('Symfony\Component\Security\Core\SecurityContextInterface');
127+
$context->expects($this->once())->method('getToken')->will($this->returnValue($this->getMock('Symfony\Component\Security\Core\Authentication\Token\TokenInterface')));
128+
129+
$listener = $this->createExceptionListener($context, $this->createTrustResolver(false), null, $this->createEntryPoint());
130+
$listener->onKernelException($event);
131+
132+
$this->assertEquals('OK', $event->getResponse()->getContent());
133+
$this->assertSame(null === $eventException ? $exception : $eventException, $event->getException()->getPrevious());
134+
}
135+
136+
public function getAccessDeniedExceptionProvider()
137+
{
138+
return array(
139+
array(new AccessDeniedException()),
140+
array(new \LogicException('random', 0, $e = new AccessDeniedException()), $e),
141+
array(new \LogicException('random', 0, $e = new AccessDeniedException('embed', new AccessDeniedException())), $e),
142+
array(new \LogicException('random', 0, $e = new AccessDeniedException('embed', new AuthenticationException())), $e),
143+
array(new AccessDeniedException('random', new \LogicException())),
144+
);
145+
}
146+
147+
private function createEntryPoint()
148+
{
149+
$entryPoint = $this->getMock('Symfony\Component\Security\Http\EntryPoint\AuthenticationEntryPointInterface');
150+
$entryPoint->expects($this->once())->method('start')->will($this->returnValue(new Response('OK')));
151+
152+
return $entryPoint;
153+
}
154+
155+
private function createTrustResolver($fullFledged)
156+
{
157+
$trustResolver = $this->getMock('Symfony\Component\Security\Core\Authentication\AuthenticationTrustResolverInterface');
158+
$trustResolver->expects($this->once())->method('isFullFledged')->will($this->returnValue($fullFledged));
159+
160+
return $trustResolver;
161+
}
162+
163+
private function createEvent(\Exception $exception, $kernel = null)
164+
{
165+
if (null === $kernel) {
166+
$kernel = $this->getMock('Symfony\Component\HttpKernel\HttpKernelInterface');
167+
}
168+
169+
$event = new GetResponseForExceptionEvent($kernel, Request::create('/'), HttpKernelInterface::MASTER_REQUEST, $exception);
170+
171+
// FIXME: to be removed in 2.4
172+
$dispatcher = $this->getMock('Symfony\Component\EventDispatcher\EventDispatcherInterface');
173+
$event->setDispatcher($dispatcher);
174+
175+
return $event;
176+
}
177+
178+
private function createExceptionListener(SecurityContextInterface $context = null, AuthenticationTrustResolverInterface $trustResolver = null, HttpUtils $httpUtils = null, AuthenticationEntryPointInterface $authenticationEntryPoint = null, $errorPage = null, AccessDeniedHandlerInterface $accessDeniedHandler = null)
179+
{
180+
return new ExceptionListener(
181+
$context ? $context : $this->getMock('Symfony\Component\Security\Core\SecurityContextInterface'),
182+
$trustResolver ? $trustResolver : $this->getMock('Symfony\Component\Security\Core\Authentication\AuthenticationTrustResolverInterface'),
183+
$httpUtils ? $httpUtils : $this->getMock('Symfony\Component\Security\Http\HttpUtils'),
184+
'key',
185+
$authenticationEntryPoint,
186+
$errorPage,
187+
$accessDeniedHandler
188+
);
189+
}
190+
}

0 commit comments

Comments
 (0)
0