From 9a288bd92180bb3bea0171d2789fd6a9b1c958fb Mon Sep 17 00:00:00 2001 From: jzhao234 Date: Mon, 14 Sep 2026 19:54:54 +0000 Subject: [PATCH] fix(auth): touch Explorer sessions at most once a minute instead of on every request TL;DR: Validating an Explorer session no longer writes to SQLite on every request. The last-seen and idle-expiry columns are rewritten only when the previous touch is more than a minute old, which turns a page load's burst of requests into a single write and keeps the stated 60-minute idle limit accurate to the minute. One minute is the convention across Auth, Explorer, and Lens. Problem: get_identity opened every check with BEGIN IMMEDIATE and issued an UPDATE, so each authenticated request took the SQLite write lock and flushed a transaction to record a timestamp that carried no new information. Explorer's inbox page fires several requests per load; with a few dozen users that is hundreds of write transactions per minute against the access-list database. Fix: - SESSION_TOUCH_SECONDS = 60, documented as the Elcano convention for service sessions and deliberately a constant. - get_identity reads first without taking the write lock, then writes only if timestamp - last_seen_at is at least the interval; the UPDATE is guarded with revoked_at IS NULL. Expiry and revocation checks are unchanged, so the idle limit behaves as "60 minutes minus at most one minute", never longer. - README notes the behaviour. Tests: - New: two validations inside the interval leave last_seen_at and idle_expires_at untouched; the first validation past the interval writes once and moves the idle clock forward from the request; a request at idle_expires_at is rejected. - Ran: pytest -q, ruff check, ruff format --check. --- README.md | 4 +++- app/central_auth.py | 31 ++++++++++++++++++++++--------- tests/test_central_auth.py | 26 ++++++++++++++++++++++++++ 3 files changed, 51 insertions(+), 10 deletions(-) diff --git a/README.md b/README.md index 31015e8..f936d9f 100644 --- a/README.md +++ b/README.md @@ -200,7 +200,9 @@ sudo explorer access revoke user@example.com Allowed users receive a random 256-bit, app-only session. Only its SHA-256 hash is stored in `/var/lib/explorer/access.db`; the host-only `__Host-explorer_session` cookie is `Secure`, `HttpOnly`, `SameSite=Lax`, and -scoped to `/`. Sessions expire after 60 minutes idle or 12 hours total. +scoped to `/`. Sessions expire after 60 minutes idle or 12 hours total; the +idle clock is refreshed at most once a minute (the Elcano convention for +service sessions), so a session can end up to a minute early but never late. Revoking an email immediately invalidates all of that email's Explorer sessions. Logout is CSRF-protected and revokes only the current Explorer session. Auth's signed back-channel endpoint also revokes every local session diff --git a/app/central_auth.py b/app/central_auth.py index 599e032..9178ba0 100644 --- a/app/central_auth.py +++ b/app/central_auth.py @@ -33,6 +33,14 @@ CENTRAL_AUTH_COOKIE_NAME = "__Host-explorer_session" DEFAULT_IDLE_SECONDS = 60 * 60 DEFAULT_ABSOLUTE_SECONDS = 12 * 60 * 60 +# How often a validated session rewrites last_seen_at / idle_expires_at. Every +# request reads the session; only a request more than this long after the +# previous touch writes. The idle limit therefore behaves as "60 minutes minus +# at most one minute", never longer, and a page's burst of requests costs one +# SQLite write instead of one per request. One minute is the convention for +# every Elcano service with its own sessions (Auth, Explorer, Lens, and +# anything built later); keep it a constant, not a setting. +SESSION_TOUCH_SECONDS = 60 LOGIN_TRANSACTION_SECONDS = 10 * 60 SCHEMA_VERSION = 2 MAX_TOKEN_RESPONSE_BYTES = 64 * 1024 @@ -681,10 +689,11 @@ def get_identity( return None timestamp = int(time.time() if now is None else now) with self._connect() as connection: - connection.execute("BEGIN IMMEDIATE") + # Plain read first: most requests fall inside the touch interval + # and must not take the write lock. row = connection.execute( """ - SELECT s.subject, s.email, s.idle_expires_at, + SELECT s.subject, s.email, s.last_seen_at, s.idle_expires_at, s.absolute_expires_at, a.enabled FROM sessions AS s JOIN access_entries AS a ON a.email = s.email @@ -704,13 +713,17 @@ def get_identity( (timestamp, token_hash), ) return None - next_idle = min( - timestamp + self.idle_seconds, int(row["absolute_expires_at"]) - ) - connection.execute( - "UPDATE sessions SET last_seen_at = ?, idle_expires_at = ? WHERE token_hash = ?", - (timestamp, next_idle, token_hash), - ) + if timestamp - int(row["last_seen_at"]) >= SESSION_TOUCH_SECONDS: + next_idle = min( + timestamp + self.idle_seconds, int(row["absolute_expires_at"]) + ) + connection.execute( + """ + UPDATE sessions SET last_seen_at = ?, idle_expires_at = ? + WHERE token_hash = ? AND revoked_at IS NULL + """, + (timestamp, next_idle, token_hash), + ) return CentralIdentity(subject=str(row["subject"]), email=str(row["email"])) def revoke_session(self, token: str | None, *, now: int | None = None) -> bool: diff --git a/tests/test_central_auth.py b/tests/test_central_auth.py index 67214ff..a1704db 100644 --- a/tests/test_central_auth.py +++ b/tests/test_central_auth.py @@ -480,3 +480,29 @@ def test_replay_table_is_pruned_after_retention(store: CentralAuthStore) -> None for row in connection.execute("SELECT event_id FROM revocation_events") } assert ids == {"new-event"} + + +def test_session_touch_is_rate_limited_to_one_write_per_minute( + store: CentralAuthStore, +) -> None: + store.grant_access("alice@example.com", now=1_000) + issued = store.create_session("account-123", "alice@example.com", now=1_000) + + def stamps(): + with sqlite3.connect(store.path) as connection: + return connection.execute( + "SELECT last_seen_at, idle_expires_at FROM sessions WHERE token_hash = ?", + (issued.token_hash,), + ).fetchone() + + assert stamps() == (1_000, 1_000 + store.idle_seconds) + # Inside the interval: validated, but no write. + assert store.get_identity(issued.token, now=1_030) is not None + assert store.get_identity(issued.token, now=1_059) is not None + assert stamps() == (1_000, 1_000 + store.idle_seconds) + # Past the interval: one write, idle clock moves forward from the request. + assert store.get_identity(issued.token, now=1_060) is not None + assert stamps() == (1_060, 1_060 + store.idle_seconds) + # The idle limit is enforced against the last touch, never longer than the + # limit: a request at exactly idle_expires_at is rejected. + assert store.get_identity(issued.token, now=1_060 + store.idle_seconds) is None