Skip to content

Gh 6053 support jwt bearer grant type #9505

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

Conversation

H-LREB
Copy link
Contributor

@H-LREB H-LREB commented Mar 16, 2021

This PR is related to gh-6053.

I have been using the classes from Joe Grandja POC for a while and they are working fine. This PR aims at integrating those classes into Spring Security :

  • Bring classes from POC
  • Change method hasTokenExpired in JwtBearerOAuth2AuthorizedClientProvider to avoid error when including skew time (same as gh-7511)
  • Rename classes to follow conventions
  • Define and use constants in AuthorizationGrantType, OAuth2ParameterNames and OAuth2AuthorizationContext
  • Add jwtBearer builder in OAuth2AuthorizedClientProviderBuilder
  • Add Junit tests, mainly copied and adapted from Password grant

H-LREB added 2 commits March 14, 2021 15:46
- Change method hasTokenExpired in JwtBearerOAuth2AuthorizedClientProvider to avoid error when including skew time (same as spring-projectsgh-7511)
- Rename classes to follow conventions
- Define and use constants in AuthorizationGrantType, OAuth2ParameterNames and OAuth2AuthorizationContext
- Add jwtBearer builder in OAuth2AuthorizedClientProviderBuilder
- Add Junit tests, mainly copied and adapted from Password grant
@pivotal-issuemaster
Copy link

@H-LREB Please sign the Contributor License Agreement!

Click here to manually synchronize the status of this Pull Request.

See the FAQ for frequently asked questions.

@pivotal-issuemaster
Copy link

@H-LREB Thank you for signing the Contributor License Agreement!

@jgrandja
Copy link
Contributor

Thank you @H-LREB ! Please see comment.

@jgrandja jgrandja added in: oauth2 An issue in OAuth2 modules (oauth2-core, oauth2-client, oauth2-resource-server, oauth2-jose) type: enhancement A general enhancement and removed status: waiting-for-triage An issue we've not yet triaged labels Mar 17, 2021
@jgrandja jgrandja added this to the 5.5.0-RC1 milestone Mar 17, 2021
@jgrandja
Copy link
Contributor

jgrandja commented Apr 1, 2021

@H-LREB I just submitted gh-9535 which includes the changes in this PR and adds a polish commit. I ended up removing some of the code so please let me know if you have any questions. I'm going to have @rwinch review gh-9535 and then we will merge before RC1.

I'm going to close this PR.

@jgrandja jgrandja closed this Apr 1, 2021
@jgrandja jgrandja added status: duplicate A duplicate of another issue and removed type: enhancement A general enhancement labels Apr 1, 2021
@jgrandja jgrandja removed this from the 5.5.0-RC1 milestone Apr 1, 2021
@H-LREB
Copy link
Contributor Author

H-LREB commented Apr 2, 2021

Thank you @jgrandja. That is a great news. I am so happy the feature is likely to be part of the next release.

However, could you reconsider the choice of not allowing the reuse of the obtained access token in class JwtBearerOAuth2AuthorizedClientProvider?

I think the reason behind this choice is mainly the phrase "a reasonably short lifetime" in the spec. However, this does not necessarily imply that the access token cannot be reused. Besides, a "reasonably short" time is relative to the application context (could be a few minutes, several minutes, 1h et.c).

Thus, I think the Spring Framework still can allow reusing the obtained access token as long as it is valid, and let the OAuth Server decide about the reasonable validity interval. In high frequency services, even the reuse of the access token for 5 or 10 minutes may spare a lot of requests to the OAuth server.

jgrandja added a commit to jgrandja/spring-security that referenced this pull request Apr 8, 2021
jgrandja added a commit that referenced this pull request Apr 9, 2021
@jgrandja
Copy link
Contributor

jgrandja commented Apr 9, 2021

@H-LREB

could you reconsider the choice of not allowing the reuse of the obtained access token in class JwtBearerOAuth2AuthorizedClientProvider?

The JwtBearerOAuth2AuthorizedClientProvider does allow for reuse of the previously exchanged token.

I think the Spring Framework still can allow reusing the obtained access token as long as it is valid, and let the OAuth Server decide about the reasonable validity interval.

Yes, this is how it was implemented.

@jgrandja
Copy link
Contributor

jgrandja commented Apr 9, 2021

@H-LREB Thanks for the push on getting this merged !

@H-LREB
Copy link
Contributor Author

H-LREB commented Apr 9, 2021

@jgrandja Thank you for these answers. Indeed, I have not read the code carefully enough before submitting my previous comment.

I have another question though. I posted it in 9535 discussion.

akohli96 pushed a commit to akohli96/spring-security that referenced this pull request 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) status: duplicate A duplicate of another issue
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants