kilork / openid

OpenID Connect Rust Library
The Unlicense
61 stars 22 forks source link

Reqwest (Hyper) refusing to load HTTPS issuer #14

Closed Armiixteryx closed 3 years ago

Armiixteryx commented 3 years ago

Hello.

I am currently experiencing the following error when using this crate:

thread 'main' panicked at 'called `Result::unwrap()` on an `Err` value: Http(reqwest::Error { kind: Request, url: Url { scheme: "https", host: Some(Domain("api.fusionfabric.cloud")), port: None, path: "/login/v1/sandbox/.well-known/openid-configuration", query: None, fragment: None }, source: hyper::Error(Connect, "invalid URL, scheme is not http") })', src/main.rs:17:13

The panic fires from the Client's discover fn, when entering an HTTPS issuer, exactly the following: https://api.fusionfabric.cloud/login/v1/sandbox

If I use actix and add its TLS component as shown in this crate's documentation, error does not show, but in my case I do not need actix

To workaround this, I have cloned this repo and added the following in Cargo.toml, to make reqwest use native TLS:

[dependencies.reqwest]
version = '0.11'
default-features = false
features = ['json', 'native-tls']

when original Cargo.toml does not contain the native-tls feature for reqwest.

So I want to ask if there is a better to fix this without patching crate manually?

Thanks and grettings!

kilork commented 3 years ago

Hi, I plan to update to reqwest 0.11 in 0.0.8, working on this right now.

openid 0.0.7 and before depend on 0.10 and another tokio version. I would check later with small example exactly your case (I am currently building example with warp), but I think you can just add reqwest = 0.10 dependency to your Cargo.toml (and of course old tokio version).

kilork commented 3 years ago

Version 0.8.0 is out, you may try it, please submit your feedback, so I can close this safely.

https://crates.io/crates/openid/0.8.0

Armiixteryx commented 3 years ago

Hey, thanks for the update!

Unfortunately it didn't work for me. Probably because of following:

I am using currently reqwest 0.9.x branch due to legacy reasons. Since openid uses its own reqwest without tls enabled, probably that's the root of this issue. Also I am not using wrap, just tokio runtime since right now I just need to login with clientCredentials flow.

My current workaround is to construct the discover client (using this fn)[https://docs.rs/openid/0.8.0/openid/struct.Client.html#method.discover_with_client] to add a renamed 0.11 reqwest client with native-tls flag enabled. That way the issue is gone, although it is probably not ideal.

kilork commented 3 years ago

I'm glad you found a workaround (it is even difficult to call "workaround")!

In your case I would actually recommend to fork project and switch dependency to 0.9. We should not mix different library versions, even if it works somehow. If I use version 1.2 and 2.2 of url crate for example - it just does not compile. I just wonder how did you manage to compile it? Update: you are using renamed version, ok.

Probably I would reexport internal dependencies and allow to enable native-tls with features. Enabling this by default is probably not for everyone, I personally do not want to use native-tls, I really want to see rustls on first place.

kilork commented 3 years ago

Example which works for me without using discover_with_client:

Cargo.toml:

[package]
name = "openid-clear-example"
version = "0.1.0"
authors = ["Alexander Korolev <alexander.korolev.germany@gmail.com>"]
edition = "2018"

[dependencies]
url = "2.2"
anyhow = "1"
openid = "0.8"
reqwest = "0.9"
reqwest2 = { version = "0.11", package = "reqwest" }
tokio = { version = "1", features = [ "full" ] }
dotenv = "0.15"

src/main.rs:

use std::env;

use openid::DiscoveredClient;

#[tokio::main]
async fn main() -> anyhow::Result<()> {
    dotenv::dotenv().ok();

    let client_id = env::var("CLIENT_ID").unwrap_or("<client id>".to_string());
    let client_secret = env::var("CLIENT_SECRET").unwrap_or("<client secret>".to_string());
    let issuer_url = env::var("ISSUER").unwrap_or("https://accounts.google.com".to_string());
    let redirect = Some(host("/login/oauth2/code/oidc"));
    let issuer = url::Url::parse(&issuer_url)?;

    eprintln!("redirect: {:?}", redirect);
    eprintln!("issuer: {}", issuer);

    let client = DiscoveredClient::discover(client_id, client_secret, redirect, issuer).await?;

    eprintln!("discovered config: {:?}", client.config());

    Ok(())
}

fn host(path: &str) -> String {
    env::var("REDIRECT_URL").unwrap_or("http://localhost:8080".to_string()) + path
}
kilork commented 3 years ago

So this line

reqwest2 = { version = "0.11", package = "reqwest" }

made a trick ;)