Expected Behavior
AbstractOAuth2AuthorizationGrantRequestEntityConverter is a base implementation converter to convert the OAuth2 Access token request for the Authorization Grant.
But when a new AuthorizationGrantType is added, the XXXGrantRequestEntityConverter can not inherit AbstractOAuth2AuthorizationGrantRequestEntityConverter class, so the current authorization grant type does not support fully extending a new authorization grant type process.
The XXXGrantRequestEntityConverter can inherit the base implementation class, and can add an additional parameter converter, such as, when supporting JWT client authentication, the new converter can add special parameters for the token request.
Current Behavior Only partially can enhance the built-in authorization grant type.
Comment From: sjohnr
Hi @moarychan, thanks for your interest in the project!
AbstractOAuth2AuthorizationGrantRequestEntityConverter is designed to support internal implementations of converters for authorization grant requests. Spring Security takes a conservative approach to exposing APIs publicly, but we do evaluate the opportunity to expose APIs for a specific reason on a case-by-case basis. In this case, I don't believe we would want to expose that abstract class and encourage users to extend it.
Ideally, any new grant types could be incorporated directly into the framework through a pull request, where it could make use of that abstract class. As you know, it's best to start by opening an issue to discuss it first.
Also, see NimbusJwtClientAuthenticationParametersConverter which adds support for JWT client authentication. Also note that you can add or override parameters converters on existing subclasses of the AbstractOAuth2AuthorizationGrantRequestEntityConverter. See examples of addParametersConverter() and setParametersConverter() in the docs. As you can see, there's quite a bit of flexibility in the existing classes already.
I'm going to close this issue for now, but if you feel I've missed anything feel free to add additional comments and we can re-open if necessary.
Comment From: moarychan
Hi @sjohnr , thanks for your response.
I have an additional question about the authorization grant extension consistency:
- Such as the AuthorizationGrantType class, maybe use the enum type instead of class, so the new grant types will be added through a pull request because the developers can create a new type, or only use the built-in authorization grant types enough.
- Such as the AbstractOAuth2AuthorizationGrantRequest class, maybe nonpublic, and this XXXGrantRequest class should not be available.
- Developers can implement other classes, such as OAuth2AccessTokenResponseClient<XXXGrantRequest> and XXXOAuth2AuthorizedClientProvider.
Is it necessary to keep them consistent to ensure that users behave within the period? 😊
Comment From: moarychan
Hi @sjohnr , we have added an extension AadOboOAuth2AuthorizedClientProvider for ON_BEHALF_OF grant type, and it's the same as JWT_BEARER grant type, we are migrating to use JWT_BEARER and want to support the two types.
The current OBO provider is not fully following the base class provided by the security, but when I want to inherit to AbstractOAuth2AuthorizationGrantRequestEntityConverter class, it's not allowed to achieve.
There's an exception process in the provider and I can not find the hook point to migrate this into the JwtBearerOAuth2AuthorizedClientProvider class.
https://github.com/Azure/azure-sdk-for-java/blob/f69cc4531667326175084caba66d2580a086fb78/sdk/spring/spring-cloud-azure-autoconfigure/src/main/java/com/azure/spring/cloud/autoconfigure/aad/implementation/webapi/AadOboOAuth2AuthorizedClientProvider.java#L146-L153
Comment From: sjohnr
The OAuth2AuthorizedClientProvider is responsible for returning an OAuth2AuthorizedClient instance. Any failure handling would be in the OAuth2AuthorizedClientManager. For example, in DefaultOAuth2AuthorizedClientManager we see:
https://github.com/spring-projects/spring-security/blob/f3c96fa9cd542763180121be64e72ca88302d346/oauth2/oauth2-client/src/main/java/org/springframework/security/oauth2/client/web/DefaultOAuth2AuthorizedClientManager.java#L175-L182
So you can plug in a failure handler for your OAuth2AuthorizedClientManager that could handle exceptions.
Comment From: moarychan
Hi @sjohnr , yes, it will be more convenient if we can plug in a failure handler in each provider level, such as in org.springframework.security.oauth2.client.JwtBearerOAuth2AuthorizedClientProvider#getTokenResponse.