Improve customization of DefaultOAuth2UserService to handle other content types

[knoobie]

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.

spring-security/oauth2/oauth2-client/src/main/java/org/springframework/security/oauth2/client/userinfo/DefaultOAuth2UserService.java

Lines 88 to 117 in a325216

	public OAuth2User loadUser(OAuth2UserRequest userRequest) throws OAuth2AuthenticationException {
	Assert.notNull(userRequest, "userRequest cannot be null");
	if (!StringUtils
	.hasText(userRequest.getClientRegistration().getProviderDetails().getUserInfoEndpoint().getUri())) {
	OAuth2Error oauth2Error = new OAuth2Error(MISSING_USER_INFO_URI_ERROR_CODE,
	"Missing required UserInfo Uri in UserInfoEndpoint for Client Registration: "
	+ userRequest.getClientRegistration().getRegistrationId(),
	null);
	throw new OAuth2AuthenticationException(oauth2Error, oauth2Error.toString());
	}
	String userNameAttributeName = userRequest.getClientRegistration().getProviderDetails().getUserInfoEndpoint()
	.getUserNameAttributeName();
	if (!StringUtils.hasText(userNameAttributeName)) {
	OAuth2Error oauth2Error = new OAuth2Error(MISSING_USER_NAME_ATTRIBUTE_ERROR_CODE,
	"Missing required \"user name\" attribute name in UserInfoEndpoint for Client Registration: "
	+ userRequest.getClientRegistration().getRegistrationId(),
	null);
	throw new OAuth2AuthenticationException(oauth2Error, oauth2Error.toString());
	}
	RequestEntity<?> request = this.requestEntityConverter.convert(userRequest);
	ResponseEntity<Map<String, Object>> response = getResponse(userRequest, request);
	Map<String, Object> userAttributes = response.getBody();
	Set<GrantedAuthority> authorities = new LinkedHashSet<>();
	authorities.add(new OAuth2UserAuthority(userAttributes));
	OAuth2AccessToken token = userRequest.getAccessToken();
	for (String authority : token.getScopes()) {
	authorities.add(new SimpleGrantedAuthority("SCOPE_" + authority));
	}
	return new DefaultOAuth2User(authorities, userAttributes, userNameAttributeName);
	}

Context

Related to #9583

[jgrandja]

@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?

[knoobie]

@jgrandja Interesting idea! Do you mind if I take a look at it, or do you wanna do it?

[jgrandja]

@knoobie It would be great if you could submit a PR for this.

[knoobie]

@jgrandja Created a first draft here knoobie@f1a86cf - 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.

[knoobie]

@sjohnr Do you think your draft is revisited and hopefully to be included in 6.0?

[sjohnr]

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.