OpenID Connect Userinfo not fetched for custom claims

[furti]

Summary

@jgrandja
We have a Problem retrieving the Userinfo data for our custom Identity Provider that implements OpenID Connect.
I stumbled accross #4451 and can totally understand why you made the retrieval of the userinfo optional.

But as far as I understand the specification, one can define custom claims https://openid.net/specs/openid-connect-core-1_0.html#AdditionalClaims

And also requesting of claims is optional. A IDP can decide what claims to send when nothing is requested.
https://openid.net/specs/openid-connect-core-1_0.html#ClaimsParameter

Support for the claims parameter is OPTIONAL. Should an OP not support this parameter and an RP uses it, the OP SHOULD return a set of Claims to the RP that it believes would be useful to the RP and the End-User using whatever heuristics it believes are appropriate. The claims_parameter_supported Discovery result indicates whether the OP supports this parameter.

Actual Behavior

With the implementation introduced in the issue linked above, it is not possible to retrieve the userinfo without specifying one of the default scopes.
But as said above, requesting a claim is totally optional. And the scope values are not the only way of requesting a claim.

But maybe my interpretation of the Spec is wrong and everything is totally fine :)

Expected Behavior

It should be possible to retrieve the userinfo without specifying a default scope.
Maybe some configurable "UserInfoRetrievalMatcher" interface. The default implementation is the current implementation, but applications can provide a custom implementation to decide if the userinfo should be loaded or not.

[jgrandja]

@furti If you look at the logic in OidcUserService.shouldRetrieveUserInfo(), you will see that the UserInfo request is initiated only if the user-info-uri is configured and at least one of profile, email, address and phone scopes was requested in the Authorization Request.

It sounds like you have user-info-uri configured but do not have any of the standard scopes configured. In this case, the UserInfo endpoint will not be called.

IMO, it makes sense to have at least profile configured given that the UserInfo response should return basic information of the user. Can you not configure the profile scope at minimum? Doesn't your custom Identity Provider support this scope?

Alternatively, you can always supply a custom OidcUserService via oauth2Login().userInfoEndpoint().oidcUserService() that determines whether the UserInfo endpoint is called or not based on your requirements.

As an FYI, requesting claims using the "claims" request parameter is not currently supported by OidcUserService. However, you do have the ability to customize the UserInfo request by supplying a custom DefaultOAuth2UserService.setRequestEntityConverter() and than setting the DefaultOAuth2UserService via OidcUserService.setOauth2UserService().

I'm going to close this issue as Works as Designed. However, we can re-open and discuss further if the solutions I provided do not work for your setup.

[furti]

@jgrandja thanks for your response.

It sounds like you have user-info-uri configured but do not have any of the standard scopes configured. In this case, the UserInfo endpoint will not be called.

Thats true. We don't need the profile as our identity provider decides on its own what data a application is allowed to get. It is not really needed that an application requests something.

Can you not configure the profile scope at minimum? Doesn't your custom Identity Provider support this scope?

Our Identity Provider simply ignores the requested scopes. Adding the scope makes everything work like it should. But it feels a bit like a Workaround to specify a scope that is known to be ignored anyways.

Alternatively, you can always supply a custom OidcUserService

Thanks for the hint. I think this is a good solution to simply use a custom implementation of the OidcUserService.
Unfortunately private boolean shouldRetrieveUserInfo(OidcUserRequest userRequest) { is private. Its not possible to simply extend the default implementation and override the method to fetch the userinfo. Maybe this method could be made protected so it is easier to reuse the already existing code?

[jgrandja]

@furti I see your point now. OidcUserService.shouldRetrieveUserInfo() should not restrict to standard scopes only. I'll address this shortly.

[jgrandja]

@furti Would you be interested in submitting a PR for this? I'm debating on whether making shouldRetrieveUserInfo() protected OR removing the logic that restricts on the standard scopes. What are your thoughts?

[furti]

@jgrandja I never used the OpenID Client Implementation of Spring Security myself. I'm maintaining our Identity Provider and somebody else had a Problem connecting to us. So I don't think if I'm the right one to submit a PR for this.

But I would aim for a combination of both.

Making the shouldRetrieveUserInfo() protected makes the service pretty flexible. So one can decide to load the userinfo based on any information available to the application. Maybe only load the userinfo between 07:00 and 16:00. No Problem when the method is protected.

But for simple usage scenarios the current logic could be preserved by creating a constructor that takes the list of scopes to load the userinfo for.

public class OidcUserService implements OAuth2UserService<OidcUserRequest, OidcUser> {
	...

	private final Set<String> userInfoScopes;
    
    ...

    public OidcUserService() {
        this(OidcScopes.PROFILE, OidcScopes.EMAIL, OidcScopes.ADDRESS, OidcScopes.PHONE)
    }

    public OidcUserService(String... userInfoScopes) {
        this(Arrays.asList(userInfoScopes));
    }

    public OidcUserService(Collection<String> userInfoScopes) {
        this.userInfoScopes = new HashSet<>(userInfoScopes);
    }
    ...
}

So it is pretty easy to load the userinfo for different scopes. And maybe change shouldRetrieveUserInfo() to load the userinfo when this.userInfoScopes.isEmpty() == true. So one can easily configure to always load the userinfo by using the constructor new OidcUserService(Collections.emptySet()).

This changes will preserve the current logic and make nearly all scenarios possible that I can think of :)