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: add possibility to ignore missing "exp" claim in introspection #161

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

Conversation

buehler
Copy link

@buehler buehler commented Jul 18, 2022

Hey there!

I'm currently having an issue with an identity provider that does not return the "exp" claim in the introspection response. Since the "exp" claim is not required by the RFC, this may happen to other identity providers.

This option should allow a configuration to ignore the fact that the exp claim is missing.

Then, the default duration is used.

@leastprivilege
Copy link
Contributor

Thanks! We will look into it.

@buehler
Copy link
Author

buehler commented May 2, 2023

ping @leastprivilege :-)

@leastprivilege
Copy link
Contributor

@brockallen Could you review?

Not sure we need an additional option - if no exp claim is present, the default cache duration could be used?!

@brockallen
Copy link
Member

Not sure we need an additional option - if no exp claim is present, the default cache duration could be used?!

Which default cache duration? Of the cache implementation itself?

@brockallen
Copy link
Member

Ok, missed that... so yea, agreed on:

Not sure we need an additional option - if no exp claim is present, the default cache duration could be used?!

@buehler
Copy link
Author

buehler commented May 5, 2023

Hey @brockallen, @leastprivilege

I changed the logic according to your comments. I removed the additional options field and "just" removed the expClaim null check. Now if there is no exp claim, the passed duration TimeSpan is used.

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

Successfully merging this pull request may close these issues.

3 participants