Apollo-ios: Provide a pattern for dynamic reconfiguration of network transport / auth headers

[timbotnik]

It's not clear how to use the API to implement login / logout, as it would require us to reconfigure headers. We’ll need a way to dynamically configure / reconfigure the client instance to provide different auth headers, perhaps other transport options as well.

[idris]

FYI I just have a shared Apollo client instance, and any time the session changes (login/logout/expired, etc), I just re-configure a new Apollo client and replace it.

[xiekevin]

For what it's worth, the solution @idris brought up is pretty much what we're doing as well

[fruitcoder]

How do you handle queries that fail because of invalid/expired tokens?
Do you catch a 401 in the completion handler of the apollo client and refresh the token? Then the request would still have failed and just the next request might go through (if token refresh was successful).
I would prefer to have a delegate method with an incoming url request and a closure that takes another url request. Here I would refresh my token, update the header for the request and call the closure with the now authenticated header request which is consumed by the apollo client.

[MrAlek]

I'd prefer this to not be in the network transport as a delegate but rather maybe an authentication delegate on the client. Currently, the network transport is the only customization point (see discussion in #6) and if you'd implement your own transport, you would then have to re-implement authentication.

Some cases the authentication mechanism should be able to handle (long-term):

• Inject means of authentication to the operation before it's sent to the network transport
• Blocking an outgoing operation from being sent to the network transport (if unauthorized)
• Re-authenticating before an operation is sent (if token, etc needs to refresh)
• (optionally) Handle unauthorized responses by either failing or re-authenticating. (could be handled by the application)

[martijnwalraven]

@MrAlek: Thanks for writing these cases down! The more I think about this, the more I realize this will take some effort to get right. And we should probably think about this in conjunction with #6.

One idea would be to implement something akin to RequestAdapter and RequestRetrier in Alamofire. These are fairly low-level callbacks, but they do offer quite a bit of flexibility. You could then perform the authentication yourself, or plug in something like p2/OAuth. (Or perform exponential backoff on connection failures, etc.)

My first thought would be to put these on a HTTPNetworkTransportDelegate, because the callbacks are tied to URLRequest, and not every network transport will use this:

public protocol HTTPNetworkTransportDelegate: class {
  func networkTransport(_ networkTransport: HTTPNetworkTransport, shouldSend request: URLRequest) -> Bool
  func networkTransport(_ networkTransport: HTTPNetworkTransport, willSend request: inout URLRequest)
  func networkTransport(_ networkTransport: HTTPNetworkTransport, request: URLRequest, didFailWithError error: Error, retryHandler: (_ shouldRetry: Bool) -> Void)
}

I'm not entirely happy with this because URLRequest doesn't provide any information about the GraphQL operation, and you may want that to decide your authentication or retry policy. We could pass the operation in as well, although that requires making these methods generic (because GraphQLOperation has an associated type).

Also, as you mentioned, this ties it to one particular network transport. An alternative might be to consider HTTPNetworkTransport abstract and allow for implementations using different network stacks.

The main issue seems to be that the 'means of authentication' is actually network transport specific. So it is hard to do this in a generic way. A web socket transport for example, wouldn't allow you to perform authentication on a per-request basis, but only for the connection as a whole.

There is also some work to be done to still allow network operations to be Cancellable, if they are not tied to a specific network task. So we may want to introduce a GraphQLRequest subclass of NSOperation. At that point, I'm wondering if we can't somehow take advantage of operation dependencies and OperationQueue to simplify some of this.

What do you think? How does this compare to the network stack you're currently using?

[fruitcoder]

Wow, I didn't know about the RequestAdapter and RequestRetrier in Alamofire but they are exactly how I would want the authentication process to work. Whether you put it in the Network or the Client doesn't really matter to me, both have pros and cons.

Using NSOperations always sounds promising at first, but for me it often felt too complicated wrapping URLSessionTasks in operations. Also, since UrlSession uses an operation queue under the hood anyways, it felt like duplicating and overcomplicating things. But the dependencies and limiting the number of concurrent operations are a huge benefit of using operations in general.

I think it makes sense to have generic callbacks so you can base your authentication/retry policy based on the queries/operations!

[fruitcoder]

Could we - until a better solution is found - add another initializer to HTTPNetworkTransport to inject an NSURLSession instead of just the configuration? That way, one could handle authentication challenges in the url session delegate methods.

[fruitcoder]

