Skip to content
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

support tokens without audience and scope from claims when using JwtBearerGrant #13

Merged
merged 4 commits into from
Oct 7, 2020

Conversation

ybelMekk
Copy link
Contributor

@ybelMekk ybelMekk commented Oct 6, 2020

I ran in to this problem, adding mockOAuth2Sever to my integration tests in maskinporten repo, integration with idporten.

I make a token request to exchange my assertion with an access_token.
(I know the implementation right now do no authenticate, as it is implicit in the assertion signature, using JWTs as Authorization Grants, the scope MAY be used, the standard also says: Authentication of the client is optional, ref: https://tools.ietf.org/html/rfc7523).

Before implementation, an request with form post with body containing only grant_type and assertion (no scope). The OAuth2TokenResponse fail and mockOAuth2Sever return 500 on /token endpoint.

Ive added the ability to first check the claim("scope") as for maskinporten/idporten assertions the "scope" value is in the jwtclaims, now not failing with a "maskinporten" request. As for audience, i was thinking to implement the ability to have a token without audience, tho i ran in some smaller problems,

  1. ExchangeAccessToken uses as a default all claims from assertion and overwrite values according to response. Making the aud claim default, as i filtered i found that the verify method for tokenResponse, has aud claim as a default when validating token, so i stopped...
  2. May need som guidance how to solve the issue with "aud" free token responses?
  3. Also added check for setting aud from claims "resource" as it how maskinporten/idporten solves the "aud" not being "mandatory" field in the token, but is extracted from claims.

To add, this solves my problem. But is it the right way? :-D

…ion tests in maskinporten, integration with maskinporten/idporten. I make a token request to exchange my assertion with an access_token. (I know the implementation right now do no authenticate, as it is implicit in the assertion signature, Using JWTs as Authorization Grants, the scope MAY be used, the standard also says: Authentication of the client is optional, ref: https://tools.ietf.org/html/rfc7523). Before implementation a request with form post with body containing only grant_type and assertion (no scope). The OAuth2TokenResponse fail and mockOAuth2Sever return 500 on /token endpoint. Ive added the ability to first check the claim("scope") as for maskinporten assertions the "scope" value is in the jwtclaims, now not failing with a "maskinporten" request. As for audience, i was thinking to implement the ability to have a token without audience, tho i ran in some smaller problems,

1. exchangeAccessToken uses as a default all claims from assertion and overwrite values according to response. Making the aud claim default, as i filtered i found that the verify method for tokenResponse, has aud claim as a default when validating token, so i stopped... May need som guidance how to solve the issue with "aud" free tokens? To match the reality of using idporten/maskinporten? :D I also added check for setting aud from claims "resource" as it how maskinporten solves the "aud" not being "mandatory" field in the token, but is extracted from claims.
@ybelMekk ybelMekk requested a review from a team as a code owner October 6, 2020 22:34
Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Scan Summary

Tool Critical High Medium Low Status
Kotlin Security Audit 0 0 1 0
Kotlin Static Analysis 0 0 3 0

Recommendation

Looks good ✔️

ybelMekk and others added 2 commits October 7, 2020 10:29
…th2TokenCallback.kt

* api change on OAuth2TokenCallback.kt, audience now returns List<String> instead of String
* an empty list for audience() in OAuth2TokenCallback.kt will yield a token without audience
* support returned response scope from assertion claim
@tommytroen tommytroen changed the title Using JWTs as Authorization Grants support tokens without audience and scope from claims when using JwtBearerGrant Oct 7, 2020
@tommytroen tommytroen added the enhancement New feature or request label Oct 7, 2020
* added verifyWith with customizable required claims when verifying tokens
# Conflicts:
#	src/test/kotlin/no/nav/security/mock/oauth2/e2e/JwtBearerGrantIntegrationTest.kt
@tommytroen tommytroen merged commit 600c2ec into master Oct 7, 2020
@tommytroen tommytroen deleted the idporten_jwt_grant branch October 7, 2020 13:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants