Consider renaming ProviderSettings to something more specific and enhance javadoc

[asaikali]

The ProviderSettings class

spring-authorization-server/oauth2-authorization-server/src/main/java/org/springframework/security/oauth2/server/authorization/config/ProviderSettings.java

Line 31 in 718b86c

public final class ProviderSettings extends AbstractSettings {

does not give any clues about its purpose. The word provider is too generic and raises questions about exactly is being provided. ProviderSettings extends from AbstractSettings which again does not provide hints on what this for, beyond it is just something to hold settings. The ClientSettings and TokenSettings extend AbstractSettings and it is much clearer from the name what they do.

I suggest that the ProviderSettings should be renamed to AuthorizationServerSettings to clearly explain its purpose and make it easy for users to know what is is used for.

[jgrandja]

@asaikali I did originally consider AuthorizationServerSettings but it was specific to OAuth2 only and is not inclusive to OIDC settings.

You will notice ProviderSettings.getOidcUserInfoEndpoint() and ProviderSettings.getOidcClientRegistrationEndpoint, which are OIDC specific settings, whereas, ProviderSettings.getAuthorizationEndpoint() and ProviderSettings.getTokenEndpoint() are OAuth2 specific settings.

It was difficult to come up with a name to accomodate both OAuth2 and OIDC settings, so the end result was a generic name Provider* to accommodate both configuration scenarios.

I could potentially introduce a hierarchy AuthorizationServerSettings extends AbstractSettings and OidcProviderSettings extends AuthorizationServerSettings but this introduces some complexity in a "should be" simple design.

If we improved the javadoc, would this help? Any other suggestions?

[asaikali]

I think improving the Javadoc helps for sure.

When I see AuthorizationServerSettings I don't think of OIDC or OAuth2 I just think this is settings required by the server. I can see why AuthorizationServerSettings might be too generic and might raise the question why do I have to setup other beans the AuthorizationServerSettings` should have all the server settings . I think introducing a class hierarchy is too much complexity.

How about:

• ProtocolSettings - Sets the exceptions that is being used to configure protocol related settings so endpoints fits in while it stays generic of OIDC or OAuth2 and allows for non end point settings.
• ProtocolEndpointSettings - describes exactly all the settings that exist in ProviderSettings today, if you have non endpoint settings in the future you will need to create a settings class with another name.

[jgrandja]

@asaikali I tried out a few different names for ProviderSettings and ended up with AuthorizationServerSettings. This name makes the most sense and clearly defines its purpose. Although I was hung up with the fact that it holds OIDC specific settings as well, the fact is that an OIDC Provider is an OAuth2 Authorization Server so this alone solidified the naming decision. Thank you for raising this issue and the naming suggestion!

Closing this as a duplicate in favour of gh-864.

See related commits gh-865, gh-866, gh-867