fix(#5022): surface auth persistence failures
This commit is contained in:
parent
3d30ae75b9
commit
3c4ef72a51
2 changed files with 128 additions and 19 deletions
80
api/auth.py
80
api/auth.py
|
|
@ -14,6 +14,7 @@ import secrets
|
|||
import tempfile
|
||||
import threading
|
||||
import time
|
||||
from pathlib import Path
|
||||
|
||||
from api.config import STATE_DIR, load_settings
|
||||
|
||||
|
|
@ -85,6 +86,18 @@ def _resolve_cookie_name() -> str:
|
|||
return COOKIE_NAME
|
||||
|
||||
|
||||
def _warn_auth_persistence_failure(prefix: str, artifact: Path, exc: Exception, consequence: str) -> None:
|
||||
logger.warning(
|
||||
'%s at %s (STATE_DIR=%s): %s: %s; %s',
|
||||
prefix,
|
||||
artifact,
|
||||
STATE_DIR,
|
||||
exc.__class__.__name__,
|
||||
exc,
|
||||
consequence,
|
||||
)
|
||||
|
||||
|
||||
_SESSIONS_FILE = STATE_DIR / '.sessions.json'
|
||||
|
||||
|
||||
|
|
@ -95,16 +108,38 @@ def _load_sessions() -> dict[str, float]:
|
|||
blocked by a corrupt or missing sessions file.
|
||||
"""
|
||||
try:
|
||||
if _SESSIONS_FILE.exists():
|
||||
data = json.loads(_SESSIONS_FILE.read_text(encoding='utf-8'))
|
||||
if not isinstance(data, dict):
|
||||
raise ValueError('malformed sessions file — expected dict')
|
||||
now = time.time()
|
||||
return {t: exp for t, exp in data.items()
|
||||
if isinstance(t, str) and isinstance(exp, (int, float)) and exp > now}
|
||||
except Exception as e:
|
||||
logger.debug("Failed to load sessions file, starting fresh: %s", e)
|
||||
return {}
|
||||
if not _SESSIONS_FILE.exists():
|
||||
return {}
|
||||
raw = _SESSIONS_FILE.read_text(encoding='utf-8')
|
||||
except OSError as e:
|
||||
_warn_auth_persistence_failure(
|
||||
'Auth session store read failed',
|
||||
_SESSIONS_FILE,
|
||||
e,
|
||||
'starting fresh with an empty session table',
|
||||
)
|
||||
return {}
|
||||
try:
|
||||
data = json.loads(raw)
|
||||
except json.JSONDecodeError as e:
|
||||
_warn_auth_persistence_failure(
|
||||
'Ignoring malformed auth session store',
|
||||
_SESSIONS_FILE,
|
||||
e,
|
||||
'starting fresh with an empty session table',
|
||||
)
|
||||
return {}
|
||||
if not isinstance(data, dict):
|
||||
_warn_auth_persistence_failure(
|
||||
'Ignoring malformed auth session store',
|
||||
_SESSIONS_FILE,
|
||||
ValueError('malformed sessions file: expected dict'),
|
||||
'starting fresh with an empty session table',
|
||||
)
|
||||
return {}
|
||||
now = time.time()
|
||||
return {t: exp for t, exp in data.items()
|
||||
if isinstance(t, str) and isinstance(exp, (int, float)) and exp > now}
|
||||
|
||||
|
||||
def _save_sessions(sessions: dict[str, float]) -> None:
|
||||
|
|
@ -128,7 +163,12 @@ def _save_sessions(sessions: dict[str, float]) -> None:
|
|||
pass
|
||||
raise
|
||||
except Exception as e:
|
||||
logger.debug("Failed to persist sessions: %s", e)
|
||||
_warn_auth_persistence_failure(
|
||||
'Auth session persistence failed',
|
||||
_SESSIONS_FILE,
|
||||
e,
|
||||
'keeping the in-process session table available',
|
||||
)
|
||||
|
||||
|
||||
# Active sessions: token -> expiry timestamp (persisted across restarts via STATE_DIR)
|
||||
|
|
@ -231,15 +271,25 @@ def _load_key(filename: str) -> bytes:
|
|||
raw = key_file.read_bytes()
|
||||
if len(raw) >= 32:
|
||||
return raw[:32]
|
||||
except OSError:
|
||||
logger.debug("Failed to read key %s", filename)
|
||||
except OSError as e:
|
||||
_warn_auth_persistence_failure(
|
||||
'Auth key read failed',
|
||||
key_file,
|
||||
e,
|
||||
'generating a new key and continuing',
|
||||
)
|
||||
key = secrets.token_bytes(32)
|
||||
try:
|
||||
STATE_DIR.mkdir(parents=True, exist_ok=True)
|
||||
key_file.write_bytes(key)
|
||||
key_file.chmod(0o600)
|
||||
except OSError:
|
||||
logger.debug("Failed to persist key %s", filename)
|
||||
except OSError as e:
|
||||
_warn_auth_persistence_failure(
|
||||
'Auth key persistence failed',
|
||||
key_file,
|
||||
e,
|
||||
'returning the generated key so startup can continue',
|
||||
)
|
||||
return key
|
||||
|
||||
|
||||
|
|
|
|||
|
|
@ -12,6 +12,7 @@ import sys
|
|||
import tempfile
|
||||
import time
|
||||
import unittest
|
||||
from unittest import mock
|
||||
from pathlib import Path
|
||||
|
||||
# Isolate state dir so tests never touch real sessions
|
||||
|
|
@ -28,9 +29,12 @@ class TestSessionPersistence(unittest.TestCase):
|
|||
|
||||
def setUp(self) -> None:
|
||||
auth._sessions.clear()
|
||||
sessions_file = _TEST_STATE / '.sessions.json'
|
||||
if sessions_file.exists():
|
||||
sessions_file.unlink()
|
||||
auth._PBKDF2_KEY_CACHE = None
|
||||
auth._SIGNING_KEY_CACHE = None
|
||||
for name in ('.sessions.json', '.signing_key', '.pbkdf2_key'):
|
||||
path = _TEST_STATE / name
|
||||
if path.exists():
|
||||
path.unlink()
|
||||
|
||||
def _simulate_restart(self) -> None:
|
||||
"""Reload auth module to simulate a fresh process start.
|
||||
|
|
@ -98,9 +102,64 @@ class TestSessionPersistence(unittest.TestCase):
|
|||
"""A corrupt sessions file must not crash auth — start with empty dict."""
|
||||
sessions_file = _TEST_STATE / '.sessions.json'
|
||||
sessions_file.write_text("not valid json {{{{")
|
||||
self._simulate_restart()
|
||||
with self.assertLogs('api.auth', level='WARNING') as captured:
|
||||
self._simulate_restart()
|
||||
self.assertEqual(auth._sessions, {},
|
||||
"Corrupt sessions file must result in empty session dict")
|
||||
warning = '\n'.join(captured.output)
|
||||
self.assertIn('Ignoring malformed auth session store', warning)
|
||||
self.assertIn(str(sessions_file), warning)
|
||||
self.assertIn(str(_TEST_STATE), warning)
|
||||
|
||||
def test_session_read_failure_warns_with_state_dir_and_starts_fresh(self) -> None:
|
||||
"""Unreadable sessions files must warn and start with an empty dict."""
|
||||
sessions_file = _TEST_STATE / '.sessions.json'
|
||||
sessions_file.write_text(json.dumps({"token": time.time() + 3600}))
|
||||
with mock.patch.object(Path, 'read_text', side_effect=OSError('read failed')):
|
||||
with self.assertLogs('api.auth', level='WARNING') as captured:
|
||||
self._simulate_restart()
|
||||
self.assertEqual(auth._sessions, {})
|
||||
warning = '\n'.join(captured.output)
|
||||
self.assertIn('Auth session store read failed', warning)
|
||||
self.assertIn(str(sessions_file), warning)
|
||||
self.assertIn(str(_TEST_STATE), warning)
|
||||
|
||||
def test_session_save_failure_warns_with_state_dir_and_keeps_in_process_session(self) -> None:
|
||||
"""Write failures must warn but keep the live session usable in-process."""
|
||||
with mock.patch.object(auth.os, 'replace', side_effect=OSError('replace failed')):
|
||||
with self.assertLogs('api.auth', level='WARNING') as captured:
|
||||
cookie = auth.create_session()
|
||||
self.assertTrue(auth.verify_session(cookie))
|
||||
warning = '\n'.join(captured.output)
|
||||
self.assertIn('Auth session persistence failed', warning)
|
||||
self.assertIn('.sessions.json', warning)
|
||||
self.assertIn(str(_TEST_STATE), warning)
|
||||
|
||||
def test_signing_key_read_failure_warns_with_state_dir_and_generates_key(self) -> None:
|
||||
"""Unreadable signing keys must warn and fall back to a fresh key."""
|
||||
key_file = _TEST_STATE / '.signing_key'
|
||||
key_file.write_text('stub')
|
||||
with mock.patch.object(Path, 'read_bytes', side_effect=OSError('read failed')):
|
||||
with self.assertLogs('api.auth', level='WARNING') as captured:
|
||||
key = auth._load_key('.signing_key')
|
||||
self.assertIsInstance(key, bytes)
|
||||
self.assertEqual(len(key), 32)
|
||||
warning = '\n'.join(captured.output)
|
||||
self.assertIn('Auth key read failed', warning)
|
||||
self.assertIn('.signing_key', warning)
|
||||
self.assertIn(str(_TEST_STATE), warning)
|
||||
|
||||
def test_signing_key_persist_failure_warns_with_state_dir(self) -> None:
|
||||
"""Key write failures must warn and still return a generated key."""
|
||||
with mock.patch.object(Path, 'write_bytes', side_effect=OSError('write failed')):
|
||||
with self.assertLogs('api.auth', level='WARNING') as captured:
|
||||
key = auth._load_key('.signing_key')
|
||||
self.assertIsInstance(key, bytes)
|
||||
self.assertEqual(len(key), 32)
|
||||
warning = '\n'.join(captured.output)
|
||||
self.assertIn('Auth key persistence failed', warning)
|
||||
self.assertIn('.signing_key', warning)
|
||||
self.assertIn(str(_TEST_STATE), warning)
|
||||
|
||||
def test_sessions_file_wrong_type_starts_fresh(self) -> None:
|
||||
"""A sessions file containing a non-dict must be ignored."""
|
||||
|
|
|
|||
Loading…
Reference in a new issue