I believe I have found a unused/unnecessary bit of code in the okta-java-sdk
and would like to bring it your attention.
In the DefaultClientBuilder#build
method, if you are configuring the client for OAuth2 with privateKey
, the following logic gets hit (see here in Github → okta/okta-sdk-java → impl/src/main/java/com/okta/sdk/impl/client/DefaultClientBuilder.java#L378):
this.clientConfig.setClientCredentialsResolver(new DefaultClientCredentialsResolver(oAuth2ClientCredentials));
but it doesn’t actually do anything from what I can tell. I’d attach pictures showing evidence of this in my IDE, but I’m not allowed to as a new user… but the getCredentialsResolver
method seems to never be called subsequently…meaning this is unused and unnecessary.
From what I can tell, this is just a leftover artifact from a previous version of the code where an underlying base class required the ClientCredentialsResolver
to be not null.
See here where this code was first added, and this line was discussed: Add OAuth2 Support by arvindkrishnakumar-okta · Pull Request #354 · okta/okta-sdk-java · GitHub. You can see that the BaseClient
being discussed requires a DefaultDataStore
(link omitted because limited), and then DefaultDataStore
requires the passed in clientCredentialsResolver
to be non-null (link omitted because limited).
The mentioned classes making this required were then removed/refactored in PR #776 (link omitted because limited); I believe this PR should have also removed the line I’m questioning.
I would have posted this straight to the Github repo, but technically it’s a question and the Issue template there directed me to post here. If this would be more appropriate to discuss in Github, I’m happy to transpose this over there.