Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Auth] How to re-request an operation with new context? #220

Open
Kyle-Mendes opened this issue Apr 16, 2019 · 22 comments
Open

[Auth] How to re-request an operation with new context? #220

Kyle-Mendes opened this issue Apr 16, 2019 · 22 comments
Labels
🚄 doc & examples requests for documentation, usage examples, and support flutter Issue relates mainly to graphql_flutter investigate We are looking into the issue link Relates to link functionality question Further information is requested

Comments

@Kyle-Mendes
Copy link

I'm trying to write my own AuthLink to allow me to handle expired access_tokens and refresh them. In the ideal implementation, I would try the request again, like in Apollo.

In Apollo I'm able to use the onError link to just call forward(newOperation) after passing the updated headers. This will pass the operation along and request it again.

I tried implementing a similar approach in our flutter project, and I think I'm pretty close.

I correctly catch when the operation fails, but I'm not able to get the operation to fire again. My approach was to call await controller.addStream(forward(operation)); after calling operation.setContext to update the headers. This doesn't work, though. All future GQL requests will succeed, but the initial request that failed will not fire again unless I trigger the operation again.

It might be because Request is already filled in on the operation but, I'm not sure.

class CustomAuthLink extends Link {
  CustomAuthLink()
      : super(
          request: (Operation operation, [NextLink forward]) {
            StreamController<FetchResult> controller;
            final storage = new FlutterSecureStorage();

            Future<void> onListen() async {
              try {
                String accessToken = await storage.read(key: 'access_token');

                operation.setContext(<String, Map<String, String>>{
                  'headers': {
                    'Authorization': 'Bearer $accessToken',
                  }
                });
              } catch (error) {
                controller.addError(error);
              }

              await controller.addStream(forward(operation));

              // We have a response from the API 
              int statusCode = operation.getContext()['response'].statusCode;
              if (statusCode < 200 || statusCode >= 400) {
                // If the request failed, try getting a new Access Token
                try {
                  final response = await Auth.refresh(); // hit the api and get a new pair of tokens

                  operation.setContext(<String, Map<String, String>>{
                    'headers': {
                      'Authorization': 'Bearer ${response.accessToken}',
                    }
                  });
                  await controller.addStream(forward(operation)); // <--- I thought, re-make the request
                } catch (e) {
                  // handle that we can't log them in
                }
              }
              await controller.close();
            }

            controller = StreamController<FetchResult>(onListen: onListen);
            return controller.stream;
          },
        );
}
@Kyle-Mendes
Copy link
Author

Sorry to bump this with a comment, but I would love if any of the contributors / maintainers could provide some direction here. I'm new to dart, so I might be missing something in how the Streams work.

I would love to contribute to this project in terms of documentation or a PR if I'm able to get this to work.

We're wrapping up work on our app soon, and this is the last major thing we have left to figure out, so any direction would be greatly appreciated!

@mainawycliffe mainawycliffe added the 🚄 doc & examples requests for documentation, usage examples, and support label May 6, 2019
@mainawycliffe
Copy link
Collaborator

Have you tried stepping through the code using a debugger and see why the second request doesn't work. I will try and re-create your example when i get sometime tomorrow.

@Kyle-Mendes
Copy link
Author

@mainawycliffe

I'll see if I can get that to work, but right now I'm struggling. I might just be misunderstanding the order that things get resolved, and how Streams work. I'll make sure to update here if I have a breakthrough!

@Kyle-Mendes
Copy link
Author

@mainawycliffe

I think I'm getting to the bottom of this. I believe that the above code snippet actually works, and the operation is added back to the stream and requested again.

However, I think that when the new operation resolves successfully, it is not triggering an update in state any where, so the builder for gql isn't getting the new results, and therefore isn't re-rendering.

I'm not 100% sure, but that's my best working theory right now. Does this sound plausible?

If so, any ideas on what I might be able to do to get the builder to re-render with the new results?

@Nazacheres
Copy link

