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

Implement all properties for TokenIntrospectionResponse optional claims #55

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

0xced
Copy link
Contributor

@0xced 0xced commented Nov 17, 2024

As described by RFC 7662 - OAuth 2.0 Token Introspection

(Taking over IdentityModel/IdentityModel#577 to the new repository, adapted to the new Duende.IdentityModel namespace.)

@damianh damianh added area/identity-model Issues related to Identity Model state/needs-triage Needs triaging by the maintainers labels Nov 17, 2024
@damianh damianh force-pushed the TokenIntrospectionResponseAdditions branch from 07c9fad to 731cd48 Compare November 19, 2024 11:36
@0xced 0xced force-pushed the TokenIntrospectionResponseAdditions branch 4 times, most recently from 37e0dfa to 979497f Compare November 21, 2024 22:04
@0xced
Copy link
Contributor Author

0xced commented Nov 21, 2024

🎉 All tests are finally green ✅!

@0xced 0xced force-pushed the TokenIntrospectionResponseAdditions branch from 979497f to a11fd24 Compare December 20, 2024 19:45
Copy link
Member

@josephdecock josephdecock left a comment

Choose a reason for hiding this comment

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

I'd like to remove the use of Lazy in this PR. That way, developers can make their own choice about if they need Lazy access to these properties. My guess is that in practice, there wouldn't be much gained by not evaluating the properties when they're not needed, and depending on access patterns, the allocations needed by Lazy and the locking it does to provide thread safety might introduce other overhead. If we leave it out of the library, then individual applications can always add whatever optimizations they want.

@0xced 0xced force-pushed the TokenIntrospectionResponseAdditions branch from ea65138 to 2a8c7d4 Compare January 21, 2025 16:48
@0xced
Copy link
Contributor Author

0xced commented Jan 21, 2025

Fair point. Addressed in f75f651.

@0xced 0xced force-pushed the TokenIntrospectionResponseAdditions branch from 2a8c7d4 to f75f651 Compare February 5, 2025 19:49
@0xced
Copy link
Contributor Author

0xced commented Feb 5, 2025

I just rebased this pull request on the main branch and resolved the conflicts that arose from #86.

I also slightly modified how the exp, iat and nbf claims are parsed in c01e68b to avoid throwing in case those contain an invalid number. With the previous Lazy implementation it would not throw before accessing them.

@0xced 0xced force-pushed the TokenIntrospectionResponseAdditions branch from 84f02f1 to c01e68b Compare February 5, 2025 20:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/identity-model Issues related to Identity Model state/needs-triage Needs triaging by the maintainers
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants