fix(permissions): bound delegated global permission edits by platform-level access

CHANGELOG.md

@@ -15,6 +15,11 @@ All notable changes to this project are documented in this file. The format is b
 ## [Unreleased]
 
 ### Security
+- **Behaviour change (#687):** delegated permission edits are bounded by
+  platform-level access. Global user and role permission writes require
+  platform-level `permissions.manage`; tenant-role mappings do not contribute
+  to delegation or target-access checks. API-token scope caps still apply;
+  unscoped platform admins remain unrestricted.
 - The release workflow passes dispatch inputs to its scripts through the
   environment instead of templating them into shell source, and runs
   read-only except for the publish jobs (OIDC) and `finalize` (push + GitHub

docs/framework/permissions.md

@@ -73,6 +73,24 @@ Host apps customize this by:
 
 The framework ships only `admin: ["*"]`. The wildcard grants every declared permission.
 
+## Delegated permission editing
+
+User and role permission assignments are global. Delegated edits require
+`permissions.manage` at the platform level and are bounded by platform-level
+access: global roles, principal claims, and direct grants (including registered
+grant sources), capped by API-token scopes. Synthetic `tenant:*` role mappings
+do not contribute to this ceiling, even when the active tenant grants the same
+permission. `resolve_principal_permissions(..., include_tenant_roles=False)`
+resolves this ceiling without changing ordinary tenant-aware authorization or
+the request's cached permissions. Credential validity still uses tenant-aware
+permissions, so a token's required `api_tokens.use` may come from its tenant.
+
+Delegated actors may add only keys within that ceiling and may edit only users
+or roles whose existing permissions are within it. This also bounds retaining
+or removing existing grants. Admin targets and users whose provider cannot
+report their roles remain out of reach; unscoped platform admins remain
+unrestricted. API refusals return 403; view writes redirect with a form error.
+
 ## Runtime permission sources
 
 `register_permissions` runs before the database is open, so it cannot declare permissions for resources admins create later (per-record-type keys such as `records.product.edit`). Register a **source** instead:

docs/modules/permissions.md

@@ -29,14 +29,16 @@ module still depends on `Users`.
 All require authentication. Read endpoints need `permissions.view`; mutate endpoints need `permissions.manage`.
 
 **Behaviour change (#687):** `permissions.manage` alone does not allow escalation.
-Role and user updates refuse added keys the actor does not hold, using the same
-resolved permissions as authorization (roles, direct grants, other grant sources,
-and token scopes). User updates also refuse targets with permissions beyond the
-actor's, admin targets, or targets whose provider cannot report their roles.
-Unscoped admins are unrestricted. Refusals return HTTP 403 with a reason in
-`detail`, on both API and Inertia write routes. Existing role keys may be retained
-or removed without holding them; unregistered submitted keys are still ignored and
-stored grants for unavailable modules are preserved.
+Role and user updates require platform-level `permissions.manage` and refuse
+added keys outside the actor's platform-level access (global roles, claims,
+direct grants, other grant sources, and token scopes). Synthetic `tenant:*`
+roles do not contribute to this ceiling. Updates also refuse user or role
+targets with permissions beyond that ceiling, admin targets, or users whose
+provider cannot report their roles. Unscoped admins are unrestricted. API
+refusals return HTTP 403 with a reason in `detail`; Inertia writes redirect with
+a form error. Existing role keys may be retained or removed only when the role
+is within reach; unregistered submitted keys are still ignored and stored grants
+for unavailable modules are preserved. See [delegated permission editing](/framework/permissions#delegated-permission-editing).
 
 | Method + path | Body / response | Permission |
 |---|---|---|

framework/hosting/simple_module_hosting/permissions.py

@@ -9,6 +9,7 @@
 from simple_module_core.app_state import get_services
 from simple_module_core.permission_denied import PERMISSION_DENIED_PREFIX, PermissionDenied
 from simple_module_core.permissions import DEFAULT_ROLE_PERMISSIONS, WILDCARD, grants
+from simple_module_core.tenancy import is_tenant_role
 
 from simple_module_hosting._anonymous import demote_to_anonymous
 from simple_module_hosting.token_guard import GATE_PERMISSION
@@ -48,14 +49,22 @@ def resolve_permissions(
     return permissions
 
 
-def _role_and_claimed(user: Any, role_map: dict[str, list[str]] | None) -> set[str]:
+def _role_and_claimed(
+    user: Any,
+    role_map: dict[str, list[str]] | None,
+    *,
+    include_tenant_roles: bool = True,
+) -> set[str]:
     """The role-derived set plus the principal's own ``permissions`` (GH #436).
 
     Read duck-typed (``UserContext.permissions`` lives in the auth module, which
     framework code must not import). The claimed keys never include the
     wildcard: like a grant source, a token or IdP claim cannot make an admin.
     """
-    permissions = resolve_permissions(getattr(user, "roles", None) or [], role_map=role_map)
+    roles = getattr(user, "roles", None) or []
+    if not include_tenant_roles:
+        roles = [r for r in roles if not is_tenant_role(r)]
+    permissions = resolve_permissions(roles, role_map=role_map)
     permissions.update(p for p in getattr(user, "permissions", None) or () if p != WILDCARD)
     return permissions
 
@@ -87,6 +96,8 @@ async def resolve_principal_permissions(
     request: Request,
     user: Any,
     registry: PermissionRegistry | None,
+    *,
+    include_tenant_roles: bool = True,
 ) -> set[str]:
     """Everything *user* holds: its roles' permissions plus every grant source.
 
@@ -101,9 +112,20 @@ async def resolve_principal_permissions(
     A token principal's set is then capped at its scopes (:func:`apply_scope_cap`),
     and is empty when its account no longer holds the permission the
     credential requires (:func:`lacks_required_permission`).
+
+    Global delegation checks exclude synthetic tenant roles with
+    ``include_tenant_roles=False``. Credential validity remains tenant-aware;
+    scope caps still apply, and the request's cached set is left untouched.
     """
-    held = await _held_permissions(request, user, registry)
-    if lacks_required_permission(user, held):
+    held = await _held_permissions(
+        request, user, registry, include_tenant_roles=include_tenant_roles
+    )
+    credential_held = held
+    # Credential validity stays tenant-aware; only resolve the rest when needed.
+    if not include_tenant_roles and getattr(user, "required_permission", None):
+        role_map = registry.role_map if registry is not None else None
+        credential_held = held | resolve_permissions(getattr(user, "roles", None) or [], role_map)
+    if lacks_required_permission(user, credential_held):
         return set()
     return apply_scope_cap(held, user)
 
@@ -112,10 +134,12 @@ async def _held_permissions(
     request: Request,
     user: Any,
     registry: PermissionRegistry | None,
+    *,
+    include_tenant_roles: bool = True,
 ) -> set[str]:
     """What *user*'s account holds right now, before any scope cap."""
     role_map = registry.role_map if registry is not None else None
-    permissions = _role_and_claimed(user, role_map)
+    permissions = _role_and_claimed(user, role_map, include_tenant_roles=include_tenant_roles)
     if registry is None or WILDCARD in permissions:
         return permissions
     for source in registry.grant_sources:

framework/hosting/tests/test_hosting_platform_permissions.py

@@ -0,0 +1,54 @@
+"""Platform permission resolution preserves credential scope and validity."""
+
+from types import SimpleNamespace
+
+import pytest
+from simple_module_core.permissions import PermissionRegistry
+from simple_module_hosting.permissions import resolve_principal_permissions
+
+
+@pytest.mark.parametrize(
+    "scopes, expected",
+    [
+        (None, {"global.read", "direct.read", "claim.read"}),
+        (["global.read"], {"global.read"}),
+        ([], set()),
+    ],
+)
+async def test_platform_resolution_excludes_tenant_roles(scopes, expected):
+    registry = PermissionRegistry()
+    registry.map_role("editor", ["global.read"])
+    registry.map_role("tenant:owner", ["tenant.read", "api_tokens.use"])
+
+    async def direct_grants(request, user):
+        return {"direct.read", "*"}
+
+    registry.add_grant_source(direct_grants)
+    user = SimpleNamespace(
+        roles=["editor", "tenant:owner"],
+        permissions=["claim.read", "*"],
+        scopes=scopes,
+        required_permission="api_tokens.use",
+    )
+    held = await resolve_principal_permissions(None, user, registry, include_tenant_roles=False)
+    assert held == expected
+    assert user.roles == ["editor", "tenant:owner"]
+    assert "tenant:owner" in registry.role_map
+    if scopes is None:
+        assert await resolve_principal_permissions(None, user, registry) == {
+            "global.read",
+            "direct.read",
+            "claim.read",
+            "tenant.read",
+            "api_tokens.use",
+        }
+
+
+async def test_platform_resolution_still_rejects_invalid_credential():
+    registry = PermissionRegistry()
+    registry.map_role("editor", ["global.read"])
+    user = SimpleNamespace(roles=["editor"], required_permission="api_tokens.use")
+    assert (
+        await resolve_principal_permissions(None, user, registry, include_tenant_roles=False)
+        == set()
+    )

modules/permissions/permissions/write_safety.py

@@ -1,23 +1,24 @@
-"""Delegated grant writes are bounded by the actor's effective access (#687)."""
+"""Delegated permission edits are bounded by platform-level access (#687)."""
 
 from fastapi import HTTPException, Request, status
 from simple_module_core.permissions import ADMIN_ROLE, PermissionRegistry, grants
 from simple_module_core.user_directory import DirectoryUser
-from simple_module_hosting.permissions import (
-    ensure_resolved_permissions,
-    resolve_principal_permissions,
-)
+from simple_module_hosting.permissions import resolve_principal_permissions
+
+from permissions.constants import PERM_MANAGE
 
 
 class PermissionWriteRefused(HTTPException):
-    """A 403 with a reason but no ``required_permission``: the actor holds
-    ``permissions.manage``, so nothing may claim it is missing. View routes turn
-    it into a form error so the edit screen can show the reason."""
+    """A 403 with a reason that view routes turn into a form error."""
 
     def __init__(self, reason: str) -> None:
         super().__init__(status_code=status.HTTP_403_FORBIDDEN, detail=reason)
 
 
+_PLATFORM_ONLY = "Global permission edits require platform-level access."
+_MORE_PRIVILEGED_ACCOUNT = "You cannot manage a more privileged account."
+
+
 def _refuse(reason: str) -> PermissionWriteRefused:
     return PermissionWriteRefused(reason)
 
@@ -32,7 +33,8 @@ async def check_permission_write(
 ) -> None:
     """Trusted in-process calls and unscoped admins may assign any registered key.
 
-    A delegated actor may change a user or role only if it already holds
+    A delegated actor needs platform-level ``permissions.manage`` and may
+    change a user or role only if its platform-level access already holds
     everything the target holds: otherwise it could strip access from (or
     reshape) principals stronger than itself. ``role`` is ``(name, keys)``.
     """
@@ -41,14 +43,21 @@ async def check_permission_write(
     actor = getattr(request.state, "user", None)
     if actor is not None and ADMIN_ROLE in actor.roles and not actor.is_scoped:
         return
-    held = await ensure_resolved_permissions(request)
+    if actor is None:
+        # No principal: skip the grant sources, which expect a real user.
+        raise _refuse(_PLATFORM_ONLY)
+    held = await resolve_principal_permissions(request, actor, registry, include_tenant_roles=False)
+    if not grants(held, PERM_MANAGE):
+        raise _refuse(_PLATFORM_ONLY)
     if target is not None:
         # Unknown provider roles cannot prove that the target is within reach.
         if target.roles is None or ADMIN_ROLE in target.roles:
-            raise _refuse("You cannot manage a more privileged account.")
-        target_permissions = await resolve_principal_permissions(request, target, registry)
+            raise _refuse(_MORE_PRIVILEGED_ACCOUNT)
+        target_permissions = await resolve_principal_permissions(
+            request, target, registry, include_tenant_roles=False
+        )
         if any(not grants(held, key) for key in target_permissions):
-            raise _refuse("You cannot manage a more privileged account.")
+            raise _refuse(_MORE_PRIVILEGED_ACCOUNT)
     if role is not None:
         name, role_keys = role
         if name == ADMIN_ROLE or any(not grants(held, key) for key in role_keys):

modules/permissions/tests/test_permissions_write_safety.py

@@ -1,6 +1,8 @@
 """Delegated permission writes cannot escalate access (#687)."""
 
+import json
 import uuid
+from base64 import b64decode
 from types import SimpleNamespace
 
 import httpx
@@ -12,6 +14,7 @@
 from permissions.endpoints.views import router as view_router
 from permissions.models import RolePermission, UserPermission
 from simple_module_core.permissions import PermissionRegistry
+from simple_module_core.tenancy import TenantRole, tenant_role
 from simple_module_core.user_directory import DirectoryUser, NullUserDirectory
 from sqlalchemy import select
 from starlette.middleware.sessions import SessionMiddleware
@@ -182,3 +185,98 @@ async def test_cannot_edit_admin_role(writes):
         f"/api/permissions/roles/{admin.id}", json={"permissions": ["test.held"]}
     )
     assert response.status_code == 403
+
+
+def _tenant_actor(writes):
+    writes.registry.add_group("Files", ["file_storage.manage"])
+    writes.registry.map_role(tenant_role(TenantRole.OWNER), ["file_storage.manage"])
+    writes.actor.roles = [tenant_role(TenantRole.OWNER)]
+
+
+def _assert_write_response(response, prefix, allowed):
+    api = prefix.startswith("/api")
+    if api:
+        assert response.status_code == (200 if allowed else 403)
+    else:
+        assert response.status_code == 303
+    if not allowed and not api:
+        session = json.loads(b64decode(response.cookies["session"].split(".")[0]))
+        assert session["_errors"]["permissions"]
+
+
+@pytest.mark.parametrize("prefix", ["/api/permissions", "/admin/permissions"])
+@pytest.mark.parametrize("kind", ["roles", "users", "self"])
+@pytest.mark.parametrize(
+    "authority", ["tenant", "role", "direct", "claim", "admin", "scoped", "scoped_allowed"]
+)
+async def test_global_grants_require_platform_authority(writes, prefix, kind, authority):
+    _tenant_actor(writes)
+    allowed = authority not in {"tenant", "scoped"}
+    if authority in {"role", "scoped", "scoped_allowed"}:
+        writes.registry.map_role("platform_editor", ["file_storage.manage"])
+        writes.actor.roles.append("platform_editor")
+    if authority == "scoped":
+        writes.actor.scopes = ["permissions.manage"]
+    elif authority == "scoped_allowed":
+        writes.actor.scopes = ["permissions.manage", "file_storage.manage"]
+        writes.actor.required_permission = "api_tokens.use"
+        writes.registry.map_role(tenant_role(TenantRole.OWNER), ["api_tokens.use"])
+    elif authority == "direct":
+        writes.db.add(
+            UserPermission(user_id=uuid.UUID(writes.actor.id), permission_key="file_storage.manage")
+        )
+        await writes.db.flush()
+    elif authority == "claim":
+        writes.actor.permissions.append("file_storage.manage")
+    elif authority == "admin":
+        writes.actor.roles.append("admin")
+    target_id = {"roles": writes.role.id, "self": writes.actor.id, "users": writes.target.id}[kind]
+    route_kind = "roles" if kind == "roles" else "users"
+    response = await writes.client.put(
+        f"{prefix}/{route_kind}/{target_id}", json={"permissions": ["file_storage.manage"]}
+    )
+    _assert_write_response(response, prefix, allowed)
+    model = RolePermission if kind == "roles" else UserPermission
+    condition = (
+        model.role_name == "user" if kind == "roles" else model.user_id == uuid.UUID(str(target_id))
+    )
+    keys = (await writes.db.execute(select(model.permission_key).where(condition))).scalars().all()
+    # A self edit of an existing direct grant is allowed too.
+    assert keys == (["file_storage.manage"] if allowed else [])
+
+
+@pytest.mark.parametrize("prefix", ["/api/permissions", "/admin/permissions"])
+@pytest.mark.parametrize("kind", ["roles", "users"])
+@pytest.mark.parametrize("kept", [[], ["file_storage.manage"]])
+async def test_global_target_reach_uses_platform_authority(writes, prefix, kind, kept):
+    _tenant_actor(writes)
+    if kind == "roles":
+        writes.db.add(RolePermission(role_name="user", permission_key="file_storage.manage"))
+        target_id = writes.role.id
+    else:
+        writes.db.add(
+            UserPermission(
+                user_id=uuid.UUID(writes.target.id), permission_key="file_storage.manage"
+            )
+        )
+        target_id = writes.target.id
+    await writes.db.flush()
+    response = await writes.client.put(f"{prefix}/{kind}/{target_id}", json={"permissions": kept})
+    _assert_write_response(response, prefix, False)
+    model = RolePermission if kind == "roles" else UserPermission
+    keys = (await writes.db.execute(select(model.permission_key))).scalars().all()
+    assert keys == ["file_storage.manage"]
+
+
+@pytest.mark.parametrize("prefix", ["/api/permissions", "/admin/permissions"])
+@pytest.mark.parametrize("kind", ["roles", "users"])
+@pytest.mark.parametrize("keys", [[], ["test.held"]])
+async def test_tenant_manage_does_not_authorize_global_writes(writes, prefix, kind, keys):
+    writes.actor.permissions = ["test.held"]
+    writes.actor.roles = [tenant_role(TenantRole.ADMIN)]
+    writes.registry.map_role(tenant_role(TenantRole.ADMIN), ["permissions.manage"])
+    target_id = writes.role.id if kind == "roles" else writes.target.id
+    response = await writes.client.put(f"{prefix}/{kind}/{target_id}", json={"permissions": keys})
+    _assert_write_response(response, prefix, False)
+    model = RolePermission if kind == "roles" else UserPermission
+    assert (await writes.db.execute(select(model.permission_key))).scalars().all() == []