Current implementation

Current OidcReactiveOAuth2UserService will retrieve user info even if access token does not contain scopes like profile, email, address, phone.

https://github.com/spring-projects/spring-security/blob/5.6.0/oauth2/oauth2-client/src/main/java/org/springframework/security/oauth2/client/oidc/userinfo/OidcReactiveOAuth2UserService.java#L125-L128

https://github.com/spring-projects/spring-security/blob/5.6.0/oauth2/oauth2-client/src/main/java/org/springframework/security/oauth2/client/oidc/userinfo/OidcUserRequestUtils.java#L45-L66

Expected implementation

Expected behavior is: Let OidcReactiveOAuth2UserService do like OidcUserService: Not retrieve user info if access token does not contain scopes like profile, email, address, phone.

https://github.com/spring-projects/spring-security/blob/5.6.0/oauth2/oauth2-client/src/main/java/org/springframework/security/oauth2/client/oidc/userinfo/OidcUserService.java#L157-L179

Comment From: chenrujun

Hi, @eleftherias . Can I create a PR for this? Target branch is main.

Comment From: eleftherias

Thanks @chenrujun. Before accepting a PR I need to discuss with the rest of the team which of the two approaches we want to make the default.

In practice this should not be an issue if you have created the ClientRegistration with the correct OIDC scopes. Do you have a scenario where this is causing a problem?

Comment From: chenrujun

@eleftherias

Thank you for your quick response.

Here is sample application.yml:

spring:
  security:
    oauth2:
      client:
        provider:
          azure-active-directory:
            issuer-uri: https://login.microsoftonline.com/${tenant-id}/v2.0 
            user-name-attribute: name
        registration:
          client-1-resource-server-1:
            provider: azure-active-directory
            client-name: client-1-resource-server-1
            client-id: ${client-1-client-id}
            client-secret: ${client-1-client-secret}
            scope: openid, profile, api://${resource-server-1-client-id}/resource-server-1.scope-1

In application.yml, scope contains 3 tiems: openid, profile, api://${resource-server-1-client-id}/resource-server-1.scope-1. In the access token, it only contain 1 item: api://${resource-server-1-client-id}/resource-server-1.scope-1.

This is expected behavior of Azure Active Directory. Azure Active Directory developers think that one access token can only have one audience. openid and profile belong to one audience, and api://${resource-server-1-client-id}/resource-server-1.scope-1 belong to another audience. So the 3 scopes can not be contained in one access token.

Comment From: sjohnr

@chenrujun I may not fully understand your use case (apologies if so), but I believe what you're asking is for the OIDC support in the framework to check the scopes (beyond the openid scope) and conditionally make a request to the user info endpoint based on what scopes are provided.

I could be wrong, but I think the solution would be to split this usage out into two separate clients, one for OIDC/login, and another for protected calls to the resource server.

The reason for this (in my mind) would be that disabling the user info request depending on scopes would be akin to disabling OIDC verification of the user. Again, perhaps I'm misunderstanding something, but hopefully not. It seems to me that you should not be using OpenID for both scenarios. Calling the resource server is purely OAuth 2.0, while logging in is OIDC, and this seems to be reinforced by the assumption made in Azure Active Directory's design.

Does any of that sound accurate?

Comment From: chenrujun

Hi, @sjohnr .

Thank you for your quick response. Now I think you are right, split this usage out into two separate clients is the right way to handle my sample scenario.

Here is another request: How about sync the logic of shouldRetrieveUserInfo in OidcReactiveOAuth2UserService.java and OidcUserService.java? There is no reason that the 2 classes use different logic.

Comment From: sjohnr

Thanks @chenrujun. There may be some work on or around OidcUserService soon. See #9629. I'll take that suggestion under advisement. :smile:

Comment From: chenrujun

Hi, @sjohnr

After accept your suggestion, I updated application.yml from

