-
-
Notifications
You must be signed in to change notification settings - Fork 9.6k
Convert InsufficientAuthenticationException to HttpException with 401 status code #28801
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍 when tests will be added
e709469
to
df9cf29
Compare
df9cf29
to
4503ac8
Compare
seems like this PR also fixes #8467, but not sure |
@Koc I'm linking your issue as being fixed by this PR. I'll let you reopen an issue if that's not the case. |
Thank you @vincentchalamon. |
…on with 401 status code (vincentchalamon) This PR was merged into the 2.8 branch. Discussion ---------- Convert InsufficientAuthenticationException to HttpException with 401 status code | Q | A | ------------- | --- | Branch? | 2.8 | Bug fix? | yes | New feature? | no | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed ticket | #8467 | License | MIT I was trying to implement the `json_login` authentication and test it with an API Platform project. When I call a secured endpoint without authentication, an InsufficientAuthenticationException is thrown with a 500 status code instead of a 401. After some researches with @dunglas, there is no default `entrypoint` on the security firewall. As one already exists for `form_login` in the FormLoginFactory, this component might need a default one to convert this 500 exception to a correct 401 HTTP error. This fixes #25806 (comment). Commits ------- 4503ac8 Convert InsufficientAuthenticationException to HttpException
The problem also exist in 4+ branch, does the fix solve the issue in recent version of Sf ? |
@Rebolon I think so, old branches are merged in the newer ones on a regular basis. |
Hey guys, this actually broke some of our code as we caught this specific exception and converted it to a 401 ourselves. With this change it turned into a 500 essentially reversing our scenario. |
@gitnik nice, your case was bound to happen as this issue has been present for quite some time now. I must say your original fix is now the root of your problem. Do you mind telling how you fixed your new issue? |
Now we just catch the generic |
There is the same kind of missing feature with this Symfony Exception: I opened an issue in ApiPlatform https://github.com/api-platform/api-platform/issues/1213 |
I was trying to implement the
json_login
authentication and test it with an API Platform project. When I call a secured endpoint without authentication, an InsufficientAuthenticationException is thrown with a 500 status code instead of a 401.After some researches with @dunglas, there is no default
entrypoint
on the security firewall. As one already exists forform_login
in the FormLoginFactory, this component might need a default one to convert this 500 exception to a correct 401 HTTP error.This fixes #25806 (comment).