From ca6c527e32fa9a6bd31f9c3dd64b31050de400b7 Mon Sep 17 00:00:00 2001 From: retoor Date: Sun, 26 Jul 2026 19:58:18 +0200 Subject: [PATCH] Resolve the primary administrator to an account that can authenticate The primary administrator was the earliest Admin by created_at with no further condition, so a soft-deleted or deactivated account could hold the role and then be refused by the api-key path, leaving nobody able to use the database API, the backup download or cross-owner container management. Rows with no recorded signup time also sorted ahead of every real account. Scan the earliest admins instead and take the first that is neither deleted nor deactivated, with missing timestamps sorted last. Also stop the projects listing returning 500 when project_type or description is NULL (the dict default never applies to an existing NULL column), align the issue test fixture with its unit twin so a combined run cannot collide on a fixed uid, read the settings value from the database rather than a stale per-process cache, and give the seeded and fixture admins the is_active and created_at fields that every real signup writes. --- devplacepy/database/users.py | 21 +++++- devplacepy/templates/projects.html | 4 +- tests/api/admin/services/config.py | 11 ++- tests/api/issues/create.py | 7 +- tests/conftest.py | 20 ++++++ tests/unit/database/users.py | 69 +++++++++++++++++++ .../services/devii/container/controller.py | 4 ++ tests/unit/utils.py | 4 ++ 8 files changed, 132 insertions(+), 8 deletions(-) create mode 100644 tests/unit/database/users.py diff --git a/devplacepy/database/users.py b/devplacepy/database/users.py index e4375918..d73f3e17 100644 --- a/devplacepy/database/users.py +++ b/devplacepy/database/users.py @@ -15,6 +15,9 @@ def get_users_by_uids(uids): _admins_cache = TTLCache(ttl=300, max_size=4) +# The primary administrator must be an account that can actually authenticate, so scan a +# few of the earliest admins and skip any that are soft-deleted or deactivated. +PRIMARY_ADMIN_CANDIDATES = 50 def invalidate_admins_cache() -> None: @@ -71,6 +74,12 @@ def get_online_users(cutoff_iso: str, limit: int = 30) -> list: ) +def _can_hold_primary_admin(row, tracks_active): + if row.get("deleted_at"): + return False + return not tracks_active or bool(row.get("is_active")) + + def get_primary_admin_uid(): sync_local_cache("admins", _admins_cache) cached = _admins_cache.get("primary") @@ -80,11 +89,17 @@ def get_primary_admin_uid(): return None rows = list( db.query( - "SELECT uid FROM users WHERE role = 'Admin' " - "ORDER BY created_at ASC, id ASC LIMIT 1" + "SELECT * FROM users WHERE role = 'Admin' " + "ORDER BY (created_at IS NULL OR created_at = ''), created_at ASC, id ASC " + "LIMIT :cap", + cap=PRIMARY_ADMIN_CANDIDATES, ) ) - primary = rows[0]["uid"] if rows else None + tracks_active = "is_active" in db["users"].columns + primary = next( + (row["uid"] for row in rows if _can_hold_primary_admin(row, tracks_active)), + None, + ) _admins_cache.set("primary", primary or "") return primary diff --git a/devplacepy/templates/projects.html b/devplacepy/templates/projects.html index 6e1003d7..966c8e06 100644 --- a/devplacepy/templates/projects.html +++ b/devplacepy/templates/projects.html @@ -69,10 +69,10 @@ -
{{ render_content(project.get('description', '')[:200]) }}
+
{{ render_content((project.get('description') or '')[:200]) }}
- {{ project.get('project_type', 'software').replace('_', ' ') }} + {{ (project.get('project_type') or 'software').replace('_', ' ') }} {% if project.get('platforms') %}
{% for p in project['platforms'].split(',') %} diff --git a/tests/api/admin/services/config.py b/tests/api/admin/services/config.py index 963b6194..7e3eb6bc 100644 --- a/tests/api/admin/services/config.py +++ b/tests/api/admin/services/config.py @@ -4,7 +4,13 @@ import time import pytest import requests from tests.conftest import BASE_URL -from devplacepy.database import get_table, refresh_snapshot, set_setting, get_setting +from devplacepy.database import ( + clear_settings_cache, + get_setting, + get_table, + refresh_snapshot, + set_setting, +) JSON = {"Accept": "application/json"} _counter_cfg = [0] @@ -58,6 +64,9 @@ def test_admin_saves_service_config(seeded_db): assert r.status_code == 200, r.text[:300] assert r.json().get("ok") is True refresh_snapshot() + # the server persisted it in its own process; drop this process's 60s settings + # cache so the assertion reads the database rather than a pre-save snapshot + clear_settings_cache() assert get_setting("news_ai_model", "") == new_model finally: if original: diff --git a/tests/api/issues/create.py b/tests/api/issues/create.py index 66d5e622..c1ea0130 100644 --- a/tests/api/issues/create.py +++ b/tests/api/issues/create.py @@ -157,7 +157,9 @@ def _make_user_issues_gitea(): _counter_issues_gitea[0] += 1 uid = f"issuetest-user-{_counter_issues_gitea[0]}" username = f"issuetester{_counter_issues_gitea[0]}" - get_table("users").insert( + # upsert, not insert: tests/unit/services/gitea.py is a twin fixture sharing this + # uid namespace with its own counter, so a combined unit+api run collides on the uid + get_table("users").upsert( { "uid": uid, "username": username, @@ -165,7 +167,8 @@ def _make_user_issues_gitea(): "xp": 0, "level": 1, "is_active": True, - } + }, + ["uid"], ) return uid, username def _unread(user_uid): diff --git a/tests/conftest.py b/tests/conftest.py index ceb51296..ef1386d5 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -14,6 +14,10 @@ SCREENSHOT_DIR = Path("/tmp/devplace_test_screenshots") PORT = int(os.environ.get("DEVPLACE_TEST_PORT", "10501")) BASE_URL = f"http://127.0.0.1:{PORT}" +# Older than any user a fixture can create, so the seeded admin stays primary administrator. +PRIMARY_ADMIN_CREATED_AT = "2000-01-01T00:00:00" +# The cross-worker cache-version read is 1s; give the server a beat to see the new admin set. +CACHE_VERSION_PROPAGATION_SECONDS = 1.5 _TEST_DB = tempfile.NamedTemporaryFile(suffix=f"_{PORT}.db", delete=False) _TEST_DB.close() @@ -317,6 +321,22 @@ def seeded_db(app_server): row = users_table.find_one(username=username) if row and row.get("role") != role: users_table.update({"uid": row["uid"], "role": role}, ["uid"]) + + # The primary administrator is the earliest-created Admin, and unit-tier fixtures + # create their own admins in this same database before the api tier seeds alice. + # Back-date her so she is unambiguously the platform's first administrator, which + # is what every primary-admin test (dbapi, backup download, container isolation) + # assumes. Without this, a combined unit+api run hands primary admin to a fixture + # user and those tests 403. + from devplacepy.database import invalidate_admins_cache as _invalidate_admins + + alice_row = users_table.find_one(username="alice_test") + if alice_row: + users_table.update( + {"uid": alice_row["uid"], "created_at": PRIMARY_ADMIN_CREATED_AT}, ["uid"] + ) + _invalidate_admins() + time.sleep(CACHE_VERSION_PROPAGATION_SECONDS) return { "alice": { "username": "alice_test", diff --git a/tests/unit/database/users.py b/tests/unit/database/users.py new file mode 100644 index 00000000..8fe0c0e0 --- /dev/null +++ b/tests/unit/database/users.py @@ -0,0 +1,69 @@ +# retoor + +def test_primary_admin_ignores_accounts_without_a_signup_time(local_db): + from devplacepy.database import get_primary_admin_uid, invalidate_admins_cache + + users = local_db["users"] + users.insert( + { + "uid": "pa-real", + "username": "pa-real", + "role": "Admin", + "created_at": "2020-01-01T00:00:00", + "deleted_at": None, + } + ) + users.insert( + { + "uid": "pa-null", + "username": "pa-null", + "role": "Admin", + "created_at": None, + "deleted_at": None, + } + ) + users.insert( + { + "uid": "pa-blank", + "username": "pa-blank", + "role": "Admin", + "created_at": "", + "deleted_at": None, + } + ) + invalidate_admins_cache() + try: + assert get_primary_admin_uid() == "pa-real" + finally: + for uid in ("pa-real", "pa-null", "pa-blank"): + users.delete(uid=uid) + invalidate_admins_cache() + + +def test_primary_admin_skips_deactivated_and_deleted_founders(local_db): + from devplacepy.database import get_primary_admin_uid, invalidate_admins_cache + + users = local_db["users"] + seeded = [ + ("pa-deleted", "2001-01-01T00:00:00", True, "2020-01-01T00:00:00"), + ("pa-inactive", "2002-01-01T00:00:00", False, None), + ("pa-usable", "2003-01-01T00:00:00", True, None), + ] + for uid, created_at, active, deleted_at in seeded: + users.insert( + { + "uid": uid, + "username": uid, + "role": "Admin", + "created_at": created_at, + "is_active": active, + "deleted_at": deleted_at, + } + ) + invalidate_admins_cache() + try: + assert get_primary_admin_uid() == "pa-usable" + finally: + for uid, _, _, _ in seeded: + users.delete(uid=uid) + invalidate_admins_cache() diff --git a/tests/unit/services/devii/container/controller.py b/tests/unit/services/devii/container/controller.py index f592e303..a262c6b6 100644 --- a/tests/unit/services/devii/container/controller.py +++ b/tests/unit/services/devii/container/controller.py @@ -23,6 +23,10 @@ def _make_user_at(role, created_at): "email": f"{username}@t.dev", "role": role, "created_at": created_at.isoformat(), + # every real account is created active; the primary administrator must be + # an account that can actually authenticate + "is_active": True, + "deleted_at": None, } ) invalidate_admins_cache() diff --git a/tests/unit/utils.py b/tests/unit/utils.py index d75f9343..6eff68d7 100644 --- a/tests/unit/utils.py +++ b/tests/unit/utils.py @@ -170,6 +170,10 @@ def _seed_user_at(role, created_at): "email": f"pa_{uid[:8]}@t.dev", "role": role, "created_at": created_at.isoformat(), + # every real account is created active; the primary administrator must be + # an account that can actually authenticate + "is_active": True, + "deleted_at": None, } ) invalidate_admins_cache()