fix(session): make request.session.clear() log out regardless of data - #258
Merged
Merged
Conversation
clear() on a loaded session now always deletes it, even when its data was already empty, and keys written after clear() start a new anonymous session. Removing the last key with del/pop() no longer logs a user out: a session with a user is saved with empty data, an anonymous one is still deleted. Closes #227
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 #227.
Problem
FastAPICacheXSessionMiddlewaredecided what an emptiedrequest.sessionmeant from whether the loaded data started out empty. That caused two bugs:clear()did not log out a session with empty data (the issue). A user session created withcreate_session(user)and no data hasdata == {}, sorequest.session.clear()changed nothing observable. The session stayed alive and the user stayed logged in.request.session.pop("flash")on a user session whose only key was a flash message took the delete branch and destroyed the whole session.Fix
request.sessionis now a smallSessionsubclass that records an explicitclear().clear()on a loaded session is a logout.Set-Cookie.clear()in the same request go into a new anonymous session under a new ID, never the logged-out one.del/pop()is not a logout.The response handling moved into a
_persist()helper so that__call__stays under ruff's statement limit.Tests
New tests in
tests/session/test_starlette_middleware.py:test_clear_logs_out_a_session_with_empty_data[cookie|header]test_popping_the_last_key_keeps_a_user_logged_intest_popping_the_last_key_deletes_an_anonymous_session(parity guard, passes before and after)test_writing_after_clear_starts_a_new_anonymous_session[cookie|header]Mutation check: with the old middleware, exactly the five new behaviour tests fail and every existing test passes. The full suite passes against live Redis and Memcached (932 passed, 99.96% coverage). Both docs builds pass with
--strict.Docs
request.sessionbullets:clear()is a logout, anddel/pop()is not.### Securityentry.