Any update? :)

[martijnwalraven]

@fruitcoder: Hey, sorry, I've been preoccupied with the new store and caching API, which has taken much longer than expected. Once this lands, we're in a better position to redesign the network layer, also thinking about the WebSocket transport and subscriptions.

[fruitcoder]

Cool, then I'll stick with my fork for now. Nice work on the store and caching!

[sciutand]

Hi, I just bumped in to this. Personally I think that the use of the NetworkTransport protocol gives enough customisation opportunities without making Apollo too opinionated in handling requests.

Here is an example of using Alamofire and SessionManager.

Once you are in control of the SessionManager you can add your RequestAdapter, RequestRetriers, background sessions etc etc

import Foundation
import Apollo
import Alamofire


enum GQLError: Error {
  case failedToParseResponseToJSONObject
}

class AlamofireNetworkTransport: NetworkTransport, URLConvertible {

  let sessionManager: SessionManager
  let url: URL

  init(url: URL, sessionManager: SessionManager = SessionManager.default) {
    self.sessionManager = sessionManager
    self.url = url
  }

  func send<Operation>(operation: Operation, completionHandler: @escaping (GraphQLResponse<Operation>?, Error?) -> Void) -> Cancellable where Operation : GraphQLOperation {

    let body: GraphQLMap = ["query": type(of: operation).queryDocument, "variables": operation.variables]
    return sessionManager
      .request(self, method: .post, parameters: body, encoding: JSONEncoding.default)
      .validate(statusCode: [200])
      .responseJSON { response in
        let gqlResult = response.result.flatMap{ value -> GraphQLResponse<Operation> in
          guard let value = value as? JSONObject else {
            throw AFError.responseSerializationFailed(reason: .jsonSerializationFailed(error: GQLError.failedToParseResponseToJSONObject))
          }
          return GraphQLResponse(operation: operation, body: value)
        }
        completionHandler(gqlResult.value, gqlResult.error)

    }.task!
  }

  func asURL() throws -> URL {
    return url
  }
}

[vishal-p]

Hi @martijnwalraven

Good work on SQLLite persistence and I'm using 0.6.0 beta. We need network layer customization for updating Auth headers. When do you think it will be available?

[martijnwalraven]

@vishal-p: The network layer is already customizable by providing your own implementation of NetworkTransport, see the AlamofireNetworkTransport code above for example. I'd still like to spend to some time to make auth easier out of the box however, hopefully next week.

[vishal-p]

Thanks for the reply @martijnwalraven, tried above code but got crash on return GraphQLResponse(operation: operation, body: value) will investigate more on this . Will look forward to out of box solution.

[vishal-p]

hi @martijnwalraven Is this enhancement available now with 0.6.0 release ?

[jasonsilberman]

Just saw this thread, has anyone had a chance to work on this?

[MaximusMcCann]

Same, this is quite important here :)

[ultrazhangster]

Any update, folks?

[sergiog90]

Hi,

my solution is based on original HTTPNetworkTransport form Apollo.
I make a custom HTTPNetworkTransport with dynamic custom headers injection.

AuthHTTPNetworkTransport

You only need to add your headers (line 37) based on your local settings, like Authorization header.

The client creation is like original
let client = ApolloClient(networkTransport: AuthHTTPNetworkTransport(url: url))

[matteinn]

@sergiog90 so aren't you sharing the same ApolloClient instance for all your requests?

[sergiog90]

@matteinn I'm sharing a single instance for all requests and adding dynamically headers for any performed request through the client.

[DarkDust]

I'd like to share our use-case why we would really love to see support for this and how we'd like to use it:

We're using OAuth2 for authorization, via AppAuth. For each network request sent to the server, the token may need to be refreshed which is an asynchronous operation.

So Apollo needs to ask our app for the additional headers which would be an asynchronous operation. For example, Apollo could pass a completion block that our app then has to call once the tokens have been refreshed. We would then pass the additional headers as parameters to the completion block.

Right now we're using a modified version of Apollo and need to wrap the Apollo calls so the tokens get refreshed and can be passed to Apollo… not a good solution.

[DarkDust]

So I was playing around with an implementation to allow a delegate of HTTPNetworkTransport to asynchronously modify a request before it's sent. The biggest "issue" would be that almost every function that returns a Cancellable would need to instead return a Promise<Cancellable> (likewise, some instance variables would need to changed accordingly. I'm not sure if that would be acceptable. Thoughts?

