Supporting secure websockets

[chelliwell]

An essential requirement of my system is to use certificate-based TLS security between client and server.
I've been able to achieve this on queries by adding the certificates to a HttpClientHandler, using that to instantiate a HttpClient, then using the overridden GraphQLHttpClient.Default().
However I can't see a similar route for GraphQLSubscriptionWebSocketClient.Default(). One idea is to expose the ClientWebSocket, or at least the Options, from the GraphQLSubscriptionWebSocketClient class. To allow for an equivalent to

        public GraphQLSubscriptionWebSocketClient(IGraphQLFieldBuilder fieldBuilder, IGraphQLQueryGeneratorFromFields queryGenerator, IGraphQLDeserialization deserialization, CancellationToken cancellationToken)
        {
            _cancellationToken = cancellationToken;
            _webSocket.Options.AddSubProtocol("graphql-ws");

            X509CertificateCollection certCollection = new X509CertificateCollection
            {    new X509Certificate2(pfxFile, pfxPassword));   };
            _webSocket.Options.ClientCertificates = certCollection;

            this.fieldBuilder = fieldBuilder;
            this.queryGenerator = queryGenerator;
            this.deserialization = deserialization;
        }

Obviously that itself is the wrong way to do it lol. I just wanted to illustrate the aim, and I've also used this code to check that the basic principle is correct - which it seems to be since it does a wss connection to my server.

I'd be interested to hear your thoughts. Note that I am no expert C# programmer, so there may be a better way to achieve it than simply exposing the ClientWebSocket.

[sahb1239]

Hi, Think the best way would be to support a new static Default method on GraphQLSubscriptionWebSocketClient which support passing the ClientWebSocket as a parameter. I think this is what corresponds closest to the GraphQLHttpClient default method which support passing in a HttpClient.

Thanks for your suggestion :)

[chelliwell]

I'll take a look at that route, and see if I can put together something acceptable. Will fire in a Pull Request if I get summat sorted.

[chelliwell]

Just having a look at this - I've forked the master, but when I open the solution it has loads of 'type or namespace name does not exist' errors. I don't want to mess about incorrectly with project settings etc - is there something more simple that I'm missing?

[sahb1239]

Try installing Visual Studio 2019 with dotnet core. If it does not solve the problem try running the command "dotnet build" and send the errors to me.

Thanks for your help with improving the client! :)

[chelliwell]

I'm using 2019 (with preview SDK enabled).
Tried a 'dotnet build' - in the sln directory - doesn't give errors which appear to relate to the VS issues, but does complain about Version.props being missing. So I've removed the include from Common.props, and 'dotnet build' then seems to work.
And VS is now able to parse/understand everything..... who'd have thought that would have thrown up a load of 'does not exist' errors in VS Intellisense!
(Now just need to figure out how to set '_websocket' (if that's the correct approach?) from a static "Default" func...)

[chelliwell]

I guess you generate Version.props as a scripted pre-build step?

[sahb1239]

Ah okay. Yes thats correct - please see the steps which is required here:
https://github.com/sahb1239/SAHB.GraphQLClient/blob/develop/appveyor.yml

I Will update the readme

[sahb1239]

You could create a new static Default method which could call a new constructor which takes the websocket as a input parameter. I think that would be a good way to do it.

[chelliwell]

You could create a new static Default method which could call a new constructor which takes the websocket as a input parameter. I think that would be a good way to do it.

Ah, an additional GraphQLSubscriptionWebSocketClient constructor, coupled with an additional static IGraphQLSubscriptionWebSocketClient Default() ?

[sahb1239]

Yep that would solve the problem assigning to the readonly variable

[sahb1239]

This should now be fixed by #72