add AccessDeniedException and InsufficientAuthentificationException c… #1653
Conversation
…lass to be allowed in the configuration exception_to_status exception_to_status api-platform/api-platform#519
There was a problem hiding this comment.
Sorry I didn't noticed that it wasn't an exception from the Http namespace. I think @stof is right in symfony/symfony#25806, it shouldn't be handled this way.
|
@dunglas i'm quite agree with you, symfony should manage this finely (which is not the cas with json_login system). But, as Symfony security let the user to override the status_code i thought it could be something wished by community of api-platform. |
|
@dunglas the weird thing is that the firewall should have a listener converting these exception into the HTTP ones if they cannot be handled in another way (redirecting to the entry point for instance). So the right fix would first involve understanding why this conversion does not happen (it might be a bug somewhere) |
| @@ -245,6 +247,8 @@ private function addExceptionToStatusSection(ArrayNodeDefinition $rootNode) | |||
| ->defaultValue([ | |||
| ExceptionInterface::class => Response::HTTP_BAD_REQUEST, | |||
| InvalidArgumentException::class => Response::HTTP_BAD_REQUEST, | |||
| AccessDeniedException::class => Response::HTTP_FORBIDDEN, | |||
| InsufficientAuthenticationException::class => Response::HTTP_FORBIDDEN, | |||
There was a problem hiding this comment.
Response::HTTP_UNAUTHORIZED ? More like a 401 than 403. He can retry the request other credentials. 403 is really "forbidden, even if you're the all-authority".
|
could you please check why the tests are failing ? |
|
Thanks for your contribution, however I'm closing here since we got no feedback, feel free to re-open when working on it ! |
|
@Simperfit the question is: does the PR has a chance to be accepted with fixed tests ? in fact it seems that the content of the original PR was not the rigth way to go on. |
…lass to be allowed in the configuration exception_to_status exception_to_status
api-platform/api-platform#519
Following @dunglas recommandation here : api-platform/api-platform#519