Allow graceful reconnection in the client

[asmeikal]

Story

I have the necessity of gracefully reconnecting the client.

The scenario in our application is the following:

• a user logs in with our application, and obtains a JWT with some claims
• the user connects to a GraphQL subscriptions service, and sends the JWT via the connection params
• the user starts some subscriptions while using the app
• the subscription service forwards events to the user based on the claims contained in the token
• later on, the user refreshes its JWT, which now contains some additional claims
• the app now needs to reconnect to the subscriptions service to send the new token via connection params, while keeping all active subscriptions, which may depend on, e.g., the page that the user is currently on

From my understanding, the dispose method "forgets" all currently active subscriptions, and they are not re-established upon reconnection. This is perfectly fine, e.g., for when a user logs off and a new user logs in in the same application, but does not cover our use case.

For our use case, we need a way to close and reopen the WebSocket connection without losing the internal state of the client, so that all subscriptions are re-established upon reconnection. The behaviour should be similar to what the client currently does when the connection goes down for network problems.

Acceptance criteria

• the client can restart the connection, without losing the currently active subscriptions

[enisdenjo]

Hey there! You already hinted the solution: simply close the socket connection with a non-fatal code. The client will retry automatically picking up the new connectionParams. Here is how PostGraphile's GraphiQL does it.

Quick and dirty example:

import { createClient, Client } from 'graphql-ws';
import { giveMeAFreshToken } from './token-giver';

let restartRequestedBeforeConnected = false;
let gracefullyRestart = () => {
  restartRequestedBeforeConnected = true;
};

const client = createClient({
  url: 'wss://graceful.restart/is/a/non-fatal/close-code',
  connectionParams: async () => {
    const token = await giveMeAFreshToken();
    return { token };
  },
  on: {
    connected: (socket) => {
      gracefullyRestart = () => {
        if (socket.readyState === WebSocket.OPEN) {
          socket.close(4205, 'Client Restart');
        }
      };

      // just in case you were eager to restart
      if (restartRequestedBeforeConnected) {
        restartRequestedBeforeConnected = false;
        gracefullyRestart();
      }
    },
  },
});

// all subscriptions through `client.subscribe` will resubscribe on graceful restarts

Hmm, I think it would be helpful to add this to the Recipes. 🤔

[Blemicek]

Let's look at the problem from a different perspective.

What if the server stores a token and re-validates it on any outgoing message, because it can expire or an identity provider changes the set of keys. In that case, the server can close the connection with something like 4401 Unauthorized error, but the message is lost. A client usually knows when to refresh the token, but there is no possibility to update technical information on the server using the protocol. A possible solution for this is adding a connection update message that can send any time during the session, e.g. before a token expires. The benefit of this solution is that the connection hasn't to be reestablished, so there is a smaller overhead. What do you think about this? My assumptions may be wrong and there is no reason to close the connection when the token is invalid, but clients can benefit from the connection update message anyway.

[asmeikal]

Hi @enisdenjo, I'm trying to integrate your suggestion into our application, and I've had some problems while writing tests. I'm reporting it here, if necessary I can open separate issues for each problem (if they are actually problems). A premise: we are currently using subscriptions-transport-ws, and we are trying to migrate away from it.

The server example with Apollo Express Server did not work straight away.

Here is a recap of what I had to do to get it to work.

The change I had to do was going from this:

// create a http server using express
const server = https.createServer(app);

// create websocket server
const wsServer = new ws.Server({
  server,
  path: '/graphql',
});

app.listen(443, () => {
  useServer(
    {
      schema,
      execute,
      subscribe,
    },
    wsServer,
  );
});

to this:

const server = app.listen(443, () => {
  const wsServer = new ws.Server({
    server,
    path: '/graphql',
  });

  useServer(
    {
      schema,
      execute,
      subscribe,
    },
    wsServer
  );
});

The example kept giving me a 400.

I've noticed that disposing of the client before calling unsubscribe from each subscription results in an error: each active subscription tries to send a complete message after the socket was closed. I'm wrapping the client inside ApolloClient, using the example. I don't know if this is caused by Apollo.

When disposing of the client manually, the closed event is never emitted by the client.

If these are actual problems and not desired behavior, I can try and reproduce them via some tests in this repo.

Btw, thanks for this library!!

[enisdenjo]

Let's look at the problem from a different perspective.

What if the server stores a token and re-validates it on any outgoing message, because it can expire or an identity provider changes the set of keys. In that case, the server can close the connection with something like 4401 Unauthorized error, but the message is lost. A client usually knows when to refresh the token, but there is no possibility to update technical information on the server using the protocol. A possible solution for this is adding a connection update message that can send any time during the session, e.g. before a token expires. The benefit of this solution is that the connection hasn't to be reestablished, so there is a smaller overhead. What do you think about this? My assumptions may be wrong and there is no reason to close the connection when the token is invalid, but clients can benefit from the connection update message anyway.

@Blemicek I feel like offering a ConnectionUpdate message in the Protocol is too much. It should stay lean and small. You can achieve the same thing by adding a custom implementation detail to the Next message where you "subscribe" to token updates and pass them to the client through a subscriber.

