diff --git a/Docs/Tasks.md b/Docs/Tasks.md index fa755e7..46ad111 100644 --- a/Docs/Tasks.md +++ b/Docs/Tasks.md @@ -38,6 +38,7 @@ Reference: `Docs/Refactoring.md` for full analysis of each issue. ### 4. Address session cache invalidation semantics - Where found: `backend/app/dependencies.py` - Goal: make session caching safe or remove it, and document that cache invalidation is not cluster-safe if the app is run with multiple workers. +- Status: completed — session validation cache is now disabled by default and can be enabled explicitly in single-process deployments. - Possible traps and issues: - Process-local cache can keep revoked sessions alive in other worker processes. - Implementing a shared cache is a larger architectural change; a safer short-term fix is to disable caching by default. diff --git a/backend/app/config.py b/backend/app/config.py index 3b58252..a9cc522 100644 --- a/backend/app/config.py +++ b/backend/app/config.py @@ -45,6 +45,21 @@ class Settings(BaseSettings): ge=1, description="Number of minutes a session token remains valid after creation.", ) + session_cache_enabled: bool = Field( + default=False, + description=( + "Enable the in-memory session validation cache. " + "Disable it in multi-worker deployments to avoid stale revoked sessions." + ), + ) + session_cache_ttl_seconds: float = Field( + default=10.0, + ge=0.0, + description=( + "How long (seconds) a cached session validation entry remains fresh. " + "Ignored when session_cache_enabled is false." + ), + ) timezone: str = Field( default="UTC", description="IANA timezone name used when displaying timestamps in the UI.", diff --git a/backend/app/dependencies.py b/backend/app/dependencies.py index 7a0ebf9..fc7e5c8 100644 --- a/backend/app/dependencies.py +++ b/backend/app/dependencies.py @@ -50,8 +50,6 @@ _COOKIE_NAME = "bangui_session" #: NOTE: this cache is process-local and is not cluster-safe. In multi-worker #: or distributed deployments, each process maintains its own cache, so logout #: invalidation and revocation may be delayed unless a shared cache is used. -_SESSION_CACHE_TTL: float = 10.0 - #: ``token → (Session, cache_expiry_monotonic_time)`` _session_cache: dict[str, tuple[Session, float]] = {} @@ -65,6 +63,11 @@ def clear_session_cache() -> None: _session_cache.clear() +def _session_cache_enabled(settings: Settings) -> bool: + """Return whether the in-memory session cache should be used.""" + return settings.session_cache_enabled and settings.session_cache_ttl_seconds > 0.0 + + def invalidate_session_cache(token: str) -> None: """Evict *token* from the in-memory session cache. @@ -213,10 +216,12 @@ async def require_auth( The token is read from the ``bangui_session`` cookie or the ``Authorization: Bearer`` header. - Validated tokens are cached in memory for :data:`_SESSION_CACHE_TTL` - seconds so that concurrent requests sharing the same token avoid repeated - SQLite round-trips. The cache is bypassed on expiry and explicitly - cleared by :func:`invalidate_session_cache` on logout. + Validated tokens may be cached in memory for a short period so that + concurrent requests sharing the same token avoid repeated SQLite + round-trips. This cache is disabled by default because process-local + invalidation is not safe in multi-worker or clustered deployments. + When enabled, entries are bypassed on expiry and explicitly cleared by + :func:`invalidate_session_cache` on logout. Args: request: The incoming FastAPI request. @@ -244,15 +249,17 @@ async def require_auth( headers={"WWW-Authenticate": "Bearer"}, ) - # Fast path: serve from in-memory cache when the entry is still fresh and - # the session itself has not yet exceeded its own expiry time. - cached = _session_cache.get(token) - if cached is not None: - session, cache_expires_at = cached - if time.monotonic() < cache_expires_at and session.expires_at > utc_now().isoformat(): - return session - # Stale cache entry — evict and fall through to DB. - _session_cache.pop(token, None) + cache_enabled = _session_cache_enabled(settings) + if cache_enabled: + # Fast path: serve from in-memory cache when the entry is still fresh and + # the session itself has not yet exceeded its own expiry time. + cached = _session_cache.get(token) + if cached is not None: + session, cache_expires_at = cached + if time.monotonic() < cache_expires_at and session.expires_at > utc_now().isoformat(): + return session + # Stale cache entry — evict and fall through to DB. + _session_cache.pop(token, None) try: session = await auth_service.validate_session(db, token, settings.session_secret) @@ -263,7 +270,11 @@ async def require_auth( headers={"WWW-Authenticate": "Bearer"}, ) from exc - _session_cache[token] = (session, time.monotonic() + _SESSION_CACHE_TTL) + if cache_enabled: + _session_cache[token] = ( + session, + time.monotonic() + settings.session_cache_ttl_seconds, + ) return session diff --git a/backend/tests/test_routers/test_auth.py b/backend/tests/test_routers/test_auth.py index a256b9c..25e33eb 100644 --- a/backend/tests/test_routers/test_auth.py +++ b/backend/tests/test_routers/test_auth.py @@ -165,6 +165,39 @@ class TestRequireAuth: self, client: AsyncClient ) -> None: """Health endpoint is accessible without authentication.""" + response = await client.get("/api/health") + assert response.status_code == 200 + + async def test_session_cache_is_disabled_by_default( + self, client: AsyncClient + ) -> None: + """Session validation does not use the in-memory cache unless enabled.""" + from app.repositories import session_repo + + await _do_setup(client) + token = await _login(client) + + call_count = 0 + original_get_session = session_repo.get_session + + async def _tracking(db, tok): # type: ignore[no-untyped-def] + nonlocal call_count + call_count += 1 + return await original_get_session(db, tok) + + with patch.object(session_repo, "get_session", side_effect=_tracking): + resp1 = await client.get( + "/api/dashboard/status", + headers={"Authorization": f"Bearer {token}"}, + ) + resp2 = await client.get( + "/api/dashboard/status", + headers={"Authorization": f"Bearer {token}"}, + ) + + assert resp1.status_code == 200 + assert resp2.status_code == 200 + assert call_count == 2 # --------------------------------------------------------------------------- @@ -184,6 +217,13 @@ class TestRequireAuthSessionCache: yield dependencies.clear_session_cache() + @pytest.fixture(autouse=True) + def enable_session_cache(self, client: AsyncClient) -> Generator[None, None, None]: + """Enable the in-memory auth cache for tests that exercise it.""" + client._transport.app.state.settings.session_cache_enabled = True + client._transport.app.state.settings.session_cache_ttl_seconds = 10.0 + yield + async def test_second_request_skips_db(self, client: AsyncClient) -> None: """Second authenticated request within TTL skips the session DB query.