@Kyle-Mendes Greetings, I got exactly the same issue. I currently can't understand why does the class which calls this API, doesn't wait for the listener to finish its work. So currently, after
await controller.addStream(forward(operation));
Data is being processed in my API class and it doesn't wait for the second call. @Kyle-Mendes how have you solved it?

@Nazacheres
Copy link

@Kyle-Mendes Aha, I just reviewed your last comment. Seems like we are at the same point. @mainawycliffe I guess the problem here, is that
await controller.addStream(forward(operation));
Doesn't stop the stream from returning request's response. Correspondingly even if you will redo the request, Future won't be interested in the other result, cause probably you call it same as me:

response = await _client.query(QueryOptions(
        document: me,
        fetchPolicy: FetchPolicy.networkOnly,
        errorPolicy: ErrorPolicy.all,
      ));

@Nazacheres
Copy link

@mainawycliffe Can I please get some help on that? What else can I do to help you investigate that?

@mainawycliffe
Copy link
Collaborator

@Nazacheres this slipped my mind, I have yet to implement similar functionality, so I don't have a working sample but I will investigate and report back over the weekend. If you can share a reproducible sample repo, it would really save some time.

@Kyle-Mendes did you ever find a solution, and if so can you please share.

@mainawycliffe mainawycliffe added flutter Issue relates mainly to graphql_flutter investigate We are looking into the issue question Further information is requested labels Aug 20, 2019
@Kyle-Mendes
Copy link
Author

I did not find a solution yet. I am still unable to get the widget to re-render when my added stream completes.

We did a little work-around for now, though. We added to the auth-link to check if the token was valid BEFORE setting the authentication header. It's not ideal, but it works for now.

@Nazacheres
Copy link

Nazacheres commented Aug 20, 2019

@Kyle-Mendes So on each request, you check if the token is correct? Do you do it by decoding the token and calculating the time, or making an additional request?

@Kyle-Mendes
Copy link
Author

@Nazacheres For our API, we get back an expires_at time stamp. So, we check to see if the token is expired before setting the header. If it is, we refresh the token, attach it, and then make the request.

It's not ideal, since tokens can expire for other reasons, so a true solution using the stream would definitely be preferable.

@Nazacheres
Copy link

@Kyle-Mendes I am currently working on another workaround will let you know till the end of tomorrow. Thank you for your help.

@Nazacheres
Copy link

Nazacheres commented Aug 20, 2019

@Kyle-Mendes I managed to fix it! My understanding of streams in dart probably far from perfect, however here is my solution based on how I imagine streams working. This is my whole custom link:

final Link retryLink = Link(request: (
      Operation operation, [
      NextLink forward,
    ]) {
      StreamController<FetchResult> controller;
      Future<void> onListen() async {
        await controller.addStream(refreshToken(tokenManager, tokenAPI, controller, forward, operation).asStream());
        await controller.close();
      }

      controller = StreamController<FetchResult>.broadcast(onListen: onListen);

      return controller.stream;
    });

And here is method refreshToken:

Future<FetchResult> refreshToken(ITokenManager tokenManager, TokenAPI tokenAPI,
      StreamController<FetchResult> controller, NextLink forward, Operation operation) async {
    try {
      var mainStream = forward(operation);
      var firstEvent = await whenFirst(mainStream);

        return firstEvent;
    } catch (e) {
      Logger.root.severe(e.toString());

      if (e is ClientException && e.message.contains("401") && (await tokenManager.hasTokens())) {
        Logger.root.info('User logged out. But token persents. Refreshing token');
        final Token token = await tokenAPI.refreshToken();
        if (token.isValid()) {
          await tokenManager.setAccessToken(token.accessToken);
          await tokenManager.setRefreshToken(token.refreshToken);

          return whenFirst(forward(operation));
        } else {
          await tokenManager.removeCredentials();
          return whenFirst(forward(operation));
        }
      } else {
        return Future.error(e);
      }
    }
  }