[DarkDust]

I came up with a design that doesn't need the Promise<Cancellable>. Will make a pull request in a couple of days.

[martijnwalraven]

That would be great! Sorry, have been really busy with GraphQL Summit.

[MrAlek]

@martijnwalraven How about going for that middleware pattern Apollo Link uses? That would solve most of this.

[martijnwalraven]

@MrAlek Yes, I think that makes sense. Want to take a stab at a design so we can discuss here? I haven't had any time to think about this since our last conversation.

[MrAlek]

@martijnwalraven I don't have much time right now either I'm afraid :/ We've worked around this particular problem right now by not using ApolloClient in the few cases we need to manually insert authentication per-request.

@DarkDust have you looked at Apollo Link? I think that context-passing middleware concept would work in the iOS client too.

[DarkDust]

I was not aware of Apollo Link and only briefly looked at it. I'm not sure I understand the benefit of that design yet.

I've submitted a pull request with my proposal at solving this issue in the meantime.

[MaxDesiatov]

If you use Alamofire library in your app and/or don't mind having it as a dependency, I've just published a package that solves this problem: https://github.com/maxdesiatov/ApolloAlamofire

When there's a need to change authentication headers, you can just change headers property on AlamofireTransport instance:

let token = "blah"
let u = URL(string: "http://localhost:8080/graphql")!
let h = ["Authorization": "Bearer \(token)"]
let t = AlamofireTransport(url: u, headers: h)
let client = ApolloClient(networkTransport: t)
// ... auth token changed and is stored in `newToken`:
t.headers = ["Authorization": "Bearer \(newToken)"]

New headers property will be applied to all subsequent request from that point.

[NiltiakSivad]

I'm new to GraphQL (I just got started switching over some of my service endpoints to GraphQL endpoints this past week), but to me this issue is much bigger than authentication, though authentication is a good place to start.

I think ideally someone should be able to manipulate the request or perform side effects (like starting the network activity indicator, or forcing version upgrades) before a request is sent and after it returns. Then have that behavior abstracted out so that it can be easily added or not across different service endpoints. Not sure how helpful this is, but I really enjoyed using an abstraction made by the Moya library for doing just this. https://github.com/Moya/Moya/blob/master/Sources/Moya/Plugin.swift

[cocojoe]

Hi @martijnwalraven this issue has been open for a long time. Does this issue still align with the current product priorities?

I've seen the issues/PRs with @MrAlek and @johntmcintosh that also contribute to this topic and add desirable additional requirements.

Would it be worthwhile to collate and confirm the Jobs to be Done and any related PR progress in these? These could then be broken out _ideally_ into individual Issue/Tasks with clear goals that should hopefully make it more accessible for community contribution.

Bonus: Should be able to close a bunch of issues and improve clarity 😄

[cocojoe]

What I'm hearing:

1. Library should support authentication use cases
   
   
   
   1. Pre request handling
   
   
   2. Post request handling
   
   
2. Library should support network activity logging

Examples:

Pre request handler:

1. Check if current AT (Access Token) has expired
2. Pause Operations
3. Retrieve the new AT using the RT (Refresh Token)
4. Resume Operations
5. Apply AT to requests

Post request handler:

1. Request returns 401
2. Clear Operations / Tokens
3. Logout user

Logging
Everyone has that moment when they need visibility of network requests for debugging 😆

Should the Library add this?
Technically anyone is free to implement the NetworkTransport protocol and create their own functionality...
However, should the Library expand the surface to support a popular use case. If one of the product goals of this Library is to provide a frictionless developer/integration experience then yes.

Implementation
There have been a number of PRs/suggestions that address that tackle the first issue of adding pre/post hooks typically through delegation or callbacks.
Although the real issue is handling the state of the Operations queue while an async hook is processed. I don't think the developer should have to worry about the internals of this.
Would prefer they can assign a callback to perform their pre/post logic and a method is made available to pause/resume operations.

[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)

Library should support authentication use cases

There is so many use cases - adding custom headers, certificate pinning etc. The most flexible option will be to allow users to build their own transport. With this any other library like alamofire could be used that has battle proven implementation. Replicating this in apollo will be hard and possibly outside of the scope of the core apollo. Community can provide separate library like apollo-alamofire-transport that will support advanced use cases, while core will have simple transport that works with the most of the general use cases.

[borisdering]

