Allow to specify web proxy

[moozzyk]

Currently we don't allow to specify a web proxy. The underlying http/websocket stack allows specifying a proxy and the users may sometimes need to do that.

[ibr23]

Is there any progress on adding this functionality? We need it for an upcoming project.

[moozzyk]

There were no asks so far so it was not implemented yet.

[ibr23]

We are happy to help with the testing once the functionality has been added!

[moozzyk]

I will be adding some things to the client soon so will take a look at this as well. Stay tuned.

[ChristophAlbert]

I also would appreciate this functionality a lot, along with the possibility to set credentials. Actually, I'd need detailed control over the options offered by the C++ Rest SDK (including things like set_nativehandle_options).

I need to set options both for http_client_config and websocket_client_config; settings for proxy and credentials would typically be identical. Therefore, I'd propose to introduce a class signalr_client_config which simply bundles both and provides some convenience functions to keep the settings synchronized:

struct signalr_client_config
  {
      void set_proxy(web::web_proxy proxy)
      {
          http_client_config.set_proxy(proxy);
          websocket_client_config.set_proxy(proxy);
      }

      void set_credentials(const web::credentials &cred)
      {
          http_client_config.set_credentials(cred);
          websocket_client_config.set_credentials(cred);
      }

      web:http::client::http_client_config http_client_config;
      web::websockets::client::websocket_client_config websocket_client_config;
  };

I'd extend the constructors of connection and hub_connection as follows:

  SIGNALRCLIENT_API explicit connection(const utility::string_t& url, const utility::string_t& query_string = _XPLATSTR(""),
      trace_level trace_level = trace_level::all, std::shared_ptr<log_writer> log_writer = nullptr,
      signalr_client_config config = signalr_client_config());
    
  SIGNALRCLIENT_API explicit hub_connection(const utility::string_t& url, const utility::string_t& query_string = U(""),
    trace_level trace_level = trace_level::all, std::shared_ptr<log_writer> log_writer = nullptr, bool use_default_url = true,
    signalr_client_config config = signalr_client_config());

Of course, this would put Casablanca types into the SignalR API - but since the PPLX classes are already part of it, maybe that's Ok?

If you want this API, I could send a pull request.

[moozzyk]

@ChristophAlbert - thanks. I will get back to you soon.

[moozzyk]

I think it is fine - hub_proxy already exposes types from the web namespace (e.g. https://github.com/aspnet/SignalR-Client-Cpp/blob/dev/include/signalrclient/hub_proxy.h#L42). This has to be optional however. Also, I don't think it should be allowed to change this configuration after the connection has started.

[ChristophAlbert]

@moozzyk - While I have the code basically ready (at least setting a proxy works ;) ), I have some trouble writing tests for it. I'd like to write a test where the credentials are used to perform Basic Authentication (negotiated by a 401 challenge). The SignalR negotiation seems to work, but the Websocket connection can not be established. I'm testing on Windows 7, the error I get is set_fail_handler: 20: Invalid HTTP status. On server side, the Basic Auth Middleware is homegrown and hacked for testing purposes, but the C# client can handle it fine.

After looking at the Casablanca-Source, I noticed that in ws_client_wspp.cpp, there is no reference to websocket_client_config::credentials(). It seems the credentials are set in ws_client_winrt.cpp, though.

Do you happen to know if there is any way this can be fixed in the SignalR code, or what the right place would be? The only workaround I currently have is to manually calculate the Basic Auth Header and set it explicitly, but that sort of defeats the purpose of the authentication challenge...

[moozzyk]

@ChristophAlbert I guess this is one of these changes which will be quite hard to test. What I would try doing to test it end to end would be to set a header in the config and then send the value back to the client from the server (empty if header did not exist) and verify that the values match.

I don't think it's a good idea to bake basic authentication in SignalR (some valid concerns here: aspnet/Security#209) - if Casablanca does not support it the user will have to do it on their side. You could raise this on the cpprestsdk github. Looks like they took a PR where someone add support for basic auth for the http_client (microsoft/cpprestsdk#97)

[moozzyk]

Fixed in 3bc6b06 and 2021090.