Skip to content

Incorrect javadoc in AuthorizationCodeOAuth2AuthorizedClientProvider #9708

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
talentedasian opened this issue May 3, 2021 · 4 comments
Closed
Assignees
Labels
in: oauth2 An issue in OAuth2 modules (oauth2-core, oauth2-client, oauth2-resource-server, oauth2-jose) type: enhancement A general enhancement
Milestone

Comments

@talentedasian
Copy link
Contributor

https://github.com/spring-projects/spring-security/blob/main/oauth2/oauth2-client/src/main/java/org/springframework/security/oauth2/client/AuthorizationCodeOAuth2AuthorizedClientProvider.java

The code in the link above returns either only a null or an exception. While the documentation says it also returns an Oauth2AuthorizedClient

@talentedasian talentedasian added status: waiting-for-triage An issue we've not yet triaged type: bug A general bug labels May 3, 2021
@jzheaux
Copy link
Contributor

jzheaux commented May 6, 2021

Thanks for the report, @talentedasian. Would you be able to submit a PR updating the description?

I imagine it might read something like this:

/**
  * ...
  * @return {@code null} if authorization is not supported
  * @throws ClientAuthorizationRequiredException in order to trigger authorization
  */

@jzheaux jzheaux added in: oauth2 An issue in OAuth2 modules (oauth2-core, oauth2-client, oauth2-resource-server, oauth2-jose) and removed status: waiting-for-triage An issue we've not yet triaged labels May 6, 2021
@jzheaux jzheaux self-assigned this May 6, 2021
@talentedasian
Copy link
Contributor Author

Was it intentional to just either return null or throw an exception? If so, do you mind explaining why as I think that it if it does really return just a null then it would be redundant

@talentedasian
Copy link
Contributor Author

Thanks for the report, @talentedasian. Would you be able to submit a PR updating the description?

I imagine it might read something like this:

/**
  * ...
  * @return {@code null} if authorization is not supported
  * @throws ClientAuthorizationRequiredException in order to trigger authorization
  */

I think it's best if someone else updates the description :)

@talentedasian
Copy link
Contributor Author

Was it intentional to just either return null or throw an exception? If so, do you mind explaining why as I think that it if it does really return just a null then it would be redundant

After reading through the code again, I now get it. The name was confusing though. But yeah, changing the description should suffice

@jgrandja jgrandja added type: enhancement A general enhancement and removed type: bug A general bug labels May 13, 2021
@jgrandja jgrandja assigned talentedasian and unassigned jzheaux May 13, 2021
@jgrandja jgrandja added this to the 5.5.0 milestone May 13, 2021
@jgrandja jgrandja changed the title AuthorizationCodeOAuth2AuthorizedClientProvider's code only either returns a null or throws an exception and not an Oauth2AuthorizedClient at all Incorrect javadoc in AuthorizationCodeOAuth2AuthorizedClientProvider May 13, 2021
marcusdacoregio pushed a commit to marcusdacoregio/spring-security that referenced this issue May 13, 2021
akohli96 pushed a commit to akohli96/spring-security that referenced this issue Aug 25, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
in: oauth2 An issue in OAuth2 modules (oauth2-core, oauth2-client, oauth2-resource-server, oauth2-jose) type: enhancement A general enhancement
Projects
None yet
Development

No branches or pull requests

3 participants