ramosbugs / oauth2-rs

Extensible, strongly-typed Rust OAuth2 client library
Apache License 2.0
939 stars 163 forks source link

Allow selecting the cert roots used by reqwest #287

Closed sjoerdsimons closed 1 month ago

sjoerdsimons commented 1 month ago

When using rustls for tls support various options are available for root certificate sources. Allow for those to be selected explicitly rather then just using the default

ramosbugs commented 1 month ago

Hey @sjoerdsimons, thanks for the PR.

Please correct me if I'm wrong, but my understanding of Cargo's feature unification is that it shouldn't be necessary for each crate to mirror the features available in the crates they depend on.

I believe a top-level crate can use oauth2 with any of reqwest's certificate sources by doing something along the lines of the following in its Cargo.toml:

oauth2 = { version = "4", default-features = false, features = ["reqwest"] }
reqwest = { version = "0.12", default-features = false, features = ["rustls-tls-manual-roots"] }

If that's the case, then I'd prefer not to introduce additional feature's to this crate's Cargo.toml. Does that solution work for you?

sjoerdsimons commented 1 month ago

That does indeed work; And could also be used instead of the existing native-tls and rust-tls features.. The downside is ofcourse now the user has to ensure to also depend on a semver compatible version of reqwest to enable the right features even if they don't use reqwest directly.

ramosbugs commented 1 month ago

I think applications that need this level of control over reqwest are already pretty tightly coupled to the specifics of that crate, so it doesn't seem unreasonable to require the extra line in Cargo.toml. I'd prefer that over having to maintain this feature set in the future, even though I previously added rustls-tls as a convenience. Thanks for the helpful discussion!