Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Appearance settings

Commit e9d12dd

Browse filesBrowse 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
Copy full SHA for e9d12dd

File tree

Expand file treeCollapse file tree

2 files changed

+246
-57
lines changed
Filter options
Expand file treeCollapse file tree

2 files changed

+246
-57
lines changed

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

Copy file name to clipboardExpand all lines: src/Symfony/Component/Security/Http/Firewall/ExceptionListener.php
+56-57Lines 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, AuthenticationException $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

Copy file name to clipboard
+190Lines 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)
Morty Proxy This is a proxified and sanitized view of the page, visit original site.