From 1f7f9fbd012efd5afae177f2d434a7ab9b6d841b Mon Sep 17 00:00:00 2001 From: allen0099 Date: Sun, 27 Sep 2026 12:02:03 +0000 Subject: [PATCH 1/2] fix(session): keep responses that carry a session token out of shared caches When FastAPICacheXSessionMiddleware sent a token (new session, sliding renewal, regenerated ID) or a clearing cookie, nothing stopped a CDN or reverse proxy from storing it: Vary was only added when the handler accessed request.session and Cache-Control was left as the route set it, so a @cache(public=True) route could hand a valid session cookie to the next visitor. Such responses now get Cache-Control: private, no-store (replacing any existing value) and Vary on the transport headers, whatever the handler did with the session. The deprecated SessionMiddleware does the same when it sends a token. Vary names already present are no longer duplicated. Closes #297 --- docs/SESSION.md | 7 + fastapi_cachex/session/middleware.py | 75 ++++- i18n/zh-TW/docs/SESSION.md | 1 + tests/session/test_token_response_caching.py | 327 +++++++++++++++++++ 4 files changed, 398 insertions(+), 12 deletions(-) create mode 100644 tests/session/test_token_response_caching.py diff --git a/docs/SESSION.md b/docs/SESSION.md index e1836c0..251ea3a 100644 --- a/docs/SESSION.md +++ b/docs/SESSION.md @@ -411,6 +411,13 @@ async def me(session=Depends(get_session)): the headers checked in `token_source_priority` order (`header_name`, and `Authorization` when bearer tokens are enabled) up to the one that carried the token. `Cookie` is added only when no header carried a token, because only then is the cookie read. +- A response that carries a session token (a new session, a sliding renewal, a regenerated ID) + or a `Set-Cookie` that expires the session cookie is never cacheable. The middleware sets + `Cache-Control: private, no-store`, replacing whatever the route set (a `@cache(public=True)` + route included), and adds the same `Vary` names as above even when the handler never touched + `request.session`. Otherwise a CDN or reverse proxy could store the token and hand it to the + next visitor. Responses without a token keep their headers. The deprecated `SessionMiddleware` + does the same when it sends a token in its response header. The cookie is always `HttpOnly`; `Secure`, `SameSite`, `Domain`, `Path` and `Max-Age` follow the `cookie_*` settings. diff --git a/fastapi_cachex/session/middleware.py b/fastapi_cachex/session/middleware.py index 8e1ae20..9942e72 100644 --- a/fastapi_cachex/session/middleware.py +++ b/fastapi_cachex/session/middleware.py @@ -146,6 +146,43 @@ def _read_header_token( return None, consulted +def _add_vary(headers: MutableHeaders, names: list[str]) -> None: + """Add header names to ``Vary``, keeping the values already there. + + Starlette's ``add_vary_header`` appends unconditionally, so names the + response already varies on (compared case-insensitively) are skipped, as + is everything when it already varies on ``*``. + + Args: + headers: Mutable response headers to write to + names: Request header names the response depends on + """ + present = { + value.strip().lower() + for line in headers.getlist("vary") + for value in line.split(",") + } + if "*" in present: + return + for name in names: + if name.lower() not in present: + headers.add_vary_header(name) + present.add(name.lower()) + + +def _forbid_storing(headers: MutableHeaders) -> None: + """Keep a response that carries a session token out of every cache. + + The token is a credential: a shared cache that stored the response would + hand it to the next visitor. This replaces any ``Cache-Control`` the route + set, ``public`` and ``max-age`` included. + + Args: + headers: Mutable response headers to write to + """ + headers["Cache-Control"] = "private, no-store" + + def _stash_session_manager(app: Any, manager: SessionManager) -> None: """Register the session manager on ``app.state`` for dependency injection. @@ -254,12 +291,15 @@ async def dispatch( if session is not None and session.session_id != loaded_session_id: # The handler regenerated the session ID; a renewed token would # name the deleted record, so send a token for the new ID. - response.headers[self.config.header_name] = ( - self.session_manager.issue_token(session) - ) - elif renewed_token is not None: + response_token: str | None = self.session_manager.issue_token(session) + else: # Propagate renewed token to client so its JWT exp stays in sync - response.headers[self.config.header_name] = renewed_token + response_token = renewed_token + + if response_token is not None: + response.headers[self.config.header_name] = response_token + _add_vary(response.headers, _read_header_token(request, self.config)[1]) + _forbid_storing(response.headers) return response @@ -408,11 +448,7 @@ async def send_wrapper(message: Message) -> None: backend_session, loaded_session_id, loaded_token, renewed_token ) - if session.accessed: - for name in vary_on: - headers.add_vary_header(name) - - await self._persist( + sent_token = await self._persist( session, headers, connection, @@ -422,6 +458,11 @@ async def send_wrapper(message: Message) -> None: from_header=from_header, ) + if session.accessed or sent_token: + _add_vary(headers, vary_on) + if sent_token: + _forbid_storing(headers) + await send(message) await self.app(scope, receive, send_wrapper) @@ -436,8 +477,13 @@ async def _persist( # noqa: PLR0913, PLR0917 fresh_token: str | None, *, from_header: bool, - ) -> None: - """Save, delete or renew the session according to what the request did to it.""" + ) -> bool: + """Save, delete or renew the session according to what the request did to it. + + Returns: + True if a token or a clearing cookie was written to the response + """ + sent_token = False target = backend_session if session.cleared and target is not None: # clear() logs out, whatever the data held. Anything @@ -448,6 +494,7 @@ async def _persist( # noqa: PLR0913, PLR0917 # Cookie transport: expire the cookie. A header-based # client simply drops its now-dangling token. headers.append("Set-Cookie", self._build_clear_cookie_header()) + sent_token = True if session.modified and ( session or (target is not None and target.user is not None) @@ -467,16 +514,20 @@ async def _persist( # noqa: PLR0913, PLR0917 token_to_emit = new_token if from_header else cookie_token if token_to_emit is not None: self._emit_token(headers, token_to_emit, from_header=from_header) + sent_token = True elif session.modified and target is not None: # An anonymous session left empty holds nothing to keep. await self.session_manager.delete_session(target.session_id) if not from_header: headers.append("Set-Cookie", self._build_clear_cookie_header()) + sent_token = True elif fresh_token is not None: # Sliding expiration renewed the token, or the ID was # regenerated, even though the dict itself was untouched; # propagate it via the same transport. self._emit_token(headers, fresh_token, from_header=from_header) + sent_token = True + return sent_token def _token_sources( self, connection: HTTPConnection diff --git a/i18n/zh-TW/docs/SESSION.md b/i18n/zh-TW/docs/SESSION.md index 51e5899..ff5fe24 100644 --- a/i18n/zh-TW/docs/SESSION.md +++ b/i18n/zh-TW/docs/SESSION.md @@ -348,6 +348,7 @@ async def me(session=Depends(get_session)): - 以 `del` 或 `pop()` 移除最後一個鍵並不是登出。帶有使用者的 Session 會以空資料儲存；匿名 Session 已無任何內容，會和 `clear()` 一樣被刪除。 - 以寫入 `request.session` 的方式登入時，會沿用請求帶來的 Session ID。Starlette 的中介軟體中 Cookie *就是* Session，因此登入回應會取代任何被植入的 Cookie；這裡的 Cookie 只是指向伺服器端紀錄的名稱，被植入的 Cookie 會跟著受害者一起登入。請在附加使用者之前呼叫 `await rotate_session_id(request)`（見[登入後重新產生 Session ID](#5-regenerate-the-session-id-after-login)）。 - 只要存取 `request.session`，就會為了尋找權杖而讀取過的每個請求標頭加入 `Vary`：依 `token_source_priority` 順序檢查的標頭（`header_name`，以及啟用 Bearer 權杖時的 `Authorization`），直到攜帶權杖的那一個為止。只有在沒有任何標頭攜帶權杖時才會讀取 Cookie，因此也只有這時才會加入 `Cookie`。 +- 帶有 Session 權杖的回應（新建立的 Session、滑動續期、重新產生的 ID），或帶有讓 Session Cookie 失效之 `Set-Cookie` 的回應，一律不可快取。中介軟體會設定 `Cache-Control: private, no-store`，取代路由原本設定的值（包括 `@cache(public=True)` 的路由），並且即使處理函式沒有碰過 `request.session`，也會加入與上一項相同的 `Vary` 名稱。否則 CDN 或反向 proxy 可能存下權杖，再交給下一位訪客。不帶權杖的回應則維持原本的標頭。已棄用的 `SessionMiddleware` 在回應標頭送出權杖時也會這麼做。 Cookie 一律為 `HttpOnly`；`Secure`、`SameSite`、`Domain`、`Path` 與 `Max-Age` 則依 `cookie_*` 設定。 diff --git a/tests/session/test_token_response_caching.py b/tests/session/test_token_response_caching.py new file mode 100644 index 0000000..d007c33 --- /dev/null +++ b/tests/session/test_token_response_caching.py @@ -0,0 +1,327 @@ +"""Responses that carry a session token must never be stored by a cache (#297). + +A token is a credential: a CDN or reverse proxy that stored a response with +``Set-Cookie`` or the token header would hand it to the next visitor. Every +response the session middleware writes a token or a clearing cookie to gets +``Cache-Control: private, no-store`` and ``Vary`` on the transport headers, +whether or not the handler touched ``request.session``. +""" + +from typing import Any + +import pytest +from fastapi import Depends +from fastapi import FastAPI +from fastapi import Request +from fastapi import Response +from fastapi.testclient import TestClient + +from fastapi_cachex import cache +from fastapi_cachex.backends.memory import MemoryBackend +from fastapi_cachex.session.config import SessionConfig +from fastapi_cachex.session.dependencies import get_session +from fastapi_cachex.session.manager import SessionManager +from fastapi_cachex.session.middleware import FastAPICacheXSessionMiddleware +from fastapi_cachex.session.middleware import SessionMiddleware +from fastapi_cachex.session.models import SessionUser + +_NO_STORE = "private, no-store" +_PUBLIC = "public, max-age=60" + + +@pytest.fixture +def sliding_config() -> SessionConfig: + """A config that renews the token on every load.""" + return SessionConfig(secret_key="a" * 32, sliding_threshold=1.0) + + +@pytest.fixture +def sliding_manager( + backend: MemoryBackend, sliding_config: SessionConfig +) -> SessionManager: + return SessionManager(backend, sliding_config) + + +def _vary(response: Any) -> list[str]: + """The response's Vary header as lowercased names, duplicates kept.""" + return [ + name.strip().lower() + for name in response.headers.get("vary", "").split(",") + if name.strip() + ] + + +def _app( + manager: SessionManager, + config: SessionConfig, + middleware: Any = FastAPICacheXSessionMiddleware, +) -> FastAPI: + """An app with a publicly cacheable route and routes that touch the session.""" + app = FastAPI() + app.add_middleware(middleware, session_manager=manager, config=config) + + @app.get("/public") + @cache(ttl=60, public=True) + async def public_route() -> dict[str, bool]: + return {"ok": True} + + @app.get("/vary-accept") + async def vary_accept_route(response: Response) -> dict[str, bool]: + response.headers["Vary"] = "Accept-Encoding, Cookie" + response.headers["Cache-Control"] = _PUBLIC + return {"ok": True} + + @app.get("/vary-star") + async def vary_star_route(response: Response) -> dict[str, bool]: + response.headers["Vary"] = "*" + return {"ok": True} + + @app.get("/write") + async def write_route(request: Request) -> dict[str, bool]: + request.session["cart"] = [1] + return {"ok": True} + + @app.get("/logout") + async def logout_route(request: Request) -> dict[str, bool]: + request.session.clear() + return {"ok": True} + + @app.get("/pop") + async def pop_route(request: Request) -> dict[str, Any]: + return {"flash": request.session.pop("flash", None)} + + @app.post("/login") + async def login_route(session=Depends(get_session)) -> dict[str, bool]: + await manager.regenerate_session_id(session) + return {"ok": True} + + return app + + +def _assert_not_storable(response: Any, vary: set[str]) -> None: + assert response.headers["cache-control"] == _NO_STORE + assert set(_vary(response)) == vary + + +def test_a_new_session_cookie_is_not_storable( + manager: SessionManager, config: SessionConfig +) -> None: + client = TestClient(_app(manager, config)) + + response = client.get("/write") + + assert config.cookie_name in response.headers["set-cookie"] + _assert_not_storable( + response, {config.header_name.lower(), "authorization", "cookie"} + ) + + +async def test_a_sliding_renewal_cookie_overrides_a_public_route( + sliding_manager: SessionManager, sliding_config: SessionConfig +) -> None: + """The repro from #297: the route never touches the session.""" + _session, token = await sliding_manager.create_session( + user=SessionUser(user_id="u") + ) + client = TestClient(_app(sliding_manager, sliding_config)) + client.cookies.set(sliding_config.cookie_name, token) + + response = client.get("/public") + + assert sliding_config.cookie_name in response.headers["set-cookie"] + _assert_not_storable( + response, {sliding_config.header_name.lower(), "authorization", "cookie"} + ) + + +async def test_a_sliding_renewal_header_overrides_a_public_route( + sliding_manager: SessionManager, sliding_config: SessionConfig +) -> None: + _session, token = await sliding_manager.create_session( + user=SessionUser(user_id="u") + ) + client = TestClient(_app(sliding_manager, sliding_config)) + + response = client.get("/public", headers={sliding_config.header_name: token}) + + assert sliding_config.header_name in response.headers + assert "set-cookie" not in response.headers + _assert_not_storable(response, {sliding_config.header_name.lower()}) + + +async def test_a_sliding_renewal_over_bearer_varies_on_authorization( + sliding_manager: SessionManager, sliding_config: SessionConfig +) -> None: + _session, token = await sliding_manager.create_session( + user=SessionUser(user_id="u") + ) + client = TestClient(_app(sliding_manager, sliding_config)) + + response = client.get("/public", headers={"Authorization": f"Bearer {token}"}) + + assert sliding_config.header_name in response.headers + _assert_not_storable( + response, {sliding_config.header_name.lower(), "authorization"} + ) + + +async def test_existing_vary_values_are_kept_and_not_duplicated( + sliding_manager: SessionManager, sliding_config: SessionConfig +) -> None: + _session, token = await sliding_manager.create_session( + user=SessionUser(user_id="u") + ) + client = TestClient(_app(sliding_manager, sliding_config)) + client.cookies.set(sliding_config.cookie_name, token) + + response = client.get("/vary-accept") + + assert response.headers["cache-control"] == _NO_STORE + vary = _vary(response) + assert sorted(vary) == sorted( + { + "accept-encoding", + "cookie", + sliding_config.header_name.lower(), + "authorization", + } + ) + + +async def test_vary_star_is_left_as_it_is( + sliding_manager: SessionManager, sliding_config: SessionConfig +) -> None: + """``Vary: *`` already covers every header; adding names to it is noise.""" + _session, token = await sliding_manager.create_session( + user=SessionUser(user_id="u") + ) + client = TestClient(_app(sliding_manager, sliding_config)) + client.cookies.set(sliding_config.cookie_name, token) + + response = client.get("/vary-star") + + assert response.headers["cache-control"] == _NO_STORE + assert response.headers["vary"] == "*" + + +@pytest.mark.parametrize("transport", ["cookie", "header"]) +async def test_a_regenerated_session_id_is_not_storable( + manager: SessionManager, config: SessionConfig, transport: str +) -> None: + _session, token = await manager.create_session(user=SessionUser(user_id="u")) + client = TestClient(_app(manager, config)) + + if transport == "cookie": + client.cookies.set(config.cookie_name, token) + response = client.post("/login") + assert config.cookie_name in response.headers["set-cookie"] + vary = {config.header_name.lower(), "authorization", "cookie"} + else: + response = client.post("/login", headers={config.header_name: token}) + assert response.headers[config.header_name] != token + vary = {config.header_name.lower()} + + _assert_not_storable(response, vary) + + +async def test_a_logout_clearing_cookie_is_not_storable( + manager: SessionManager, config: SessionConfig +) -> None: + _session, token = await manager.create_session(user=SessionUser(user_id="u")) + client = TestClient(_app(manager, config)) + client.cookies.set(config.cookie_name, token) + + response = client.get("/logout") + + assert "1970" in response.headers["set-cookie"] + _assert_not_storable( + response, {config.header_name.lower(), "authorization", "cookie"} + ) + + +async def test_an_emptied_anonymous_session_clearing_cookie_is_not_storable( + manager: SessionManager, config: SessionConfig +) -> None: + _session, token = await manager.create_anonymous_session(flash="hi") + client = TestClient(_app(manager, config)) + client.cookies.set(config.cookie_name, token) + + response = client.get("/pop") + + assert "1970" in response.headers["set-cookie"] + _assert_not_storable( + response, {config.header_name.lower(), "authorization", "cookie"} + ) + + +async def test_an_emptied_anonymous_header_session_sends_nothing_to_forbid( + manager: SessionManager, config: SessionConfig +) -> None: + """A header client just drops its dangling token, so no header is sent.""" + _session, token = await manager.create_anonymous_session(flash="hi") + client = TestClient(_app(manager, config)) + + response = client.get("/pop", headers={config.header_name: token}) + + assert response.json() == {"flash": "hi"} + assert config.header_name not in response.headers + assert "set-cookie" not in response.headers + assert "cache-control" not in response.headers + + +@pytest.mark.filterwarnings("ignore::DeprecationWarning") +async def test_the_deprecated_middleware_renewal_is_not_storable( + sliding_manager: SessionManager, sliding_config: SessionConfig +) -> None: + _session, token = await sliding_manager.create_session( + user=SessionUser(user_id="u") + ) + client = TestClient( + _app(sliding_manager, sliding_config, middleware=SessionMiddleware) + ) + + response = client.get("/public", headers={sliding_config.header_name: token}) + + assert sliding_config.header_name in response.headers + _assert_not_storable(response, {sliding_config.header_name.lower()}) + + +@pytest.mark.filterwarnings("ignore::DeprecationWarning") +async def test_the_deprecated_middleware_leaves_a_tokenless_response_alone( + manager: SessionManager, config: SessionConfig +) -> None: + _session, token = await manager.create_session(user=SessionUser(user_id="u")) + client = TestClient(_app(manager, config, middleware=SessionMiddleware)) + + response = client.get("/public", headers={config.header_name: token}) + + assert config.header_name not in response.headers + assert response.headers["cache-control"] == _PUBLIC + assert "vary" not in response.headers + + +async def test_a_response_without_a_token_keeps_its_cache_control( + manager: SessionManager, config: SessionConfig +) -> None: + """A loaded session outside the renewal window sends no token.""" + _session, token = await manager.create_session(user=SessionUser(user_id="u")) + client = TestClient(_app(manager, config)) + client.cookies.set(config.cookie_name, token) + + response = client.get("/public") + + assert "set-cookie" not in response.headers + assert response.headers["cache-control"] == _PUBLIC + assert "vary" not in response.headers + + +def test_a_response_without_a_session_keeps_its_cache_control( + manager: SessionManager, config: SessionConfig +) -> None: + client = TestClient(_app(manager, config)) + + response = client.get("/public") + + assert "set-cookie" not in response.headers + assert response.headers["cache-control"] == _PUBLIC + assert "vary" not in response.headers From a2c4287b9a3dfa107d57de81b8e22fe85b0fd79f Mon Sep 17 00:00:00 2001 From: allen0099 Date: Sun, 27 Sep 2026 12:15:18 +0000 Subject: [PATCH 2/2] docs(changelog): add the #297 fragment --- changelog.d/297.security.md | 10 ++++++++++ 1 file changed, 10 insertions(+) create mode 100644 changelog.d/297.security.md diff --git a/changelog.d/297.security.md b/changelog.d/297.security.md new file mode 100644 index 0000000..06df7b3 --- /dev/null +++ b/changelog.d/297.security.md @@ -0,0 +1,10 @@ +**Responses that carry a session token are never cacheable.** When +`FastAPICacheXSessionMiddleware` sends a token (a new session, a sliding +renewal, a regenerated ID) or a cookie-clearing `Set-Cookie`, it now sets +`Cache-Control: private, no-store`, replacing whatever the route set, and adds +`Vary` for the token transport even when the handler never touched +`request.session`. Before, a `@cache(public=True)` route could return +`Cache-Control: public` with a valid session cookie, and a CDN or reverse proxy +could hand that cookie to the next visitors. The deprecated `SessionMiddleware` +does the same when it sends a token, and neither middleware repeats a `Vary` +name the response already has.