Support Certificate Pinning

[tsorencraig]

Public key pinning is a great step for apps to reduce MITM vulnerabilities. The current instantiation of the Apollo Client allows a parameter for the URLSession configuration. In order to implement SSL pinning, however, an entire URLSession object should be able to be passed in. This would allows the URLSession object to register a delegate that handles the pinning by implementing

optional public func urlSession(_ session: URLSession, didReceive challenge: URLAuthenticationChallenge, completionHandler: @escaping (URLSession.AuthChallengeDisposition, URLCredential?) -> Swift.Void)

This can be done simply by adding another constructor to the HttpNetworkTransport object that accepts the URLSession object instead of just it's configuration.

After that, injecting the session object with the appropriate delegate is simple.

let configuration = URLSessionConfiguration.default
        
let networkTransport = HTTPNetworkTransport(url: "https://some-gql-endpoint/graphql",
                                            session: URLSession(configuration: configuration,
                                                                                delegate: NSURLSessionPinningDelegate(),
                                                                                delegateQueue: nil))

let client = ApolloClient(networkTransport: networkTransport)

[martijnwalraven]

I don't think we should give out control of URLSession completely, because HTTPNetworkTransport should have the ability to implement URLSessionDataDelegate methods. But maybe we could add a HTTPNetworkTransportDelegate protocol that would pass through the authentication challenge.

This is also one of the possible solutions to handling token authentication. See this issue for a long discussion about this :) We should really make some decisions here and solve these issues. @MrAlek, what do you think?

[MrAlek]

Though adding a HTTPNetworkTransportDelegate would be one way to open up the API, I think people will find more and more cases for customization. Maybe HTTPNetworkTransport should be opened up for subclassing instead, so you can override delegate methods, inspect the HTTPURLResponse & modify outgoing URLRequests? That could be a good intermediary solution before links are in place.

[johntmcintosh]

I'd second the idea of opening HTTPNetworkTransport for subclassing.

In our app, we've needed to re-implement HTTPNetworkTransport to have some hooks into both outgoing requests and incoming responses. Our use cases are:

• Injecting logging of the request and response objects
• Automatically triggering de-authentication if a 403 response is received
• Hook for SSL Pinning

I think that a subclass of HTTPNetworkTransport would enable us to accomplish the same things without needing to re-implement that class ourselves, which would be a nice simplification.

@martijnwalraven , I'm happy to make a pass at a PR for that if you think that could be a viable option.

Regarding the token authentication piece, I'll take some time to digest the #37 thread and add some thoughts there as well, as that's a pretty urgent topic for us right now. Related to this thread, my concern with solving token auth via the subclass approach is that for us we have token refresh logic in a separate layer, and retrieving the user's current tokens is an asynchronous operation. As a result, this makes appending them in the network transport's send function problematic, since send is looking for a synchronous return value. I'll think about that some separately, but for now, I'd be thinking about this PR in terms of just opening up the existing structure to provide a little more flexibility for consumers of the library. Thoughts?

[crafterm]

I agree with @johntmcintosh - had the same requirement here and ended up reimplementing a new NetworkTransport class, where most of the code was largely HTTPNetworkTransport including pinning support. Being able to subclass, or access the URLSession property to become the delegate would be really useful.

[ericmarkmartin]

Same here. I can'e help but notice that this issue was opened almost 5 months ago. Is the reason there hasn't been any progress because the maintainers aren't sure if opening for subclassing is the right move or something else?

[wtrocki]

This feature will be possible to be written by users if library will allow to provide custom NetworkTransport.
It is really easy fix and it could be game changer for apollo on ios. More about it in comment: #297 (comment)

[designatednerd]

Apologies for the long radio silence on this issue. I'm going to be chatting with @martijnwalraven next week about the implications of giving up the ability to use the session delegate internally.

[designatednerd]

As of 0.15.0, we now support passing in a URLSession rather than just a URLSessionConfiguration, so you are able to take advantage of the URLSessionDelegate methods in order to facilitate this.

If you are still having problems with this after upgrading, please open a new issue. Thank you!

[ts95]

I recently updated Apollo to v0.37.0 and had to refactor the auth layer because of breaking changes. In the process of refactoring I noticed that there's seemingly no way to supply a custom URLSession instance anymore to the network transport class. Previously it could be supplied to HTTPNetworkTransport, but since that class has been replaced by RequestChainNetworkTransport whose initializer doesn't take a URLSession parameter, I can no longer specify a URLSessionDelegate for certificate pinning.

Am I missing something?

[ts95]

I found out that Apollo's URLSessionClient class implements URLSessionDelegate, so I can simply subclass it and override this method:

override func urlSession(
    _ session: URLSession,
    didReceive challenge: URLAuthenticationChallenge,
    completionHandler: @escaping (URLSession.AuthChallengeDisposition, URLCredential?) -> Void
)

The URLSessionClient instance can be passed to the LegacyInterceptorProvider initializer 👍

[designatednerd]

You beat me to it! Yep, subclassing URLSessionClient is the answer.