fix(session): keep responses that carry a session token out of shared caches - #303
Merged
Merged
Conversation
… 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
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
When
FastAPICacheXSessionMiddlewaresends a session token to the client (a new session, a sliding renewal, a regenerated ID) viaSet-Cookieor theX-Session-Tokenheader, nothing stops a shared cache from storing it.Varywas only added when the handler accessedrequest.session, andCache-Controlwas left as the route set it. On a@cache(ttl=60, public=True)route, a request that triggered sliding renewal gotCache-Control: public, max-age=60, noVary, andSet-Cookie: session=<valid token>, so a CDN or reverse proxy could hand that token to the next visitors. #168 fixed which header names go inVary, but it only covered responses where the session was accessed.Fix
fastapi_cachex/session/middleware.py:_persistnow returns whether it wrote a token or a clearing cookie. It covers every_emit_tokencall and both clear-cookie appends.send_wrappersetsCache-Control: private, no-store, replacing any existing value, and addsVaryfor the transport headers consulted (vary_on: the token header,Authorizationwhen bearer tokens are enabled, andCookiefor cookie transport), whether or not the session was accessed.SessionMiddleware.dispatchdoes the same when it sets the token response header._add_varyhelper skips names that are already present (compared case-insensitively) and leavesVary: *alone. Starlette'sMutableHeaders.add_vary_headerappends without checking, so a route that already sentVary: Cookiewould otherwise getCookietwice. The existingsession.accessedpath uses the helper too.Responses that carry no token keep their headers unchanged, so a
@cache(public=True)response that doesn't renew or create a session is still cacheable.The behaviour is documented in
docs/SESSION.mdandi18n/zh-TW/docs/SESSION.md(in therequest.sessionsection, next to the existingVarybullet).Tests
New file
tests/session/test_token_response_caching.py(15 tests):@cache(public=True)route over cookie, header and bearer transportSessionMiddlewarerenewalVaryvalues kept and not duplicatedVary: *left as it ispublic, max-age=60and get noVary: a loaded session outside the renewal window, no session at all, the deprecated middleware without renewal, and an emptied anonymous header sessionMutation check: with
middleware.pyreverted tomaster, 11 of the new tests fail. The 4 that pass are the no-token guards, and they are expected to pass onmastertoo. Targeted mutations:Varyonly when accessed: 6 failCache-Controloverride: 11 fail*check: 1 failsVaryin the deprecated middleware: 1 failsFull suite: 872 passed, 191 skipped (live Redis/Memcached). Coverage 93.71%;
session/middleware.pyis at 100%. ruff check, ruff format --check,mypy --strictand pre-commit all pass.CHANGELOG
The entry is in
changelog.d/297.security.mdand is merged intoCHANGELOG.mdat release time.Closes #297