Repository navigation
Pass verifyClient result to connection event #377
Description
Activity
+1
+1
This would be a nice addition. I'm using jwt's in my
verifyClientcode and it would be nice to cleanly save the decoded result fo use in the connection handler. Something like this:const wss = new WebSocketServer({ server: server, verifyClient: function(info, done) { let query = url.parse(info.req.url, true).query; jwt.verify(query.token, config.jwt.secret, function(err, decoded) { if (err) return done(false, 403, 'Not valid token'); // Saving the decoded JWT on the client would be nice done(true); }); } }); wss.on('connection', ws => { // get decoded JWT here? });
Reacted by Stellan Haglund, Mark Essel, Christian Howe, Attila Komaromi, valdemars, Kevin, reboxer, marxangels, apiel, sofuetakuma112 and 8 moreReacted by Amir Tadros@pwnall do you store the jwt as a cookie or do you use it as part of the url?
I'm implementing jwt into my code but I'm not quite sure how to deliver the token.
Any suggestions?
oh and +1 ;)
Reacted by cyberdelika@stefanocudini I'm not using jwt. Here is what I'm doing.
https://github.com/pwnall/w3gram-server/blob/70a3024527e72f184cfb4d0139de218f96690848/src/ws_connection.coffee#L44
https://github.com/pwnall/w3gram-server/blob/70a3024527e72f184cfb4d0139de218f96690848/src/ws_connection.coffee#L58
https://github.com/pwnall/w3gram-server/blob/70a3024527e72f184cfb4d0139de218f96690848/src/ws_connection.coffee#L22I hope this helps.
Perhaps #1099 makes this functionality a little more public? Out of curiosity, how are you handling the invalid token on the client side? Using the 1006 error?
@ChrisZieba I ended up verifying the JWT in my connection handler as well to get the decoded data. Did you find a better solution?
To avoid double JWT encoding, I used global object (pertainInfosThroughConnectionProcess) where I store info’s that I want to retrieve upon opening connection. To distinguish to point right connection as key name I use JWT token itself.
var pertainInfosThroughConnectionProcess = {}; const wss = new WebSocketServer({ server: server, verifyClient: function(info, done) { let query = url.parse(info.req.url, true).query; jwt.verify(query.token, config.jwt.secret, function(err, decoded) { if (err) return done(false, 403, 'Not valid token'); // Using jwt as key name and storing uid pertainInfosThroughConnectionProcess[jwt] = decoded.uid; // Using jwt as key name and storing numerous values in object pertainInfosThroughConnectionProcess[jwt] = { uid: decoded.uid, anotherKey: 'another value', oneMoreKey: 'one more value' }; done(true); }); } }); wss.on('connection', ws => { // Now we can use uid from global obejct pertainInfosThroughConnectionProcess // Note: I used 'sec-websocket-protocol' to send JWT in header, so upon opening connection I can access it with ws.protocol var uid = pertainInfosThroughConnectionProcess[ws.protocol]; // or if you saved numerous values in object var uid = pertainInfosThroughConnectionProcess[ws.protocol].uid; var anotherKey = pertainInfosThroughConnectionProcess[ws.protocol].anotherKey; var oneMoreKey = pertainInfosThroughConnectionProcess[ws.protocol].oneMoreKey; // After retrieving data, we can delete this key value as is no longer needed // Note: delete is costly operation on object and there is way to optimize it, however for this purpose is not too bad delete pertainInfosThroughConnectionProcess[ws.protocol]; });
Is there a better way to do it rather than setting up global var?
@marcelijanowski yep:
verifyClient: function({ req }, done) { req.jwt = jwt.verify( // ... ); done(true); }); wss.on('connection', (ws, req) => { const jwt = req.jwt; });
Reacted by David Fairbanks, Andrew Watts-Curnow, Miles Stötzner, Oscar Stefanini, Judy Yang, Denys V., Max Greenwald, Ash, ckvv, Rajab Mohammadi and 4 moreI solved this problem using the request object and a WeakMap.
const userRequestMap = new WeakMap(); const server = new ws.Server({ port, verifyClient: (info, done) => { const user = authenticateUser(info); userRequestMap.set(info.req, user); done(user !== null); }, }); server.on('connection', (connection, request) =>{ const user = userRequestMap.get(request); });
Reacted by Pavel K., cyberdelika, dogusdeniz, Eduardo García Sanz, Joon Park and Cory Robinson+1 on this.
I will most likely end up with mutating
info.reqapproach, but it seems fragile and doesn't play well with TypeScript out of the box. It also requires theon('connection', ...)handler to process the second argument which I wouldn't need otherwise (the same concern as in #1099).
Instead it would be nice to have some documented way to approach this.Right now the async
verifyClientcan invoke the callback with up to 4 arguments in case whenresultisfalse, but the truthy case suddenly doesn't care about the other 3 arguments.
I'd propose to utilize one of these 3 and haveconnectionobject extended with some new property (say,verificationResult), so that whatever the developer passes to the second argument of the callback appears on that new property.For example,
const server = new ws.Server({ verifyClient: (info, callback) => { const verificationResult = { userId: 123 }; callback(true, verificationResult); }, }); server.on('connection', (connection) =>{ const { userId } = connection.verificationResult; });
I'd keep the sync implementation of
verifyClientas it is now, because sync computations IMHO either fairly cheep to be repeated insideon('connection', ...)handler or may not be needed there at all. And for those rare cases when the precomputed values may actually be needed, the developer should be able to refactor the code to use the callback instead.This doesn't seem like a breaking change. Any concerns?
@nilfalse that's what i'm looking for !
Are there any plans to implement that feature ? It would definetly help...
I would be glad to investigate & contributing this feature.
But until any indication from a maintainer, it doesn't make sense to even start implementing it.43 remaining items
Hello @trasherdk:
Thanks for the examples.
Out of curiosity, I checked your repo https://github.com/trasherdk/ws and saw that you mention:
This branch is up to date with websockets/ws:master.
Could you please share the motivation for your branch?
Thanks.
@rooton You can find a bunch of examples: Suggested handleUpgrade and connection flow
Thank you, but you are just copy/paste example. And my question is about socket.write. It is impossible to read message on client side to ensure thats an auth error.
Reacted by Sámal Rasmussen, Vitor Leal and rootonmy 5 cents: verifyClient is the only place to deploy connection rate limits, even tough it is not in the spec it is the ideal place to ban connections with 429 before authentication
@farr64 The reason for my clone/fork is test snippets
It's pretty much answers to other peoples issues, routed into folders on my mail-server. This way I don't have to look for answers in closed issues, but have a collection of stuff I find interesting.
The
trasherdk-snippetsbranch was to avoid locked issue template, but opted for discussions later 😄Reacted by Vitor Leal@constantind You are probably right about the rate-limit and verifyClient thingy. It makes sense to do that as early as possible.
@trasherdk wrote:
@rooton You can find a bunch of examples: Suggested handleUpgrade and connection flow
I'm trying to figure out how to get an
authentication failedclose reason from a browser WebSocket client, just like @rooton did, and the linked examples don't answer this at all. The browser websocket doesn't produce a http response of any sort that you can inspect for status codes, sosocket.write("HTTP/1.1 401 Unauthorized\r\n\r\n")andsocket.destroy()will just kill the websocket without giving any close reason to the browser websocket.I've moved the authentication handling inside the
wss.handleUpgradelike this because the ws.close call can respond with a code and reason that the websocket.onclose handler on the browser websocket will actually get.httpServer.on("upgrade", (request, socket, head) => { wss.handleUpgrade(request, socket, head, (ws) => { const authResult = authenticate(); if (!authResult.ok) { ws.close(4000, "authentication failed"); } wss.emit("connection", ws, request); }); });
I don't know if there are real costs/drawbacks to handling it here and not a level above in the
httpServer.on("upgrade"handler. I would love it if anyone could share some wisdom on this.Reacted by rooton and Saikat Dey@samal-rasmussen did you ever determine if there were any cons associated with this? I'm also in the same boat and would love to know what's the best practice here!
Does anyone know how do I get the server response (handshake response) in the client? I can only get 1006 (abnormal closure) in the
closeevent. Is there really not a way to inspect the handshake response in this library?EDIT: I had to use the
unexpected-responseevent@samal-rasmussen did you ever determine if there were any cons associated with this? I'm also in the same boat and would love to know what's the best practice here!
Works fine so far 🤷♂️
@lpinca Can you comment on the approach @samal-rasmussen is using to actually return failure codes back to a client? I've struggled to see the point of writing an HTTP response code to the socket before destroying it in the http server
upgradehandler because it doesn't appear that the client ever receives any response at all.Yet such a pattern (writing a response code to the socket) seems to be used in all of the documentation involving authentication.
Reacted by Saikat DeyThat is "ok" but in that case the authentication happens after the WebSocket connection is established. The
'open'event is emitted on the client. If you want to prevent the connection from being established you have to close it during the handshake.Yes, that appears to be the trade-off. So are you confirming that there is no way to return an error code to the client (at least in a way that a browser can see it) without first establishing a websocket connection?
In the browser client, no. Other clients (like
ws) might allow you to read the HTTP response.Reacted by Davison Long and Saikat DeyThat is "ok" but in that case the authentication happens after the WebSocket connection is established. The
'open'event is emitted on the client. If you want to prevent the connection from being established you have to close it during the handshake.Yes. This also means that you cannot assume that you are authenticated when you get the open event on the client. The client must wait for a message from the server than confirms the validation first.
Reacted by Saikat Dey
I'm doing some expensive work in
verifyClientand I'd like to reuse the result in theconnectionevent handler.I'm currently using the fact that the undocumented
WebSocketpropertyupgradeReqis the same request asinfo.reqinverifyClient, and I'm modifying the request. This feels dirty though.Will you please consider allowing any truthy
verifyClientresult, and passing it into theconnectionevent handler?If this seems reasonable, I'd be glad to prepare a pull request.