soynatan / django-easy-audit

Yet another Django audit log app, hopefully the simplest one.
GNU General Public License v3.0
745 stars 182 forks source link

Fix handling of IP when setting DJANGO_EASY_AUDIT_REMOTE_ADDR_HEADER to HTTP_X_FORWARDED_FOR #294

Open mitchconnermeow opened 5 months ago

mitchconnermeow commented 5 months ago

For users who are running their app behind a reverse proxy such as Cloudflare, in order to log accurate IPs, they have to rely on other headers rather than Djangos default REMOTE_ADDR. HTTP_X_FORWARDED_FOR often times returns a list of IPs. It's typical for the first IP in this list to be the actual client IP.

Can you fix this so the auth_signals.py and request_signals.py can handle receiving a list of IPs and only using the first one?

There might be more elegant ways of handling this for other use cases. I know django-axes and djano-ipware can handle lists of IPs when setting their default ip setting to HTTP_X_FORWARDED_FOR. Maybe you can look to those for examples of how to best handle it if need be.

Otherwise, simply setting it to use the first IP in a given list should work fine for most use cases I would imagine.

mschoettle commented 5 months ago

I've noticed this too. When running app and reverse proxy in a container you get the container IP.

There is also the HTTP_X_REAL_IP header. Would that work? In your case @mitchconnermeow, does it still contain a list?

Here is an example excerpt from request.META:

{
    [...]
    "HTTP_X_FORWARDED_FOR": "<actualIP>",
    "HTTP_X_FORWARDED_HOST": "<actualHost>",
    "HTTP_X_FORWARDED_PORT": "443",
    "HTTP_X_FORWARDED_PROTO": "https",
    "HTTP_X_FORWARDED_SERVER": "d1d76468231d",
    "HTTP_X_REAL_IP": "<actualIP>",
    "HTTP_ACCEPT_ENCODING": "gzip",
    "wsgi.url_scheme": "http",
    "REMOTE_ADDR": "172.18.0.12",
    "REMOTE_PORT": "50372",
    "SERVER_NAME": "0.0.0.0",
    "SERVER_PORT": "8000",
    [...]
}
mitchconnermeow commented 5 months ago

I've noticed this too. When running app and reverse proxy in a container you get the container IP.

There is also the HTTP_X_REAL_IP header. Would that work? In your case @mitchconnermeow, does it still contain a list?

Here is an example excerpt from request.META:

{
    [...]
    "HTTP_X_FORWARDED_FOR": "<actualIP>",
    "HTTP_X_FORWARDED_HOST": "<actualHost>",
    "HTTP_X_FORWARDED_PORT": "443",
    "HTTP_X_FORWARDED_PROTO": "https",
    "HTTP_X_FORWARDED_SERVER": "d1d76468231d",
    "HTTP_X_REAL_IP": "<actualIP>",
    "HTTP_ACCEPT_ENCODING": "gzip",
    "wsgi.url_scheme": "http",
    "REMOTE_ADDR": "172.18.0.12",
    "REMOTE_PORT": "50372",
    "SERVER_NAME": "0.0.0.0",
    "SERVER_PORT": "8000",
    [...]
}

Unfortunately in my case I do not have the HTTP_X_REAL_IP header. In my particular case, the only header that contains the real client IP is coming from HTTP_X_FORWARDED_FOR. At the end of the day I can just modify the package to handle this case for my program but it really should be included in the default package.

mschoettle commented 5 months ago

Good to know. It could look at all of them in the following order of preference: HTTP_X_REAL_IP, HTTP_X_FORWARDED_FOR, REMOTE_ADDR.

Could you show an example of how the HTTP_X_FORWARDED_FOR header looks like in your case (with fake IPs)?

mitchconnermeow commented 5 months ago

'HTTP_X_FORWARDED_FOR': 'xxx.xxx.xxx.xxx, xxx.xxx.xxx.xxx'

This is how it's shown when viewing data from request.META on the front end. Back end is just a list ['ip','ip','ip']

thegumbyman commented 1 month ago

To add to this, if the first address (the real client ip) in the list is IPv6, you run afoul of the 50 character constraint on the remote_ip field. e.g. some of ours look like this: "xxxx:xxxx:xxxx:xxxx:xxxx:xxxx:xxxx:xxxx, xxx.xx.xxx.xxx" where the second is the cloudflare IPv4 address.