Make OidcUserService overrideable

[AB-xdev]

Expected Behavior

I would like to extend OidcUserService and override certain methods e.g. shouldRetrieveUserInfo

Current Behavior

You can't override like ~80% of the class because it's method/fields final or private

Context

How has this issue affected you?
What are you trying to accomplish?
What other alternatives have you considered?
Are you aware of any workarounds?

I have to copy paste the complete class -which will likely break when I update the library.
Or I get the reflection "crowbar" and make methods somehow accessible.
Or I create a lot of issues here that are related to my use-case which just costs a lot of time and effort and then I still have to wait for a release.

It also doesn't matter that this was changed in 96e3e4f, this is a general problem.

[sjohnr]

@AB-xdev, see this comment on the corresponding PR that was opened. Can you please describe your use case here and why you are unable to achieve your goal with the recent enhancement to this class? Or better still, can you provide a minimal reproducible example demonstrating what you had to do to accomplish it?

[AB-xdev]

Can you please describe your use case here and why you are unable to achieve your goal with the recent enhancement to this class?

We are using spring-boot 3.2.4 which contains spring-security 6.2.3.
Currently I would like to override shouldRetrieveUserInfo to additionally check if the required data (in my case email + fullname) is already present and if so don't perform the retrieve operation.

Code looks like this:

@Override
protected boolean shouldRetrieveUserInfo(final OidcUserRequest userRequest)
{
	// Check if required data is NOT already present
	if(Optional.ofNullable(userRequest.getIdToken())
		.map(t -> t.getEmail() == null || t.getFullName() == null)
		.orElse(true))
	{
		return super.shouldRetrieveUserInfo(userRequest);
	}
	// If data is already present don't fetch additional data
	return false;
}

I see that 96e3e4f added an option for that in release 6.3 however there are a few problems:

1. The release is not available yet - I can't use it
2. The changes from the commit mentioned above still don't allow me to access the default method (which is still private).
3. It doesn't address the general problem that when I need to override something inside the class due to various reasons I currently can't as mentioned above.

However, opening up classes to extension in this way isn't the solution that we're looking for.

Could you please give some additional context why this isn't an solution?

[sjohnr]

Hi @AB-xdev, thank you for the additional information!

I see that 96e3e4f added an option for that in release 6.3 however there are a few problems:

1. The release is not available yet - I can't use it

This would still be an issue for you with the proposed change. If you need a change sooner than the release, we generally recommend you simply copy the source of the class in the latest commit and add it to your project (and configure the custom class to be used) as a temporary workaround. The class can be removed once you have upgraded to the released version.

2. The changes from the commit mentioned above still don't allow me to access the default method (which is still private).

The logic in the default implementation is minimal and handles only the general case. Your case is more specific, and therefore is usually safe to use in place of the default. You don't need a fallback to the default if you know you have configured the ClientRegistration to contain one of the valid scopes for making the UserInfo request (profile, phone, email or address). If you can't make that assumption in your own code, you can replace the super call with the following which I feel is quite reasonable:

var hasUserInfoEndpoint = StringUtils.hasLength(userRequest.getClientRegistration()
		.getProviderDetails()
		.getUserInfoEndpoint()
		.getUri());
var hasEmptyScopes = CollectionUtils.isEmpty(userRequest.getAccessToken().getScopes());
var hasAccessibleScopes = CollectionUtils.containsAny(
		userRequest.getAccessToken().getScopes(),
		Set.of(OidcScopes.PROFILE, OidcScopes.EMAIL, OidcScopes.ADDRESS, OidcScopes.PHONE));
return hasUserInfoEndpoint && (hasEmptyScopes || hasAccessibleScopes);

3. It doesn't address the general problem that when I need to override something inside the class due to various reasons I currently can't as mentioned above.

Could you please give some additional context why this isn't an solution?

As maintainers of Spring Security, we're looking to be consistent across the codebase, which generally prefers delegation over inheritance.

Given the above explanation and workaround I mentioned, I'm going to close this issue. Please give the suggested workaround a try while you wait for the next release. Once the update is available, you can provide your own customization using one of the two options I explained depending on your configuration, both of which I feel are reasonable. Our primary goal with gh-13259 was to allow for customization which is now possible and will be available with the next release.