As you can see instead of forwarding request, I am forwarding custom future (which then is converted to stream).
In this method, I am forwarding the request to other links, but not giving it to the main stream. It kinda breaks the logic of streams, cause in whenFirst I do:

Future<T> whenFirst<T>(Stream<T> source) async {
  try {
    await for (T value in source) {
      if (value != null) {
        return value;
      }
    }
  } catch (e) {
    return Future.error(e);
  }
}

and it throws ClientException, not graphql error.

However this is the concept how I managed it to work for me, if you guys @Kyle-Mendes and @mainawycliffe see how this can be improved please let me know.

@aduryagin
Copy link

@Nazacheres can you create repository with working sample, if your code actually works?

@PetreaLoredana
Copy link

Did anyone find a solution to the @Kyle-Mendes 's problem?

@klavs klavs added the link Relates to link functionality label Feb 16, 2020
@jamonkko
Copy link

For anyone still looking for pre-made solution: The fresh_graphql package seems to be updated to use flutter graphql v4: https://github.com/felangel/fresh/tree/master/packages/fresh_graphql

Depending of your use case it might work out of the box, and if not, you should be able to tune it to suite your needs. The link implementation looked pretty straightforward.

Have not tested it though, since managed to get refresh to work by using Dio as a http client with the gql_dio_link. Dio 's interceptor middlewares are little bit simpler to implement and they allow queuing (locking) new requests and response error handling that makes sure only one refresh-request is made simultaneously and new graphql requests can wait for the refresh before trying. That is however extra nice stuff that you might not need.

@Aristidios
Copy link

Hello @jamonkko would you kindly share with us your refresh implementation using Dio as a http client with the gql_dio_link

fresh_graphql didn't work out for me I've had to many issues with it

@jamonkko
Copy link

@Aristidios you can find some code examples how to do it with dio interceptors from there cfug/dio#50
Nice thing with that solution is that it locks new requests until you get a refreshed token, preventing multiple sequential refresh calls.

Note that the earliest examples in that issue require small porting to latest Dio v4. There seems to be some posts in that thread how to fix that too.

You can also google for "dio interceptor token refresh" which will yield multiple promising hits for more examples.

One thing that might not be in the examples is that even if you lock new requests from being sent while refreshing the token, you still might initially send more than one requests concurrently that will all fail with 401, in that case the example code will probably make multiple jwt refresh calls. To avoid that you would need to somehow throttle the refresh requests e.g. by a shared future to hold refreshed jwt which also tracks if refresh call is already on the way. That will make the code more complex though and might want to think if that is needed or not.

@Faiyyaz
Copy link

Faiyyaz commented Sep 11, 2022

Hey can anybody share their example of refresh token with locking the request and response thread similar to Dio.

Thanks!

@satvikpendem
Copy link

You can also actually do what @Gujie-Novade mentions in #672 which is to use a custom AuthLink:

final AuthLink authLink = AuthLink(
    getToken: () async  {
       final token = await getAccessToken();
       return `Bearer $token`
    },
  );

...

void getAccessToken() {
    // retrieve the access token from wherever you stored it
    
    // parse it and check the expiry

    // if it's expired, renew it by making a HTTP call to your server with your refresh token
    
    // return the access token
}

That way you don't even need to get a 401 back since you know whether the access token expired already or not, to your app it will seem as though everything is as it normally is when using a non-currently-expired access token.

@iamnabink
Copy link

@satvikpendem what if token expired during request (401) and we have to refresh current token (with refresh token and get new one and continue the request) ? how do we do that in graphql ?

@x-ji
Copy link

x-ji commented May 16, 2024

@satvikpendem I don't think that works when you have multiple requests failing at the same time due to expired token though, as they'll all try to call the geAccesstToken function at the same time. That's also what in #672 was further asked.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🚄 doc & examples requests for documentation, usage examples, and support flutter Issue relates mainly to graphql_flutter investigate We are looking into the issue link Relates to link functionality question Further information is requested
Projects
None yet
Development

No branches or pull requests