search

panva / openid-client

OAuth 2 / OpenID Connect Client API for JavaScript Runtimes
MIT License
1.83k stars 392 forks source link

Returned error from jose.importJWK is squashed #552

Closed simon-dk closed 1 year ago

simon-dk commented 1 year ago

We had an issue where we were trying to debug the following error:

no valid key found in issuer's jwks_uri for key parameters {"kid":"jwa2A0ZH0Y4M_9HD-spX","alg":"RS256"}

The error is thrown at openid-client/lib/helpers/issuer.js:94:11.

After digging some time, I located the real source: https://github.com/panva/node-openid-client/blob/b3c0f3c5f800253dfd5fd793c63faccc0951148e/lib/helpers/keystore.js#L231

Our bug was missing "n" (modulus) and "e" (exponent) in some dev jwks keys. Because the error was squashed though, we could not see what happened.

I'll be willing to make a PR for this if the owners agree the errors should be handled differently.

panva commented 1 year ago

I don't think it needs to be handled differently, at face value the error says everything right, there was no valid key for the given parameters.

simon-dk commented 1 year ago

I found the message confusing because we had a key with the printed out properties (kid and alg). The error did not say that we were missing properties in said key.

In contrast the error from Jose reads: The "key.n" property must be of type string. Received undefined

Edit: Just wanted to add that, and that is just my opinion, it is often better to fail fast. The rest of the code did not have to be running at all as there was already an error.

panva commented 1 year ago

There is more than one moving part when it comes to selecting a key from an IdP hosted JWKS, just because in this case there was no need to continue iterating doesn't mean that holds true for all scenarios.

panva commented 1 year ago

Likewise the error from jose is only ever good in one scenario that you happened to fall under but would completely throw off developers in others. It is not that common that authorization servers don't know how to well form a JWK ;)

simon-dk commented 1 year ago

Thats a great point, and if that is the case it would of course not make sense to throw an error. A tiny bit more info in the log would have saved my day though. I'll leave it at that and close this issue. Others who might get the same error now has a chance of reading about it here 🙃

It is not that common that authorization servers don't know how to well form a JWK ;)

😄 Gladly it was some dev keys in a dev environment, not sure how they were generated, but I'll have to look into that next.