[BUG] AadClientRegistrationRepository#toClientRegistration did not set userInfoUri

[chenrujun]

Context

In current AadClientRegistrationRepository#toClientRegistration , userInfoUri is not set, which caused OidcUserService#shouldRetrieveUserInfo always returns false.

Problem

OidcUserService#shouldRetrieveUserInfo always returns false, it is unexpected behavior.

Goal

1. Make userInfoUri configured and use it to get user information.
2. Confirm proxy setting will work when send http request to userInfoUri. DefaultOAuth2UserService's RestTemplate. Refs: [BUG] Make sure all RestTemplate are managed by AadRestTemplateCreator #31347 (comment)

Useful links

https://learn.microsoft.com/en-us/azure/active-directory/develop/userinfo

[chenrujun]

In spring-boot-autoconfigure, userInfoUri will be got from issuerUri

[chenrujun]

Screenshot of quick fix:

Need add more unit tests.

[backwind1233]

The userInfoUri is used to get OidcUserInfo,

• and then the OidcUserInfo will be used to construct OidcUser .

As the java doc of OidcUser says:

An OidcUser contains "claims" about the authentication of the End-User.  
The claims are aggregated from the OidcIdToken and the OidcUserInfo (if available).

So whether to set userInfoUri, it depends, we need to check whether there are different "claims" in OidcUserInfo rather than in the OidcIdToken, and then make a decision whether to set the value by default, or provide an option to user.

Also, our AadOAuth2UserService#loadUser doesn't use the oidcUser information returned by OidcUserService#loadUser.
So we need to redesign(confirm), what's the infomation we decide to return by AadOAuth2UserService#loadUser exactly.

[backwind1233]

We need to make a design(decision) what is expected to return in AadOAuth2UserService#loadUser .

• Update code logic.
• Update java doc.

[backwind1233]

The javadoc does not match the implementation.

1. UserInfo Endpoint is not set.
2. It does not use the userInfo to build the result.

[backwind1233]

• The userInfoUri in the AadClientRegistrationRepository#toClientRegistration class, which means for applications using our aad starter, when the applications' users login and call AadOAuth2UserService#loadUser() to load OidcUser, the method call will never obtain the user attributes of the end-user from the UserInfo Endpoint. This means that our Java doc is not correct.
  

• Also in our implementation of the loadUser method, we actually did not use the userInfo to construct the return information.

  We build and return a DefaultOidcUser instance by using the below DefaultOidcUser constructor, which will always takes a null value for userInfo .
  

  This means no matter whether the userInfoUri is set or not, we will never use or return the userInfo.

  This logic has not changed since the first commit.

I think we have two options:

1. Keep the logic.
   • Since we are using the constructor for DefaultOidcUser that does not take userInfo explicitly, and no users have raised an issue, we can keep things as they are, but we need to refactor our code and modify our javadoc to reflect our current logic.
2. Provide an implementation of the loadUser method that returns a DefaultOidcUser instance containing userInfo.

I prefer to the second option.

• From the javadoc of the interface, we can infer that it is expected to obtain the UserInfo from the UserInfo Endpoint and return it.

  • 

• The Spring official documentation also provides a sample implementation that includes setting the userInfoUri, by default it will return userInfo.

  • 

What do you think @saragluna ?

[saragluna]

IMO, we should figure out the following questions before making a decision:

• What's the API (input/output) of a OAuth2UserService#loadUser method
• What's the difference between the API and our current implementation
• What's the difference between the responses from the user info endpoint and what we call from the graph endpoint (our current implementation)?

[backwind1233]

What's the API (input/output) of an OAuth2UserService#loadUser method

OAuth2UserService

@FunctionalInterface 
public interface OAuth2UserService<R extends OAuth2UserRequest, U extends OAuth2User> {
    U loadUser(R userRequest) throws OAuth2AuthenticationException;
}

• Input: an OAuth2UserRequest

• Output: an OAuth2User (it extends OAuth2AuthenticatedPrincipal), it contains below APIs

   public interface OAuth2User extends OAuth2AuthenticatedPrincipal {
   }
   public interface OAuth2AuthenticatedPrincipal extends AuthenticatedPrincipal {
       default <A> A getAttribute(String name) {
           return (A) getAttributes().get(name);
       }
       Map<String, Object> getAttributes();
       Collection<? extends GrantedAuthority> getAuthorities();
   } 
       

