FriendsOfSymfony / FOSRestBundle

This Bundle provides various tools to rapidly develop RESTful API's with Symfony
http://symfony.com/doc/master/bundles/FOSRestBundle/index.html
MIT License
2.79k stars 702 forks source link

Symfony\Component\Security\Http\Firewall\ExceptionListener overrides AccessDeniedListener #1538

Closed bblue closed 4 years ago

bblue commented 8 years ago

The logic in the AccessDeniedListener is effectively overriden by Symfony\Component\Security\Http\Firewall\ExceptionListener

The priority of the Symfony listener is lower than that of FOSRest, thus the following method in Symfony's listener overrides the $event->setException($e);

private function handleAuthenticationException(GetResponseForExceptionEvent $event, AuthenticationException $exception)
{
    if (null !== $this->logger) {
        $this->logger->info('An AuthenticationException was thrown; redirecting to authentication entry point.', array('exception' => $exception));
    }

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

This will trigger a redirect to the login page, and in some cases (my case) this will trigger yet another authentication exception (due to faulty JWT token for instance), but this new exception will be caught in the above method, the $event->exception will be overridden, and hence no conversion to the http exceptions are done. I will still get JSON data, but it will come with a 500 error.

geoffreytran commented 8 years ago

Running into this same issue here

pppdns commented 7 years ago

Same here, although I get HTML data (coming from RedirectResponse#L85)) while redirecting to the login page.

btw I'm using FrameworkExtraBundle's @SECURITY annotation so the code of my controller action does not even start executing, the AccessDeniedHttpException is thrown automatically and handled by Symfony\Component\Security\Http\Firewall\ExceptionListener.

ExceptionListener then stops propagation so FOSRestBundle's ExceptionListener is not called. From the logs:

DEBUG - Listener "Symfony\Component\Security\Http\Firewall\ExceptionListener::onKernelException" stopped propagation of the event "kernel.exception".
DEBUG - Listener "FOS\RestBundle\EventListener\ExceptionListener::onKernelException" was not called for event "kernel.exception".

@geoffreytran, @bblue have you solved the problem?

lsmith77 commented 7 years ago

FYI I created a PR but I am not sure yet what the side effects will be https://github.com/FriendsOfSymfony/FOSRestBundle/pull/1692

scott-r-lindsey commented 5 years ago

Any movement on this? Or, a suggested workaround?

bblue commented 5 years ago

Just realized I had been referenced in this thread a while ago. Haven't worked on that project for a loooong time, and its dependencies and symfony has not been updated since 2017.

I recall initally making a compiler pass to override the FOSRest listener, but was not happy with that solution. I ended up instead solving the root cause that threw my initial exception (not linked to fosrest), thus I really just side-steped the real bug, but I then would not have to override any FOSRest classes.

So to summarize: you can probably avoid a particular use-case error with a compiler pass, but it would be fragile. Unfortunately I have deleted my compiler pass object, so I can't tell you what I actually wrote in it... I probably just rethrew the exception.

I'm guessing the PR by @lsmith77 is the way to go, but have not looked at it.

xabbuh commented 4 years ago

I was trying to write a test case for this issue to ensure that #2109 is indeed the right fix, but unfortunately I wasn't able to do so. Can anyone of you suffering from this issue please create a small example application that allows to do so?

lsmith77 commented 4 years ago

i will try to have a look today