search

NuSkooler / enigma-bbs

ENiGMA½ BBS Software
https://nuskooler.github.io/enigma-bbs/
BSD 2-Clause "Simplified" License
529 stars 104 forks source link

Fix for x-forwarded-for not being applied #533

Open notepid opened 4 months ago

notepid commented 4 months ago

this.proxied is not set before calling setClient() resulting in a "undefined" value for this.proxied. This in turn prevents the proxy headers to be used when resolving the client IP.

In a scenario where a reverse proxy is used the client IP will always show as 127.0.0.1 instead of the X-Forwarded-For header.

Also consider using request.socket instead of connection in setClient() Ref: https://stackoverflow.com/questions/8107856/how-to-determine-a-users-ip-address-in-node

NuSkooler commented 4 months ago

@notepid Thanks for this!

RE: request.socket, is the current method causing issues?

notepid commented 4 months ago

@notepid Thanks for this!

You're welcome :)

RE: request.socket, is the current method causing issues?

No issues as I see. Just noted while looking into this that connection is being deprecated.

I tested with this code and it looked to work but that was just a quick test: this.resolvedRemoteAddress = (this.client.proxied && httpRequest.headers['x-forwarded-for'] || '').split(',').pop().trim() || httpRequest.socket.remoteAddress;

Apparently x-forwarded-for can be a list of addresses.

NuSkooler commented 4 months ago

@notepid Thanks for this! No issues as I see. Just noted while looking into this that connection is being deprecated.

Ah gotcha, feel free to open a PR or update here. I'm happy either way if you want to update it. It can land in a ticket otherwise. I have a bit list of things I need to get back to when I get some more time on my plate.

Apparently x-forwarded-for can be a list of addresses.

Never can be just "that easy" :)