ramosbugs / openidconnect-rs

OpenID Connect Library for Rust
MIT License
426 stars 103 forks source link

`Send` issue when migrating from v3.5 to v4 (using async http client) #171

Closed brocaar closed 5 months ago

brocaar commented 6 months ago

First of all thanks for providing this crate!

I'm trying to upgrade from the v3.5 version to the v4 alpha, but I'm running into some issues.

I am refactoring:

async fn get_client() -> Result<CoreClient> {
    let conf = config::get();

    if conf.user_authentication.enabled != "openid_connect" {
        return Err(anyhow!("OIDC is not enabled"));
    }

    let client_id = ClientId::new(conf.user_authentication.openid_connect.client_id.clone());
    let client_secret = ClientSecret::new(
        conf.user_authentication
            .openid_connect
            .client_secret
            .clone(),
    );
    let provider_url =
        IssuerUrl::new(conf.user_authentication.openid_connect.provider_url.clone())?;
    let redirect_url =
        RedirectUrl::new(conf.user_authentication.openid_connect.redirect_url.clone())?;

    let provider_metadata =
        CoreProviderMetadata::discover_async(provider_url, async_http_client).await?;
    let client =
        CoreClient::from_provider_metadata(provider_metadata, client_id, Some(client_secret))
            .set_redirect_uri(redirect_url);

    Ok(client)
}

Into:

type Client = CoreClient<
    EndpointSet,
    EndpointNotSet,
    EndpointNotSet,
    EndpointNotSet,
    EndpointMaybeSet,
    EndpointMaybeSet,
>;

async fn get_client() -> Result<Client> {
    let conf = config::get();

    if conf.user_authentication.enabled != "openid_connect" {
        return Err(anyhow!("OIDC is not enabled"));
    }

    let http_client = reqwest::ClientBuilder::new()
        .redirect(reqwest::redirect::Policy::none())
        .build()?;

    let provider_metadata = CoreProviderMetadata::discover_async(
        IssuerUrl::new(conf.user_authentication.openid_connect.provider_url.clone())?,
        &http_client,
    )
    .await?;

    let client = CoreClient::from_provider_metadata(
        provider_metadata,
        ClientId::new(conf.user_authentication.openid_connect.client_id.clone()),
        Some(ClientSecret::new(
            conf.user_authentication
                .openid_connect
                .client_secret
                .clone(),
        )),
    )
    .set_redirect_uri(RedirectUrl::new(
        conf.user_authentication.openid_connect.redirect_url.clone(),
    )?);

    Ok(client)
}

While I believe the code is correct, it no longer works within my application because the returned future is not Send, which is required by a higher-up function that is invoking the get_client function.

166 |       let provider_metadata = CoreProviderMetadata::discover_async(
    |  _____________________________^
167 | |         IssuerUrl::new(conf.user_authentication.openid_connect.provider_url.clone())?,
168 | |         &http_client,
169 | |     )
    | |_____^ await occurs here on type `Pin<Box<dyn futures_util::Future<Output = Result<ProviderMetadata<EmptyAdditionalProviderMetadata, CoreAuthDisplay, CoreClientAuthMethod, CoreClaimName, CoreClaimType, CoreGrantType, CoreJweContentEncryptionAlgorithm, CoreJweKeyManagementAlgorithm, CoreJsonWebKey, CoreResponseMode, CoreResponseType, CoreSubjectIdentifierType>, DiscoveryError<HttpClientError<reqwest::Error>>>>>>`, which is not `Send`
    = note: required for the cast from `Pin<Box<{async block@chirpstack/src/api/internal.rs:433:68: 513:6}>>` to `Pin<Box<dyn futures_util::Future<Output = Result<tonic::Response<OpenIdConnectLoginResponse>, tonic::Status>> + std::marker::Send>>`

This error was not raised when using the v3.5 crate version.

ramosbugs commented 5 months ago

I believe this was fixed in #158 but I still need to cut a release with it, which I'll try to do this week.

brocaar commented 5 months ago

Thanks for your response @ramosbugs, I will re-try against the next release then :)

ramosbugs commented 5 months ago

https://crates.io/crates/openidconnect/4.0.0-alpha.2 is now released