search

spring-projects / spring-security

Spring Security
http://spring.io/projects/spring-security
Apache License 2.0
8.86k stars 5.92k forks source link

Expose getter for nameAttributeKey in OAuth2AuthenticatedPrincipal #16003

Open andreblanke opened 1 month ago

andreblanke commented 1 month ago

Given a DefaultOidcUser or DefaultOAuth2User, it is currently not possible to create a faithful copy of the instance since the constructors require a nameAttributeKey for the all-arg constructor:

public DefaultOidcUser(Collection<? extends GrantedAuthority> authorities, OidcIdToken idToken,
        String nameAttributeKey)
public DefaultOAuth2User(Collection<? extends GrantedAuthority> authorities, Map<String, Object> attributes,
        String nameAttributeKey)

which is not accessible.

This issue has been mentioned before in https://github.com/spring-projects/spring-security/issues/14461#issuecomment-1898069450 where a custom ExtendedOidcUser decorator class is used to work around it. While that works, I feel that this should be possible without introducing a separate class using just the public API.

The PR aims to change this by adding the OAuth2AuthenticatedPrincipal.getNameAttributeKey method in 464b078d82632d3b8e162042fc6d27fb323a1fc2. This is a breaking change for classes implementing the interface.

With the nameAttributeKey now available from within the interface, I figured it also makes sense to provide a default implementation for OAuth2AuthenticatedPrincipal.getName in c7bcf865715d3f78017131e8e7470022f1508842.

jzheaux commented 1 week ago

Hi, @andreblanke! Are you able to make the requested changes? No rush, if I don't hear from you in about a week, I'm happy to make the changes myself.

andreblanke commented 5 days ago

Hi there @jzheaux. First of all thank you for your feedback. Sorry, I've been postponing this. I'd like to finish the changes in the next few days once we've decided on an implementation (getter for nameAttributeKey vs. new constructors).