diff --git a/CHANGELOG.md b/CHANGELOG.md index cdfb8ed..26c42c0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -54,6 +54,10 @@ Note that 0.3.3 was never released; 0.3.4 follows 0.3.2. the same pipeline as its value and returns the absolute expiry the memory backend reports. A key that disappears between the scan and the fetch is left out. ([#74](https://github.com/allen0099/FastAPI-CacheX/issues/74)) +- The client IP used for `ip_binding` now walks every `X-Forwarded-For` header + line, not only the first. A proxy that adds its own line instead of appending + to the caller's left a caller-chosen first line in charge of the walk, so a + forged address could satisfy the binding. ([#104](https://github.com/allen0099/FastAPI-CacheX/issues/104)) ## [0.3.6] - 2026-09-25 diff --git a/docs/SESSION.md b/docs/SESSION.md index 189ec55..81f2955 100644 --- a/docs/SESSION.md +++ b/docs/SESSION.md @@ -545,7 +545,8 @@ config = SessionConfig( The client address is then the **rightmost `X-Forwarded-For` entry that is not listed in `trusted_proxies`**: proxies append to the header, so the leftmost entry is whatever the caller -chose to send and cannot be trusted. If every entry in the chain is a trusted proxy, the direct +chose to send and cannot be trusted. When the header arrives on several lines, they are read as +one comma-separated chain. If every entry in the chain is a trusted proxy, the direct peer address is used. `X-Real-IP` is written by the proxy itself and has no chain to walk, so it is used only when `X-Forwarded-For` yields no usable value. diff --git a/fastapi_cachex/session/middleware.py b/fastapi_cachex/session/middleware.py index 5c44c55..bfe781c 100644 --- a/fastapi_cachex/session/middleware.py +++ b/fastapi_cachex/session/middleware.py @@ -60,7 +60,9 @@ def get_client_ip(connection: HTTPConnection, config: SessionConfig) -> str | No peer = connection.client.host if connection.client else None if peer is not None and config.is_trusted_proxy(peer): - forwarded_for = connection.headers.get("x-forwarded-for") + # A proxy may add its own header line instead of appending to the + # caller's, so the chain is every line joined, not just the first one. + forwarded_for = ",".join(connection.headers.getlist("x-forwarded-for")) if forwarded_for: for entry in reversed(forwarded_for.split(",")): candidate = entry.strip() diff --git a/tests/session/test_client_ip.py b/tests/session/test_client_ip.py index 93d9c1c..57eb30f 100644 --- a/tests/session/test_client_ip.py +++ b/tests/session/test_client_ip.py @@ -166,3 +166,20 @@ def test_model_copy_update_uses_the_new_ranges(): assert copied.is_trusted_proxy("192.168.1.1") assert not copied.is_trusted_proxy("10.0.0.1") + + +def test_forwarded_chain_spans_every_header_line(): + """A caller-sent first line must not hide the line the proxy added.""" + config = SessionConfig(secret_key="a" * 32, trusted_proxies=["10.0.0.9"]) + connection = HTTPConnection( + { + "type": "http", + "client": ("10.0.0.9", 1234), + "headers": [ + (b"x-forwarded-for", b"198.51.100.5"), # sent by the caller + (b"x-forwarded-for", b"203.0.113.7"), # added by the proxy + ], + } + ) + + assert get_client_ip(connection, config) == "203.0.113.7" diff --git a/tests/session/test_middleware.py b/tests/session/test_middleware.py index 4a63792..43ef618 100644 --- a/tests/session/test_middleware.py +++ b/tests/session/test_middleware.py @@ -11,6 +11,7 @@ from fastapi import Request from fastapi import Response from fastapi.testclient import TestClient +from starlette.datastructures import Headers from fastapi_cachex.backends.memory import MemoryBackend from fastapi_cachex.session.config import SessionConfig @@ -151,7 +152,7 @@ async def app(scope, receive, send): middleware = SessionMiddleware(app, manager, config) request = MagicMock(spec=Request) - request.headers = {"x-forwarded-for": "192.168.1.1, 10.0.0.1"} + request.headers = Headers({"x-forwarded-for": "192.168.1.1, 10.0.0.1"}) client = MagicMock() client.host = "10.0.0.9" request.client = client @@ -172,7 +173,7 @@ async def app(scope, receive, send): request = MagicMock(spec=Request) # The attacker sent the first entry themselves; nginx appended the second. - request.headers = {"x-forwarded-for": "198.51.100.5, 203.0.113.99"} + request.headers = Headers({"x-forwarded-for": "198.51.100.5, 203.0.113.99"}) client = MagicMock() client.host = "10.0.0.9" request.client = client @@ -194,7 +195,7 @@ async def app(scope, receive, send): middleware = SessionMiddleware(app, manager, config) request = MagicMock(spec=Request) - request.headers = {"x-forwarded-for": "10.0.0.1"} + request.headers = Headers({"x-forwarded-for": "10.0.0.1"}) client = MagicMock() client.host = "10.0.0.9" request.client = client @@ -214,7 +215,7 @@ async def app(scope, receive, send): middleware = SessionMiddleware(app, manager, config) request = MagicMock(spec=Request) - request.headers = {"x-real-ip": "192.168.1.1"} + request.headers = Headers({"x-real-ip": "192.168.1.1"}) client = MagicMock() client.host = "10.0.0.9" request.client = client