ramosbugs / openidconnect-rs

OpenID Connect Library for Rust
MIT License
372 stars 98 forks source link

Integrate functionality from oidc-jwt-validator #164

Closed colemickens closed 2 months ago

colemickens commented 2 months ago

This library looks compelling, but it seems to be missing some critical functionality that is contained in oidc-jwt-validator:

https://crates.io/crates/oidc-jwt-validator

Particularly, I don't see anything that:

  1. Indicates that this library attempts to keep the JWKS set updated.
  2. Any sort of easy way to validate an JWT, regardless of whether I know/whitelist the issuer.

The closest thing I'm seeing is: https://docs.rs/openidconnect/latest/openidconnect/trait.JsonWebKey.html#tymethod.verify_signature

which requires me to:

Am I missing a more convenient option?

ramosbugs commented 2 months ago
  • Indicates that this library attempts to keep the JWKS set updated.

See #25. HTTP cache headers are the only official way for OIDC providers to indicate a JWKS TTL to clients. This crate's Client struct is intended to be short-lived (e.g., while handling a single HTTP request), and by default the OIDC DIscovery implementation will fetch the JWKS each time, so it's always up to date.

Since fetching each time can add unacceptable overhead, an HTTP caching layer can be added underneath this library using something like https://crates.io/crates/http-cache-reqwest. That should integrate easily with the new stateful HTTP client API coming out in 4.0 of this crate (see upgrade guide). As I've suggested in other issues, I believe caching functionality belongs at the HTTP client layer, not in the application layer above it.

Combining a long-lived, stateful, caching HTTP client with a short-lived openidconnect::Client should provide the desired behavior.

  • Any sort of easy way to validate an JWT, regardless of whether I know/whitelist the issuer.

This crate is not a general-purpose JWT library. Its JWT functionality is intentionally limited to ID tokens and user info JWTs explicitly defined in the OpenID Connect specs.

What other types of JWTs are you intending to verify using this crate?

See also #160 for a recent related discussion.

colemickens commented 2 months ago

Wow, what an answer, thank you for such a great, thoughtful answer, and so quickly.

  1. Love all the thoughts about caching, I think that all makes a ton of sense and I agree.

  2. What other types of JWTs are you intending to verify using this crate?

Well, indeed it would just be id_tokens; we are wanting to accept id_tokens from (now that I think about it, probably hard-coded/white-listed) issuers, that come to us in Authorization header(s). The current thought it that I'd look at the iss, use the caching reqwest client to get the discovery info/keys, and then validate the full token.

It does appear that I just didn't search the API well enough. It looks like what I'm after is probably is some sequence of:

Thanks for the nudges in the right direction, sorry for the noise, this wound up mostly being support. (Which I really appreciate). Library looks great, and the answer really inspires further confidence. Thanks a bunch!

colemickens commented 2 months ago

Closing, as the feature request was mis-guided and/or already covered by functionality that I simply didn't find for searching "verify" instead of "verif" :sweat: .

ramosbugs commented 2 months ago

It does appear that I just didn't search the API well enough. It looks like what I'm after is probably is some sequence of:

  • get ProviderMetadata from discover_async
  • Client -> from_provider_metadata
  • Client -> id_token_verifier

That should work. I'll just add to that:

Also, if you don't need any other Client functionality, you could construct the IdTokenVerifier directly from the provider metadata using IdTokenVerifier::new_public_client or IdTokenVerifier::new_confidential_client as appropriate to your use case.

colemickens commented 2 months ago

I mostly have this working, but I haven't managed to use the caching reqwest client yet.

Do you have an example of async_http_client that re-uses a client from a struct? I guess the method would need to return a closure that reference's the struct's client?

ramosbugs commented 2 months ago

If you're using the latest alpha release, the AsyncHttpClient trait is implemented for reqwest::Client, so you should be able to just pass a reference to one instead of using a closure.

In 3.x, you would need to pass in a closure, which you can base on the implementation here (without instantiating a new Client each time).