Client authentication errors cannot be serialised differently without reimplementing onAuthenticationFailure
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.
Source: spring-projects/spring-security