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

hperl/per-client-token-types #3446

Merged
merged 1 commit into from
Mar 2, 2023
Merged

Conversation

hperl
Copy link
Contributor

@hperl hperl commented Feb 22, 2023

Related issue(s)

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
    security@ory.sh) 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

@hperl hperl requested a review from aeneasr February 22, 2023 09:37
@hperl hperl self-assigned this Feb 22, 2023
@codecov
Copy link

codecov bot commented Feb 22, 2023

Codecov Report

Merging #3446 (602b423) into master (375bd5a) will decrease coverage by 0.01%.
The diff coverage is 84.28%.

❗ Current head 602b423 differs from pull request most recent head 2ec7a94. Consider uploading reports for the commit 2ec7a94 to get more accurate results

@@            Coverage Diff             @@
##           master    #3446      +/-   ##
==========================================
- Coverage   76.84%   76.83%   -0.01%     
==========================================
  Files         123      123              
  Lines        9073     9105      +32     
==========================================
+ Hits         6972     6996      +24     
- Misses       1659     1664       +5     
- Partials      442      445       +3     
Impacted Files Coverage Δ
client/validator.go 69.82% <50.00%> (-3.33%) ⬇️
driver/config/provider.go 81.85% <55.55%> (-0.71%) ⬇️
client/client.go 76.41% <85.71%> (+0.65%) ⬆️
fositex/token_strategy.go 100.00% <100.00%> (ø)
oauth2/fosite_store_helpers.go 100.00% <100.00%> (ø)
oauth2/handler.go 66.50% <100.00%> (ø)
persistence/sql/persister_oauth2.go 82.26% <100.00%> (-0.63%) ⬇️
jwk/handler.go 67.77% <0.00%> (+3.33%) ⬆️

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

@aeneasr
Copy link
Member

aeneasr commented Feb 23, 2023

For the tests, please check that jwt->jwt, jwt->opaque, opaque->jwt, opaque->opaque and flows:

  • client credentials: issue token, validate token, invalidate token
  • auth code: issue token, validate token, refresh token, validate token, invalidate token, call userinfo endpoint
  • jwt-bearer flow: does it issue the token in the correct format?

@hperl hperl force-pushed the hperl/per-client-token-types branch 3 times, most recently from d0cae03 to 1130b7f Compare February 27, 2023 16:05
@hperl hperl marked this pull request as ready for review February 27, 2023 16:13
@hperl hperl force-pushed the hperl/per-client-token-types branch from 1130b7f to af50266 Compare February 28, 2023 08:58
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.

Please make this not available in OIDC dynamic client registration.

@hperl hperl linked an issue Mar 1, 2023 that may be closed by this pull request
6 tasks
@hperl hperl force-pushed the hperl/per-client-token-types branch from 0b24070 to ca93750 Compare March 1, 2023 12:35
@hperl hperl requested a review from aeneasr March 1, 2023 12:35
@hperl hperl force-pushed the hperl/per-client-token-types branch from ca93750 to a4612f1 Compare March 1, 2023 14:38
aeneasr
aeneasr previously approved these changes Mar 2, 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.

Great job, looks good!

@aeneasr
Copy link
Member

aeneasr commented Mar 2, 2023

merge conflicts 🙄

The access token type (`jwt` or `opaque`) can now be set in the
client configuration.  The value set here will overwrite the
global value for all flows concerning that client.
@hperl hperl force-pushed the hperl/per-client-token-types branch from a4612f1 to 2ec7a94 Compare March 2, 2023 14:28
@hperl
Copy link
Contributor Author

hperl commented Mar 2, 2023

@aeneasr all resolved now :)

@aeneasr aeneasr merged commit a6beed4 into master Mar 2, 2023
@aeneasr aeneasr deleted the hperl/per-client-token-types branch March 2, 2023 15:17
harnash pushed a commit to Wikia/ory-hydra that referenced this pull request Apr 12, 2023
The access token type (`jwt` or `opaque`) can now be set in the client configuration. The value set here will overwrite the global value for all flows concerning that client.
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.

Support per-client configuration of token type (opaque/jwt)
2 participants