spring-projects
Improve customization of `DefaultOAuth2UserService` to handle other content types
Expected Behavior
DefaultOAuth2UserService can be extended to e.g. allow for custom body parsing to handle application/jwtfor signed and/or encrypted UserInfo Response.
Rough draft:
public class CustomOAuth2UserService extends DefaultOAuth2UserService {
@Override
protected ResponseEntity<Map<String, Object>> getResponse(OAuth2UserRequest userRequest, RequestEntity<?> request) {
// Custom code to handle requests that aren't simple application/json
return ...;
}
}
We are open for other solutions as well and happy to contribute, if that's something you see worth it as addition to spring-security.
Current Behavior
DefaultOAuth2UserService has to be copied and "rewritten" - because getResponse() is called inside loadUser(OAuth2UserRequest userRequest) which forces us to re-create the whole loadUser(OAuth2UserRequest userRequest) method.
https://github.com/spring-projects/spring-security/blob/a325216f19277d4191c97afee1c66f82d056f9dc/oauth2/oauth2-client/src/main/java/org/springframework/security/oauth2/client/userinfo/DefaultOAuth2UserService.java#L88-L117
Context
Related to #9583
@knoobie Take a look at the implementations of OAuth2AccessTokenResponseClient, e.g. DefaultAuthorizationCodeTokenResponseClient, as the associated RestTemplate requires OAuth2AccessTokenResponseHttpMessageConverter to parse the response body into OAuth2AccessTokenResponse.
We could apply the same pattern where we extract the response processing logic in DefaultOAuth2UserService into a new (default) implementation of HttpMessageConverter. This work would allow a custom configured HttpMessageConverter to support gh-9583.
Makes sense?
@jgrandja Interesting idea! Do you mind if I take a look at it, or do you wanna do it?
@jgrandja Created a first draft here https://github.com/knoobie/spring-security/commit/f1a86cf69625482caa4222f77c4014d1aa3b8e3d - using OAuth2UserAuthority as target for the HttpMessageConverter. Was this the way you had in mind?
Currently only two tests are failing and I'm struggling to understand why.
@sjohnr Do you think your draft is revisited and hopefully to be included in 6.0?
Hi @knoobie! Thanks for checking in, I'm very sorry not to have updated you before now. At this time, the team is prioritizing breaking changes only, and until we can work through that list of issues, we wouldn't be prioritizing other enhancements for 6.0. If there's time after, we could certainly look at it, but the schedule is pretty tight. The good news is that if this issue misses 6.0, it could still go into 6.1.
