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

feat: support different jwt scope claim strategies #3531

Merged
merged 3 commits into from
Jul 5, 2023

Conversation

brett-patterson
Copy link
Contributor

Adds a strategies.jwt.scope_claim optional configuration property that controls how the scope claim is represented in JWT access tokens. It can be set to one of list, string, or both. list (the default behavior) matches the current behavior of using a scp claim that is an array of strings. string uses a scope claim as defined in this RFC, a single space-delimited string. both will include both the scp and scope claim in the token.

Related issue(s)

#3524

Checklist

  • I have read the contributing guidelines.
  • I have referenced an issue containing the design document if my change
    introduces a new feature.
  • I am following the
    contributing code guidelines.
  • I have read the security policy.
  • I confirm that this pull request does not address a security
    vulnerability. If this pull request addresses a security vulnerability, I
    confirm that I got the approval (please contact
    [email protected]) from the maintainers to push
    the changes.
  • I have added tests that prove my fix is effective or that my feature
    works.
  • I have added or changed the documentation.

Further Comments

These different scope claim strategies were already available in fosite, this PR is just exposing them through a Hydra configuration property. Open to suggestions on configuration property naming! Will update docs but wanted to get this PR settled first to know what to add to the docs.

@brett-patterson brett-patterson marked this pull request as ready for review June 1, 2023 19:29
@codecov
Copy link

codecov bot commented Jun 1, 2023

Codecov Report

Merging #3531 (58c06b0) into master (116c1e8) will decrease coverage by 0.02%.
The diff coverage is 83.33%.

❗ Current head 58c06b0 differs from pull request most recent head 3c986de. Consider uploading reports for the commit 3c986de to get more accurate results

@@            Coverage Diff             @@
##           master    #3531      +/-   ##
==========================================
- Coverage   76.64%   76.62%   -0.02%     
==========================================
  Files         129      129              
  Lines        9630     9640      +10     
==========================================
+ Hits         7381     7387       +6     
- Misses       1748     1751       +3     
- Partials      501      502       +1     
Impacted Files Coverage Δ
cmd/cmd_list_clients.go 80.64% <ø> (ø)
driver/config/provider.go 82.75% <ø> (ø)
driver/config/provider_fosite.go 81.81% <80.00%> (-0.54%) ⬇️
fositex/config.go 86.11% <100.00%> (ø)
persistence/sql/persister.go 85.00% <100.00%> (ø)

... and 1 file with indirect coverage changes

aeneasr
aeneasr previously approved these changes Jun 26, 2023
Copy link
Member

@aeneasr aeneasr left a comment

Choose a reason for hiding this comment

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

Awesome, thank you! 🎉 Your contribution makes Ory better :)

aeneasr
aeneasr previously approved these changes Jun 26, 2023
@aeneasr aeneasr force-pushed the feat/jwt-scope-claim-strategy branch from d587c93 to 3c986de Compare June 26, 2023 09:44
@aeneasr aeneasr merged commit 45da11e into ory:master Jul 5, 2023
@vinckr
Copy link
Member

vinckr commented Jul 12, 2023

Hello @brett-patterson
Thanks so much for the contribution 🙌

Let me know if I can support you in adding something to the documentation. I have never used the feature so I would need some rough pointers/drafts.

Thanks!

@Lelekova
Copy link

Lelekova commented Aug 8, 2023

Hi there!
@brett-patterson thanks a lot for the feature :)

I have a question for the Ory guys: is this feature already a part of any release? If yes, which one? If no, when should we expect it?
@aeneasr I guess you might be able to answer this question :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants