- changed status to open
Make AuthorizationGrant implementation classes extensible
Issue #105
resolved
We extend some of the SDK classes to enable our scenarios and the ability to extended different grant implementations was one of them. I would be really helpful if you could remove the final keyword from the grant type classes. This will unblock us from adopting the latest and greatest changes!
Comments (19)
-
-
Another class that has been set to final is OIDCAccessTokenResponse.
-
Is there any nightly available so that we can unblock ourselves until a release comes out?
-
No, there's no nightly. We'll do the changes and we'll push them to Maven Central shortly. We'll let you know here once it's done.
-
Removed final from AuthorizationCodeGrant (commit db47b51).
-
Removed final from OIDCAccessTokenResponse (commit 77fd34b).
-
I see. Is there any specific reason to have classes final though? We would also like to support saml 1.1, so we would need to create a custom grant type.
-
Could you please remove final from all grant implementation classes?
-
Removes final from remaining AuthorizationGrant classes (commit 202854e).
-
Is there any specific reason to have classes final though?
The idea has been to create a grant class structure that would cover all possible std grants, and wouldn't need extending.
But apparently that didn't work out in your case :)
-
One problem I see with SAML 1.1- the GrantType has a private constructor.
That was done to ensure only the constants are used, which include two important parameters -- whether client auth and an explicit client_id is required in the token request.
-
Hi Kanishk,
The changes got pushed to Maven Central as version 4.4.3 a minute ago.
-
If you remove final, make your constructor public and make default constructor private then it would work the same way. Right?
-
Thanks for the suggestion.
There is a static parse method in the AuthorizationGrant class, that needs to know its extending classes:
Any suggestions how to deal with this?
-
The static TokenRequest.parse method also needs to have prior knowledge of the extending AuthorizationGrant classes:
Perhaps that was part of the reason these classes were declared final.
We'll try to get Vladimir's input on this.
-
We just had an internal consultation on this issue. One way around it is to devise a parser or lookup interface, and use that as an optional argument to the static parse methods. That, however, would take some time to resolve.
The good implication is that this only affects server-side processing of TokenRequests. If you're using the SDK on the client side only, we could just open up the GrantType constructor (as you suggested), so you can continue your work, and we'll deal with the server-side issue later.
Please, let us know what your situation is.
-
I am using the sdk only on the client side. Thanks a lot for the changes! You guys rule!
-
The GrantType has now got a default public constructor, and the static parse method was unbounded:
https://bitbucket.org/connect2id/oauth-2.0-sdk-with-openid-connect-extensions/commits/bfa4ad7
The new changes got pushed to Maven Central as version 4.5.
If you have any other suggestions, just let us know!
Cheers!
-
- changed status to resolved
- Log in to comment
Accepted :)