From 494ee694454ad921243e92cb9714cf029598ab2b Mon Sep 17 00:00:00 2001 From: allen0099 Date: Sat, 26 Sep 2026 13:32:33 +0000 Subject: [PATCH] fix(session): vary on the headers read to find the session token FastAPICacheXSessionMiddleware added Vary: Cookie whenever a handler touched request.session, even when the token came in X-Session-Token or Authorization. Those requests never read the cookie, and a shared cache keyed on Cookie could hand one header client's response to another. Vary on every request header read to find the token, in token_source_priority order, and add Cookie only when no header carried one, since only then is the cookie read. Closes #168 --- docs/SESSION.md | 5 +- fastapi_cachex/session/middleware.py | 46 ++++++++++-- i18n/zh-TW/docs/SESSION.md | 2 +- tests/session/test_starlette_middleware.py | 85 ++++++++++++++++++++++ 4 files changed, 131 insertions(+), 7 deletions(-) diff --git a/docs/SESSION.md b/docs/SESSION.md index 7a1e8ba..8d76bdd 100644 --- a/docs/SESSION.md +++ b/docs/SESSION.md @@ -366,7 +366,10 @@ async def me(session=Depends(get_session)): replacing `Session.data` with the dict's contents. - Clearing it (`request.session.clear()`) on a session that had data deletes the backend session; a cookie client also receives a `Set-Cookie` that expires the cookie. -- Any access to `request.session` adds `Vary: Cookie` to the response. +- Any access to `request.session` adds `Vary` for every request header read to find the token: + 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. 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 bcbd4d5..efc9208 100644 --- a/fastapi_cachex/session/middleware.py +++ b/fastapi_cachex/session/middleware.py @@ -99,18 +99,40 @@ def _extract_header_token( Returns: Session token or None """ + return _read_header_token(connection, config)[0] + + +def _read_header_token( + connection: HTTPConnection, config: SessionConfig +) -> tuple[str | None, list[str]]: + """Extract a session token and name the request headers that were read. + + The response depends on every header read before the token was found, so + those are the names it must ``Vary`` on. + + Args: + connection: Incoming HTTP connection + config: Session configuration + + Returns: + ``(token, consulted)``: the token or None, and the header names read, + in the order they were checked + """ + consulted: list[str] = [] # `token_source_priority` is a list of Literals, so pydantic has already # rejected anything that is neither branch; the chain stays an `elif` so a # source added later falls through instead of being read as a bearer token. for source in config.token_source_priority: if source == "header": + consulted.append(config.header_name) token = connection.headers.get(config.header_name) if token: logger.debug("Token extracted from header") - return token + return token, consulted elif source == "bearer": if config.use_bearer_token: + consulted.append("Authorization") # The scheme name is case-insensitive (RFC 9110 §11.1) and is # followed by one or more spaces (RFC 6750 §2.1). scheme, _, token_value = connection.headers.get( @@ -119,9 +141,9 @@ def _extract_header_token( token_value = token_value.lstrip(" ") if scheme.lower() == "bearer" and token_value: logger.debug("Token extracted from bearer auth") - return token_value + return token_value, consulted - return None + return None, consulted def _stash_session_manager(app: Any, manager: SessionManager) -> None: @@ -340,7 +362,7 @@ async def __call__(self, scope: Scope, receive: Receive, send: Send) -> None: # `header_token` is captured so the response is routed by transport: a # header-sourced token is echoed back via the response header, otherwise # via Set-Cookie (see send_wrapper). - header_token = _extract_header_token(connection, self.config) + header_token, vary_on = self._token_sources(connection) token_value = header_token or connection.cookies.get(self.config.cookie_name) if token_value: try: @@ -381,7 +403,8 @@ async def send_wrapper(message: Message) -> None: ) if session.accessed: - headers.add_vary_header("Cookie") + for name in vary_on: + headers.add_vary_header(name) if session.modified and session: cookie_token, new_token = await self._write_session( @@ -421,6 +444,19 @@ async def send_wrapper(message: Message) -> None: await self.app(scope, receive, send_wrapper) + def _token_sources( + self, connection: HTTPConnection + ) -> tuple[str | None, list[str]]: + """The header-carried token, if any, and the request headers to Vary on. + + The response depends on every header read to find the token. The + cookie is read only when no header carried one. + """ + header_token, vary_on = _read_header_token(connection, self.config) + if header_token is None: + vary_on.append("Cookie") + return header_token, vary_on + def _response_tokens( self, backend_session: "Session | None", diff --git a/i18n/zh-TW/docs/SESSION.md b/i18n/zh-TW/docs/SESSION.md index b9bc94e..05dd0a6 100644 --- a/i18n/zh-TW/docs/SESSION.md +++ b/i18n/zh-TW/docs/SESSION.md @@ -326,7 +326,7 @@ async def me(session=Depends(get_session)): - 在沒有載入任何 Session 時寫入 `request.session`，會建立一個新的**匿名** Session（`SessionManager.create_anonymous_session()`，並依設定套用 IP / User-Agent 綁定），並透過該請求的傳輸方式傳回其權杖。 - 修改已載入 Session 的 `request.session`，會透過 `update_session()` 將新內容儲存到後端，以 dict 的內容取代 `Session.data`。 - 在原本有資料的 Session 上清除它（`request.session.clear()`），會刪除後端的 Session；Cookie 用戶端還會收到一個使 Cookie 過期的 `Set-Cookie`。 -- 只要存取 `request.session`，就會在回應中加入 `Vary: Cookie`。 +- 只要存取 `request.session`，就會為了尋找權杖而讀取過的每個請求標頭加入 `Vary`：依 `token_source_priority` 順序檢查的標頭（`header_name`，以及啟用 Bearer 權杖時的 `Authorization`），直到攜帶權杖的那一個為止。只有在沒有任何標頭攜帶權杖時才會讀取 Cookie，因此也只有這時才會加入 `Cookie`。 Cookie 一律為 `HttpOnly`；`Secure`、`SameSite`、`Domain`、`Path` 與 `Max-Age` 則依 `cookie_*` 設定。 diff --git a/tests/session/test_starlette_middleware.py b/tests/session/test_starlette_middleware.py index 030150d..e006e3e 100644 --- a/tests/session/test_starlette_middleware.py +++ b/tests/session/test_starlette_middleware.py @@ -836,3 +836,88 @@ async def test_deprecated_middleware_sends_regenerated_token( await manager.get_session(new_token) with pytest.raises(SessionNotFoundError): await manager.get_session(old_token) + + +def _vary(response: Any) -> set[str]: + """The response's Vary header as a set of lowercased header names.""" + return { + name.strip().lower() + for name in response.headers.get("vary", "").split(",") + if name.strip() + } + + +def _session_reading_app(manager: SessionManager, config: SessionConfig) -> FastAPI: + app = FastAPI() + app.add_middleware( + FastAPICacheXSessionMiddleware, session_manager=manager, config=config + ) + + @app.get("/read") + async def read_route(request: Request) -> dict[str, Any]: + return dict(request.session) + + @app.get("/untouched") + async def untouched_route() -> dict[str, bool]: + return {"ok": True} + + return app + + +@pytest.mark.asyncio +async def test_header_token_varies_on_the_token_header_not_cookie( + manager: SessionManager, config: SessionConfig +) -> None: + """The cookie is never read when the header carries the token (#168).""" + _session, token = await manager.create_session(user=SessionUser(user_id="u")) + client = TestClient(_session_reading_app(manager, config)) + + response = client.get("/read", headers={config.header_name: token}) + + assert _vary(response) == {config.header_name.lower()} + + +@pytest.mark.asyncio +async def test_bearer_token_varies_on_every_header_consulted( + manager: SessionManager, config: SessionConfig +) -> None: + """The custom header is checked first, so the response depends on it too.""" + _session, token = await manager.create_session(user=SessionUser(user_id="u")) + client = TestClient(_session_reading_app(manager, config)) + + response = client.get("/read", headers={"Authorization": f"Bearer {token}"}) + + assert _vary(response) == {config.header_name.lower(), "authorization"} + + +@pytest.mark.asyncio +async def test_cookie_token_varies_on_cookie_and_the_headers_checked_first( + manager: SessionManager, config: SessionConfig +) -> None: + """A token header, had one been sent, would have won over the cookie.""" + _session, token = await manager.create_session(user=SessionUser(user_id="u")) + client = TestClient(_session_reading_app(manager, config)) + client.cookies.set(config.cookie_name, token) + + response = client.get("/read") + + assert _vary(response) == {config.header_name.lower(), "authorization", "cookie"} + + +def test_disabled_bearer_transport_is_left_out_of_vary(manager: SessionManager) -> None: + config = SessionConfig(secret_key="a" * 32, use_bearer_token=False) + client = TestClient(_session_reading_app(manager, config)) + + response = client.get("/read") + + assert _vary(response) == {config.header_name.lower(), "cookie"} + + +def test_no_vary_when_the_session_is_not_accessed( + manager: SessionManager, config: SessionConfig +) -> None: + client = TestClient(_session_reading_app(manager, config)) + + response = client.get("/untouched") + + assert "vary" not in response.headers