Skip to content

Conversation

nov
Copy link

@nov nov commented Sep 23, 2022

same with omniauth/omniauth_openid_connect#124

when passing OpenIDConnect::Discovery::Provider::Config::Response instance to OpenIDConnect::ResponseObject::IdToken.decode, it fetches JWK Set using JSON::JWK::Set::Fetcher.

JSON::JWK::Set::Fetcher tries to cache JWKS by given kid when JSON::JWK::Set::Fetcher.cache is setup like below.

JSON::JWK::Set::Fetcher.cache = Rails.cache

@cben
Copy link
Collaborator

cben commented Sep 23, 2022

@rhysm @benlangfeld you know more than me about OIDC, could you review?

@nov Does this deserve any additions to README? Or is it "if you've set up a JWK cache it'll automagically do the obvious Right Thing, and if not you don't care about this?"

@nov
Copy link
Author

nov commented Sep 23, 2022

If OIDC login flow is executed very often, it deserve to be added to README too.

@cben
Copy link
Collaborator

cben commented Jan 15, 2023

OK, I don't know how to review this myself, but I shouldn't have blocked this either. Superficially, makes sense 👍
[I'm just getting too little time for kubeclient, so trying to outsource... nagging is welcome!]

@nov please add at least a CHANGELOG.md entry (as I'm not sure how to explain this change well), and I'll merge.

@cben cben added enhancement doc Need to add/improve documentation labels Jan 16, 2023
@cben
Copy link
Collaborator

cben commented Mar 20, 2023

@nov friendly ping — the only thing blocking merge is I don't know how to explain this change in CHANGELOG.md, please add an entry there 🙏

FYI, #606 is looking to auto-renew credentials, and intends to call OIDCAuthProvider.token on every kubeclient request. We need more eyes on that.

[EDIT: all this time I haven't realized you're are the author of openid_connect 😳 👏 Still, I don't know enough about this area, and I need someone to explain the implications at least to the users...]

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
doc Need to add/improve documentation enhancement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants