From e091490f7c7b46328716f66361a16205852b6a01 Mon Sep 17 00:00:00 2001 From: adrianhopebailie Date: Sun, 4 Aug 2019 11:19:10 +0200 Subject: [PATCH 1/7] docs: deprecate verifyClient --- README.md | 38 +++++++++++++++++++ doc/ws.md | 7 +++- examples/express-session-parse/index.js | 24 ++++++------ examples/express-session-parse/public/app.js | 10 +++++ .../express-session-parse/public/index.html | 3 ++ lib/websocket-server.js | 2 +- 6 files changed, 70 insertions(+), 14 deletions(-) diff --git a/README.md b/README.md index db7b6280d..c769c690a 100644 --- a/README.md +++ b/README.md @@ -249,6 +249,44 @@ server.on('upgrade', function upgrade(request, socket, head) { server.listen(8080); ``` +### Client authentication + +```js +const http = require('http'); +const WebSocket = require('ws'); +const url = require('url'); + +const server = http.createServer(); +const wss = new WebSocket.Server({ noServer: true }); + +wss.on('connection', function(ws, request, client) { + ws.on('message', function(message) { + console.log(`WS message ${message} from user ${client}`); + }); +}); + +server.on('upgrade', function upgrade(request, socket, head) { + try { + authenticate(request, (client) => { + if (!client) { + socket.destroy(); + } + wss.handleUpgrade(request, socket, head, function done(ws) { + wss.emit('connection', ws, request, client); + }); + }); + } catch (e) { + socket.destroy(); + return; + } +}); + +server.listen(8080); +``` + +Also see the provided [example](./examples/express-session-parse) using +`express-session`. + ### Server broadcast A client WebSocket broadcasting to all connected WebSocket clients, including diff --git a/doc/ws.md b/doc/ws.md index 77b3e42e0..289ce459a 100644 --- a/doc/ws.md +++ b/doc/ws.md @@ -57,7 +57,8 @@ This class represents a WebSocket server. It extends the `EventEmitter`. - `backlog` {Number} The maximum length of the queue of pending connections. - `server` {http.Server|https.Server} A pre-created Node.js HTTP/S server. - `verifyClient` {Function} A function which can be used to validate incoming - connections. See description below. + connections. See description below. (Deprecated: See + [Issue #337](https://github.com/websockets/ws/issues/377#issuecomment-462152231)) - `handleProtocols` {Function} A function which can be used to handle the WebSocket subprotocols. See description below. - `path` {String} Accept only connections matching this path. @@ -75,6 +76,10 @@ started manually. The "noServer" mode allows the WebSocket server to be completly detached from the HTTP/S server. This makes it possible, for example, to share a single HTTP/S server between multiple WebSocket servers. +> **NOTE:** Use of `verifyClient` is discouraged. Rather handle client +> authentication in the `upgrade` event of the HTTP server. See examples for +> more details. + If `verifyClient` is not set then the handshake is automatically accepted. If it is provided with a single argument then that is: diff --git a/examples/express-session-parse/index.js b/examples/express-session-parse/index.js index 761987dc9..a452a461a 100644 --- a/examples/express-session-parse/index.js +++ b/examples/express-session-parse/index.js @@ -48,20 +48,20 @@ app.delete('/logout', function(request, response) { // const server = http.createServer(app); -const wss = new WebSocket.Server({ - verifyClient: function(info, done) { - console.log('Parsing session from request...'); - sessionParser(info.req, {}, () => { - console.log('Session is parsed!'); +const wss = new WebSocket.Server({ noServer: true }); - // - // We can reject the connection by returning false to done(). For example, - // reject here if user is unknown. - // - done(info.req.session.userId); +server.on('upgrade', async function upgrade(request, socket, head) { + console.log('Parsing session from request...'); + sessionParser(request, {}, () => { + if (!request.session.userId) { + socket.destroy(); + return; + } + console.log('Session is parsed!'); + wss.handleUpgrade(request, socket, head, function done(ws) { + wss.emit('connection', ws, request); }); - }, - server + }); }); wss.on('connection', function(ws, request) { diff --git a/examples/express-session-parse/public/app.js b/examples/express-session-parse/public/app.js index 56abc591a..3e0ec1b34 100644 --- a/examples/express-session-parse/public/app.js +++ b/examples/express-session-parse/public/app.js @@ -1,6 +1,7 @@ (function() { const messages = document.querySelector('#messages'); const wsButton = document.querySelector('#wsButton'); + const wsSendButton = document.querySelector('#wsSendButton'); const logout = document.querySelector('#logout'); const login = document.querySelector('#login'); @@ -50,6 +51,15 @@ }; ws.onclose = function() { showMessage('WebSocket connection closed'); + ws = null; }; }; + + wsSendButton.onclick = function() { + if (!ws) { + showMessage('No WebSocket connection'); + } + ws.send('Hello World!'); + showMessage('Sent "Hello World!"'); + }; })(); diff --git a/examples/express-session-parse/public/index.html b/examples/express-session-parse/public/index.html index c99949c77..c07aa2e87 100644 --- a/examples/express-session-parse/public/index.html +++ b/examples/express-session-parse/public/index.html @@ -15,6 +15,9 @@

Choose an action.

+

     
   
diff --git a/lib/websocket-server.js b/lib/websocket-server.js
index 057a12be5..768f2aacc 100644
--- a/lib/websocket-server.js
+++ b/lib/websocket-server.js
@@ -35,7 +35,7 @@ class WebSocketServer extends EventEmitter {
    *     permessage-deflate
    * @param {Number} options.port The port where to bind the server
    * @param {http.Server} options.server A pre-created HTTP/S server to use
-   * @param {Function} options.verifyClient A hook to reject connections
+   * @deprecated @param {Function} options.verifyClient A hook to reject connections
    * @param {Function} callback A listener for the `listening` event
    */
   constructor(options, callback) {

From f3ea92584e7563e1404a51a16c630031fee73aa5 Mon Sep 17 00:00:00 2001
From: Adrian Hope-Bailie 
Date: Sun, 4 Aug 2019 20:55:35 +0200
Subject: [PATCH 2/7] Update examples/express-session-parse/public/app.js

Co-Authored-By: Luigi Pinca 
---
 examples/express-session-parse/public/app.js | 1 +
 1 file changed, 1 insertion(+)

diff --git a/examples/express-session-parse/public/app.js b/examples/express-session-parse/public/app.js
index 3e0ec1b34..99a6e5cf2 100644
--- a/examples/express-session-parse/public/app.js
+++ b/examples/express-session-parse/public/app.js
@@ -58,6 +58,7 @@
   wsSendButton.onclick = function() {
     if (!ws) {
       showMessage('No WebSocket connection');
+      return;
     }
     ws.send('Hello World!');
     showMessage('Sent "Hello World!"');

From 0d1cf38b63e1972e4532501482648c36c86e3107 Mon Sep 17 00:00:00 2001
From: Adrian Hope-Bailie 
Date: Sun, 4 Aug 2019 20:55:41 +0200
Subject: [PATCH 3/7] Update doc/ws.md

Co-Authored-By: Luigi Pinca 
---
 doc/ws.md | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/doc/ws.md b/doc/ws.md
index 289ce459a..ef4ec0fbd 100644
--- a/doc/ws.md
+++ b/doc/ws.md
@@ -57,7 +57,7 @@ This class represents a WebSocket server. It extends the `EventEmitter`.
   - `backlog` {Number} The maximum length of the queue of pending connections.
   - `server` {http.Server|https.Server} A pre-created Node.js HTTP/S server.
   - `verifyClient` {Function} A function which can be used to validate incoming
-    connections. See description below. (Deprecated: See
+    connections. See description below. (Usage is discouraged: see
     [Issue #337](https://github.com/websockets/ws/issues/377#issuecomment-462152231))
   - `handleProtocols` {Function} A function which can be used to handle the
     WebSocket subprotocols. See description below.

From 75776114eed973a2f22c4f7bbf837c5363adb743 Mon Sep 17 00:00:00 2001
From: Adrian Hope-Bailie 
Date: Sun, 4 Aug 2019 20:55:56 +0200
Subject: [PATCH 4/7] Update lib/websocket-server.js

Co-Authored-By: Luigi Pinca 
---
 lib/websocket-server.js | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lib/websocket-server.js b/lib/websocket-server.js
index 768f2aacc..057a12be5 100644
--- a/lib/websocket-server.js
+++ b/lib/websocket-server.js
@@ -35,7 +35,7 @@ class WebSocketServer extends EventEmitter {
    *     permessage-deflate
    * @param {Number} options.port The port where to bind the server
    * @param {http.Server} options.server A pre-created HTTP/S server to use
-   * @deprecated @param {Function} options.verifyClient A hook to reject connections
+   * @param {Function} options.verifyClient A hook to reject connections
    * @param {Function} callback A listener for the `listening` event
    */
   constructor(options, callback) {

From 26c13b092bbc4f387f7e745f76ddc35bf5dc50a7 Mon Sep 17 00:00:00 2001
From: Adrian Hope-Bailie 
Date: Sun, 4 Aug 2019 21:01:50 +0200
Subject: [PATCH 5/7] Fix example

---
 README.md | 20 ++++++++------------
 1 file changed, 8 insertions(+), 12 deletions(-)

diff --git a/README.md b/README.md
index c769c690a..f9044235a 100644
--- a/README.md
+++ b/README.md
@@ -32,6 +32,7 @@ can use one of the many wrappers available on npm, like
   - [Simple server](#simple-server)
   - [External HTTP/S server](#external-https-server)
   - [Multiple servers sharing a single HTTP/S server](#multiple-servers-sharing-a-single-https-server)
+  - [Client authentication](#client-authentication)
   - [Server broadcast](#server-broadcast)
   - [echo.websocket.org demo](#echowebsocketorg-demo)
   - [Use the Node.js streams API](#use-the-nodejs-streams-api)
@@ -266,19 +267,14 @@ wss.on('connection', function(ws, request, client) {
 });
 
 server.on('upgrade', function upgrade(request, socket, head) {
-  try {
-    authenticate(request, (client) => {
-      if (!client) {
-        socket.destroy();
-      }
-      wss.handleUpgrade(request, socket, head, function done(ws) {
-        wss.emit('connection', ws, request, client);
-      });
+  authenticate(request, (err, client) => {
+    if (err || !client) {
+      socket.destroy();
+    }
+    wss.handleUpgrade(request, socket, head, function done(ws) {
+      wss.emit('connection', ws, request, client);
     });
-  } catch (e) {
-    socket.destroy();
-    return;
-  }
+  });
 });
 
 server.listen(8080);

From ddfc6b62603274a075ed7a0eaf3c4b269343a3c6 Mon Sep 17 00:00:00 2001
From: Adrian Hope-Bailie 
Date: Sun, 4 Aug 2019 21:04:34 +0200
Subject: [PATCH 6/7] Update README.md

Co-Authored-By: Luigi Pinca 
---
 README.md | 1 +
 1 file changed, 1 insertion(+)

diff --git a/README.md b/README.md
index f9044235a..2b422be3a 100644
--- a/README.md
+++ b/README.md
@@ -270,6 +270,7 @@ server.on('upgrade', function upgrade(request, socket, head) {
   authenticate(request, (err, client) => {
     if (err || !client) {
       socket.destroy();
+      return;
     }
     wss.handleUpgrade(request, socket, head, function done(ws) {
       wss.emit('connection', ws, request, client);

From af74e10ef59caa2d668f99430ff30163b8453457 Mon Sep 17 00:00:00 2001
From: Adrian Hope-Bailie 
Date: Sun, 4 Aug 2019 21:04:56 +0200
Subject: [PATCH 7/7] Update examples/express-session-parse/index.js

Co-Authored-By: Luigi Pinca 
---
 examples/express-session-parse/index.js | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/examples/express-session-parse/index.js b/examples/express-session-parse/index.js
index a452a461a..4e4c4789c 100644
--- a/examples/express-session-parse/index.js
+++ b/examples/express-session-parse/index.js
@@ -50,7 +50,7 @@ const server = http.createServer(app);
 
 const wss = new WebSocket.Server({ noServer: true });
 
-server.on('upgrade', async function upgrade(request, socket, head) {
+server.on('upgrade', function upgrade(request, socket, head) {
   console.log('Parsing session from request...');
   sessionParser(request, {}, () => {
     if (!request.session.userId) {