Skip to content

Commit 717d4f4

Browse files
committed
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
1 parent 0b5b4d4 commit 717d4f4

4 files changed

Lines changed: 131 additions & 7 deletions

File tree

‎docs/SESSION.md‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -371,7 +371,10 @@ async def me(session=Depends(get_session)):
371371
replacing `Session.data` with the dict's contents.
372372
- Clearing it (`request.session.clear()`) on a session that had data deletes the backend session;
373373
a cookie client also receives a `Set-Cookie` that expires the cookie.
374-
- Any access to `request.session` adds `Vary: Cookie` to the response.
374+
- Any access to `request.session` adds `Vary` for every request header read to find the token:
375+
the headers checked in `token_source_priority` order (`header_name`, and `Authorization` when
376+
bearer tokens are enabled) up to the one that carried the token. `Cookie` is added only when
377+
no header carried a token, because only then is the cookie read.
375378

376379
The cookie is always `HttpOnly`; `Secure`, `SameSite`, `Domain`, `Path` and `Max-Age` follow the
377380
`cookie_*` settings.

‎fastapi_cachex/session/middleware.py‎

Lines changed: 41 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -99,18 +99,40 @@ def _extract_header_token(
9999
Returns:
100100
Session token or None
101101
"""
102+
return _read_header_token(connection, config)[0]
103+
104+
105+
def _read_header_token(
106+
connection: HTTPConnection, config: SessionConfig
107+
) -> tuple[str | None, list[str]]:
108+
"""Extract a session token and name the request headers that were read.
109+
110+
The response depends on every header read before the token was found, so
111+
those are the names it must ``Vary`` on.
112+
113+
Args:
114+
connection: Incoming HTTP connection
115+
config: Session configuration
116+
117+
Returns:
118+
``(token, consulted)``: the token or None, and the header names read,
119+
in the order they were checked
120+
"""
121+
consulted: list[str] = []
102122
# `token_source_priority` is a list of Literals, so pydantic has already
103123
# rejected anything that is neither branch; the chain stays an `elif` so a
104124
# source added later falls through instead of being read as a bearer token.
105125
for source in config.token_source_priority:
106126
if source == "header":
127+
consulted.append(config.header_name)
107128
token = connection.headers.get(config.header_name)
108129
if token:
109130
logger.debug("Token extracted from header")
110-
return token
131+
return token, consulted
111132

112133
elif source == "bearer":
113134
if config.use_bearer_token:
135+
consulted.append("Authorization")
114136
# The scheme name is case-insensitive (RFC 9110 §11.1) and is
115137
# followed by one or more spaces (RFC 6750 §2.1).
116138
scheme, _, token_value = connection.headers.get(
@@ -119,9 +141,9 @@ def _extract_header_token(
119141
token_value = token_value.lstrip(" ")
120142
if scheme.lower() == "bearer" and token_value:
121143
logger.debug("Token extracted from bearer auth")
122-
return token_value
144+
return token_value, consulted
123145

124-
return None
146+
return None, consulted
125147

126148

127149
def _stash_session_manager(app: Any, manager: SessionManager) -> None:
@@ -340,7 +362,7 @@ async def __call__(self, scope: Scope, receive: Receive, send: Send) -> None:
340362
# `header_token` is captured so the response is routed by transport: a
341363
# header-sourced token is echoed back via the response header, otherwise
342364
# via Set-Cookie (see send_wrapper).
343-
header_token = _extract_header_token(connection, self.config)
365+
header_token, vary_on = self._token_sources(connection)
344366
token_value = header_token or connection.cookies.get(self.config.cookie_name)
345367
if token_value:
346368
try:
@@ -381,7 +403,8 @@ async def send_wrapper(message: Message) -> None:
381403
)
382404

383405
if session.accessed:
384-
headers.add_vary_header("Cookie")
406+
for name in vary_on:
407+
headers.add_vary_header(name)
385408

386409
if session.modified and session:
387410
cookie_token, new_token = await self._write_session(
@@ -421,6 +444,19 @@ async def send_wrapper(message: Message) -> None:
421444

422445
await self.app(scope, receive, send_wrapper)
423446

447+
def _token_sources(
448+
self, connection: HTTPConnection
449+
) -> tuple[str | None, list[str]]:
450+
"""The header-carried token, if any, and the request headers to Vary on.
451+
452+
The response depends on every header read to find the token. The
453+
cookie is read only when no header carried one.
454+
"""
455+
header_token, vary_on = _read_header_token(connection, self.config)
456+
if header_token is None:
457+
vary_on.append("Cookie")
458+
return header_token, vary_on
459+
424460
def _response_tokens(
425461
self,
426462
backend_session: "Session | None",

‎i18n/zh-TW/docs/SESSION.md‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -328,7 +328,7 @@ async def me(session=Depends(get_session)):
328328
- 在沒有載入任何 Session 時寫入 `request.session`,會建立一個新的**匿名** Session(`SessionManager.create_anonymous_session()`,並依設定套用 IP / User-Agent 綁定),並透過該請求的傳輸方式傳回其權杖。
329329
- 修改已載入 Session 的 `request.session`,會透過 `update_session()` 將新內容儲存到後端,以 dict 的內容取代 `Session.data`。
330330
- 在原本有資料的 Session 上清除它(`request.session.clear()`),會刪除後端的 Session;Cookie 用戶端還會收到一個使 Cookie 過期的 `Set-Cookie`。
331-
- 只要存取 `request.session`,就會在回應中加入 `Vary: Cookie`。
331+
- 只要存取 `request.session`,就會為了尋找權杖而讀取過的每個請求標頭加入 `Vary`:依 `token_source_priority` 順序檢查的標頭(`header_name`,以及啟用 Bearer 權杖時的 `Authorization`),直到攜帶權杖的那一個為止。只有在沒有任何標頭攜帶權杖時才會讀取 Cookie,因此也只有這時才會加入 `Cookie`。
332332

333333
Cookie 一律為 `HttpOnly`;`Secure`、`SameSite`、`Domain`、`Path` 與 `Max-Age` 則依 `cookie_*` 設定。
334334

‎tests/session/test_starlette_middleware.py‎

Lines changed: 85 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -836,3 +836,88 @@ async def test_deprecated_middleware_sends_regenerated_token(
836836
await manager.get_session(new_token)
837837
with pytest.raises(SessionNotFoundError):
838838
await manager.get_session(old_token)
839+
840+
841+
def _vary(response: Any) -> set[str]:
842+
"""The response's Vary header as a set of lowercased header names."""
843+
return {
844+
name.strip().lower()
845+
for name in response.headers.get("vary", "").split(",")
846+
if name.strip()
847+
}
848+
849+
850+
def _session_reading_app(manager: SessionManager, config: SessionConfig) -> FastAPI:
851+
app = FastAPI()
852+
app.add_middleware(
853+
FastAPICacheXSessionMiddleware, session_manager=manager, config=config
854+
)
855+
856+
@app.get("/read")
857+
async def read_route(request: Request) -> dict[str, Any]:
858+
return dict(request.session)
859+
860+
@app.get("/untouched")
861+
async def untouched_route() -> dict[str, bool]:
862+
return {"ok": True}
863+
864+
return app
865+
866+
867+
@pytest.mark.asyncio
868+
async def test_header_token_varies_on_the_token_header_not_cookie(
869+
manager: SessionManager, config: SessionConfig
870+
) -> None:
871+
"""The cookie is never read when the header carries the token (#168)."""
872+
_session, token = await manager.create_session(user=SessionUser(user_id="u"))
873+
client = TestClient(_session_reading_app(manager, config))
874+
875+
response = client.get("/read", headers={config.header_name: token})
876+
877+
assert _vary(response) == {config.header_name.lower()}
878+
879+
880+
@pytest.mark.asyncio
881+
async def test_bearer_token_varies_on_every_header_consulted(
882+
manager: SessionManager, config: SessionConfig
883+
) -> None:
884+
"""The custom header is checked first, so the response depends on it too."""
885+
_session, token = await manager.create_session(user=SessionUser(user_id="u"))
886+
client = TestClient(_session_reading_app(manager, config))
887+
888+
response = client.get("/read", headers={"Authorization": f"Bearer {token}"})
889+
890+
assert _vary(response) == {config.header_name.lower(), "authorization"}
891+
892+
893+
@pytest.mark.asyncio
894+
async def test_cookie_token_varies_on_cookie_and_the_headers_checked_first(
895+
manager: SessionManager, config: SessionConfig
896+
) -> None:
897+
"""A token header, had one been sent, would have won over the cookie."""
898+
_session, token = await manager.create_session(user=SessionUser(user_id="u"))
899+
client = TestClient(_session_reading_app(manager, config))
900+
client.cookies.set(config.cookie_name, token)
901+
902+
response = client.get("/read")
903+
904+
assert _vary(response) == {config.header_name.lower(), "authorization", "cookie"}
905+
906+
907+
def test_disabled_bearer_transport_is_left_out_of_vary(manager: SessionManager) -> None:
908+
config = SessionConfig(secret_key="a" * 32, use_bearer_token=False)
909+
client = TestClient(_session_reading_app(manager, config))
910+
911+
response = client.get("/read")
912+
913+
assert _vary(response) == {config.header_name.lower(), "cookie"}
914+
915+
916+
def test_no_vary_when_the_session_is_not_accessed(
917+
manager: SessionManager, config: SessionConfig
918+
) -> None:
919+
client = TestClient(_session_reading_app(manager, config))
920+
921+
response = client.get("/untouched")
922+
923+
assert "vary" not in response.headers

0 commit comments

Comments
 (0)