Furthermore, I really dont feel that updating the connection details within one is necessary simply because an active WebSocket connection cannot be hijacked. This is a nifty feature which helps you guarantee that the client you initially approved/connected is still the client you're communicating with. Keeping this in mind, I dont see that updating a token would ever be necessary. People are accustomed to looking at stuff through HTTP's request-response eyes where you HAVE to identify every single time; however, in WebSocket's world, the connection is persisted during its whole lifetime - eradicating the authentication overhead. The initial HTTP WS connection upgrade request and/or the ConnectionInit message are the ones which should identify and authorize the user.

---

The server example with Apollo Express Server did not work straight away.

@asmeikal good catch, I've updated the recipes to reflect your findings, thanks!

I've noticed that disposing of the client before calling unsubscribe from each subscription results in an error: each active subscription tries to send a complete message after the socket was closed. I'm wrapping the client inside ApolloClient, using the example. I don't know if this is caused by Apollo.

@asmeikal no reason why the client shouldnt check if the socket is still open before dispatching a Complete message. I'll fix this soon (just want to write a failing test first).

When disposing of the client manually, the closed event is never emitted by the client.

@asmeikal the closed event is emitted once per connection when the WS socket is closed. It does not necessarily mean the client got disposed, or even that the socket connection existed when the client got disposed (like if using lazy mode). If the WebSocket connect really gets closed and the closed event is not emitted, please do make a separate issue and a repro (if possible).

[asmeikal]

@enisdenjo, I opened a separate bug for the last problem. I believe the same test should highlight the problem with Complete messages sent after disposing of the client, but it doesn't happen, so it may be related to the integration with ApolloClient. I'll try and dig deeper.

[enisdenjo]

@asmeikal good news, v4.1.3 fixes all mentioned issues! Check the release notes. 😄

[asmeikal]

Thanks!! I'm still not sure how ApolloClient managed to call the cleanup function with a closed socket, but the fix works!

About the original question, your proposal seems to work without a problem!

[enisdenjo]

Awesome! Glad to hear everything works as expected. If you're satisfied with the answer, please close the issue. 😄

[Blemicek]

@enisdenjo, I think there is some misunderstanding, probably from my side. The proposed connection update message should be used in the direction from a client to the server similar to the connection init message and should be used only for technical things. Anyway, I understand that it's probably out of the scope of the protocol.

My concerns aren't about hijacking of a connection, but about the privileges, that are carried by the token, and connection livespan. As the connection is persistent, it can live, let say, forever (which is not true, I know) with initial privileges and it's not safe to push sensitive data to untrusted client even after authentication, bacause privileges can change in identity provider or client can leave connection open after an user logged out. I know this is quite naive or paranoic view on the problem and there are many ways how to solve it, e.g. hard closing of connections, updating of privileges on server side etc., but I think (or hope) the proposed solution is also valid as it's mitigates some problems introduced by the other options.

Thanks for your opinion! It's always pleasure to discuss about WS/Subscriptions with you! :)

[enisdenjo]

@Blemicek I'd just like to kick it off with: you can never be too paranoid about online security! 😉

Your arguments make a lot of sense, especially how a connection can live "forever" even in cases where the end user's identity changes. Like for example: I create a WS connection and keep it alive for the whole browser's session. A single browser session can have had multiple identity updates in the meantime (one user logs out, another one logs in).

Keeping this in mind, your suggestion is very convenient. You simply push a ConnectionUpdate with the updated payload and you're off! However, this does introduce a few problems IMHO:

• It is convenient, not secure. A much more secure approach would be, indeed, to close the WS connection because then the client is forced to pass 2 authentication/identification "doors": the HTTP upgrade request and the actual ConnectionInit. Furthermore, we devs are quite lazy (myself included). I fear that conveniences get abused or misinterpreted as a quick lets-be-done-with-it solutions that, often enough, get swallowed with upcoming "more important" tasks. Yes, this is more of a psychology game; but, I feel that you might understand my point. 😅
• What happens if the user has existing streaming operations (like an active subscription) while requesting a connection update? Identity changes should be reflected by either:
  • Completing all subscriptions. Why not just close the WS connection?
  • Doing some mumbo jumbo to check if the subscriptions are valid AFTER the identity change and keep them running. If you do this, then you effectively dont care about the identity, you might as well stream to anonymous clients.

I kinda' feel that ConnectionUpdate is synonymous to "close the connection and re-connect with new parameters", but with a twist: the latter is more secure and less implementation intensive.

Note that I've been talking exclusively about identity changes. Privileges, or all other user-centric details, should live on the server. This is a bias opinion and I dont mind enforcing it through constructive arguments and Protocol's small footprint. I hope you understand (and hopefully agree!).

Beware, my arguments are very opinionated - please dont take them for granted. The Protocol itself too, that is why I am pushing an RFC over at GraphQL over HTTP. The work I've done is in no way for me, it is for us! So, if you'd like to discuss this in further depths, I'd recommend leaving a comment over there and have the other, super-smart and awesome, developers engage too!

Right back at ya! I love exercising my decision making synapses through reflective feedback of the community. Especially by smart, forward-looking individuals, like yourself. 😄