aboutsummaryrefslogtreecommitdiffstats
path: root/packages/meshbay-hub
diff options
context:
space:
mode:
authorChristophe Besson <cbesson@gmail.com>2026-09-01 17:48:56 +0200
committerChristophe Besson <cbesson@gmail.com>2026-09-01 17:48:56 +0200
commit9f95bf2deaec0c31e5fac95e1068db40b878e217 (patch)
treed8a0f5f5ca2270d047a272b02f04796ee2606bba /packages/meshbay-hub
parent8a6294b0412a86f378c6e2e937c28de64a903c91 (diff)
downloadmeshbay-9f95bf2deaec0c31e5fac95e1068db40b878e217.tar.gz
fix(hub): moderator can no longer grant admin or hard-revoke accounts
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 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011pG75yGK3NthNfyjH74omG
Diffstat (limited to 'packages/meshbay-hub')
-rw-r--r--packages/meshbay-hub/src/meshbay_hub/api/admin.py21
-rw-r--r--packages/meshbay-hub/src/meshbay_hub/api/deps.py17
-rw-r--r--packages/meshbay-hub/tests/test_admin.py39
3 files changed, 71 insertions, 6 deletions
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
diff --git a/packages/meshbay-hub/tests/test_admin.py b/packages/meshbay-hub/tests/test_admin.py
index 51b233a..ad48487 100644
--- a/packages/meshbay-hub/tests/test_admin.py
+++ b/packages/meshbay-hub/tests/test_admin.py
@@ -5,7 +5,6 @@ Integration tests for the admin/moderation panel API.
import pytest
from cryptography.hazmat.primitives.asymmetric.ed25519 import Ed25519PrivateKey
from cryptography.hazmat.primitives.asymmetric.x25519 import X25519PrivateKey
-
from meshbay_common.crypto import pk_to_b64
from meshbay_hub.api.deps import set_admin_usernames
@@ -162,6 +161,44 @@ async def test_admin_change_role(client):
@pytest.mark.asyncio
+async def test_moderator_cannot_change_roles_or_revoke(client):
+ """A moderator suspends and restores (reversible); it cannot promote anyone
+ or hard-revoke, which would be a path from the moderation role to full
+ instance control."""
+ _, admin_token = await _setup_admin(client, "boss")
+ mod_id = await _register(client, "moduser")
+ await client.patch(f"/v1/admin/users/{mod_id}", json={"role": "moderator"},
+ headers={"Authorization": f"Bearer {admin_token}"})
+ mod_token = await _login(client, "moduser")
+ mod_h = {"Authorization": f"Bearer {mod_token}"}
+
+ victim = await _register(client, "victim", email="v@x.com")
+
+ # No promoting an accomplice.
+ r = await client.patch(f"/v1/admin/users/{victim}", json={"role": "admin"},
+ headers=mod_h)
+ assert r.status_code == 403
+
+ # No hard revocation.
+ r = await client.patch(f"/v1/admin/users/{victim}", json={"status": "revoked"},
+ headers=mod_h)
+ assert r.status_code == 403
+
+ # No touching an admin's account.
+ admin2 = await _register(client, "admin2", email="a2@x.com")
+ await client.patch(f"/v1/admin/users/{admin2}", json={"role": "admin"},
+ headers={"Authorization": f"Bearer {admin_token}"})
+ r = await client.patch(f"/v1/admin/users/{admin2}", json={"status": "suspended"},
+ headers=mod_h)
+ assert r.status_code == 403
+
+ # Suspending a plain user is still fine.
+ r = await client.patch(f"/v1/admin/users/{victim}", json={"status": "suspended"},
+ headers=mod_h)
+ assert r.status_code == 200
+
+
+@pytest.mark.asyncio
async def test_admin_cannot_modify_self(client):
admin_id, token = await _setup_admin(client)
r = await client.patch(f"/v1/admin/users/{admin_id}",