Are there any updates on this issue?

[MaxDesiatov]

This feature will be possible to be written by users if library will allow to provide custom NetworkTransport

Sorry @wtrocki, I'm a bit confused, could you please clarify what you mean by this? Apollo iOS library already allows providing a custom NetworkTransport, you can check an example of a custom implementation of that provided in ApolloAlamofire library.

[wtrocki]

@MaxDesiatov This is something that I was looking for (and possibly all developers who wanted to extend core networking). This library should be added to docs for people who already use Alamofire.
Really good work!

[MaxDesiatov]

Thank you very much @wtrocki, that's great to hear! I've mentioned this library multiple times in related issues of this repository and also Apollo Slack, but it doesn't look there's any interest from the maintainers of Apollo to mention ApolloAlamofire anywhere in the docs. I actually don't mind that and hope that users can still discover it directly.

[wtrocki]

@MaxDesiatov Library is amazing and it is been used quite extensively from what I see. Are you open for collaboration on this? As chances to get this functionality to core is fairly limited maybe we could move that to some aggregator where more developers can join and actively maintain it. This will be also great way to discover it (which is why I think now we still have so many PR's to the core trying to resolve it by duplicating functionality). We can put that into apollo-community organization and invite users that are actively using them.

WDYT?

[MaxDesiatov]

Sure, that's quite reasonable, I wouldn't mind having something like apollo-community and would greatly appreciate if it was somehow linked. Look forward to hearing more details on how this organization would be governed and if there are any additional criteria or requirements.

[D-Link13]

@martijnwalraven your thoughts about using ApolloAlamofire?

[postmechanical]

Came to this issue via https://www.apollographql.com/docs/ios/initialization#adding-headers. Will that doc ever be updated with the conclusions here? Will this issue actually ever turn into changes in the SDK?

[wtrocki]

I think there is wider discussion now in the community about support more advanced use cases for IOS and getting things merged.

[postmechanical]

@wtrocki What does that mean with respect to this particular issue? Which specific solution will be chosen?

[wtrocki]

ApolloAlamofire mentioned couple times here resolves a lots of issues and IMHO should be listed in documentation as alternative to internal transports. If there is any use case that this lib is not resolving lets ping team or create issue in the repo.

EDIT: Library is now available under graphql-community umbrella (https://github.com/graphql-community/ApolloAlamofire) with the number of people tracking issues. @MaxDesiatov done amazing work on the library.

The way I see it is that core networking will be working for simple use cases but if app needs to work with OAuth, cert pinning, send metrics etc. ApolloAlamofire provides a great alternative, especially that most of the apps will already have Alamofire configured. Couple PR's created for Apollo-ios are targeting the problem that ApolloAlamofire resolves.

[postmechanical]

There's plenty of us who don't want yet another dependency, particularly Alamofire. Not an acceptable solution at all. @wtrocki

[wtrocki]

Yes. It is just alternative that has features that still did not landed in apollo-ios library.

[MaxDesiatov]

I'm wondering if anyone would be interested in using a new potential package similar to ApolloAlamofire without Alamofire as a dependency, which would use NSURLSession directly?

[RolandasRazma]

would be great if it would build in as any auth requires it. Don't want to reply on another "package"...

[designatednerd]

Very much still subject to change but a draft on cancel-before-flight and swap-out-info-on-the-request stuff is up as #602

[designatednerd]

Shipped with v0.11.0!

[Drusy]

Hey @designatednerd, from what I understand, this will work only in case of malformed response or netwok error ?
The delegate HTTPNetworkTransportRetryDelegate won't be called in case of correct GraphQL response but functional errors filled in GraphQLResult.errors from body["errors"]?

[designatednerd]

Yes, that's correct - because the same request is retried, there's no reason to think that if you get errors back from the server, you won't get those same errors sending again.

The only exception I can think of is for Automatic Persisted Queries, which will be special cased. Right now #608 is looking like the most promising implementation for that.

[Drusy]

In our case, the missing authentication error (401) is located in body["errors"] and the HTTPNetworkTransportRetryDelegate would allow us to refresh the JWT token and retry the request.

[designatednerd]

Dammit, I love that I wrote this code last week and I already forgot what it does 🙃.

Wherever the error is created when dealing with a response, it hits the handleErrorOrRetry method, which will hit the retrier if it exists. Only if the retrier determines a retry should not be done will it then actually return the original error.