fix(session): vary on the headers read to find the session token - #209
Merged
Merged
Conversation
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
20 tasks
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.
Closes #168
Problem
FastAPICacheXSessionMiddlewareaddedVary: Cookiewhenever a handler touchedrequest.session, whatever transport carried the token. A request authenticated withX-Session-TokenorAuthorization: Bearernever reads the cookie. Its response depends on those headers, so a shared cache that honoursVarywould key it onCookiealone. Repro: an app that readsrequest.session, requested with onlyX-Session-Token: <token>, returnsVary: Cookie, and nothing forX-Session-Token.Fix
_read_header_token(connection, config)returns the token and the header names it read, intoken_source_priorityorder, stopping at the one that carried the token._extract_header_tokenkeeps its signature and delegates to it.Varyfor each of those headers. It addsCookieonly when no header carried a token, because only then is the cookie read.Resulting
Varywith the default config (token_source_priority=["header", "bearer"]):VaryX-Session-TokenX-Session-TokenAuthorization: BearerX-Session-Token, AuthorizationX-Session-Token, Authorization, CookieThe headers checked before the cookie are listed on cookie responses too: had the client sent one, it would have won over the cookie, so the response depends on its absence. When
use_bearer_token=False,Authorizationis left out. As before, nothing is added unless the handler accessedrequest.session.The deprecated
SessionMiddlewarehas never addedVary; this PR leaves it unchanged.Tests
New tests in
tests/session/test_starlette_middleware.py:Varyset;Varywhen the session is not accessed (guards existing behaviour).With
middleware.pyrestored from master, exactly the four transport tests fail; they all get{'cookie'}.ruff,
mypy --strict, and the full suite against live Redis and Memcached (CACHEX_REQUIRE_LIVE_SERVERS=1) pass: 866 passed, 100% coverage.docs/SESSION.mdand its zh-TW version describe the new rule.CHANGELOG entry
Section: Fixed (to be added via #206)