diogo
/
symfony
Archived
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

bug #36974 [Security] Fixed handling of CSRF logout error (wouterj) 

This PR was merged into the 3.4 branch.

Discussion
----------

[Security] Fixed handling of CSRF logout error

| Q             | A
| ------------- | ---
| Branch?       | 3.4
| Bug fix?      | yes
| New feature?  | no
| Deprecations? | no
| Tickets       | Fix #36814
| License       | MIT
| Doc PR        | -

8 years ago, a typo was made while refactoring the `ExceptionListener`, loosing this logic (46071f3238). I think we should fix it.

The `LogoutException` is a very generic name for something only used when the CSRF token is invalid. Should we match the exception message to make sure only this CSRF error is transformed into 403? I didn't yet do it because any usage of `LogoutException` would have resulted in 500, which always is worse than 403.

Commits
-------

50348f2eb7 Fixed handling of CSRF logout error
This commit is contained in:
Robin Chalas 2020-05-26 17:59:18 +02:00
parent 4f40da5809 50348f2eb7
commit ce61bb0750
2 changed files with 17 additions and 3 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

8
src/Symfony/Component/Security/Http/Firewall/ExceptionListener.php
View File

@ -102,7 +102,7 @@ class ExceptionListener
}
if ($exception instanceof LogoutException) {
$this->handleLogoutException($exception);
$this->handleLogoutException($event, $exception);
return;
}
@ -172,10 +172,12 @@ class ExceptionListener
}
}
private function handleLogoutException(LogoutException $exception)
private function handleLogoutException(GetResponseForExceptionEvent $event, LogoutException $exception)
{
$event->setException(new AccessDeniedHttpException($exception->getMessage(), $exception));
if (null !== $this->logger) {
$this->logger->info('A LogoutException was thrown.', ['exception' => $exception]);
$this->logger->info('A LogoutException was thrown; wrapping with AccessDeniedHttpException', ['exception' => $exception]);
}
}

12
src/Symfony/Component/Security/Http/Tests/Firewall/ExceptionListenerTest.php
View File

@ -21,6 +21,7 @@ use Symfony\Component\Security\Core\Authentication\AuthenticationTrustResolverIn
use Symfony\Component\Security\Core\Authentication\Token\Storage\TokenStorageInterface;
use Symfony\Component\Security\Core\Exception\AccessDeniedException;
use Symfony\Component\Security\Core\Exception\AuthenticationException;
use Symfony\Component\Security\Core\Exception\LogoutException;
use Symfony\Component\Security\Http\Authorization\AccessDeniedHandlerInterface;
use Symfony\Component\Security\Http\EntryPoint\AuthenticationEntryPointInterface;
use Symfony\Component\Security\Http\Firewall\ExceptionListener;
@ -160,6 +161,17 @@ class ExceptionListenerTest extends TestCase
$this->assertSame(null === $eventException ? $exception : $eventException, $event->getException()->getPrevious());
}
public function testLogoutException()
{
$event = $this->createEvent(new LogoutException('Invalid CSRF.'));
$listener = $this->createExceptionListener();
$listener->onKernelException($event);
$this->assertEquals('Invalid CSRF.', $event->getException()->getMessage());
$this->assertEquals(403, $event->getException()->getStatusCode());
}
public function getAccessDeniedExceptionProvider()
{
return [