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

Update auth header on Websocket connection, automatic reconnect of subscription? #1178

Closed
gazzer82 opened this issue May 1, 2020 · 8 comments

Comments

@gazzer82
Copy link

gazzer82 commented May 1, 2020

I have this working in a project to a Hasura backend using subscriptions. I am authenticating by adding an ID Token from google auth to the connectingPayload.

The validity of the ID token from google is 1 hour, and I am trying to honour that in Hasura with only allowing the connection to remain valid for the remaining validity of the token when received.

Unfortunately that means I need to change the token in the payload when it has expired, and reconnect the web socket.

I am currently thinking the only way to do this is to create a new WebSocketTransport in

func webSocketTransport(_ webSocketTransport: WebSocketTransport, didDisconnectWithError error:Error?)

But the problem is I have some active subscriptions going on when this happens, and they don't reconnect unless manually made to do so.

Am I missing something here, or is their no easy/clean way way to handle the currently in the framework?

Thanks

Gareth

@designatednerd
Copy link
Contributor

Yeah - as I understand it, this is a problem with short-lived auth tokens and web sockets in general, since there's not really a way to constantly re-authenticate.

We do have a closeConnection() method that closes out the connection, but that would require you to manually resubscribe to everything, which leaves you where you are now.

I'm going to tag in @fassko here, who's contributed to both ApolloWebSocket and the underlying Starscream library, and is wayyyy more familiar with WebSockets than I am: Kristaps, have you got any suggestions for Gareth on what the best way to handle this would be? Is there anything Apollo can do to make this easier for our clients or is it more of a conceptual difference between an HTTP connection and a WebSocket connection?

@gazzer82
Copy link
Author

gazzer82 commented May 2, 2020

Thanks @designatednerd for that, I have had a dig through the framework, and adding this to, and calling when the token needs to be refreshed seems to work in initial testing. It seems a bit hacky, but by doing it this way calling connect re-starts the subscriptions that where active when disconnect is called it seems.

in WebSocketTransport.swift

public func updateHeaderValue(value: String?, for field: String) {
        self.websocket.request.setValue(value ?? "", forHTTPHeaderField: field)
        self.websocket.disconnect()
        self.websocket.connect()
    }

Now, I could be missing something very bad I'm doing here, so please let me know if I am?

Cheers

Gareth

@designatednerd
Copy link
Contributor

I don't believe there's anything bad, but there might be a race condition since the webSocketDidDisconnect delegate method automatically reconnects if the option to auto-reconnect is on, since that will automatically call connect after whatever the reconnectInterval is.

I'm not sure if Starscream handles multiple attempts to reconnect under the hood or if we'd need to temporarily disable the auto-reconnect.

@gazzer82
Copy link
Author

Ah yes, valid point.

So maybe something more like this, where we explicitly disable reconnect for the restart of the socket, and set it back to its previous value if it was true?

/// NOTE: Setting this will cause the socket to disconnect and immediately reconnect, active subscriptions will be reconnected once the connection is re-established.
    public func updateHeaderValue(value: String?, for field: String) {
        self.websocket.request.setValue(value ?? "", forHTTPHeaderField: field)
        let autoConnectEnabled = reconnect.value
        self.reconnect.value = false
        self.websocket.disconnect()
        self.websocket.connect()
        reconnect.value = autoConnectEnabled
    }

Is this something you'd be interested in implementing in the framework, or should I make my own fork for this (which I'd really like to not have to do) as I cannot extend or sub-class due to private restrictions e.t.c

Thanks

Gareth

@designatednerd
Copy link
Contributor

Yeah, please open a PR!

@yootsubo
Copy link
Contributor

+1!

I would very much like this to be a feature. 🙏
I'm open to putting up a PR if there isn't any action being taken.

@designatednerd
Copy link
Contributor

@yootsubo I'm swamped so any help I can get would be lovely

@designatednerd
Copy link
Contributor

This shipped with 0.28.0!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants