From 9f95bf2deaec0c31e5fac95e1068db40b878e217 Mon Sep 17 00:00:00 2001 From: Christophe Besson Date: Tue, 1 Sep 2026 17:48:56 +0200 Subject: fix(hub): moderator can no longer grant admin or hard-revoke accounts MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit admin_patch_user was gated by require_moderator but wrote `role` and `status` with no further check. A moderator could promote any account (an accomplice) to admin, demote an existing admin, or set status="revoked" — a straight path from the moderation role to full instance control. Split authorization by field: status between active/suspended stays at require_moderator (reversible content moderation); role changes, status="revoked", and touching an admin's account at all now require user_is_admin(current_user) (new helper in deps.py, alongside the existing require_admin/require_moderator). Regression test: test_moderator_cannot_change_roles_or_revoke. Third security review, finding H1. Co-Authored-By: Claude Sonnet 5 Claude-Session: https://claude.ai/code/session_011pG75yGK3NthNfyjH74omG --- packages/meshbay-hub/src/meshbay_hub/api/admin.py | 21 ++++++++++++++++++++- packages/meshbay-hub/src/meshbay_hub/api/deps.py | 17 +++++++++++++---- 2 files changed, 33 insertions(+), 5 deletions(-) (limited to 'packages/meshbay-hub/src') diff --git a/packages/meshbay-hub/src/meshbay_hub/api/admin.py b/packages/meshbay-hub/src/meshbay_hub/api/admin.py index ab6fa07..4960674 100644 --- a/packages/meshbay-hub/src/meshbay_hub/api/admin.py +++ b/packages/meshbay-hub/src/meshbay_hub/api/admin.py @@ -14,7 +14,7 @@ from sqlalchemy import func, select from sqlalchemy.ext.asyncio import AsyncSession from meshbay_hub.auth import decrypt_email -from meshbay_hub.api.deps import require_admin, require_moderator +from meshbay_hub.api.deps import require_admin, require_moderator, user_is_admin from meshbay_hub.api.revocation import get_connected_node_count, is_node_connected from meshbay_hub.db.engine import get_db from meshbay_hub.db.models import Group, GroupMember, IPLog, Node, User @@ -196,6 +196,25 @@ async def admin_patch_user( if user.id == current_user.id: raise HTTPException(status_code=400, detail="Cannot modify your own account") + # A moderator suspends and restores accounts — reversible content moderation. + # Changing what someone *is* (their role), and the one irreversible status + # (`revoked`, which is signed and broadcast to every node), are administrative. + # Without this split a moderator could promote an accomplice to admin, or + # revoke every admin, entirely from the moderation role. `admin_delete_user` + # already draws this exact line for the same reason. + privileged = body.role is not None or body.status == "revoked" + if privileged and not user_is_admin(current_user): + raise HTTPException( + status_code=403, + detail="Changing a role, or revoking an account, requires admin rights") + + # An admin's account is not a moderator's to touch at all — not their role, + # not their status. + if user_is_admin(user) and not user_is_admin(current_user): + raise HTTPException( + status_code=403, + detail="Only an admin can change another admin's account") + from meshbay_hub.api.notifications import create_notification if body.role is not None: diff --git a/packages/meshbay-hub/src/meshbay_hub/api/deps.py b/packages/meshbay-hub/src/meshbay_hub/api/deps.py index 1bf57a4..907a481 100644 --- a/packages/meshbay-hub/src/meshbay_hub/api/deps.py +++ b/packages/meshbay-hub/src/meshbay_hub/api/deps.py @@ -72,11 +72,21 @@ async def require_user_scope( return current_user +def user_is_admin(user: User) -> bool: + """Admin by DB role or by the config allow-list. Use inside a handler that + already depends on `require_moderator` but has to draw the admin line for + one field (see `admin_patch_user`).""" + return user.role == "admin" or user.username in _admin_usernames + + +def user_is_moderator(user: User) -> bool: + return user.role in ("moderator", "admin") or user.username in _admin_usernames + + async def require_moderator( current_user: User = Depends(get_current_user), ) -> User: - if current_user.role not in ("moderator", "admin") \ - and current_user.username not in _admin_usernames: + if not user_is_moderator(current_user): raise HTTPException(status_code=status.HTTP_403_FORBIDDEN, detail="Moderator access required") return current_user @@ -85,8 +95,7 @@ async def require_moderator( async def require_admin( current_user: User = Depends(get_current_user), ) -> User: - if current_user.role != "admin" \ - and current_user.username not in _admin_usernames: + if not user_is_admin(current_user): raise HTTPException(status_code=status.HTTP_403_FORBIDDEN, detail="Admin access required") return current_user -- cgit v1.2.3