From a073578e3cfc208705e84ce37c4241cbdad84234 Mon Sep 17 00:00:00 2001 From: allen0099 Date: Sat, 26 Sep 2026 21:37:46 +0000 Subject: [PATCH] fix(session): make request.session.clear() log out regardless of data 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 --- CHANGELOG.md | 9 ++ CLAUDE.md | 1 + docs/SESSION.md | 8 +- fastapi_cachex/session/middleware.py | 119 +++++++++++++------- i18n/zh-TW/docs/SESSION.md | 3 +- tests/session/test_starlette_middleware.py | 122 +++++++++++++++++++++ 6 files changed, 220 insertions(+), 42 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 54b84fa..82f84e3 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -202,6 +202,15 @@ Note that 0.3.3 was never released; 0.3.4 follows 0.3.2. login CSRF although STATE.md described the states as CSRF protection. The quick start now sets and checks a binding cookie. ([#226](https://github.com/allen0099/FastAPI-CacheX/issues/226)) +- **`request.session.clear()` logs out a session whose data was empty.** With + `FastAPICacheXSessionMiddleware`, whether the data started out empty decided + what a cleared session meant, so a user session created without data + survived `clear()` and stayed logged in. `clear()` on a loaded session now + always deletes it; keys written after `clear()` go into a new anonymous + session. The same rule logged a user out when the last key was removed with + `del` or `pop()`, e.g. a flash message; such a session is now saved with + empty data. An emptied anonymous session is still deleted. + ([#227](https://github.com/allen0099/FastAPI-CacheX/issues/227)) ## [0.3.7] - 2026-09-25 diff --git a/CLAUDE.md b/CLAUDE.md index cadd4e3..ae357b7 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -71,6 +71,7 @@ The library has four independent subsystems: - `SessionManagerProxy` mirrors the `BackendProxy` pattern for managing the `SessionManager` singleton. - Key FastAPI dependencies: `get_session`, `require_session`, `get_optional_session` (in `session/dependencies.py`). - `rotate_session_id(request)` (same module) regenerates the loaded session's ID at login against session fixation; a no-op when none was loaded. The middleware notices the changed ID and sends the new token. +- `FastAPICacheXSessionMiddleware` wraps `request.session` in `_RequestSession`, which records an explicit `clear()`: that deletes the loaded session (logout) even with empty data, and later writes start a new anonymous one. Emptying via `del`/`pop()` keeps a user session (saved empty) and deletes an anonymous one. **4. State Management (`fastapi_cachex/state/`)** - `StateManager` provides one-time-use state tokens for OAuth flows. States are consumed (deleted) on first successful `consume_state()` call. diff --git a/docs/SESSION.md b/docs/SESSION.md index e14d445..8427adf 100644 --- a/docs/SESSION.md +++ b/docs/SESSION.md @@ -369,8 +369,12 @@ async def me(session=Depends(get_session)): configured) and sends its token back through the request's transport. - Modifying it on a loaded session saves the new contents to the backend via `update_session()`, replacing `Session.data` with the dict's contents. -- Clearing it (`request.session.clear()`) on a session that had data deletes the backend session; - a cookie client also receives a `Set-Cookie` that expires the cookie. +- Clearing it (`request.session.clear()`) on a loaded session logs out: the backend session is + deleted even if its data was already empty, and a cookie client also receives a `Set-Cookie` + that expires the cookie. Keys written after `clear()` in the same request go into a new + anonymous session under a new ID. +- Removing the last key with `del` or `pop()` is not a logout. A session with a user is saved + with empty data; an anonymous one holds nothing and is deleted, as with `clear()`. - Logging in by writing to `request.session` keeps the session ID the request arrived with. With Starlette's middleware the cookie *is* the session, so the login response replaces whatever cookie was planted; here the cookie only names a server-side record, and a planted diff --git a/fastapi_cachex/session/middleware.py b/fastapi_cachex/session/middleware.py index 983db0b..0f9a879 100644 --- a/fastapi_cachex/session/middleware.py +++ b/fastapi_cachex/session/middleware.py @@ -286,6 +286,21 @@ def _get_client_ip(self, request: Request) -> str | None: return get_client_ip(request, self.config) +class _RequestSession(StarletteSession): + """``request.session`` that remembers an explicit ``clear()``. + + ``clear()`` is the logout idiom, so it has to end the loaded session even + when its data was already empty, while removing the last key with + ``del``/``pop`` must not log a user out. + """ + + cleared: bool = False + + def clear(self) -> None: + self.cleared = True + super().clear() + + class FastAPICacheXSessionMiddleware: """Drop-in-compatible replacement for Starlette's ``SessionMiddleware``. @@ -343,7 +358,6 @@ async def __call__(self, scope: Scope, receive: Receive, send: Send) -> None: _stash_session_manager(scope["app"], self.session_manager) connection = HTTPConnection(scope) - initial_session_was_empty = True loaded_token: str | None = None renewed_token: str | None = None backend_session: Session | None = None @@ -368,16 +382,15 @@ async def __call__(self, scope: Scope, receive: Receive, send: Send) -> None: ) loaded_token = renewed_token or token_value loaded_session_id = backend_session.session_id - scope["session"] = StarletteSession(backend_session.data) - initial_session_was_empty = not backend_session.data + scope["session"] = _RequestSession(backend_session.data) except SessionError: logger.debug( "FastAPICacheXSessionMiddleware: token invalid/expired; " "starting empty session", ) - scope["session"] = StarletteSession() + scope["session"] = _RequestSession() else: - scope["session"] = StarletteSession() + scope["session"] = _RequestSession() scope.setdefault("state", {})["__fastapi_cachex_session"] = backend_session @@ -388,7 +401,7 @@ async def __call__(self, scope: Scope, receive: Receive, send: Send) -> None: async def send_wrapper(message: Message) -> None: if message["type"] == "http.response.start": - session: StarletteSession = scope["session"] + session: _RequestSession = scope["session"] headers = MutableHeaders(scope=message) current_token, fresh_token = self._response_tokens( @@ -399,44 +412,72 @@ async def send_wrapper(message: Message) -> None: for name in vary_on: headers.add_vary_header(name) - if session.modified and session: - cookie_token, new_token = await self._write_session( - session, - connection, - backend_session, - current_token, - fresh_token, - ) - # Header clients only need a genuinely new/renewed token (an - # unchanged one is already held); cookie clients always get a - # refreshed cookie. - token_to_emit = new_token if from_header else cookie_token - if token_to_emit is not None: - self._emit_token( - headers, token_to_emit, from_header=from_header - ) - elif session.modified and not initial_session_was_empty: - # Cleared -> delete backend session. backend_session is always - # set when initial_session_was_empty is False (both are only set - # together, after a successful get_session() call above). - assert backend_session is not None # noqa: S101 - await self.session_manager.delete_session( - backend_session.session_id - ) - if not from_header: - # Cookie transport: expire the cookie. A header-based client - # simply drops its now-dangling token (record is deleted). - headers.append("Set-Cookie", self._build_clear_cookie_header()) - elif fresh_token is not None: - # Sliding expiration renewed the token, or the ID was - # regenerated, even though the dict itself was untouched; - # propagate it via the same transport. - self._emit_token(headers, fresh_token, from_header=from_header) + await self._persist( + session, + headers, + connection, + backend_session, + current_token, + fresh_token, + from_header=from_header, + ) await send(message) await self.app(scope, receive, send_wrapper) + async def _persist( # noqa: PLR0913 + self, + session: _RequestSession, + headers: MutableHeaders, + connection: HTTPConnection, + backend_session: "Session | None", + current_token: str | None, + fresh_token: str | None, + *, + from_header: bool, + ) -> None: + """Save, delete or renew the session according to what the request did to it.""" + target = backend_session + if session.cleared and target is not None: + # clear() logs out, whatever the data held. Anything + # written after it goes into a new anonymous session. + await self.session_manager.delete_session(target.session_id) + target = current_token = fresh_token = None + if not session and not from_header: + # Cookie transport: expire the cookie. A header-based + # client simply drops its now-dangling token. + headers.append("Set-Cookie", self._build_clear_cookie_header()) + + if session.modified and ( + session or (target is not None and target.user is not None) + ): + # A logged-in session emptied with del/pop keeps its user + # and is saved with empty data. + cookie_token, new_token = await self._write_session( + session, + connection, + target, + current_token, + fresh_token, + ) + # Header clients only need a genuinely new/renewed token (an + # unchanged one is already held); cookie clients always get a + # refreshed cookie. + token_to_emit = new_token if from_header else cookie_token + if token_to_emit is not None: + self._emit_token(headers, token_to_emit, from_header=from_header) + elif session.modified and target is not None: + # An anonymous session left empty holds nothing to keep. + await self.session_manager.delete_session(target.session_id) + if not from_header: + headers.append("Set-Cookie", self._build_clear_cookie_header()) + elif fresh_token is not None: + # Sliding expiration renewed the token, or the ID was + # regenerated, even though the dict itself was untouched; + # propagate it via the same transport. + self._emit_token(headers, fresh_token, from_header=from_header) + def _token_sources( self, connection: HTTPConnection ) -> tuple[str | None, list[str]]: diff --git a/i18n/zh-TW/docs/SESSION.md b/i18n/zh-TW/docs/SESSION.md index b8dee2a..fbdac9f 100644 --- a/i18n/zh-TW/docs/SESSION.md +++ b/i18n/zh-TW/docs/SESSION.md @@ -327,7 +327,8 @@ async def me(session=Depends(get_session)): - 在沒有載入任何 Session 時寫入 `request.session`，會建立一個新的**匿名** Session（`SessionManager.create_anonymous_session()`，並依設定套用 IP / User-Agent 綁定），並透過該請求的傳輸方式傳回其權杖。 - 修改已載入 Session 的 `request.session`，會透過 `update_session()` 將新內容儲存到後端，以 dict 的內容取代 `Session.data`。 -- 在原本有資料的 Session 上清除它（`request.session.clear()`），會刪除後端的 Session；Cookie 用戶端還會收到一個使 Cookie 過期的 `Set-Cookie`。 +- 在已載入的 Session 上清除它（`request.session.clear()`）即為登出：即使資料原本就是空的，也會刪除後端的 Session；Cookie 用戶端還會收到一個使 Cookie 過期的 `Set-Cookie`。同一個請求中在 `clear()` 之後寫入的鍵，會存進一個使用新 ID 的新匿名 Session。 +- 以 `del` 或 `pop()` 移除最後一個鍵並不是登出。帶有使用者的 Session 會以空資料儲存；匿名 Session 已無任何內容，會和 `clear()` 一樣被刪除。 - 以寫入 `request.session` 的方式登入時，會沿用請求帶來的 Session ID。Starlette 的中介軟體中 Cookie *就是* Session，因此登入回應會取代任何被植入的 Cookie；這裡的 Cookie 只是指向伺服器端紀錄的名稱，被植入的 Cookie 會跟著受害者一起登入。請在附加使用者之前呼叫 `await rotate_session_id(request)`（見[登入後重新產生 Session ID](#5-regenerate-the-session-id-after-login)）。 - 只要存取 `request.session`，就會為了尋找權杖而讀取過的每個請求標頭加入 `Vary`：依 `token_source_priority` 順序檢查的標頭（`header_name`，以及啟用 Bearer 權杖時的 `Authorization`），直到攜帶權杖的那一個為止。只有在沒有任何標頭攜帶權杖時才會讀取 Cookie，因此也只有這時才會加入 `Cookie`。 diff --git a/tests/session/test_starlette_middleware.py b/tests/session/test_starlette_middleware.py index 690b026..b1d527e 100644 --- a/tests/session/test_starlette_middleware.py +++ b/tests/session/test_starlette_middleware.py @@ -1005,3 +1005,125 @@ def test_no_vary_when_the_session_is_not_accessed( response = client.get("/untouched") assert "vary" not in response.headers + + +def _clearing_app(manager: SessionManager, config: SessionConfig) -> FastAPI: + """An app that logs out with clear(), pops single keys and writes after clearing.""" + app = FastAPI() + app.add_middleware( + FastAPICacheXSessionMiddleware, session_manager=manager, config=config + ) + + @app.get("/logout") + async def logout_route(request: Request) -> dict[str, bool]: + request.session.clear() + return {"ok": True} + + @app.get("/pop-flash") + async def pop_flash_route(request: Request) -> dict[str, Any]: + return {"flash": request.session.pop("flash", None)} + + @app.get("/clear-then-write") + async def clear_then_write_route(request: Request) -> dict[str, bool]: + request.session.clear() + request.session["flash"] = "signed out" + return {"ok": True} + + return app + + +@pytest.mark.asyncio +@pytest.mark.parametrize("transport", ["cookie", "header"]) +async def test_clear_logs_out_a_session_with_empty_data( + manager: SessionManager, config: SessionConfig, transport: str +) -> None: + """clear() ends a logged-in session even when its data was already empty. + + The emptiness of the initial data used to decide whether clear() counted, + so a user session created without data survived its own logout. + """ + _session, token = await manager.create_session( + user=SessionUser(user_id="empty-data-user") + ) + + client = TestClient(_clearing_app(manager, config)) + if transport == "cookie": + client.cookies.set(config.cookie_name, token) + response = client.get("/logout") + set_cookie = response.headers["set-cookie"] + assert f"{config.cookie_name}=;" in set_cookie + assert "1970" in set_cookie + else: + response = client.get("/logout", headers={config.header_name: token}) + assert "set-cookie" not in response.headers + assert config.header_name.lower() not in response.headers + + assert response.status_code == 200 + with pytest.raises(SessionNotFoundError): + await manager.get_session(token) + + +@pytest.mark.asyncio +async def test_popping_the_last_key_keeps_a_user_logged_in( + manager: SessionManager, config: SessionConfig +) -> None: + """Removing the last key is not a logout: the user session stays, with empty data.""" + _session, token = await manager.create_session( + user=SessionUser(user_id="flash-user"), flash="hi" + ) + + client = TestClient(_clearing_app(manager, config)) + response = client.get("/pop-flash", headers={config.header_name: token}) + + assert response.json() == {"flash": "hi"} + kept, _ = await manager.get_session(token) + assert kept.user is not None + assert kept.user.user_id == "flash-user" + assert kept.data == {} + + +@pytest.mark.asyncio +async def test_popping_the_last_key_deletes_an_anonymous_session( + manager: SessionManager, config: SessionConfig +) -> None: + """Without a user an emptied session holds nothing, so it goes, as in Starlette.""" + _session, token = await manager.create_anonymous_session(flash="hi") + + client = TestClient(_clearing_app(manager, config)) + client.cookies.set(config.cookie_name, token) + response = client.get("/pop-flash") + + assert response.json() == {"flash": "hi"} + assert "1970" in response.headers["set-cookie"] + with pytest.raises(SessionNotFoundError): + await manager.get_session(token) + + +@pytest.mark.asyncio +@pytest.mark.parametrize("transport", ["cookie", "header"]) +async def test_writing_after_clear_starts_a_new_anonymous_session( + manager: SessionManager, config: SessionConfig, transport: str +) -> None: + """Keys written after clear() go to a new session, never the logged-out one.""" + _session, token = await manager.create_session( + user=SessionUser(user_id="logout-flash-user"), seen=True + ) + + client = TestClient(_clearing_app(manager, config)) + if transport == "cookie": + client.cookies.set(config.cookie_name, token) + response = client.get("/clear-then-write") + new_token = _extract_cookie_token( + response.headers["set-cookie"], config.cookie_name + ) + else: + response = client.get("/clear-then-write", headers={config.header_name: token}) + new_token = response.headers[config.header_name] + + assert response.status_code == 200 + with pytest.raises(SessionNotFoundError): + await manager.get_session(token) + fresh, _ = await manager.get_session(new_token) + assert fresh.session_id != _session.session_id + assert fresh.user is None + assert fresh.data == {"flash": "signed out"}