spring-projects / spring-projects/spring-security
Client authentication errors cannot be serialised differently without reimplementing onAuthenticationFailure
Nobody has claimed this yet.
- Dominant language
- Java
- Stars
- 9.6k
- Forks
- 6.3k
- Avg merge
- 2d 11h
- Merged PRs (30d)
- 52
Description
This is a follow-up to #18285, where the same hook came up. There the answer was:
This is already possible. See Configuring Client Authentication and the
clientAuthentication.errorResponseHandler(), which is anAuthenticationFailureHandler.
That is correct, and it is what we use. But it is the only hook, and it is all or nothing. If the thing you want to change is the serialisation of the error body, you have to take over the whole response writing.
Our case: legacy clients that expect an additional field in the error body. Nothing about status code, headers or when a failure happens, only how the OAuth2Error is written.
At the token endpoint this is a few lines, because the default failure handler is public and has a setter:
OAuth2ErrorHttpMessageConverter errorConverter = new OAuth2ErrorHttpMessageConverter();
errorConverter.setErrorParametersConverter(ourSerialisation());
OAuth2ErrorAuthenticationFailureHandler handler = new OAuth2ErrorAuthenticationFailureHandler();
handler.setErrorResponseConverter(errorConverter);
// tokenEndpoint.errorResponseHandler(handler)
ErrorSerializationTest asserts that the custom parameters really end up in the response body.
At OAuth2ClientAuthenticationFilter the same is not reachable (ClientAuthenticationFilterHooksTest):
- the filter is
final, so no subclass errorHttpResponseConverteris aprivate finalfield, built in the filter itself, and there is no setter for it. The setters aresetAuthenticationConverter,setAuthenticationSuccessHandlerandsetAuthenticationFailureHandler, nothing else- the filter writes the error response itself in
onAuthenticationFailure
So the only way to change one field is to pass an AuthenticationFailureHandler that does again what the default does. In practice that means copying the body of onAuthenticationFailure and keeping that copy correct across upgrades, for a change that has nothing to do with failure handling.
For two filters in the same package that both write an OAuth2Error this looks like an asymmetry that was not intended, especially since the class that solves it, OAuth2ErrorAuthenticationFailureHandler, is right there.
Suggestion, either one is fine for us:
- add a setter for the error converter on
OAuth2ClientAuthenticationFilter - or give the filter the same default
OAuth2ErrorAuthenticationFailureHandlerthat the token endpoint uses, so the existingerrorResponseHandler()hook reaches the serialisation
The second is smaller and makes both endpoints behave the same way. I am happy to open a PR for the one you prefer.
Tests: https://github.com/macstab/spring-authorization-server-issues - green on 7.1.1, on 7.0.7 and on standalone 1.5.3.
Contributor guide
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
Research direction
Start with OAuth2ClientAuthenticationFilter and the ClientAuthenticationFilterHooksTest, then compare its failure-response path with OAuth2ErrorAuthenticationFailureHandler and the ErrorSerializationTest. Done means a client authentication error can use a custom OAuth2Error serialization without reimplementing the complete failure handler, with tests covering the additional response field.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- java, spring
- Domain
- authentication, security
- Issue type
- Feature
- Difficulty
- 3/5
- Estimated time
- 1-2 days
- Activity status
- Active
- Clarity
- Mostly clear
- Newbie friendliness
- 74/100