diff --git a/changelog.d/362.security.md b/changelog.d/362.security.md new file mode 100644 index 0000000..7ace93a --- /dev/null +++ b/changelog.d/362.security.md @@ -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. diff --git a/docs/CACHE_FLOW.md b/docs/CACHE_FLOW.md index 105b4ca..518b0ae 100644 --- a/docs/CACHE_FLOW.md +++ b/docs/CACHE_FLOW.md @@ -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 diff --git a/docs/HTTP_CACHING.md b/docs/HTTP_CACHING.md index 9fcea31..c52da3e 100644 --- a/docs/HTTP_CACHING.md +++ b/docs/HTTP_CACHING.md @@ -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 @@ -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, diff --git a/fastapi_cachex/cache.py b/fastapi_cachex/cache.py index 99d90f0..7d0009e 100644 --- a/fastapi_cachex/cache.py +++ b/fastapi_cachex/cache.py @@ -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 @@ -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 @@ -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 diff --git a/i18n/zh-TW/docs/CACHE_FLOW.md b/i18n/zh-TW/docs/CACHE_FLOW.md index 0487734..95d7c3a 100644 --- a/i18n/zh-TW/docs/CACHE_FLOW.md +++ b/i18n/zh-TW/docs/CACHE_FLOW.md @@ -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 diff --git a/i18n/zh-TW/docs/HTTP_CACHING.md b/i18n/zh-TW/docs/HTTP_CACHING.md index fb58f07..feab9f6 100644 --- a/i18n/zh-TW/docs/HTTP_CACHING.md +++ b/i18n/zh-TW/docs/HTTP_CACHING.md @@ -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` 送出並保留其他指令，讓下游的共用快取也不會儲存它。 diff --git a/tests/test_cache_bypass_private.py b/tests/test_cache_bypass_private.py new file mode 100644 index 0000000..8af9cbc --- /dev/null +++ b/tests/test_cache_bypass_private.py @@ -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() == []