fix(session): send the new token after regenerate_session_id - #137
Merged
Merged
Conversation
FastAPICacheXSessionMiddleware re-sent the loaded token after a handler regenerated the request's session ID, so the cookie named a deleted record and the user was logged out; header clients got no token. The deprecated SessionMiddleware could overwrite the new token with a renewed one for the old ID. Both now compare the session ID with the loaded one and issue a token for the new ID. Token signing moves into SessionManager.issue_token(). Closes #103
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 #103.
Problem
docs/SESSION.mdrecommendsregenerate_session_id()after login to prevent session fixation. That call gives the loadedSessiona new ID and deletes the old record. Neither middleware noticed:FastAPICacheXSessionMiddleware:_write_session()returned the loaded, old token.SessionMiddleware: when sliding expiration had renewed the loaded token, the handler's new token in the response header was overwritten with the renewed token for the old ID.Change
SessionManager.issue_token(session) -> str: a new public helper that signs a token for the session's current ID and expiry.create_session, sliding renewal andregenerate_session_idnow all use it instead of repeating_create_token+to_string.FastAPICacheXSessionMiddleware:http.response.start,_response_tokens()compares it with the backend session's current ID. If the ID changed, the current and fresh tokens both becomeissue_token(session)._write_session, untouched data → the "fresh token" branch that sliding renewal already used.SessionMiddleware: the same comparison. A regenerated session getsissue_token(session)in the response header, ahead of any renewed token.regenerate_session_iddocstring: it now says the session is changed in place and that the middleware delivers the token.docs/SESSION.md"Regenerate the Session ID After Login":SessionDep+SessionManagerDep).ip_address/user_agent; without themget_sessionraises when bindings are on.Tests
In
tests/session/test_starlette_middleware.py, each case is parametrized over whether the handler also writes torequest.session, and whether the load triggered sliding renewal:Set-Cookiecarries a token that resolves to the session with its data, and the old token raisesSessionNotFoundError.SessionMiddleware: the header carries the new ID's token, with and without renewal.All 10 cases fail against the old code.
Checks
fastapi_cachex,testsandscriptstestsandscriptsCACHEX_REQUIRE_LIVE_SERVERS=1): 754 passed, 100% coveragezensical build --strict