fix: cache not used as object different each time

[sami-sweng]

The library used for the cache supports using objects as keys, but checks for pointer equality and not deep or JSON equality (lru-cache).

Since the key that we want to cache originate from a JSON.parse call (client.js#L642, client.js#L557), it is different each time.

Thus, the cache only works for 60 sec for a particular key (thanks to the throttle key).

[codecov]

Codecov Report

Merging #162 into master will not change coverage.
The diff coverage is 100%.

@@          Coverage Diff          @@
##           master   #162   +/-   ##
=====================================
  Coverage     100%   100%           
=====================================
  Files          18     18           
  Lines         798    802    +4     
=====================================
+ Hits          798    802    +4

[panva]

Hi @sami-sweng, thank you for taking the time to make openid-client better. I have a different fix for this in the 3.x branch that more reflects the intended logic (not just kid cache), if you want to backport that, please do so. Otherwise this I will not accept.

[sami-sweng]

Should I create a new PR?

[panva]

Just update this one and point it to master please.

[sami-sweng]

Ah, I get it,

you want me to check what you did in v.3 branch and apply it in v2?

Alright, I'll try something if it's not too time consuming.

Thank you for your work on this library.

[sami-sweng]

Alright,

here is a light version of your v3 fix based on object hash.

[panva]

can you include this part too?

const defHash = objectHash(def, {
  algorithm: 'sha256',
  ignoreUnknown: true,
  unorderedArrays: true,
  unorderedSets: true,
});

[panva]

Okay, there are more things missed, i really intend to cherry-pick the properties that matter for selecting a key, kid, kty, alg, use, key_ops. Would you mind not making this "light" but rather do a full backport of this particular fix?

[sami-sweng]

From blame it seems to be part of a big commit (7bb5847) so I'm not sure about the scope of this backport.

But I can hash based on these properties...?

[sami-sweng]

So are there more things that I should add in this PR?

@panva as a side question: I'm making this PR because I randomly have a few failed login against a Microsoft implementation. It seems that their keys route (jwks_uri) is randomly failing with ETIMEDOUT. Does this sound familiar to you?

[panva]

So are there more things that I should add in this PR?

Yes, it's incomplete, but don't worry, I'll redo this as part of my efforts to backport some of the v3.x fixes back to v2.x

It seems that their keys route (jwks_uri) is randomly failing with ETIMEDOUT. Does this sound familiar to you?

Nope.

[panva]

I'll do the backport with the last release on the v2.x line right before releasing v3.x