Cache broken when key is missing kid

[sfiodorov]

Describe the problem

node-jwks-rsa integration Have jwks endpoint with the single key without a "kid" field. Each try to validate JWT signed by this key leads to HTTP requests to jwks endpoint.

What was the expected behavior?

Since that package supports single key without kid (see JwksClient.js line :74) it should support the cache as well

[sfiodorov]

changing cache.js to have fake kid in case of kid is not defined might help : from:

return promisify(memoizer({
    hash: (kid) => kid,
    load: callbackify(client.getSigningKey.bind(client)),
    maxAge: cacheMaxAge,
    max: cacheMaxEntries
  }));

to

return promisify(memoizer({
    hash: (kid) => kid || "fakeKid",
    load: callbackify(client.getSigningKey.bind(client)),
    maxAge: cacheMaxAge,
    max: cacheMaxEntries
  }));

[adamjmcgrath]

Thanks for raising this @sfiodorov - we'll investigate. In the meantime, please continue to use your workaround

[adamjmcgrath]

Hi @sfiodorov - I'm not able to reproduce your issue

When I use a JWKS with a single JWK without a kid, the caching seems to work as expected - see https://replit.com/@AdamMcGrath/DryLightcyanNanotechnology

jose adds a kid if one is not present, and the SDK uses that for the cache key

[sfiodorov]

@adamjmcgrath , how jose adds the same kid to both jwt header and the signing key to be matched?

[adamjmcgrath]

@sfiodorov - I'm assuming your JWT header doesn't have a kid, if your JWKS doesn't have one to match against (see that getSigningKey is not being called with a value for kid in https://replit.com/@AdamMcGrath/DryLightcyanNanotechnology)

[sfiodorov]

Yes, both jwt header and key are missing kid. You are right here keys created by a call to getSigningKeys() are with the kid. But the problem that happens after the call to JWKS endpoint. When the key comes from JWKS it comes without a kid, so it is cached with undefined. Need to have some stub key for this case before calling to memoizer

[adamjmcgrath]

When the key comes from JWKS it comes without a kid, so it is cached with undefined.

The SigningKey is cached (not the JWKS) and the SigningKey is created with a kid

Perhaps you could update https://replit.com/@AdamMcGrath/DryLightcyanNanotechnology to demonstrate the issue, because I'm still not seeing one

[sfiodorov]

@adamjmcgrath thanks, i will try to reproduce with this code

[sfiodorov]

Wierd. When running your example I see that at lru-memoizer\lib\async.js that is used by the cache, the key is cached with an undefined key. The same cache does not work with the "undefined" cache key at my code.

Please let me know if it is correct that you cache wiht "undefined" as key cache key

[adamjmcgrath]

Please let me know if it is correct that you cache wiht "undefined" as key cache key

Yes - it comes from the value of the kid passed to getSigningKey

Doesn't seem to be a problem in any version of node I've tested, what happens when you do:

$ node -e "LRU=require('lru-cache');lru = new LRU(); lru.set(undefined, 'foo'); console.log(lru.get(undefined))"

[adamjmcgrath]

Or, for that matter:

$ node -e "lru = new Map(); lru.set(undefined, 'foo'); console.log(lru.get(undefined))"

[sfiodorov]

Seems like it is some issue on my side. Thanks for your assistance. I will need to recheck my implementation side.