Require a Message to authorize a SocketConnection open.
#111
Assignees
Labels
Comments
LiranCohen
changed the title
Message to authorize a SocketConnection open.
LiranCohen
added a commit
that referenced
this issue
Feb 27, 2024
This PR enhances `WebSocket` support via `JSON RPC 2.0`. This now supports long running subscriptions via `JSON RPC`. The new `JsonRpcSocket` abstracts this by providing two methods,`request` and `subscribe`. `request` sends a JSON RPC message over the socket, setting up a message handler to listen to the response, once the response is received it will clean up the listener, and resolve the response to the promise. If a response is not emitted within a given timeout, it will reject the promise. `subscribe` does something similar, however after receiving the response to the subscribe message, it will keep the handler open and look for any other messages that match the JSON RPC Id provided in the subscription request, and emit those to a message handler. The subscribe method returns a close method in order to clean up these listeners. Some things to note: - `RecordsWrite` are currently not supported via sockets, this is due to a poor handling of the data payload in the prior implementation. Would rather add this as a separate effort. TODO is tagged in code with the issue listed below. - `Subscribe` methods are currently only supported via sockets and hot `http`. - A `rpc.subscription.close` JSON RPC Method was added to close a subscription that is active for a connection. I went back and forth between making this a DWN Message vs some other signal. I landed on this for the time being and am open to discussion. More notes below. - As a part of `getDWNConfig`, `tenantGate` and `eventStream` were both made optional, as well as the `registrationManager` for `HttpApi`. I did this mostly out of convenience, but it also seems plausible that someone might run this without any registration manager. Open to discuss/change. - the `sendHttpMessage` method within `tests/utils.ts` will also be replaced by full-fledged client, listed in a separate PR below. - Current implementation allows anyone to connect, this will be addressed in a subsequent PR, issue listed below. ### Usage of `subscription.close` JSON RPC method. Q: Why not use a specific `Unsubscribe` DWN message such as `RecordsUnsubscribe`? A: This would be the first message of it's kind that would need to specifically target the DWN and potentially transport of the DWN which holds the subscription. Instead the DWN `RecordsSubscribe` message returns a close method which the transport can keep a reference to given a specific `JSON RPC Id`. This JSON RPC Id represents a specific request to a transport that was created. Later a user can issue a `rpc.subscription.close` JSON RPC Method to tell the server to close that subscription. Ultimately the owner of the JRPC Socket connection is in charge of closing the subscription, they can close all subscriptions by simply disconnecting. So it makes sense to give them the ability to close a specific JRPC Subscription. ## Initial Effort Subsequent Issues/PRs: - #111 - #109 - #110 ### Separate effort: - #108
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Currently given in PR #107 for WebSocket long running subscriptions, anyone can connect and keep a socket alive.
We have a few options:
Subscribetype message to arrive and a subscription to be accepted by the DWN.Subscribemessage with the web socket connection request and reject if not supplied or if message is not accepted by the DWN.We currently allow only on/off wrt subscription support in the DWN, we should consider potentially allowing some finer grained config options such as disallowing anonymous subscriptions or public unauthed subscriptions.
But any of the above will work with the tenant gate mechanism in place which should be enough for now.
I really like requiring the message with the initial request, however you cannot pass custom headers with a socket connection request, the common mechanism is to pass a token in the query parameters of the request. I don't mind that, but there is the caveat that the query parameters are not protected by TLS.
The only downside of 1 or 2 is that you are keeping a connection open for a timeout while waiting to receive the message through the socket.
The text was updated successfully, but these errors were encountered: