From 0ac1277479474a8e9f200c87b2bb96209be8767a Mon Sep 17 00:00:00 2001 From: timmy Date: Sat, 8 Aug 2026 07:15:37 +0000 Subject: [PATCH] perf: keep session validation off event loop (#275) --- src/dashboard_auth.py | 5 ++- src/main.py | 20 +++++----- src/session_store.py | 33 ++++++++++------ tests/test_dashboard_auth.py | 73 ++++++++++++++++++++++++++++++++++++ tests/test_session_store.py | 36 ++++++++++++++++-- 5 files changed, 141 insertions(+), 26 deletions(-) diff --git a/src/dashboard_auth.py b/src/dashboard_auth.py index 99ab08a..337154b 100644 --- a/src/dashboard_auth.py +++ b/src/dashboard_auth.py @@ -1,5 +1,6 @@ """Signed, short-lived single-operator sessions for the dashboard boundary.""" +import asyncio import base64 import hashlib import hmac @@ -139,8 +140,8 @@ def revoke_session(session: Session) -> None: _session_store().revoke(session.session_id) -def request_session(request: Request) -> Session | None: - return verify_session(request.cookies.get(SESSION_COOKIE)) +async def request_session(request: Request) -> Session | None: + return await asyncio.to_thread(verify_session, request.cookies.get(SESSION_COOKIE)) def cookie_path(request: Request) -> str: diff --git a/src/main.py b/src/main.py index b439e04..618b471 100644 --- a/src/main.py +++ b/src/main.py @@ -470,14 +470,16 @@ async def require_operator_session(request: Request, call_next): or path.startswith("/static/") or (path == "/api/v1/session" and request.method == "POST") ) - try: - session = dashboard_auth.request_session(request) - except dashboard_auth.SessionStoreError: - return JSONResponse( - {"detail": "Session registry is temporarily unavailable"}, - status_code=503, - headers={"Cache-Control": "no-store"}, - ) + session = None + if not public: + try: + session = await dashboard_auth.request_session(request) + except dashboard_auth.SessionStoreError: + return JSONResponse( + {"detail": "Session registry is temporarily unavailable"}, + status_code=503, + headers={"Cache-Control": "no-store"}, + ) if not public and session is None: if path.startswith("/api/"): return JSONResponse( @@ -615,7 +617,7 @@ async def sign_in(payload: DashboardSignIn, request: Request, response: Response @app.get("/api/v1/session") async def session_status(request: Request): - session = dashboard_auth.request_session(request) + session = request.state.dashboard_session return { "authenticated": session is not None, "csrf_token": session.csrf if session is not None else "", diff --git a/src/session_store.py b/src/session_store.py index 18d4051..de6062e 100644 --- a/src/session_store.py +++ b/src/session_store.py @@ -26,25 +26,35 @@ class SessionStore: def _digest(session_id: str) -> str: return hashlib.sha256(session_id.encode()).hexdigest() - def _connect(self) -> sqlite3.Connection: + def _connect(self, *, initialize: bool = False) -> sqlite3.Connection: try: - self.path.parent.mkdir(parents=True, exist_ok=True) - connection = sqlite3.connect(self.path, timeout=self.lock_timeout_seconds) - connection.execute( - """ - CREATE TABLE IF NOT EXISTS active_sessions ( - session_hash TEXT PRIMARY KEY, - expires_at INTEGER NOT NULL + if initialize: + self.path.parent.mkdir(parents=True, exist_ok=True) + connection = sqlite3.connect( + self.path, timeout=self.lock_timeout_seconds + ) + else: + connection = sqlite3.connect( + f"{self.path.resolve().as_uri()}?mode=rw", + timeout=self.lock_timeout_seconds, + uri=True, + ) + if initialize: + connection.execute( + """ + CREATE TABLE IF NOT EXISTS active_sessions ( + session_hash TEXT PRIMARY KEY, + expires_at INTEGER NOT NULL + ) + """ ) - """ - ) return connection except (OSError, sqlite3.Error) as exc: raise SessionStoreError("Session registry is temporarily unavailable") from exc def activate(self, session_id: str, expires_at: int) -> None: try: - with self._connect() as connection: + with self._connect(initialize=True) as connection: connection.execute( "DELETE FROM active_sessions WHERE expires_at <= ?", (int(self.clock()),) ) @@ -59,7 +69,6 @@ class SessionStore: now = int(self.clock()) try: with self._connect() as connection: - connection.execute("DELETE FROM active_sessions WHERE expires_at <= ?", (now,)) row = connection.execute( "SELECT expires_at FROM active_sessions WHERE session_hash = ?", (self._digest(session_id),), diff --git a/tests/test_dashboard_auth.py b/tests/test_dashboard_auth.py index 23afc9a..e3207b5 100644 --- a/tests/test_dashboard_auth.py +++ b/tests/test_dashboard_auth.py @@ -1,3 +1,6 @@ +import asyncio +import time + import httpx import pytest @@ -175,6 +178,30 @@ async def test_authenticated_get_reaches_private_api(access_control, monkeypatch assert response.json()["user"]["login"] == "timmy" +@pytest.mark.anyio +async def test_session_status_reuses_the_middleware_validation(access_control, monkeypatch): + transport = httpx.ASGITransport(app=main.app) + async with httpx.AsyncClient(transport=transport, base_url="https://test") as client: + await client.post( + "/api/v1/session", json={"access_token": "correct horse battery staple"} + ) + original_store = main.dashboard_auth._session_store() + lookups = 0 + + class CountingStore: + def is_active(self, session_id, expires_at): + nonlocal lookups + lookups += 1 + return original_store.is_active(session_id, expires_at) + + monkeypatch.setattr(main.dashboard_auth, "_session_store", lambda now=None: CountingStore()) + response = await client.get("/api/v1/session") + + assert response.status_code == 200 + assert response.json()["authenticated"] is True + assert lookups == 1 + + @pytest.mark.anyio async def test_authenticated_session_status_exposes_only_csrf_proof(access_control): transport = httpx.ASGITransport(app=main.app) @@ -292,6 +319,31 @@ async def test_logout_revokes_a_captured_cookie_before_gitea(access_control, mon assert calls == 1 +@pytest.mark.anyio +async def test_session_registry_latency_does_not_block_the_event_loop(access_control, monkeypatch): + transport = httpx.ASGITransport(app=main.app) + async with httpx.AsyncClient(transport=transport, base_url="https://test") as client: + await client.post( + "/api/v1/session", json={"access_token": "correct horse battery staple"} + ) + + class SlowStore: + def is_active(self, session_id, expires_at): + time.sleep(0.15) + return True + + monkeypatch.setattr(main.dashboard_auth, "_session_store", lambda now=None: SlowStore()) + private_request = asyncio.create_task(client.get("/api/v1/session")) + await asyncio.sleep(0) + started = time.perf_counter() + await asyncio.sleep(0.01) + heartbeat_elapsed = time.perf_counter() - started + response = await private_request + + assert response.status_code == 200 + assert heartbeat_elapsed < 0.08 + + @pytest.mark.anyio async def test_session_registry_read_failure_fails_closed_before_gitea( access_control, monkeypatch @@ -375,6 +427,27 @@ async def test_logout_registry_failure_does_not_claim_revocation(access_control, assert "database path" not in response.text +@pytest.mark.anyio +async def test_public_routes_skip_session_registry_validation(access_control, monkeypatch): + transport = httpx.ASGITransport(app=main.app) + async with httpx.AsyncClient(transport=transport, base_url="https://test") as client: + signed_in = await client.post( + "/api/v1/session", json={"access_token": "correct horse battery staple"} + ) + + class BrokenStore: + def is_active(self, session_id, expires_at): + raise SessionStoreError("public routes must not read the registry") + + monkeypatch.setattr(main.dashboard_auth, "_session_store", lambda now=None: BrokenStore()) + login = await client.get("/login") + manifest = await client.get("/manifest.webmanifest") + static = await client.get("/static/session.js") + + assert signed_in.status_code == 200 + assert [login.status_code, manifest.status_code, static.status_code] == [200, 200, 200] + + @pytest.mark.anyio async def test_public_routes_remain_available_and_readiness_hides_identity(access_control, monkeypatch): async def user(): diff --git a/tests/test_session_store.py b/tests/test_session_store.py index c8c20bd..9393caf 100644 --- a/tests/test_session_store.py +++ b/tests/test_session_store.py @@ -1,6 +1,9 @@ import sqlite3 -from src.session_store import SessionStore +import pytest + +from src import session_store +from src.session_store import SessionStore, SessionStoreError def test_revocation_is_durable_and_scoped_to_one_session(tmp_path): @@ -19,7 +22,34 @@ def test_revocation_is_durable_and_scoped_to_one_session(tmp_path): assert b"second-session-secret" not in database.read_bytes() -def test_expired_sessions_are_rejected_and_pruned(tmp_path): +def test_validation_does_not_create_a_missing_registry(tmp_path): + database = tmp_path / "sessions.sqlite3" + store = SessionStore(database, clock=lambda: 1_000.0) + + with pytest.raises(SessionStoreError): + store.is_active("unknown-session", 2_000) + + assert database.exists() is False + + +def test_validation_executes_only_a_read_query(tmp_path, monkeypatch): + store = SessionStore(tmp_path / "sessions.sqlite3", clock=lambda: 1_000.0) + store.activate("active-session", 2_000) + statements = [] + connect = sqlite3.connect + + def traced_connect(*args, **kwargs): + connection = connect(*args, **kwargs) + connection.set_trace_callback(statements.append) + return connection + + monkeypatch.setattr(session_store.sqlite3, "connect", traced_connect) + + assert store.is_active("active-session", 2_000) is True + assert [statement.split()[0].upper() for statement in statements] == ["SELECT"] + + +def test_expired_sessions_are_rejected_without_writing_during_validation(tmp_path): now = [1_000.0] store = SessionStore(tmp_path / "sessions.sqlite3", clock=lambda: now[0]) store.activate("expiring-session", 1_001) @@ -28,4 +58,4 @@ def test_expired_sessions_are_rejected_and_pruned(tmp_path): assert store.is_active("expiring-session", 1_001) is False with sqlite3.connect(store.path) as connection: - assert connection.execute("SELECT COUNT(*) FROM active_sessions").fetchone() == (0,) + assert connection.execute("SELECT COUNT(*) FROM active_sessions").fetchone() == (1,) -- 2.43.0