[feature request] socket.io middleware

I would like to add a backwards compatible feature to node-red-dashboard that will allow us to add custom middleware to the socket io connection. The implementation is simple and not too different from how we add http middleware. The main difference is that io.use() can only receive a single function, instead of an array of functions. But, we can call io.use() multiple times to add as many middleware functions as needed.

example code to add user configured io middleware as a single function, an array of functions, or fallback to the default:

if (typeof uiSettings.ioMiddleware === 'function') {

    io.use(uiSettings.ioMiddleware);

} else if (Array.isArray(uiSettings.ioMiddleware)) {

    uiSettings.ioMiddleware.forEach(middleware => {

        io.use(middleware);

    });

} else {
    // use the default that guards against cross domain connection attempts
    io.use(function (socket, next) {
        if (socket.client.conn.request.url.indexOf('transport=websocket') !== -1) {
            // Reject direct websocket requests
            socket.client.conn.close();
            return;
        }
        if (socket.handshake.xdomain === false) {
            return next();
        } else {
            socket.disconnect(true);
        }
    }
}

In our custom io middleware, we can look at more things that just xdomain. For example, we can get the cookies from socket.request.cookies and verify user authentication, etc. We can then take full advantage of creating an error object and calling next(error). This will send the simple error object to the client side code and trigger the connect_error event, at which point the error's data can be acted upon. The only error info that survives the trip from server to client is error.message and error.data. error.data can be an object containing additional data. It only supports simple data such as boolean, int, string, etc.

example sending error from within socket io middleware:

const err = new Error();
err.message = 'authentication fail';
err.data = {
  forceReload: true, // use this to trigger the forceReload() function in main.js
}

next(err);

example client side code in main.js to handle the error.data.forceReload value:

// inside the `connect_error` handler
if ((error.type === 'TransportError' && error.description === 401) || (error.data && error.data.forceReload === true)) {
    forceReload();
}

An alternative to using forceReload could be something like forceRedirect, where the server side could send an error with { data: { forceRedirect: '/loginpage' } }and that could trigger the connect_error handler to set window.location = error.data.forceRedirect.

If this seems reasonable, I can make a PR.

And as a reminder, this would be completely backwards compatible and not break anything because it preserves the existing behavior and the new setting is opt-in.

2 Likes

Hi Kevin. Yes PR away. Always welcome. Does this replace https://github.com/node-red/node-red-dashboard/pull/655 ?

1 Like

I believe that the Socket.IO middleware function is only ever called when the ws connection is in http or polling state, it isn't used when in ws state since ws: does not permit custom headers?

Typically, that means - I think - that the middleware function will normally only be called when a client connects.

That was strictly for http middleware. If you want, we can discard that pull request and I can do just a new one that will include that plus the socket .io middleware implementation? Also, you don't have a dev branch for node-red-dashboard. Do you want PRs coming to master?

I had thought that too. But, I have been experimenting with authentication for the last couple weeks and discovered many things. It's true that the middleware is only called when connecting, but it does not matter if it uses polling first and upgrades to ws, or if you skip polling and directly use ws (all via the socket io client). I have both situations on my node-red pi. My dashboard uses the default of polling first, whereas my mp4frag video uses ws only. They both get run through the socket .io middleware. The bad news is that skipping the polling causes socket .io handshake to always show the inbound connection is cross domain. The good news is that using polling or ws carries headers and cookies that are available on the socket.request object. I guess socket .io probably does some magic and preserves the headers which is why you have to use their client with their server.

I don't mind if one or two PR as long as it's clear.

Sent to master branch?

Yes please.

I think that is because the ws: protocol always connects over http for the initial handshake. You can see that in the browsers console using the networks tab. That is when the middleware is called. If the upgrade step fails which happens usually straight after the initial handshake, then Socket.IO falls back to polling (though you can disable that if you like). As polling happens over http, I think the middleware will continue to be called. But if the upgrade succeeds, the middleware is no longer called. Maybe Socket.IO retains the headers, I don't know, but I don't see how they could be updated since ws(s) doesn't support custom headers.

That being the case, the good news is that you can certainly do authentication and authorisation via the middleware. What you cannot do though is to continue to verify that the client is who they claim to be. So if session management or ongoing identity verification is important to your use-case, you need to do something more than just the middleware on the initial handshake.

For uibuilder, I've been looking at how to do that. I believe that the best approach is to include additional information in the msg's being exchanged over ws(s):

So for the uibuilder security features (still in development), the idea is to create a JWT on initial connection then maintain that via the msg's. I think that cookies could also be used but some people disable cookies so that isn't always reliable. Cookies, as you say, seem to be one thing that does continue to be exchanged even over ws(s).

Of course, JWT is not a security mechanism but rather a convenience. So you still need to do identity verification periodically at the back end and keep track of some session information. JWT's are susceptible to man-in-the-middle attacks and so should be given a short lifespan and regularly changed - if security is important anyway. And it should go without saying that you should never do any authentication without having TLS encrypted comms. Which is why uibuilder won't let you use its security features without https/wss in a production environment.

Be good to get your take on this since I'm not a security programmer - another opinion and some tests would be very helpful all round I think.

If you do want to exchange some additional information via the msg's, I'd be interested in us sharing ideas on standardising the msg schema. I've documented what I think is needed in uibuilder v3 (the latest release) but again, this is right at the edge of my knowledge so it might not be correct.

My authorization node repo is still private, but hopefully I can clean it up soon so I can make it public so you can review my attempts.

For my initial auth, I provide a simple login post route to verify the user and create a jwt that is set in a cookie, just as you described. The jwt only contains the username and ip address of the user. Each time the user has to use the jwt, I verify that the user is still valid with the correct level of permissions and that the ip is still the same(a mild verification).

For the sessions, I was considering that I would keep track of any connected socket and disconnect it based on the expiration time of the jwt or if changes to the user has been made server side, etc.

All of my salts and secrets are generated when the server boots. So, signed cookies and jwts will all be invalidated if for whatever reason I restart the backend, or some other arbitrary reason, etc.

I am also trying to mimic the way express makes a res.locals object available for middleware to pass data. In my socket io middleware, i think I am going to create a socket.locals = {}.

Cool. I look forward to seeing what you've done.

As I say, my own meagre attempts are in the uibuilder v3 code and docs which are part of the repo. If you get a chance to have a look, that would be fab.

I created a new PR that includes the middleware changes for http and socket io. When I updated my fork from your recently updated master, it seems like I am trying to merge an extra commit, but it shows that I only edited the ui.js file. Let me know if this PR can't be merged and I will redo it.

1 Like