Support using an existing reqwest::Client when making requests

[cdhowie]

The oauth2::reqwest::async_http_client function creates a new reqwest::Client on every invocation. This completely precludes the possibility of using HTTP Keep-Alive or DNS caching as supported by reqwest::Client, so every single request is going to make a DNS query, create a new TCP connection, and perform an SSL handshake. This is horribly inefficient and can cause problems in applications making a large number of requests.

We can take the existing function and copy-paste it to support using an existing client, but this doesn't seem very DRY.

I would suggest splitting this function into two: one that creates a new client and one that uses an existing one (the former can delegate to the latter). For example (this code is untested):

pub async fn async_http_client(
    request: HttpRequest,
) -> Result<HttpResponse, Error<reqwest::Error>> {
    let client = {
        let builder = reqwest::Client::builder();

        // Following redirects opens the client up to SSRF vulnerabilities.
        // but this is not possible to prevent on wasm targets
        #[cfg(not(target_arch = "wasm32"))]
        let builder = builder.redirect(reqwest::redirect::Policy::none());

        builder.build().map_err(Error::Reqwest)?
    };

    async_http_client_with_client(&client, request).await
}

pub async fn async_http_client_with_client(
    client: &reqwest::Client,
    request: HttpRequest,
) -> Result<HttpResponse, Error<reqwest::Error>> {
    let mut request_builder = client
        .request(request.method, request.url.as_str())
        .body(request.body);
    for (name, value) in &request.headers {
        request_builder = request_builder.header(name.as_str(), value.as_bytes());
    }
    let request = request_builder.build().map_err(Error::Reqwest)?;

    let response = client.execute(request).await.map_err(Error::Reqwest)?;

    let status_code = response.status();
    let headers = response.headers().to_owned();
    let chunks = response.bytes().await.map_err(Error::Reqwest)?;
    Ok(HttpResponse {
        status_code,
        headers,
        body: chunks.to_vec(),
    })
}

Now if we want to use our own client, we can do .request_async(|request| async_http_client_with_client(&client, request)).

[ramosbugs]

Thanks for filing this issue! I agree this should be fixed, and now is a good time since I've already started merging breaking changes for the next major release.

I'm planning to change the request/response API to use traits as described here, and we can simply make the reqwest implementation of those traits stateful to allow client reuse.

[cdhowie]

That sounds ideal! I look forward to that landing. For now we'll have to deal with the copy-paste but I'll link back to this issue so we can migrate when possible.

[ramosbugs]

This is now released in 5.0.0-alpha.1. If you can, please try it out and let me know if you find any issues that should be fixed before stabilizing the 5.0 API. There's an Upgrade Guide to help.