Disable session cache by default and make it opt-in for single-process deployments
This commit is contained in:
@@ -38,6 +38,7 @@ Reference: `Docs/Refactoring.md` for full analysis of each issue.
|
|||||||
### 4. Address session cache invalidation semantics
|
### 4. Address session cache invalidation semantics
|
||||||
- Where found: `backend/app/dependencies.py`
|
- 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.
|
- 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:
|
- Possible traps and issues:
|
||||||
- Process-local cache can keep revoked sessions alive in other worker processes.
|
- 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.
|
- Implementing a shared cache is a larger architectural change; a safer short-term fix is to disable caching by default.
|
||||||
|
|||||||
@@ -45,6 +45,21 @@ class Settings(BaseSettings):
|
|||||||
ge=1,
|
ge=1,
|
||||||
description="Number of minutes a session token remains valid after creation.",
|
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(
|
timezone: str = Field(
|
||||||
default="UTC",
|
default="UTC",
|
||||||
description="IANA timezone name used when displaying timestamps in the UI.",
|
description="IANA timezone name used when displaying timestamps in the UI.",
|
||||||
|
|||||||
@@ -50,8 +50,6 @@ _COOKIE_NAME = "bangui_session"
|
|||||||
#: NOTE: this cache is process-local and is not cluster-safe. In multi-worker
|
#: 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
|
#: or distributed deployments, each process maintains its own cache, so logout
|
||||||
#: invalidation and revocation may be delayed unless a shared cache is used.
|
#: invalidation and revocation may be delayed unless a shared cache is used.
|
||||||
_SESSION_CACHE_TTL: float = 10.0
|
|
||||||
|
|
||||||
#: ``token → (Session, cache_expiry_monotonic_time)``
|
#: ``token → (Session, cache_expiry_monotonic_time)``
|
||||||
_session_cache: dict[str, tuple[Session, float]] = {}
|
_session_cache: dict[str, tuple[Session, float]] = {}
|
||||||
|
|
||||||
@@ -65,6 +63,11 @@ def clear_session_cache() -> None:
|
|||||||
_session_cache.clear()
|
_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:
|
def invalidate_session_cache(token: str) -> None:
|
||||||
"""Evict *token* from the in-memory session cache.
|
"""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
|
The token is read from the ``bangui_session`` cookie or the
|
||||||
``Authorization: Bearer`` header.
|
``Authorization: Bearer`` header.
|
||||||
|
|
||||||
Validated tokens are cached in memory for :data:`_SESSION_CACHE_TTL`
|
Validated tokens may be cached in memory for a short period so that
|
||||||
seconds so that concurrent requests sharing the same token avoid repeated
|
concurrent requests sharing the same token avoid repeated SQLite
|
||||||
SQLite round-trips. The cache is bypassed on expiry and explicitly
|
round-trips. This cache is disabled by default because process-local
|
||||||
cleared by :func:`invalidate_session_cache` on logout.
|
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:
|
Args:
|
||||||
request: The incoming FastAPI request.
|
request: The incoming FastAPI request.
|
||||||
@@ -244,15 +249,17 @@ async def require_auth(
|
|||||||
headers={"WWW-Authenticate": "Bearer"},
|
headers={"WWW-Authenticate": "Bearer"},
|
||||||
)
|
)
|
||||||
|
|
||||||
# Fast path: serve from in-memory cache when the entry is still fresh and
|
cache_enabled = _session_cache_enabled(settings)
|
||||||
# the session itself has not yet exceeded its own expiry time.
|
if cache_enabled:
|
||||||
cached = _session_cache.get(token)
|
# Fast path: serve from in-memory cache when the entry is still fresh and
|
||||||
if cached is not None:
|
# the session itself has not yet exceeded its own expiry time.
|
||||||
session, cache_expires_at = cached
|
cached = _session_cache.get(token)
|
||||||
if time.monotonic() < cache_expires_at and session.expires_at > utc_now().isoformat():
|
if cached is not None:
|
||||||
return session
|
session, cache_expires_at = cached
|
||||||
# Stale cache entry — evict and fall through to DB.
|
if time.monotonic() < cache_expires_at and session.expires_at > utc_now().isoformat():
|
||||||
_session_cache.pop(token, None)
|
return session
|
||||||
|
# Stale cache entry — evict and fall through to DB.
|
||||||
|
_session_cache.pop(token, None)
|
||||||
|
|
||||||
try:
|
try:
|
||||||
session = await auth_service.validate_session(db, token, settings.session_secret)
|
session = await auth_service.validate_session(db, token, settings.session_secret)
|
||||||
@@ -263,7 +270,11 @@ async def require_auth(
|
|||||||
headers={"WWW-Authenticate": "Bearer"},
|
headers={"WWW-Authenticate": "Bearer"},
|
||||||
) from exc
|
) 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
|
return session
|
||||||
|
|
||||||
|
|
||||||
|
|||||||
@@ -165,6 +165,39 @@ class TestRequireAuth:
|
|||||||
self, client: AsyncClient
|
self, client: AsyncClient
|
||||||
) -> None:
|
) -> None:
|
||||||
"""Health endpoint is accessible without authentication."""
|
"""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
|
yield
|
||||||
dependencies.clear_session_cache()
|
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:
|
async def test_second_request_skips_db(self, client: AsyncClient) -> None:
|
||||||
"""Second authenticated request within TTL skips the session DB query.
|
"""Second authenticated request within TTL skips the session DB query.
|
||||||
|
|
||||||
|
|||||||
Reference in New Issue
Block a user