Support custom OAuth2AuthenticatedPrincipal in Jwt-based authentication flow

[therepanic]

This PR implements a simpler approach, as suggested by @jzheaux, to support OAuth2AuthenticatedPrincipal injection into JwtAuthenticationToken

Resolves: #6237

[therepanic]

I think the current solution can't be merged now since at the very least we don't have tests. I would like to hear feedback if I have understood the solution to the current problem correctly.

Also I guess we can't add a constructor to (Jwt jwt, Object principal, Collection<GrantedAuthority> authorities) as written in #32 because we already have a constructor with 3 arguments (Jwt jwt, Collection<? extends GrantedAuthority> authorities, String name). Because of this we have a build failing now.

[jzheaux]

Thanks for the PR, @therepanic! I've left some feedback inline that I hope will also address your question about the new JwtAuthenticationToken constructor.

After you review my comments, if we are agreed, will you also please add tests that confirm the new setter methods work?

[jzheaux]

This should be @since 7.0 as that's the current feature release (now that 6.5 is released, it will only take bug fixes).

[jzheaux]

With the help of a private inner class, I believe we can adapt Jwt into an OAuth2AuthenticatedPrincipal so that this if statement is not necessary. Consider adding a class like this:

private static final class JwtAuthenticatedPrincipal extends Jwt implements OAuth2AuthenticatedPrincipal {

	private final String principalClaimName;

	JwtAuthenticatedPrincipal(Jwt jwt) {
		this(jwt, JwtClaimNames.SUB);
	}

	JwtAuthenticatedPrincipal(Jwt jwt, String principalClaimName) {
		super(jwt.getTokenValue(), jwt.getIssuedAt(), jwt.getExpiresAt(), jwt.getHeaders(), jwt.getClaims());
		this.principalClaimName = principalClaimName;
	}

	@Override
	public Map<String, Object> getAttributes() {
		return getClaims();
	}

	@Override
	public Collection<? extends GrantedAuthority> getAuthorities() {
		return List.of();
	}

	@Override
	public String getName() {
		return getClaimAsString(this.principalClaimName);
	}

}

Then, I believe jwtPrincipalConverter can default to JwtAuthenticatedPrincipal::new and principalClaimName can be removed by having setPrincipalClaimName use JwtAuthenticatedPrincipal as well.

[jzheaux]

This should use the principal provided by the principal conversion instead of constructing a new DefaultOAuth2AuthenticatedPrincipal.

It might be a little simpler at this point to change this class to have its own Converter<Jwt, OAuth2AuthenticatedPrincipal> and Converter<Jwt, Collection<GrantedAuthority>> references.

[therepanic]

Adding your own links to converters will indeed be easier, but I have a question. Do we want to create the same jwtPrincipalConverter in JwtBearerTokenAuthenticationConverter as in JwtAuthenticationConverter? That is, by default with the type JwtAuthenticatedPrincipal::new? If so, then in the version you suggested, this class is private, and therefore we will not be able to create a converter by default. Should we make JwtAuthenticatedPrincipal package-private? Or we can just create getters for converters in JwtAuthenticationConverter and use them?

[jzheaux]

I'd recommend that the principal converter in JwtBearerTokenAuthenticationConverter be one that creates a DefaultOAuth2AuthenticatedPrincipal in order to preserve existing behavior.

And I'm not sure, but I believe that may require calling the authorities converter twice, once to populate DefaultOAuth2AuthenticatedPrincipal and once to populate BearerTokenAuthentication; however, I don't think that's a big issue as the point is to allow the strategy to be overridden.

[jzheaux]

Will you please update this to @since 7.0?

[jzheaux]

Will you please state the default value as well? For example:

* <p>By default, constructs a {@link DefaultOAuth2AuthenticatedPrincipal} based on the claims and authorities derived from the {@link Jwt}.