Key not found result is not cached resulting in extra jwks requests

[tzuge]

Describe the problem

Failure to find the key from the jwks endpoint leads to a SigningKeyNotFoundError, and lru-memoizer does not appear to cache this 'no key' result, so caching does not limit requests to the jwks endpoint when kid is not found in the response.

Expected behaviour

I would expect that a failure in the request to jwks endpoint would not be cache (i.e. if server returns 500, then trying again next time is appropriate). However, if the request is successful and the kid is not in the response, that's a valid result that should be cached for the regular TTL.

Reproduction

In my specific case, I have express applications with multiple jwt strategies with passport and it's expected that tokens will pass one or the other. The 'no key' result not being cached results in extra overhead on requests; this is partly interaction with issuer check happening after secretOrKeyProvider in jwt strategy. Workaround is possible by decoding jwt header and comparing against issuer to skip jwks key resolution.

Configure a jwks uri that is not consistent with access tokens provided in requests.

passportJwtSecret({
    jwksUri: myWrongJwksUrl,
    cache: true,
  });

Environment

jwks-rsa version: 2.1.4

[adamjmcgrath]

Hi @tzuge - thanks for raising this

Failure to find the key from the jwks endpoint leads to a SigningKeyNotFoundError, and lru-memoizer does not appear to cache this 'no key' result,

This is expected behaviour, otherwise the cache could be filled up with spurious results.

[tzuge]

@adamjmcgrath , could you clarify what you mean by spurious results?

The response coming back with 200 and a body that does not contain the kid is a legitimate result and distinguishable from a response coming back a failure status code. Assuming the caching did account for this, subsequent requests for the kid at the particular url would cache hit with the 'no key' result still subject to cache TTL. Requests that fail would continue to not be cached.

I acknowledge that this might be a pain to change since the error is used for both cases and the memoize lib is quite reasonably not memoizing that.

[adamjmcgrath]

Hi @tzuge

could you clarify what you mean by spurious results?

Sure, if you cached non existent kids - someone could generate many access tokens with different kid's and quickly fill the cache.

The SDK is not optimised to accept access tokens from the wrong issuer, you'll need to find a way to select the correct issuer based on the request (like the workaround you described)