fix: seed builtin admin only on empty DB; recover page-load sessions with one refresh
Auth seeding (last-resort guard) - `_seed_builtin_admin()` in get_db() now skips when VACUUM_WALL_SEED_BUILTIN_ADMIN=0 or when the users table already contains any user — previously a fresh service start after a non-default bootstrap (e.g. --mgmt-user alice) seeded a hard-coded `admin` with an unrecoverable random password, shadowing the operator's account - bootstrap_auth.py sets VACUUM_WALL_SEED_BUILTIN_ADMIN=0: bootstrap creates the operator user itself on a fresh install, so exactly one account exists and no seeded admin can appear Frontend (session recovery) - on page load/restore the in-memory TTL timer is gone, so a valid 7-day refresh token could sit in sessionStorage while the access token is already expired server-side: the session `check` now attempts exactly one refresh (POST /api/auth/refresh with the stored refresh token) on 401 before treating the session as dead - extract shared `_doRefresh()` used by both the `check` 401 fallback and the `refresh` action (removes the duplicated rotation logic) Tests - update seeding tests to the new any-user-present check; add test_seed_skipped_when_users_exist, test_seed_skipped_via_env, test_bootstrap_flow_creates_exactly_one_user, and the auth-model JS test suite (tests/test-auth-model.js) Docs - AGENTS.md: document VACUUM_WALL_SEED_BUILTIN_ADMIN - architecture.md / hoover.md / security.md: describe the bootstrap check 401 → one-refresh fallback path
This commit is contained in:
+61
-9
@@ -26,6 +26,7 @@ from lib.auth import (
|
||||
from lib.auth_users import (
|
||||
create_user,
|
||||
delete_user,
|
||||
find_user,
|
||||
get_user,
|
||||
list_users,
|
||||
reset_password,
|
||||
@@ -53,6 +54,7 @@ def _db_reset():
|
||||
reset_db_for_test()
|
||||
old_backend = os.environ.pop("VACUUM_WALL_DB_BACKEND", None)
|
||||
old_path = os.environ.pop("VACUUM_WALL_DB_PATH", None)
|
||||
old_seed = os.environ.pop("VACUUM_WALL_SEED_BUILTIN_ADMIN", None)
|
||||
|
||||
os.environ["VACUUM_WALL_DB_BACKEND"] = "sqlite"
|
||||
os.environ["VACUUM_WALL_DB_PATH"] = ":memory:"
|
||||
@@ -64,6 +66,8 @@ def _db_reset():
|
||||
os.environ["VACUUM_WALL_DB_BACKEND"] = old_backend
|
||||
if old_path is not None:
|
||||
os.environ["VACUUM_WALL_DB_PATH"] = old_path
|
||||
if old_seed is not None:
|
||||
os.environ["VACUUM_WALL_SEED_BUILTIN_ADMIN"] = old_seed
|
||||
|
||||
|
||||
@pytest.fixture
|
||||
@@ -1372,8 +1376,8 @@ class TestBuiltinAdminSeeding:
|
||||
assert user["permissions"] == {s: "rw" for s in ALL_SUBSYSTEMS}
|
||||
|
||||
def test_seed_noop_when_admin_exists(self) -> None:
|
||||
"""Seeding is a no-op when the admin user already exists."""
|
||||
from lib.db import Q_SELECT_USER_BY_NAME, _seed_builtin_admin
|
||||
"""Seeding is a no-op when any user (here: admin) already exists."""
|
||||
from lib.db import Q_SELECT_ALL_USERS, _seed_builtin_admin
|
||||
|
||||
db = get_db()
|
||||
_seed_builtin_admin(db)
|
||||
@@ -1382,19 +1386,20 @@ class TestBuiltinAdminSeeding:
|
||||
calls = {"n": 0}
|
||||
|
||||
def counting_query(query_id, params=()):
|
||||
if query_id == Q_SELECT_USER_BY_NAME:
|
||||
if query_id == Q_SELECT_ALL_USERS:
|
||||
calls["n"] += 1
|
||||
return real_query(query_id, params)
|
||||
|
||||
with patch.object(db, "query", side_effect=counting_query):
|
||||
_seed_builtin_admin(db)
|
||||
# Early-return path: only the existence check runs.
|
||||
# Early-return path: only the users-present check runs.
|
||||
assert calls["n"] >= 1
|
||||
|
||||
def test_seed_concurrent_lose_race(self) -> None:
|
||||
"""Concurrent seeding: if the insert loses a race, the loser re-checks,
|
||||
finds the winner's admin, and returns instead of raising IntegrityError."""
|
||||
from lib.db import Q_SELECT_USER_BY_NAME, _seed_builtin_admin
|
||||
"""Concurrent seeding: if the users-present check sees a stale (empty)
|
||||
view and the insert then loses the race, the loser re-checks, finds
|
||||
the winner's admin, and returns instead of raising IntegrityError."""
|
||||
from lib.db import Q_SELECT_ALL_USERS, _seed_builtin_admin
|
||||
|
||||
db = get_db()
|
||||
# get_db() already seeded admin for this fresh in-memory DB.
|
||||
@@ -1404,13 +1409,60 @@ class TestBuiltinAdminSeeding:
|
||||
calls = {"n": 0}
|
||||
|
||||
def fake_query(query_id, params=()):
|
||||
if query_id == Q_SELECT_USER_BY_NAME and params and params[0] == "admin":
|
||||
if query_id == Q_SELECT_ALL_USERS:
|
||||
calls["n"] += 1
|
||||
if calls["n"] == 1:
|
||||
return [] # stale view: existence check misses concurrent seeder
|
||||
return [] # stale view: users-present check misses seeder
|
||||
return real_query(query_id, params)
|
||||
|
||||
with patch.object(db, "query", side_effect=fake_query):
|
||||
_seed_builtin_admin(db) # must not raise
|
||||
|
||||
assert get_user("admin") is not None
|
||||
|
||||
def test_seed_skipped_when_users_exist(self, tmp_path) -> None:
|
||||
"""Last-resort rule: a DB that already has users gets no seeded
|
||||
admin — a non-default bootstrap user must not be shadowed."""
|
||||
from lib.auth_users import ALL_SUBSYSTEMS
|
||||
from lib.db_sqlite import SQLiteBackend
|
||||
|
||||
db_file = tmp_path / "auth.db"
|
||||
|
||||
# A prior process (the bootstrap run) created the operator account.
|
||||
first = SQLiteBackend(str(db_file))
|
||||
first.init_tables()
|
||||
first.run(Q_INSERT_USER, ("alice", hash_password("alice-pw"), "jwt-secret"))
|
||||
for sub in ALL_SUBSYSTEMS:
|
||||
first.run(Q_UPSERT_PERMISSION, ("alice", sub, "rw"))
|
||||
|
||||
# A fresh service process initializes the same DB file.
|
||||
reset_db_for_test()
|
||||
os.environ["VACUUM_WALL_DB_PATH"] = str(db_file)
|
||||
db = get_db()
|
||||
|
||||
usernames = {row["username"] for row in db.query(Q_SELECT_ALL_USERS)}
|
||||
assert usernames == {"alice"}
|
||||
assert get_user("admin") is None
|
||||
|
||||
def test_seed_skipped_via_env(self) -> None:
|
||||
"""VACUUM_WALL_SEED_BUILTIN_ADMIN=0 (set by bootstrap_auth.py)
|
||||
suppresses the seed even on a completely empty DB."""
|
||||
os.environ["VACUUM_WALL_SEED_BUILTIN_ADMIN"] = "0"
|
||||
db = get_db()
|
||||
assert db.query(Q_SELECT_ALL_USERS) == []
|
||||
assert find_user("admin") is None
|
||||
|
||||
def test_bootstrap_flow_creates_exactly_one_user(self) -> None:
|
||||
"""Simulates bootstrap_auth.py's main() on a fresh DB: with the seed
|
||||
suppressed, bootstrap creates exactly the operator account and no
|
||||
hardcoded admin shadow (regression for --mgmt-user != admin leaving
|
||||
an unrecoverable superuser)."""
|
||||
from lib.auth_users import ALL_SUBSYSTEMS
|
||||
|
||||
os.environ["VACUUM_WALL_SEED_BUILTIN_ADMIN"] = "0"
|
||||
assert find_user("alice") is None
|
||||
create_user("alice", "secret123", {sub: "rw" for sub in ALL_SUBSYSTEMS})
|
||||
|
||||
usernames = {row["username"] for row in get_db().query(Q_SELECT_ALL_USERS)}
|
||||
assert usernames == {"alice"}
|
||||
assert find_user("admin") is None
|
||||
|
||||
Reference in New Issue
Block a user