Update SocketIORouter.js

[spruce]

Here you have it :P, as you requested
We can discuss, whether it should be added some kind of security with setting an extra header in nginx which contains a secret key, so it's sure it really got forwarded and is not any scam just setting the header manually

[rhelmer]

Isn't this trivial for the client to spoof? It should be off by default and have a config option to turn it on.

[JohnMcLear]

Would !client.handshake.headers['x-forwarded-for'] be safer? IE

if(!client.handshake.headers['x-forwarded-for']){

[JohnMcLear]

@rhelmer That's a very good point.

[rhelmer]

Oh ok, actually just read the summary :) - some kind of additional header with a key specified in the config would do the trick (having a simple off/on switch like I suggested isn't enough anyway)

[spruce]

I will add this tomorrow

[rhelmer]

I mean you could get away with just on/off setting if this header is guaranteed to be overridden by the proxy (and lots of server software does this), but it seems like a footgun for people setting up the system :)

Also - peeking through the source for express and socket.io I see some mention of x-forwarded, can you look to see if it's possible to just flip a switch somewhere instead of etherpad-lite having to worry about it?

[spruce]

I have added it to both express and Socket.io.
For express I use the standard method enable('trust proxy'). so that req.ip get's set to the original IP.
But for socket.io I haven't found anything. There was once a pull request but it got never merged: socketio/socket.io#397 so I added a quickfix for that. will do a pull request there too.

[lkraav]

Here's another one interested in x-forwarded-for operating properly. Thanks @spruce, got enough from the patch for a "works for me"

[JohnMcLear]

@spruce we can't automatically merge this pull request, can you update please? :)

[marcelklehr]

what about this, now?

[JohnMcLear]

Bump

[marcelklehr]

merged.

[JohnMcLear]

Does this have a related issue?