diff --git a/CHANGELOG.md b/CHANGELOG.md index 5cb53e7..6dfbd88 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -25,6 +25,12 @@ Note that 0.3.3 was never released; 0.3.4 follows 0.3.2. - `add_routes(..., include_content_preview=False)` leaves response bodies out of `/cached-records`: `content_preview` is `null`, while keys, sizes and expiry are still reported. The default stays `True`. ([#79](https://github.com/allen0099/FastAPI-CacheX/issues/79)) +- `get_client_ip(request, config)` (exported from `fastapi_cachex.session`) and + the `ClientIPDep` dependency (`fastapi_cachex.session.dependencies`) return + the client address the session middleware checks `ip_binding` against, + honouring `trusted_proxies`. Pass it to `create_session()`: behind a trusted + proxy, `request.client.host` is the proxy's address, so a session bound to it + was rejected on its next request. ([#87](https://github.com/allen0099/FastAPI-CacheX/issues/87)) ### Fixed diff --git a/docs/SESSION.md b/docs/SESSION.md index 7acc956..ae8ce5b 100644 --- a/docs/SESSION.md +++ b/docs/SESSION.md @@ -170,6 +170,7 @@ from fastapi_cachex.session import ( SessionUser, get_session, ) +from fastapi_cachex.session.dependencies import ClientIPDep app = FastAPI() @@ -200,7 +201,7 @@ app.add_middleware( @app.post("/api/auth/login") -async def login(username: str, password: str, request: Request): +async def login(username: str, password: str, request: Request, client_ip: ClientIPDep): # Authenticate the user (should query a database) if not authenticate_user(username, password): raise HTTPException(status_code=401, detail="Invalid credentials") @@ -213,14 +214,13 @@ async def login(username: str, password: str, request: Request): roles=get_user_roles(username), ) - # Collect client information for the bindings. See "Client IP and reverse - # proxies" below if the app runs behind a proxy. - ip_address = request.client.host if request.client else None + # Collect client information for the bindings. `client_ip` is the address + # the middleware checks later, including behind trusted proxies. user_agent = request.headers.get("user-agent") session, token = await session_manager.create_session( user=user, - ip_address=ip_address, + ip_address=client_ip, user_agent=user_agent, ) @@ -544,8 +544,24 @@ used only when `X-Forwarded-For` yields no usable value. The middleware applies this logic when it checks a binding, but `create_session()` binds whatever `ip_address` you pass it. Behind a trusted proxy, `request.client.host` is the proxy's address, -so pass the same client address the middleware will derive, or the binding check fails on the -next request. +which never matches, so the binding check fails on the next request. Pass the address the +middleware derives instead, either through the `ClientIPDep` dependency or by calling +`get_client_ip()` with the same config: + +```python +from fastapi_cachex.session import get_client_ip +from fastapi_cachex.session.dependencies import ClientIPDep, SessionManagerDep + + +@app.post("/login") +async def login(manager: SessionManagerDep, client_ip: ClientIPDep): + session, token = await manager.create_session(user, ip_address=client_ip) + return {"token": token} + + +# Outside a route, with the SessionConfig you gave the middleware: +client_ip = get_client_ip(request, config) +``` ### 4. IP Binding (Optional) diff --git a/fastapi_cachex/session/__init__.py b/fastapi_cachex/session/__init__.py index 27a01bf..5855d82 100644 --- a/fastapi_cachex/session/__init__.py +++ b/fastapi_cachex/session/__init__.py @@ -3,11 +3,13 @@ from .config import SessionConfig from .dependencies import get_optional_session from .dependencies import get_session +from .dependencies import get_session_client_ip from .dependencies import get_session_manager from .dependencies import require_session from .manager import SessionManager from .middleware import FastAPICacheXSessionMiddleware from .middleware import SessionMiddleware +from .middleware import get_client_ip from .models import Session from .models import SessionUser from .proxy import SessionManagerProxy @@ -20,8 +22,10 @@ "SessionManagerProxy", "SessionMiddleware", "SessionUser", + "get_client_ip", "get_optional_session", "get_session", + "get_session_client_ip", "get_session_manager", "require_session", ] diff --git a/fastapi_cachex/session/dependencies.py b/fastapi_cachex/session/dependencies.py index 87b1813..1e47184 100644 --- a/fastapi_cachex/session/dependencies.py +++ b/fastapi_cachex/session/dependencies.py @@ -10,6 +10,7 @@ from fastapi.security import HTTPAuthorizationCredentials from fastapi.security import HTTPBearer +from .middleware import get_client_ip from .models import Session if TYPE_CHECKING: @@ -115,6 +116,38 @@ async def login( return manager +def get_session_client_ip( + request: Request, + manager: "SessionManager" = Depends(get_session_manager), +) -> str | None: + """Get the client IP address the session middleware binds sessions to. + + Resolves the address with the registered SessionManager's configuration, + honouring `trusted_proxies` exactly like the middleware does. Pass it to + `create_session()` so `ip_binding` checks later requests against the same + address. + + Example: + ```python + from fastapi_cachex.session.dependencies import ClientIPDep, SessionManagerDep + + + @app.post("/login") + async def login(manager: SessionManagerDep, client_ip: ClientIPDep): + session, token = await manager.create_session(user, ip_address=client_ip) + return {"token": token} + ``` + + Args: + request: FastAPI request object + manager: SessionManager registered by the session middleware + + Returns: + Client IP address or None + """ + return get_client_ip(request, manager.config) + + require_session = get_session # Alias for required session dependency # Type annotations for dependency injection @@ -123,3 +156,4 @@ async def login( SessionDep = Annotated[Session, Depends(get_session)] UserSessionDep = Annotated[Session, Depends(get_session)] SessionManagerDep = Annotated["SessionManager", Depends(get_session_manager)] +ClientIPDep = Annotated[str | None, Depends(get_session_client_ip)] diff --git a/fastapi_cachex/session/middleware.py b/fastapi_cachex/session/middleware.py index 8744f3e..1bb3157 100644 --- a/fastapi_cachex/session/middleware.py +++ b/fastapi_cachex/session/middleware.py @@ -29,9 +29,14 @@ logger = logging.getLogger(__name__) -def _get_client_ip(connection: HTTPConnection, config: SessionConfig) -> str | None: +def get_client_ip(connection: HTTPConnection, config: SessionConfig) -> str | None: """Get the client IP address from an HTTP connection. + This is the address the session middleware checks `ip_binding` against. + Pass the same value to `SessionManager.create_session()` so the address + stored at login matches the one checked on later requests, which + `request.client.host` does not when the app runs behind a trusted proxy. + `X-Forwarded-For` and `X-Real-IP` are believed only when the request actually arrived from one of `config.trusted_proxies`; with the default empty list they are ignored entirely, because anyone can send them. @@ -250,7 +255,7 @@ def _get_client_ip(self, request: Request) -> str | None: Returns: Client IP address or None """ - return _get_client_ip(request, self.config) + return get_client_ip(request, self.config) class FastAPICacheXSessionMiddleware: @@ -325,7 +330,7 @@ async def __call__(self, scope: Scope, receive: Receive, send: Send) -> None: token_value = header_token or connection.cookies.get(self.config.cookie_name) if token_value: try: - ip_address = _get_client_ip(connection, self.config) + ip_address = get_client_ip(connection, self.config) user_agent = connection.headers.get("user-agent") backend_session, renewed_token = await self.session_manager.get_session( token_value, @@ -439,7 +444,7 @@ async def _write_session( backend_session, loaded_token, ) = await self.session_manager.create_anonymous_session( - ip_address=_get_client_ip(connection, self.config), + ip_address=get_client_ip(connection, self.config), user_agent=connection.headers.get("user-agent"), ) new_token: str | None = loaded_token diff --git a/tests/session/test_client_ip.py b/tests/session/test_client_ip.py new file mode 100644 index 0000000..a2fc367 --- /dev/null +++ b/tests/session/test_client_ip.py @@ -0,0 +1,108 @@ +"""Tests for the public client-IP helpers used with session IP binding.""" + +import pytest +from fastapi import FastAPI +from fastapi import Request +from fastapi.testclient import TestClient +from starlette.requests import HTTPConnection + +from fastapi_cachex.backends.memory import MemoryBackend +from fastapi_cachex.session import FastAPICacheXSessionMiddleware +from fastapi_cachex.session import SessionConfig +from fastapi_cachex.session import SessionManager +from fastapi_cachex.session import SessionUser +from fastapi_cachex.session import get_client_ip +from fastapi_cachex.session.dependencies import ClientIPDep +from fastapi_cachex.session.dependencies import OptionalSession +from fastapi_cachex.session.dependencies import SessionManagerDep + + +def _connection(peer: str | None, headers: dict[str, str]) -> HTTPConnection: + return HTTPConnection( + { + "type": "http", + "client": (peer, 1234) if peer is not None else None, + "headers": [(k.lower().encode(), v.encode()) for k, v in headers.items()], + } + ) + + +def test_get_client_ip_uses_forwarded_address_behind_trusted_proxy(): + config = SessionConfig(secret_key="a" * 32, trusted_proxies=["10.0.0.9"]) + connection = _connection("10.0.0.9", {"X-Forwarded-For": "198.51.100.5"}) + + assert get_client_ip(connection, config) == "198.51.100.5" + + +def test_get_client_ip_ignores_forwarded_address_from_untrusted_peer(): + config = SessionConfig(secret_key="a" * 32) + connection = _connection("10.0.0.9", {"X-Forwarded-For": "198.51.100.5"}) + + assert get_client_ip(connection, config) == "10.0.0.9" + + +def test_get_client_ip_without_peer(): + config = SessionConfig(secret_key="a" * 32) + + assert get_client_ip(_connection(None, {}), config) is None + + +@pytest.fixture +def proxied_app() -> FastAPI: + """An app with IP binding whose only peer (TestClient) is a trusted proxy.""" + config = SessionConfig( + secret_key="a" * 32, ip_binding=True, trusted_proxies=["testclient"] + ) + manager = SessionManager(MemoryBackend(), config) + + app = FastAPI() + app.add_middleware( + FastAPICacheXSessionMiddleware, session_manager=manager, config=config + ) + + @app.post("/login") + async def login( + manager: SessionManagerDep, client_ip: ClientIPDep + ) -> dict[str, str | None]: + session, token = await manager.create_session( + user=SessionUser(user_id="u1"), ip_address=client_ip + ) + return {"token": token, "ip": session.ip_address} + + @app.post("/login-peer") + async def login_peer( + request: Request, manager: SessionManagerDep + ) -> dict[str, str | None]: + _, token = await manager.create_session( + user=SessionUser(user_id="u1"), + ip_address=request.client.host if request.client else None, + ) + return {"token": token} + + @app.get("/me") + async def me(session: OptionalSession) -> dict[str, bool]: + return {"authenticated": session is not None} + + return app + + +def test_client_ip_dep_binds_the_address_the_middleware_checks(proxied_app: FastAPI): + client = TestClient(proxied_app) + forwarded = {"X-Forwarded-For": "198.51.100.5"} + + login = client.post("/login", headers=forwarded).json() + + assert login["ip"] == "198.51.100.5" + me = client.get("/me", headers={**forwarded, "X-Session-Token": login["token"]}) + assert me.json() == {"authenticated": True} + + +def test_peer_address_binding_fails_behind_trusted_proxy(proxied_app: FastAPI): + """The problem ClientIPDep solves: the proxy's address never matches.""" + client = TestClient(proxied_app) + forwarded = {"X-Forwarded-For": "198.51.100.5"} + + token = client.post("/login-peer", headers=forwarded).json()["token"] + + me = client.get("/me", headers={**forwarded, "X-Session-Token": token}) + assert me.json() == {"authenticated": False} diff --git a/tests/session/test_starlette_middleware.py b/tests/session/test_starlette_middleware.py index 82d7bde..65ffcab 100644 --- a/tests/session/test_starlette_middleware.py +++ b/tests/session/test_starlette_middleware.py @@ -417,7 +417,7 @@ def test_get_client_ip_from_x_forwarded_for(config: SessionConfig) -> None: """Shared _get_client_ip reads X-Forwarded-For behind a trusted proxy.""" from starlette.requests import HTTPConnection - from fastapi_cachex.session.middleware import _get_client_ip + from fastapi_cachex.session.middleware import get_client_ip trusting = config.model_copy(update={"trusted_proxies": ["10.0.0.9", "10.0.0.1"]}) scope: dict[str, Any] = { @@ -427,7 +427,7 @@ def test_get_client_ip_from_x_forwarded_for(config: SessionConfig) -> None: } connection = HTTPConnection(scope) - assert _get_client_ip(connection, trusting) == "192.168.1.1" + assert get_client_ip(connection, trusting) == "192.168.1.1" def test_get_client_ip_takes_the_rightmost_untrusted_entry( @@ -436,7 +436,7 @@ def test_get_client_ip_takes_the_rightmost_untrusted_entry( """A caller-supplied entry sits to the left of the address the proxy added.""" from starlette.requests import HTTPConnection - from fastapi_cachex.session.middleware import _get_client_ip + from fastapi_cachex.session.middleware import get_client_ip trusting = config.model_copy(update={"trusted_proxies": ["10.0.0.9"]}) scope: dict[str, Any] = { @@ -446,14 +446,14 @@ def test_get_client_ip_takes_the_rightmost_untrusted_entry( } connection = HTTPConnection(scope) - assert _get_client_ip(connection, trusting) == "203.0.113.99" + assert get_client_ip(connection, trusting) == "203.0.113.99" def test_get_client_ip_from_real_ip(config: SessionConfig) -> None: """Shared _get_client_ip falls back to X-Real-IP behind a trusted proxy.""" from starlette.requests import HTTPConnection - from fastapi_cachex.session.middleware import _get_client_ip + from fastapi_cachex.session.middleware import get_client_ip trusting = config.model_copy(update={"trusted_proxies": ["10.0.0.9"]}) scope: dict[str, Any] = { @@ -463,7 +463,7 @@ def test_get_client_ip_from_real_ip(config: SessionConfig) -> None: } connection = HTTPConnection(scope) - assert _get_client_ip(connection, trusting) == "192.168.1.1" + assert get_client_ip(connection, trusting) == "192.168.1.1" def test_get_client_ip_ignores_forwarded_headers_from_untrusted_peer( @@ -472,7 +472,7 @@ def test_get_client_ip_ignores_forwarded_headers_from_untrusted_peer( """With no trusted proxies the headers are ignored entirely.""" from starlette.requests import HTTPConnection - from fastapi_cachex.session.middleware import _get_client_ip + from fastapi_cachex.session.middleware import get_client_ip scope: dict[str, Any] = { "type": "http", @@ -484,14 +484,14 @@ def test_get_client_ip_ignores_forwarded_headers_from_untrusted_peer( } connection = HTTPConnection(scope) - assert _get_client_ip(connection, config) == "10.0.0.9" + assert get_client_ip(connection, config) == "10.0.0.9" def test_get_client_ip_from_client(config: SessionConfig) -> None: """Shared _get_client_ip free function falls back to the raw client address.""" from starlette.requests import HTTPConnection - from fastapi_cachex.session.middleware import _get_client_ip + from fastapi_cachex.session.middleware import get_client_ip scope: dict[str, Any] = { "type": "http", @@ -500,19 +500,19 @@ def test_get_client_ip_from_client(config: SessionConfig) -> None: } connection = HTTPConnection(scope) - assert _get_client_ip(connection, config) == "192.168.1.1" + assert get_client_ip(connection, config) == "192.168.1.1" def test_get_client_ip_none(config: SessionConfig) -> None: """Shared _get_client_ip free function returns None when nothing is available.""" from starlette.requests import HTTPConnection - from fastapi_cachex.session.middleware import _get_client_ip + from fastapi_cachex.session.middleware import get_client_ip scope: dict[str, Any] = {"type": "http", "headers": [], "client": None} connection = HTTPConnection(scope) - assert _get_client_ip(connection, config) is None + assert get_client_ip(connection, config) is None @pytest.mark.asyncio