What's the difference between the API and our current implementation

• our current API

  public class AadOAuth2UserService implements OAuth2UserService<OidcUserRequest, OidcUser> {
      @Override
      public OidcUser loadUser(OidcUserRequest userRequest) throws OAuth2AuthenticationException {
                      // construct authorities
                      // construct idToken
                      // construct userInfo
                      // construct nameAttributeKey
      }
  }

  • Input: an OidcUserRequest which extends OAuth2UserRequest
  • Output: an OidcUser which extends OAuth2User, it contains below APIs

    public interface OidcUser extends OAuth2User, IdTokenClaimAccessor {
        @Override 
        Map<String, Object> getClaims();
        OidcUserInfo getUserInfo();
        OidcIdToken getIdToken();
        
        // ... some other APIs in IdTokenClaimAccessor 
    }

What's the difference between the responses from the user info endpoint and what we call from the graph endpoint (our current implementation)?

• This is an example of the response from the user info endpoint

Note You can't add to or customize the information returned by the UserInfo endpoint.

{
  "sub": "OLu859SGc2Sr9ZsqbkG-QbeLgJlb41KcdiPoLYNpSFA",
  "name": "Mikah Ollenburg", // all names require the “profile” scope.
  "family_name": " Ollenburg",
  "given_name": "Mikah",
  "picture": "https://graph.microsoft.com/v1.0/me/photo/$value",
  "email": "mikoll@contoso.com" // requires the “email” scope.
}

• the graph endpoint (our current implementation) only used to get group information.
  

Summary

The picture shows the difference between our Spring Cloud Azure implementaition and Spring security implementaition.

No	Step name	Implementaition In Spring Security	Implementaition In Spring Cloud Azure
Step1	Construct Authorities	Authorities from accessToken scopes + "ROLE_USER"	Authorities from groupids + groupnames + roles
Step2	Construct UserInfo	userInfo from userInfo endpoint	uri not passed, skip constructing
Step3	Construct IdToken	IdToken from input request	IdToken from input request
Step4	Construct nameAttributeKey	IdTokenClaimNames.SUB("sub")	User passed nameAttributeKey, default == "name"
Step5	Construct Result	return new DefaultOidcUser( authorities, idToken, null, nameAttributeKey)	return new DefaultOidcUser( authorities, idToken, userInfo, IdTokenClaimNames.SUB)

Step1 in detail

This is a sample to show implementaition logic in Spring Cloud Azure.

Below is a sample to show the implementaition logic in Spring Security.

[backwind1233]

Updates relates to Step2 "Construct UserInfo"

• In the interface, the getUserInfo() may be null, because the userinfo_endpoint is not a REQUIRED but RECOMMENDED claim in OpenID Provider Metadata.

  https://openid.net/specs/openid-connect-discovery-1_0.html#ProviderMetadata

  userinfo_endpoint
  RECOMMENDED. URL of the OP's UserInfo Endpoint [OpenID.Core]. This URL MUST use the https scheme and MAY contain port, path, and query parameter components.

   public interface OidcUser extends OAuth2User, IdTokenClaimAccessor {
   	/**
   	 * Returns the claims about the user. The claims are aggregated from
   	 * {@link #getIdToken()} and {@link #getUserInfo()} (if available).
   	 * @return a {@code Map} of claims about the user
   	 */
   	@Override
   	Map<String, Object> getClaims();
   
   	OidcUserInfo getUserInfo();
       // ... other coes
   } 

• In Azure Directory, the userinfo_endpoint will be returned while calling https://login.microsoftonline.com/common/v2.0/.well-known/openid-configuration.

  You can find the UserInfo endpoint programmatically by reading the userinfo_endpoint field of the OpenID configuration document at https://login.microsoftonline.com/common/v2.0/.well-known/openid-configuration.

  https://learn.microsoft.com/en-us/azure/active-directory/develop/userinfo#find-the-well-known-configuration-endpoint

• Azure AD suggest get user's information from idtoken.

  we suggest getting the user's information from the token instead of calling the UserInfo endpoint.
  https://learn.microsoft.com/en-us/azure/active-directory/develop/userinfo#consider-using-an-id-token-instead