Add Proxy Support

[visig9]

Hello, I just found this crate this morning and consider use it for my project recently. But after quick skim it seem like no proxy support (which is necessary part for me). So I try to add it.

I'm not familiar current codebase so some decision may be little weird. Here is something I'm not so sure:

• Custom Proxy struct as thin wrapper of reqwest::Proxy, because...
  • Seem this crate try to hide reqwest as implement detail, so I follow that.
  • Some request::Proxy methods may not suit for this case (e.g., request::Proxy::no_proxy())
• Proxy::new() return an Option but not Result, because ...
  • I'm not sure which Error type should be used for Client creation phase. So just use Option for now but this may not good enough.

How your feel? Please let me know. Thanks.

BTW, thanks for your cool project!

CAUTION: I believe this code can works but currently it only pass cargo check and I'm not actual run it yet. Because I don't have test environment right now. Feel free to test it by youself or I will test it later.

[visig9]

I test it today manually and it works.

User can create a client with proxy like this:

use graph_rs_sdk::{
    identity::ConfidentialClientApplication, GraphClient, GraphClientConfiguration, Proxy,
};

fn main() {
    const TANENT_ID: &str = "xxx";
    const CLIENT_ID: &str = "yyy";
    const CLIENT_SECRET: &str = "zzz";

    let confidential_client_application = ConfidentialClientApplication::builder(CLIENT_ID)
        .with_client_secret(CLIENT_SECRET)
        .with_tenant(TANENT_ID)
        .build();

    let proxy = Proxy::new("http://myporxy.com:3128").unwrap(); // <- here

    let graph_client: GraphClient = GraphClientConfiguration::new()
        .client_application(confidential_client_application)
        .proxy(proxy)  // <- here
        .into();

    // do something with graph_client ...
}

[sreeise]

I test it today manually and it works.

User can create a client with proxy like this:

use graph_rs_sdk::{
    identity::ConfidentialClientApplication, GraphClient, GraphClientConfiguration, Proxy,
};

fn main() {
    const TANENT_ID: &str = "xxx";
    const CLIENT_ID: &str = "yyy";
    const CLIENT_SECRET: &str = "zzz";

    let confidential_client_application = ConfidentialClientApplication::builder(CLIENT_ID)
        .with_client_secret(CLIENT_SECRET)
        .with_tenant(TANENT_ID)
        .build();

    let proxy = Proxy::new("http://myporxy.com:3128").unwrap(); // <- here

    let graph_client: GraphClient = GraphClientConfiguration::new()
        .client_application(confidential_client_application)
        .proxy(proxy)  // <- here
        .into();

    // do something with graph_client ...
}

Nice, i'll take a look at your PR and review it hopefully within a couple of days but it may be a little longer. Thanks for taking the initiative.

[sreeise]

I'm not familiar current codebase so some decision may be little weird.

The fact that you just found this crate and were able to find where to add the proxy support and the features for socks means your doing good haha.

Some request::Proxy methods may not suit for this case (e.g., request::Proxy::no_proxy())

Yeah that one im not sure what the use case would be. But regardless I don't think... its an issue? I could see this being a use case if someone wants to proxy specific requests to one proxy while others may go to a different proxy. So they setup an exclusion list of proxy servers for some requests. I've havn't used proxies much so thats just a guess on my end. Regardless I think its fine that they can do that.

I'm not sure which Error type should be used for Client creation phase. So just use Option for now but this may not good enough.

Yeah I think this one is solved by allowing the user to pass the reqwest::Proxy object instead as I mentioned in the pr review. That way the user has to deal with any issues regarding the proxy before adding to the configuration.

BTW, thanks for your cool project!

Thanks! I appreciate it.

[visig9]

Thanks for suggestions. I force push a new commit to fix all issues you list here.

Here is new examples:

use graph_rs_sdk::{
    identity::ConfidentialClientApplication, GraphClient, GraphClientConfiguration, http::Proxy,
};

fn main() {
    const TANENT_ID: &str = "xxx";
    const CLIENT_ID: &str = "yyy";
    const CLIENT_SECRET: &str = "zzz";

    let confidential_client_application = ConfidentialClientApplication::builder(CLIENT_ID)
        .with_client_secret(CLIENT_SECRET)
        .with_tenant(TANENT_ID)
        .build();

    let proxy = Proxy::all("http://myproxy.com:3128").unwrap(); // <- here

    let graph_client: GraphClient = GraphClientConfiguration::new()
        .client_application(confidential_client_application)
        .proxy(proxy)  // <- here
        .into();

    // do something with graph_client ...
}

This new commit also change feature flag socks-proxy to socks to match how you mapping reqwest feature names to local feature names.

[sreeise]

@visig9 Released in crate version 2.0.1 https://crates.io/crates/graph-rs-sdk/2.0.1