[bug]: X-Forwarded-For and X-Real-IP are not respected

[checkraisefold]

Is there an existing issue for this?

• [x] I have searched the existing issues

Current behavior

Currently, existing code fetches the client IP with REMOTE_ADDR from the request metadata. This is incorrect behind a reverse proxy.

Code exists to use X-Forwarded-For, but it is entirely unused. https://github.com/makeplane/plane/blob/9ee1d8cb03dc3ee061534e7a0acb18c6e1ae31d3/apiserver/plane/utils/ip_address.py#L1-L7

Steps to reproduce

1. Log in on a correctly configured reverse proxied Plane instance
2. Check last_login_ip; it will not match the sent X-Forwarded-For header

Environment

Production

Browser

Mozilla Firefox

Variant

Self-hosted

Version

v0.25.1

[janreges]

@checkraisefold, @pushya22, @vihar, @akshat5302 - gyus be careful, this looks like a security risk.

A header like X-Forwarded-For should only be taken into account if the request comes over the network from the allowed so-called "trusted proxies" IP ranges. But I don't see any work and configuration of such allowed IP ranges in this code. In that case, anyone could substitute any other IP address with the header and thus bypass some protections or other functionalities tied to the IP address.

[checkraisefold]

@checkraisefold, @pushya22, @vihar, @akshat5302 - gyus be careful, this looks like a security risk.

A header like X-Forwarded-For should only be taken into account if the request comes over the network from the allowed so-called "trusted proxies" IP ranges. But I don't see any work and configuration of such allowed IP ranges in this code. In that case, anyone could substitute any other IP address with the header and thus bypass some protections or other functionalities tied to the IP address.

I think Nginx handles this correctly, but this is definitely still a security consideration. Most other apps handle this with a trusted proxy configuration with CIDR ranges if I recall, which should be the proper solution here!

[pablohashescobar]

@checkraisefold, @janreges, thanks for flagging this. I see the error, and we will fix it for the reverse proxy as well. The IP rate limiting and other security-based checks are handled by Django internally, so it is not a security issue right now.