spring:
  security:
    oauth2:
      client:
        provider:
          azure-active-directory:
            issuer-uri: https://login.microsoftonline.com/${tenant-id}/v2.0 
            user-name-attribute: name
        registration:
          client-1-resource-server-1:
            provider: azure-active-directory
            client-name: client-1-resource-server-1
            client-id: ${client-1-client-id}
            client-secret: ${client-1-client-secret}
            scope: openid, profile, api://${resource-server-1-client-id}/resource-server-1.scope-1

to

spring:
  security:
    oauth2:
      client:
        provider:
          azure-active-directory:
            issuer-uri: https://login.microsoftonline.com/${tenant-id}/v2.0 
            user-name-attribute: name
        registration:
          client-oidc:
            provider: azure-active-directory
            client-name: client-1-resource-server-1
            client-id: ${client-1-client-id}
            client-secret: ${client-1-client-secret}
            scope: openid, profile
          client-1-resource-server-1:
            provider: azure-active-directory
            client-name: client-1-resource-server-1
            client-id: ${client-1-client-id}
            client-secret: ${client-1-client-secret}
            scope: api://${resource-server-1-client-id}/resource-server-1.scope-1
  1. For client-oidc, there is no problem. The access token can be used to access user-info endpoint.
  2. But for client-1-resource-server-1, similar problem still exists. The access token is used to access resource-server-1, it can not be used to access user-info endpoint. IMU, for client-1-resource-server-1, it should not retrieve user info. But current OidcUserRequestUtils#shouldRetrieveUserInfo returned true.

Here is related code: https://github.com/spring-projects/spring-security/blob/5.6.0/oauth2/oauth2-client/src/main/java/org/springframework/security/oauth2/client/oidc/userinfo/OidcUserRequestUtils.java#L45-L66

    static boolean shouldRetrieveUserInfo(OidcUserRequest userRequest) {
        // Auto-disabled if UserInfo Endpoint URI is not provided
        ClientRegistration clientRegistration = userRequest.getClientRegistration();
        if (StringUtils.isEmpty(clientRegistration.getProviderDetails().getUserInfoEndpoint().getUri())) {
            return false;
        }
        // The Claims requested by the profile, email, address, and phone scope values
        // are returned from the UserInfo Endpoint (as described in Section 5.3.2),
        // when a response_type value is used that results in an Access Token being
        // issued.
        // However, when no Access Token is issued, which is the case for the
        // response_type=id_token,
        // the resulting Claims are returned in the ID Token.
        // The Authorization Code Grant Flow, which is response_type=code, results in an
        // Access Token being issued.
        if (AuthorizationGrantType.AUTHORIZATION_CODE.equals(clientRegistration.getAuthorizationGrantType())) {
            // Return true if there is at least one match between the authorized scope(s)
            // and UserInfo scope(s)
            return CollectionUtils.containsAny(userRequest.getAccessToken().getScopes(),
                    userRequest.getClientRegistration().getScopes());
        }
        return false;
    }

Comment From: sjohnr

Hi @chenrujun. How are you using the client-1-resource-server-1 client registration? Since it does not contain the openid scope, it should not be used for .oauth2Login(), only for the .oauth2Client().

Comment From: chenrujun

@sjohnr Problem solved by setting .oauth2Client(), thank you very much.

Comment From: sjohnr

Awesome. Is there anything remaining that would require leaving this issue open? Or should we go ahead and close it?

Comment From: chenrujun

@sjohnr

Is there anything remaining that would require leaving this issue open? Or should we go ahead and close it?

There may be some work on or around OidcUserService soon. See #9629. I'll take that suggestion under advisement. 😄

If the logic of OidcReactiveOAuth2UserService.java and OidcUserService.java will be same after resolve #9629, I think it's OK to close this issue.

Thank you for your support.

Comment From: sjohnr

We'll have some design work to do before working on either service, but I think it would trigger changes to both versions of the service, so I would seek to make them in sync if possible, yes. I'll close this now. Glad we were able to make progress!