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

Make issuer key cache length configurable #86

Open
brianhlin opened this issue Jun 1, 2022 · 8 comments
Open

Make issuer key cache length configurable #86

brianhlin opened this issue Jun 1, 2022 · 8 comments

Comments

@brianhlin
Copy link
Contributor

We'd like to be able to specify the length of time that issuer keys are cached (currently 4 days)

@djw8605
Copy link
Contributor

djw8605 commented Jun 1, 2022

We are thinking of a configuration file, does that work for you?

@brianhlin
Copy link
Contributor Author

Hrm, I was thinking that this would be something that we could specify in the HTCondor configuration and then HTCondor would pass along its cache lifetime preference when making calls to the SciTokens library. @JaimeFrey and @timtheisen may have some opinions on whether or not a SciTokens library config file would be workable for us.

@JaimeFrey
Copy link
Contributor

I don't foresee any issues on the HTCondor side for a SciTokens configuration file. I would presume there'd be a way to specify what filename to use via the API. It'd be easy for us to have that be configured from the HTCondor config file.

@bbockelm
Copy link
Contributor

bbockelm commented Nov 3, 2022

@djw8605 - should we really have a configuration file here? Or should we expose a configuration API that the application above (for @JaimeFrey, this would be HTCondor) can invoke and populate with their config file?

What's the format and location for the Python library configuration file?

@jthiltges
Copy link
Contributor

Having recently been thinking about cache lifetimes, could this be turned around, and have key cache lifetime driven by the issuer? Use the Cache-Control max-age on the jwks_uri reply (with upper/lower guardrails).

@djw8605
Copy link
Contributor

djw8605 commented Apr 30, 2024

At one time, we discussed the minimum of either the cache-control or a configured min lifetime. I wonder if that still makes sense.

@jbasney
Copy link
Member

jbasney commented Apr 30, 2024

In WLCG-AuthZ-WG/common-jwt-profile#17 we discussed updating the WLCG profile to respect the Cache-Control header, but in the end we just updated the maximum from 1 day to 4 days.

Repeating a few observations I made in that WLCG pull request:

  1. The HTTP caching headers should take precedence, so long as the lifetime is within the minimum and maximum values. Respecting the HTTP caching headers allows providers to adjust caching as needed (e.g., during key rollover periods).
  2. We should clarify cache update times (polling frequency) versus cache eviction times. I see that implementations are polling the JWKS endpoints every 10 minutes (which is too frequent according to the 1 hour minimum in the WLCG profile) but caching keys for longer periods (hours/days) in case the JWKS endpoints are offline. This distinction between the two time periods (polling for updates versus evicting stale items from the cache) needs to be clearly specified in the configuration.
  3. We should cache key sets, not individual keys, so if the issuer removes a key from its key set (e.g., in case that individual key is compromised), that individual key is removed from the cache at the next update interval and does not stay in the cache waiting for the longer eviction period.

@jbasney
Copy link
Member

jbasney commented May 1, 2024

There's a TODO comment for looking at the cache-control header:

// TODO: take expiration time from the cache-control header in the

For comparison, the python library already looks at the cache-control header:
https://github.com/scitokens/scitokens/blob/8d60a0759620050e12ebbda8eee206de0bf93479/src/scitokens/utils/keycache.py#L303

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

No branches or pull requests

6 participants