-
Notifications
You must be signed in to change notification settings - Fork 233
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
Introduces a Custom Cache #199
Conversation
const logger = debug('jwks'); | ||
const getSigningKey = client.getSigningKey; | ||
|
||
logger(`Configured caching of signing keys. Max: ${cacheMaxEntries} / Age: ${cacheMaxAge}`); | ||
|
||
if (customCache) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The cache will need to fallback to getSigningKey
when you get a miss (and populate the cache when getSigningKey
returns something).
Otherwise your example:
client = new JwksClient({
jwksUri: `${jwksHost}/.well-known/jwks.json`,
cache: true,
customCache: new CustomCache()
});
Will never resolve anything
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ideally a custom cache should replace memoizer (the default in-memory cache) rather than sit in front of it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Perhaps using something like cache-manager
, eg
import cacheManager from 'cache-manager';
export default function(client, { cacheMaxEntries = 5, cacheMaxAge = ms('10m'), store = 'memory' } = options) {
const memoryCache = cacheManager.caching({ store, max: cacheMaxEntries, ttl: cacheMaxAge });
return (kid, cb) => memoryCache.wrap(kid, (cacheCallback) => {
client.getSigningKey(kid, cacheCallback);
}, {}, cb);
}
You'd ideally need to provide an lru cache as your custom cache, then the cache will drop the least recently used when size > cacheMaxEntries
or cache item age > cacheMaxAge
. eg https://github.com/BryanDonovan/node-cache-manager#custom-stores
Thanks for the review @adamjmcgrath. I will look at replacing the |
As mentioned in #202
|
By submitting a PR to this repository, you agree to the terms within the Auth0 Code of Conduct. Please see the contributing guidelines for how to create and submit a high-quality PR for this repo.
Description
There has been discussion around the need for a more flexible cache (issues/prs listed in references below). This PR introduces the ability for a developer to override the built-in cache and use their own Custom Cache. This replaces #141 as discussed here #141 (comment)
References
Testing
Added tests to cover the use of a custom cache. Additionally added a mock customCache for an idea of what a custom cache implementation might look like
Checklist
master