Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 9 additions & 0 deletions changelog.d/362.security.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
**`@cache` routes without a positive `ttl` now send `private` to requests with
credentials.** A route with `ttl=0` or no `ttl` skips the backend anyway, so it
did not check for `Authorization` or a session, and it answered them with its
plain `Cache-Control`: `@cache(ttl=0, must_revalidate=True)` sent `max-age=0,
must-revalidate`, which RFC 9111 §3.5 lets a shared cache such as a CDN store
and reuse for other users after revalidation. Such a response now gets `private`
like on every other route (`private, max-age=0, must-revalidate`), unless the
route is `public=True` or `cache_authorized=True`. No bypass warning is logged
for these routes, since they never read the backend.
2 changes: 1 addition & 1 deletion docs/CACHE_FLOW.md
Original file line number Diff line number Diff line change
Expand Up @@ -218,7 +218,7 @@ if no_store:

bypass = private or not ttl
# Authorization header, a session the middleware loaded, or non-empty request.session
credential = None if bypass or public or cache_authorized else request_credential(request)
credential = None if private or public or cache_authorized else request_credential(request)
if bypass or credential:
response, etag = await render() # backend neither read nor written
return not_modified(...) if etag_matches(client_etag, etag) else response
Expand Down
9 changes: 5 additions & 4 deletions docs/HTTP_CACHING.md
Original file line number Diff line number Diff line change
Expand Up @@ -139,9 +139,10 @@ A response that belongs to one caller is never stored either (#296):
caller's identity (see [Authenticated endpoints](#authenticated-endpoints)).
`must_revalidate=True` does not lift the bypass: RFC 9111 would let a shared
cache reuse such a response under `must-revalidate`, but the library
requires an explicit opt-in. Routes that skip the backend anyway
(`private=True`, or no positive `ttl`) do not check for credentials and
send their own `Cache-Control` unchanged.
requires an explicit opt-in. A route without a positive `ttl` skips the
backend anyway, but its response to such a request still gets `private`
(before 0.3.9 it was sent without it, #362); `private=True` routes send it
already.
A request has a session when `FastAPICacheXSessionMiddleware` (or the
deprecated `SessionMiddleware`) loaded one for it, from the token header, a
bearer token or the session cookie, with or without a user, or when
Expand All @@ -150,7 +151,7 @@ A response that belongs to one caller is never stored either (#296):
count, so it cannot be used to skip the cache. Before 0.3.9 only
`Authorization` did, and a plain `@cache` on a route that read the session
served one visitor's response to the next (#319). The first bypass on each
route is logged at `WARNING` (see
route that reads the backend is logged at `WARNING` (see
[Requests with credentials](#requests-with-credentials)).
- **The handler's own `Cache-Control` contains `private` or `no-store`**
(as whole directives, in any case). The response is served but not stored,
Expand Down
20 changes: 13 additions & 7 deletions fastapi_cachex/cache.py
Original file line number Diff line number Diff line change
Expand Up @@ -1020,10 +1020,11 @@ def cache(
backend as ``private=True`` does (RFC 9111 §3.5), unless
``public`` is set, and its response is sent with ``private``.
A token that does not resolve to a session does not count.
Routes that skip the backend anyway (``private=True``, or no
positive ``ttl``) do not check for credentials, so their
``Cache-Control`` is sent unchanged.
The first such bypass is logged at ``WARNING`` once per route
A route without a positive ``ttl`` skips the backend anyway, but
its response to such a request is still sent with ``private``;
``private=True`` routes send it already.
On a route that reads the backend, the first such bypass is
logged at ``WARNING`` once per route
and credential kind (the route template and the kind, never the
value), since it otherwise leaves the route with no cache hits.
RFC 9111 would also allow reuse under ``must-revalidate``, but
Expand Down Expand Up @@ -1223,10 +1224,12 @@ async def serve(*args: Any, **kwargs: Any) -> Response:
# default key carries no identity, so treat such requests as
# private unless the route is `public` or opted in. A request that
# arrived with a session is the same case, whichever transport
# carried its token (#319).
# carried its token (#319). Routes without a positive ttl skip the
# backend anyway, but their response still needs `private` for a
# downstream cache (#362); only `private=True` already sends it.
credential = (
None
if bypass_backend or public or cache_authorized
if private or public or cache_authorized
else _request_credential(req)
)
authorized_bypass = credential is not None
Expand All @@ -1236,7 +1239,10 @@ async def serve(*args: Any, **kwargs: Any) -> Response:
credential,
req.url.path,
)
warn_bypass(req, credential)
# The warning is about lost cache hits, which a route that
# never reads the backend does not have.
if not bypass_backend:
warn_bypass(req, credential)

# A private response belongs to exactly one user, so it must never
# be read from or written to the shared backend — the default cache
Expand Down
2 changes: 1 addition & 1 deletion i18n/zh-TW/docs/CACHE_FLOW.md
Original file line number Diff line number Diff line change
Expand Up @@ -166,7 +166,7 @@ if no_store:

bypass = private or not ttl
# Authorization 標頭、中介軟體載入的 Session,或不是空的 request.session
credential = None if bypass or public or cache_authorized else request_credential(request)
credential = None if private or public or cache_authorized else request_credential(request)
if bypass or credential:
response, etag = await render() # 既不讀取也不寫入後端
return not_modified(...) if etag_matches(client_etag, etag) else response
Expand Down
2 changes: 1 addition & 1 deletion i18n/zh-TW/docs/HTTP_CACHING.md
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,7 @@ GET /items → 200, Cache-Control: max-age=60, Age: 42(儲存後 42 秒送出

屬於單一呼叫者的回應同樣不會被儲存(#296):

- **請求帶有 `Authorization` 或 Session。** 依照 RFC 9111 §3.5 對共用快取的要求,這類請求會像 `private=True` 一樣繞過後端:不讀取也不寫入,handler 照常執行,`If-None-Match` 與新產生的回應比對。回應(以及 304)會以 `private` 取代 `public` 送出,並保留裝飾器的其他指令(`no_cache` 路由則為 `private, no-cache`),讓 CDN 或代理也不會儲存它。`public=True` 的路由不受此限,設定 `cache_authorized=True` 的路由也一樣;後者是給包含呼叫者身分的 key builder 使用的明確選項(見[需驗證身分的端點](#authenticated-endpoints))。`must_revalidate=True` 不會解除繞過:RFC 9111 允許共用快取在 `must-revalidate` 下重複使用這類回應,但本函式庫要求明確選擇啟用。本來就不經過後端的路由(`private=True`,或沒有正數的 `ttl`)不會檢查憑證,會原樣送出自己的 `Cache-Control`。請求「帶有 Session」是指 `FastAPICacheXSessionMiddleware`(或已棄用的 `SessionMiddleware`)為它載入了 Session(權杖來自標頭、Bearer 權杖或 Session Cookie 皆可,有沒有使用者都算),或在任何 Session 中介軟體(包括 Starlette 的)下 `request.session` 不是空的。解析不出 Session 的權杖(偽造、過期)不算,因此無法用來略過快取。0.3.9 以前只有 `Authorization` 會觸發繞過,讀取 Session 的路由只加上 `@cache` 時,會把一位訪客的回應提供給下一位(#319)。每個路由第一次繞過時會以 `WARNING` 等級記錄(見[帶有憑證的請求](#requests-with-credentials))。
- **請求帶有 `Authorization` 或 Session。** 依照 RFC 9111 §3.5 對共用快取的要求,這類請求會像 `private=True` 一樣繞過後端:不讀取也不寫入,handler 照常執行,`If-None-Match` 與新產生的回應比對。回應(以及 304)會以 `private` 取代 `public` 送出,並保留裝飾器的其他指令(`no_cache` 路由則為 `private, no-cache`),讓 CDN 或代理也不會儲存它。`public=True` 的路由不受此限,設定 `cache_authorized=True` 的路由也一樣;後者是給包含呼叫者身分的 key builder 使用的明確選項(見[需驗證身分的端點](#authenticated-endpoints))。`must_revalidate=True` 不會解除繞過:RFC 9111 允許共用快取在 `must-revalidate` 下重複使用這類回應,但本函式庫要求明確選擇啟用。沒有正數 `ttl` 的路由本來就不經過後端,但它對這類請求的回應仍會加上 `private`(0.3.9 以前不會加,#362);`private=True` 的路由本來就會送出 `private`。請求「帶有 Session」是指 `FastAPICacheXSessionMiddleware`(或已棄用的 `SessionMiddleware`)為它載入了 Session(權杖來自標頭、Bearer 權杖或 Session Cookie 皆可,有沒有使用者都算),或在任何 Session 中介軟體(包括 Starlette 的)下 `request.session` 不是空的。解析不出 Session 的權杖(偽造、過期)不算,因此無法用來略過快取。0.3.9 以前只有 `Authorization` 會觸發繞過,讀取 Session 的路由只加上 `@cache` 時,會把一位訪客的回應提供給下一位(#319)。會讀取後端的路由第一次繞過時,會以 `WARNING` 等級記錄(見[帶有憑證的請求](#requests-with-credentials))。
- **handler 自己的 `Cache-Control` 含有 `private` 或 `no-store`**(完整指令,不分大小寫)。回應照常送出但不儲存,而且 handler 的標頭會原樣送出,不會被裝飾器的標頭取代。
- **回應設定了 cookie。** 回應照常送出(包含 `Set-Cookie`),但不儲存;它(以及 304)會以 `private` 取代 `public` 送出並保留其他指令,讓下游的共用快取也不會儲存它。

Expand Down
124 changes: 124 additions & 0 deletions tests/test_cache_bypass_private.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,124 @@
"""A route without a positive ttl still marks a credentialed response private (#362).

Such routes skip the backend anyway, so the credential check was skipped with
it and the response kept the decorator's header: `@cache(ttl=0,
must_revalidate=True)` answered an `Authorization` request with `max-age=0,
must-revalidate`, which RFC 9111 §3.5 lets a shared cache store.
"""

import logging

import pytest
from fastapi import FastAPI
from fastapi import Request
from fastapi.testclient import TestClient
from starlette.middleware.sessions import (
SessionMiddleware as StarletteSessionMiddleware,
)

from fastapi_cachex import cache
from fastapi_cachex.backends import MemoryBackend
from fastapi_cachex.proxy import BackendProxy

_AUTH = {"Authorization": "Bearer alice"}


def _client(**cache_kwargs: object) -> TestClient:
app = FastAPI()

@app.get("/me")
@cache(**cache_kwargs) # type: ignore[arg-type]
async def me() -> dict[str, str]:
return {"me": "x"}

return TestClient(app)


@pytest.mark.parametrize(
("cache_kwargs", "expected"),
[
({"ttl": 0, "must_revalidate": True}, "private, max-age=0, must-revalidate"),
({"ttl": 0}, "private, max-age=0"),
({"must_revalidate": True}, "private, must-revalidate"),
({}, "private"),
({"ttl": 0, "no_cache": True}, "private, no-cache"),
],
)
def test_authorized_request_gets_private(
cache_kwargs: dict[str, object], expected: str
) -> None:
response = _client(**cache_kwargs).get("/me", headers=_AUTH)

assert response.status_code == 200
assert response.headers["cache-control"] == expected


def test_not_modified_is_private_too() -> None:
client = _client(ttl=0, must_revalidate=True)
etag = client.get("/me", headers=_AUTH).headers["etag"]

response = client.get("/me", headers={**_AUTH, "If-None-Match": etag})

assert response.status_code == 304
assert response.headers["cache-control"] == "private, max-age=0, must-revalidate"


def test_request_without_credentials_is_unchanged() -> None:
response = _client(ttl=0, must_revalidate=True).get("/me")

assert response.headers["cache-control"] == "max-age=0, must-revalidate"


@pytest.mark.parametrize(
("cache_kwargs", "expected"),
[
({"ttl": 0, "public": True}, "public, max-age=0"),
({"ttl": 0, "cache_authorized": True}, "max-age=0"),
({"ttl": 0, "private": True}, "private, max-age=0"),
],
)
def test_public_opted_in_and_private_routes_are_unchanged(
cache_kwargs: dict[str, object], expected: str
) -> None:
response = _client(**cache_kwargs).get("/me", headers=_AUTH)

assert response.headers["cache-control"] == expected


def test_session_request_gets_private() -> None:
app = FastAPI()
app.add_middleware(StarletteSessionMiddleware, secret_key="x" * 32)

@app.get("/cart")
@cache(ttl=0, must_revalidate=True)
async def cart(request: Request) -> dict[str, list[str]]:
return {"cart": request.session.get("cart", [])}

@app.get("/add")
async def add(request: Request) -> dict[str, bool]:
request.session["cart"] = ["apple"]
return {"ok": True}

client = TestClient(app)
assert client.get("/cart").headers["cache-control"] == "max-age=0, must-revalidate"
client.get("/add")

response = client.get("/cart")

assert response.json() == {"cart": ["apple"]}
assert response.headers["cache-control"] == "private, max-age=0, must-revalidate"


async def test_backend_stays_untouched_and_nothing_warns(
caplog: pytest.LogCaptureFixture,
) -> None:
"""The route never had cache hits to lose, so the bypass warning stays quiet."""
backend = BackendProxy.get()
assert isinstance(backend, MemoryBackend)
client = _client(ttl=0, must_revalidate=True)

with caplog.at_level(logging.WARNING, logger="fastapi_cachex.cache"):
client.get("/me", headers=_AUTH)

assert [r for r in caplog.records if r.levelno >= logging.WARNING] == []
assert await backend.get_all_keys() == []
Loading