diff --git a/CHANGELOG.md b/CHANGELOG.md index c754a425..d0eae773 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -101,6 +101,17 @@ All notable changes to this project are documented in this file. The format is b (#363). ### Security +- **Settings scopes on multi-tenant hosts** (#368). The settings routes checked + `settings.view`/`.edit`/`.delete` only, so any holder could write the + host-wide system scope and any tenant's or user's scope by naming it in the + URL. With `multi_tenant` on, a caller now reaches only its own tenant scope + (the request's tenant) and its own user scope; the system scope, other + scopes, `resolve` for another tenant/user, id-based CRUD, the module-settings + API and the `/admin/settings` screens need a platform settings admin — the + new `settings.system` permission (held by `admin` via the wildcard). On a + host **without** a tenant resolver, a user whose record carries a + `tenant_id` is that tenant's admin and never a platform one, even with the + `admin` role. Single-tenant hosts are unchanged. - The tenant header (`tenant_header`) is no longer honoured for an authenticated user without a tenant of their own: such a user could name any tenant. On the legacy path it applies to anonymous requests only; with the diff --git a/docs/framework/multi-tenancy.md b/docs/framework/multi-tenancy.md index e7fd3792..9bfb430f 100644 --- a/docs/framework/multi-tenancy.md +++ b/docs/framework/multi-tenancy.md @@ -289,6 +289,17 @@ async def test_same_tenant(tenant_client): ... ``` +## Host-wide settings + +The `settings` module's scopes follow the same rule (#368): with `multi_tenant` +on, `settings.*` lets a caller manage its **own** tenant scope (the request's +tenant) and its own user scope. The system scope — where every module's +DB-backed configuration lives — other tenants' and users' scopes, and the +`/admin/settings` screens need `settings.system`, which no tenant role should +be mapped to. On a host without a tenant resolver, a user with a `tenant_id` on +their record never counts as a platform admin, whatever their roles: there the +global `admin` role is the only admin role a tenant's operator can have. + ## Unique keys On a tenant-scoped table every business key is per tenant: put `tenant_id` in diff --git a/modules/settings/settings/constants.py b/modules/settings/settings/constants.py index ded7d293..2059fb33 100644 --- a/modules/settings/settings/constants.py +++ b/modules/settings/settings/constants.py @@ -90,11 +90,19 @@ PERM_CREATE: Final = "settings.create" PERM_EDIT: Final = "settings.edit" PERM_DELETE: Final = "settings.delete" -# Edit the *active* tenant's overrides of ``tenant_overridable`` keys — and -# nothing else. Mapped onto ``tenant:owner`` / ``tenant:admin``; the four -# above stay platform-operator permissions (system scope, any tenant's scope). +# Cross-scope platform administration on multi-tenant hosts (GH #368). +# Never granted to tenant roles; see ``settings.scope_guard``. +PERM_SYSTEM: Final = "settings.system" +# Self-service edits to the active tenant's overridable keys (GH #382). PERM_TENANT_EDIT: Final = "settings.tenant.edit" -ALL_PERMISSIONS: Final = (PERM_VIEW, PERM_CREATE, PERM_EDIT, PERM_DELETE, PERM_TENANT_EDIT) +ALL_PERMISSIONS: Final = ( + PERM_VIEW, + PERM_CREATE, + PERM_EDIT, + PERM_DELETE, + PERM_SYSTEM, + PERM_TENANT_EDIT, +) # ── Cache invalidation ─────────────────────────────────────────────── # Published (after commit) for every SYSTEM / TENANT write, keyed per diff --git a/modules/settings/settings/endpoints/api.py b/modules/settings/settings/endpoints/api.py index 7b2e71b8..d1b581ea 100644 --- a/modules/settings/settings/endpoints/api.py +++ b/modules/settings/settings/endpoints/api.py @@ -38,6 +38,13 @@ SettingUpsert, ) from settings.deps import get_setting_service +from settings.scope_guard import ( + require_listable, + require_own_tenant, + require_own_user, + require_platform, + require_resolvable, +) from settings.service import SettingService from settings.tenant_scope import ( is_known_tenant, @@ -57,6 +64,13 @@ _EDIT = [Depends(RequiresPermission(PERM_EDIT))] _DELETE = [Depends(RequiresPermission(PERM_DELETE))] +# On a multi-tenant host the permissions above say what, not whose: each route +# also names the scopes it may touch (GH #368, ``settings.scope_guard``). The +# permission check runs first, so a caller without it still gets its 401/403. +_PLATFORM = [Depends(require_platform)] +_OWN_TENANT = [Depends(require_own_tenant)] +_OWN_USER = [Depends(require_own_user)] + def _not_found() -> HTTPException: return HTTPException(status_code=STATUS_NOT_FOUND, detail=ERR_SETTING_NOT_FOUND) @@ -65,7 +79,7 @@ def _not_found() -> HTTPException: # ── List / filter ─────────────────────────────────────────────────── -@router.get("/", response_model=list[SettingOut], dependencies=_VIEW) +@router.get("/", response_model=list[SettingOut], dependencies=[*_VIEW, Depends(require_listable)]) async def list_settings( scope: SettingScope | None = Query(default=None, alias=QP_SCOPE), scope_id: str = Query(default=SYSTEM_SCOPE_ID, alias=QP_SCOPE_ID), @@ -79,7 +93,9 @@ async def list_settings( # ── Resolution (USER > TENANT > SYSTEM) ───────────────────────────── -@router.get(API_RESOLVE_PATH, response_model=SettingOut, dependencies=_VIEW) +@router.get( + API_RESOLVE_PATH, response_model=SettingOut, dependencies=[*_VIEW, Depends(require_resolvable)] +) async def resolve_setting( key: str, user_id: str | None = Query(default=None, alias=QP_USER_ID), @@ -95,7 +111,7 @@ async def resolve_setting( # ── Scoped (system / tenant / user) ───────────────────────────────── -@router.get(API_SYSTEM_PATH, response_model=SettingOut, dependencies=_VIEW) +@router.get(API_SYSTEM_PATH, response_model=SettingOut, dependencies=[*_VIEW, *_PLATFORM]) async def get_system_setting( key: str, service: SettingService = Depends(get_setting_service) ) -> SettingOut: @@ -105,7 +121,7 @@ async def get_system_setting( return result -@router.put(API_SYSTEM_PATH, response_model=SettingOut, dependencies=_EDIT) +@router.put(API_SYSTEM_PATH, response_model=SettingOut, dependencies=[*_EDIT, *_PLATFORM]) async def upsert_system_setting( key: str, data: SettingUpsert, @@ -114,7 +130,7 @@ async def upsert_system_setting( return await service.upsert_scoped(SettingScope.SYSTEM, SYSTEM_SCOPE_ID, key, data) -@router.delete(API_SYSTEM_PATH, status_code=STATUS_NO_CONTENT, dependencies=_DELETE) +@router.delete(API_SYSTEM_PATH, status_code=STATUS_NO_CONTENT, dependencies=[*_DELETE, *_PLATFORM]) async def delete_system_setting( key: str, service: SettingService = Depends(get_setting_service) ) -> None: @@ -122,13 +138,12 @@ async def delete_system_setting( raise _not_found() -# Platform-operator routes: the tenant comes from the URL, so it must name a -# real tenant (#382). DELETE stays unvalidated so a row left behind by a -# deleted tenant can still be cleared — but while the tenant exists, a key set -# by upload is cleared through its upload route, which reaps the file. +# Explicit tenant-id routes require either the caller's own tenant or platform +# authority (#368). Reads/writes validate that the tenant still exists (#382); +# DELETE stays unvalidated so orphaned rows can still be cleared. -@router.get(API_TENANT_PATH, response_model=SettingOut, dependencies=_VIEW) +@router.get(API_TENANT_PATH, response_model=SettingOut, dependencies=[*_VIEW, *_OWN_TENANT]) async def get_tenant_setting( scope_id: str, key: str, @@ -142,7 +157,7 @@ async def get_tenant_setting( return result -@router.put(API_TENANT_PATH, response_model=SettingOut, dependencies=_EDIT) +@router.put(API_TENANT_PATH, response_model=SettingOut, dependencies=[*_EDIT, *_OWN_TENANT]) async def upsert_tenant_setting( scope_id: str, key: str, @@ -155,7 +170,9 @@ async def upsert_tenant_setting( return await service.upsert_scoped(SettingScope.TENANT, scope_id, key, data) -@router.delete(API_TENANT_PATH, status_code=STATUS_NO_CONTENT, dependencies=_DELETE) +@router.delete( + API_TENANT_PATH, status_code=STATUS_NO_CONTENT, dependencies=[*_DELETE, *_OWN_TENANT] +) async def delete_tenant_setting( scope_id: str, key: str, @@ -165,7 +182,7 @@ async def delete_tenant_setting( raise _not_found() -@router.get(API_USER_PATH, response_model=SettingOut, dependencies=_VIEW) +@router.get(API_USER_PATH, response_model=SettingOut, dependencies=[*_VIEW, *_OWN_USER]) async def get_user_setting( scope_id: str, key: str, @@ -177,7 +194,7 @@ async def get_user_setting( return result -@router.put(API_USER_PATH, response_model=SettingOut, dependencies=_EDIT) +@router.put(API_USER_PATH, response_model=SettingOut, dependencies=[*_EDIT, *_OWN_USER]) async def upsert_user_setting( scope_id: str, key: str, @@ -187,7 +204,7 @@ async def upsert_user_setting( return await service.upsert_scoped(SettingScope.USER, scope_id, key, data) -@router.delete(API_USER_PATH, status_code=STATUS_NO_CONTENT, dependencies=_DELETE) +@router.delete(API_USER_PATH, status_code=STATUS_NO_CONTENT, dependencies=[*_DELETE, *_OWN_USER]) async def delete_user_setting( scope_id: str, key: str, @@ -200,7 +217,9 @@ async def delete_user_setting( # ── Id-based CRUD (admin tooling) ─────────────────────────────────── -@router.post("/", response_model=SettingOut, status_code=STATUS_CREATED, dependencies=_CREATE) +@router.post( + "/", response_model=SettingOut, status_code=STATUS_CREATED, dependencies=[*_CREATE, *_PLATFORM] +) async def create_setting( data: SettingCreate, request: Request, @@ -217,7 +236,7 @@ async def create_setting( raise HTTPException(status_code=STATUS_CONFLICT, detail=ERR_SETTING_EXISTS) from exc -@router.get(API_BY_ID_PATH, response_model=SettingOut, dependencies=_VIEW) +@router.get(API_BY_ID_PATH, response_model=SettingOut, dependencies=[*_VIEW, *_PLATFORM]) async def get_setting( setting_id: int, service: SettingService = Depends(get_setting_service) ) -> SettingOut: @@ -227,7 +246,7 @@ async def get_setting( return result -@router.put(API_BY_ID_PATH, response_model=SettingOut, dependencies=_EDIT) +@router.put(API_BY_ID_PATH, response_model=SettingOut, dependencies=[*_EDIT, *_PLATFORM]) async def update_setting( setting_id: int, data: SettingUpdate, @@ -243,7 +262,7 @@ async def update_setting( return result -@router.delete(API_BY_ID_PATH, status_code=STATUS_NO_CONTENT, dependencies=_DELETE) +@router.delete(API_BY_ID_PATH, status_code=STATUS_NO_CONTENT, dependencies=[*_DELETE, *_PLATFORM]) async def delete_setting( setting_id: int, service: SettingService = Depends(get_setting_service) ) -> None: diff --git a/modules/settings/settings/endpoints/module_api.py b/modules/settings/settings/endpoints/module_api.py index 08568e0d..bacf8755 100644 --- a/modules/settings/settings/endpoints/module_api.py +++ b/modules/settings/settings/endpoints/module_api.py @@ -24,6 +24,7 @@ from settings.deps import get_setting_service from settings.hydrate import hydrate_settings from settings.reload import apply_changes_and_reload +from settings.scope_guard import require_platform from settings.service import SettingService from settings.store import SettingsStore @@ -31,10 +32,13 @@ # Per-module settings UI exposes raw secret values (mailer password, JWT # signing keys, etc.) — every endpoint here is gated on the same permissions -# the scoped API uses so a non-admin can't read or mutate module config. -_VIEW = [Depends(RequiresPermission(PERM_VIEW))] -_EDIT = [Depends(RequiresPermission(PERM_EDIT))] -_DELETE = [Depends(RequiresPermission(PERM_DELETE))] +# the scoped API uses so a non-admin can't read or mutate module config. Module +# settings live in the system scope, so on a multi-tenant host they are also +# platform-only (GH #368). +_PLATFORM = Depends(require_platform) +_VIEW = [Depends(RequiresPermission(PERM_VIEW)), _PLATFORM] +_EDIT = [Depends(RequiresPermission(PERM_EDIT)), _PLATFORM] +_DELETE = [Depends(RequiresPermission(PERM_DELETE)), _PLATFORM] def _masked_fields(app: FastAPI, package: str) -> frozenset[str]: diff --git a/modules/settings/settings/endpoints/views.py b/modules/settings/settings/endpoints/views.py index 55b60d1f..51f388b5 100644 --- a/modules/settings/settings/endpoints/views.py +++ b/modules/settings/settings/endpoints/views.py @@ -55,6 +55,7 @@ from settings.contracts.schemas import SettingUpdate from settings.deps import get_setting_service from settings.endpoints._create_form import create_from_form +from settings.scope_guard import require_platform from settings.service import SettingService from settings.tenant_scope import tenant_update_error @@ -74,8 +75,10 @@ # their env var names, and now which of the two is in force. The matching JSON # API (``/api/settings/...``) has always required ``settings.view``, so leaving # these unguarded let any signed-in account read the same data by asking for -# the page instead. Mutating routes add their own stricter guard on top. -router = APIRouter(dependencies=[Depends(RequiresPermission(PERM_VIEW))]) +# the page instead. Mutating routes add their own stricter guard on top. The +# screens span every scope, so on a multi-tenant host they are platform-only +# (GH #368). +router = APIRouter(dependencies=[Depends(RequiresPermission(PERM_VIEW)), Depends(require_platform)]) @router.get(VIEW_STORE_PATH, response_model=None) diff --git a/modules/settings/settings/scope_guard.py b/modules/settings/settings/scope_guard.py new file mode 100644 index 00000000..bbd70925 --- /dev/null +++ b/modules/settings/settings/scope_guard.py @@ -0,0 +1,132 @@ +"""Who may address which settings scope on a multi-tenant host (GH #368). + +``settings.view``/``.edit``/``.delete`` say what a caller may do to settings; +they say nothing about *whose* settings. On a multi-tenant host that left any +holder free to write the host-wide system scope — where every module's +DB-backed configuration lives — and any tenant's or user's scope by naming it +in the URL. + +So on such a host: + +- a caller's **own** tenant scope (the request's tenant) and **own** user scope + need only the route's usual permission; +- everything else — the system scope, another tenant's or user's scope, and + the admin tooling that spans scopes — needs a *platform settings admin*. + +A platform settings admin holds ``settings.system``. On hosts with a resolver +(the ``tenants`` module), the request's tenant is a choice the user made and +tenant roles arrive as ``tenant:``; global ``admin`` stays a platform +role even while working inside an organisation. On hosts without a resolver, +any legacy tenant-bound identity is not a platform admin either. New users no +longer carry a ``tenant_id`` column; tenancy is defined by memberships. + +Single-tenant hosts are untouched: every check passes when ``multi_tenant`` is +off. +""" + +from __future__ import annotations + +from fastapi import HTTPException, Query, Request +from simple_module_core.permissions import grants +from simple_module_hosting.permissions import PERMISSION_DENIED_PREFIX, resolved_permissions_for + +from settings.constants import PERM_SYSTEM, QP_SCOPE, QP_SCOPE_ID, QP_TENANT_ID, QP_USER_ID +from settings.contracts.schemas import SettingScope + +_STATUS_FORBIDDEN = 403 + + +def _multi_tenant(request: Request) -> bool: + sm = getattr(request.app.state, "sm", None) + return bool(getattr(getattr(sm, "settings", None), "multi_tenant", False)) + + +def is_platform_settings_admin(request: Request) -> bool: + """Whether the caller may address every settings scope.""" + if not _multi_tenant(request): + return True + if not grants(resolved_permissions_for(request), PERM_SYSTEM): + return False + if getattr(request.app.state, "tenant_resolver", None) is not None: + return True + user = getattr(request.state, "user", None) + return getattr(user, "tenant_id", None) is None + + +def _own_tenant(request: Request) -> str | None: + return getattr(request.state, "tenant_id", None) + + +def _own_user(request: Request) -> str | None: + user = getattr(request.state, "user", None) + user_id = getattr(user, "id", None) + return str(user_id) if user_id is not None else None + + +def _deny() -> HTTPException: + return HTTPException( + status_code=_STATUS_FORBIDDEN, detail=f"{PERMISSION_DENIED_PREFIX}{PERM_SYSTEM}" + ) + + +def _require_own(request: Request, owned: str | None, scope_id: str | None) -> None: + """Pass when *scope_id* is absent or the caller's own; else need platform.""" + if scope_id is None or (owned is not None and scope_id == owned): + return + if not is_platform_settings_admin(request): + raise _deny() + + +def require_platform(request: Request) -> None: + """Dependency: the route spans scopes or touches the system scope.""" + if not is_platform_settings_admin(request): + raise _deny() + + +def require_own_tenant(request: Request, scope_id: str) -> None: + """Dependency for ``/tenant/{scope_id}/…``.""" + _require_own(request, _own_tenant(request), scope_id) + + +def require_own_user(request: Request, scope_id: str) -> None: + """Dependency for ``/user/{scope_id}/…``.""" + _require_own(request, _own_user(request), scope_id) + + +def require_resolvable( + request: Request, + user_id: str | None = Query(default=None, alias=QP_USER_ID), + tenant_id: str | None = Query(default=None, alias=QP_TENANT_ID), +) -> None: + """Dependency for ``/resolve/{key}``: only the caller's own chain. + + The chain ends at the system scope, so a caller resolving its own keys + reads system values it may not address directly — by design: that is the + effective configuration it already runs under, and secrets stay masked. + """ + _require_own(request, _own_tenant(request), tenant_id) + _require_own(request, _own_user(request), user_id) + + +def require_listable( + request: Request, + scope: SettingScope | None = Query(default=None, alias=QP_SCOPE), + scope_id: str | None = Query(default=None, alias=QP_SCOPE_ID), +) -> None: + """Dependency for the list route: one of the caller's own scopes, or platform.""" + if scope == SettingScope.TENANT and scope_id is not None: + _require_own(request, _own_tenant(request), scope_id) + elif scope == SettingScope.USER and scope_id is not None: + _require_own(request, _own_user(request), scope_id) + else: + require_platform(request) + + +__all__ = [ + "is_platform_settings_admin", + "require_listable", + "require_own_tenant", + "require_own_user", + "require_platform", + "require_resolvable", +] diff --git a/modules/settings/tests/test_settings_tenant_scope.py b/modules/settings/tests/test_settings_tenant_scope.py new file mode 100644 index 00000000..c293cfed --- /dev/null +++ b/modules/settings/tests/test_settings_tenant_scope.py @@ -0,0 +1,236 @@ +"""Settings scopes on a multi-tenant host (GH #368). + +The routes used to check ``settings.*`` only, so anyone holding them could +write the host-wide system scope and any tenant's or user's scope by naming it +in the URL. Now the system scope, the cross-scope admin tooling and every +scope other than the caller's own need a *platform* settings admin. +""" + +from __future__ import annotations + +import uuid +from collections.abc import AsyncGenerator, Callable +from contextlib import asynccontextmanager + +import httpx +import pytest +from settings.constants import ALL_PERMISSIONS, API_PREFIX, PERM_SYSTEM +from simple_module_test.session_cookie import forge_session_cookie +from sqlalchemy import select +from tenants.resolver import forget + +_SETTINGS_PERMS = ["settings.view", "settings.create", "settings.edit", "settings.delete"] + + +def _url(path: str = "") -> str: + return f"{API_PREFIX}/{path.lstrip('/')}" if path else f"{API_PREFIX}/" + + +@pytest.fixture(autouse=True) +def _fresh_membership_cache(): + forget(None) + yield + forget(None) + + +@pytest.fixture +def client_for(app) -> Callable: + """``async with client_for("a@x.io", role="admin") as (c, uid)``.""" + + @asynccontextmanager + async def factory( + email: str, *, role: str = "user" + ) -> AsyncGenerator[tuple[httpx.AsyncClient, str], None]: + from users.models import Role, User, UserRole + + async with app.state.sm.db.session_factory() as session: + user = User( + id=uuid.uuid4(), + email=email, + hashed_password="x", + is_active=True, + is_verified=True, + ) + session.add(user) + await session.flush() + row = ( + await session.execute(select(Role).where(Role.name == role)) + ).scalar_one_or_none() + if row is not None: # only ``admin`` is seeded; a plain user needs no row + session.add(UserRole(user_id=user.id, role_id=row.id)) + await session.commit() + user_id = str(user.id) + + cookie = forge_session_cookie(app.state.sm.settings.secret_key, {"user_id": user_id}) + async with httpx.AsyncClient( + transport=httpx.ASGITransport(app=app), + base_url="http://testserver", + cookies={"session": cookie}, + ) as client: + yield client, user_id + + return factory + + +def test_system_permission_is_registered(app): + assert PERM_SYSTEM in ALL_PERMISSIONS + assert PERM_SYSTEM in app.state.sm.permissions.all_permissions + + +# Tenant roles now come from memberships, not the removed User.tenant_id column. + + +@pytest.fixture +def owners_edit_settings(app): + """An org owner with the older scoped settings permissions as well as self-service.""" + app.state.sm.permissions.map_role("tenant:owner", _SETTINGS_PERMS) + return app + + +async def _org(client: httpx.AsyncClient, name: str) -> str: + resp = await client.post("/api/tenants/", json={"name": name}) + assert resp.status_code in (200, 201), resp.text + return resp.json()["id"] + + +async def test_tenant_owner_cannot_write_the_system_scope(owners_edit_settings, client_for): + async with client_for("acme-admin@x.io") as (c, _): + await _org(c, "Acme") + resp = await c.put(_url("system/records.flag"), json={"value": "true"}) + assert resp.status_code == 403, resp.text + assert PERM_SYSTEM in resp.json()["detail"] + assert (await c.get(_url("system/records.flag"))).status_code == 403 + assert (await c.delete(_url("system/records.flag"))).status_code == 403 + + +async def test_tenant_owner_cannot_write_another_tenant(owners_edit_settings, client_for): + async with client_for("acme-admin@x.io") as (c, _), client_for("globex@x.io") as (other, _): + await _org(c, "Acme") + globex = await _org(other, "Globex") + assert (await c.put(_url(f"tenant/{globex}/k"), json={"value": "x"})).status_code == 403 + assert (await c.get(_url(f"tenant/{globex}/k"))).status_code == 403 + assert (await c.delete(_url(f"tenant/{globex}/k"))).status_code == 403 + + +async def test_tenant_owner_manages_its_own_tenant(owners_edit_settings, client_for): + async with client_for("acme-admin@x.io") as (c, _): + acme = await _org(c, "Acme") + assert (await c.put(_url(f"tenant/{acme}/k"), json={"value": "x"})).status_code == 200 + assert (await c.get(_url(f"tenant/{acme}/k"))).json()["value"] == "x" + listed = await c.get(_url(), params={"scope": "tenant", "scope_id": acme}) + assert [r["scope_id"] for r in listed.json()] == [acme] + assert (await c.delete(_url(f"tenant/{acme}/k"))).status_code == 204 + + +async def test_user_scope_is_limited_to_the_caller(owners_edit_settings, client_for): + async with client_for("acme-admin@x.io") as (c, uid): + await _org(c, "Acme") + assert (await c.put(_url(f"user/{uid}/k"), json={"value": "me"})).status_code == 200 + other = str(uuid.uuid4()) + assert (await c.put(_url(f"user/{other}/k"), json={"value": "x"})).status_code == 403 + assert (await c.get(_url(f"user/{other}/k"))).status_code == 403 + + +async def test_resolve_only_for_own_tenant_and_user(owners_edit_settings, client_for): + async with client_for("acme-admin@x.io") as (c, uid), client_for("globex@x.io") as (b, _): + acme = await _org(c, "Acme") + globex = await _org(b, "Globex") + await c.put(_url(f"tenant/{acme}/k"), json={"value": "ten"}) + own = await c.get(_url("resolve/k"), params={"tenant_id": acme, "user_id": uid}) + assert own.json()["value"] == "ten" + other = await c.get(_url("resolve/k"), params={"tenant_id": globex}) + assert other.status_code == 403 + + +@pytest.mark.parametrize( + ("method", "path", "params"), + [ + ("GET", "", None), + ("GET", "", {"scope": "system"}), + ("GET", "", {"scope": "tenant", "scope_id": "globex"}), + ("POST", "", None), + ("GET", "1", None), + ("PUT", "1", None), + ("DELETE", "1", None), + ("GET", "modules", None), + ], +) +async def test_cross_scope_tooling_is_platform_only( + owners_edit_settings, client_for, method, path, params +): + body = {"key": "k", "value": "x"} if method in ("POST", "PUT") else None + async with client_for("acme-admin@x.io") as (c, _): + await _org(c, "Acme") + resp = await c.request(method, _url(path), params=params, json=body) + assert resp.status_code == 403, resp.text + + +@pytest.mark.parametrize("path", ["/admin/settings/", "/admin/settings/store"]) +async def test_settings_screens_are_platform_only(owners_edit_settings, client_for, path): + async with client_for("acme-admin@x.io") as (c, _): + await _org(c, "Acme") + assert (await c.get(path, follow_redirects=False)).status_code == 403 + + +@pytest.mark.parametrize( + ("method", "path"), + [ + ("POST", "/admin/settings/store"), + ("PUT", "/admin/settings/1"), + ("DELETE", "/admin/settings/1"), + ("POST", "/admin/settings/test-connection/settings"), + ("PUT", "/api/settings/modules/settings"), + ("DELETE", "/api/settings/modules/settings/some_field"), + ], +) +async def test_settings_writes_outside_the_scoped_api_are_platform_only( + owners_edit_settings, client_for, method, path +): + """The screens' actions and the module-settings API write the system scope.""" + async with client_for("acme-admin@x.io") as (c, _): + await _org(c, "Acme") + body = {"key": "k", "value": "x", "scope": "system"} if method != "DELETE" else None + resp = await c.request(method, path, json=body, follow_redirects=False) + assert resp.status_code == 403, resp.text + assert PERM_SYSTEM in resp.text + + +async def test_platform_admin_keeps_every_scope(authenticated_client): + c = authenticated_client + globex = await _org(c, "Globex") + assert (await c.put(_url("system/k"), json={"value": "s"})).status_code == 200 + assert (await c.put(_url(f"tenant/{globex}/k"), json={"value": "t"})).status_code == 200 + assert (await c.put(_url(f"user/{uuid.uuid4()}/k"), json={"value": "u"})).status_code == 200 + assert (await c.get(_url())).status_code == 200 + assert (await c.get("/admin/settings/", follow_redirects=False)).status_code == 200 + + +async def test_single_tenant_host_is_unchanged(app, client_for): + app.state.sm.settings.multi_tenant = False + async with client_for("acme-admin@x.io", role="admin") as (c, _): + globex = await _org(c, "Globex") + assert (await c.put(_url("system/k"), json={"value": "s"})).status_code == 200 + assert (await c.put(_url(f"tenant/{globex}/k"), json={"value": "t"})).status_code == 200 + + +# ── tenants module: membership roles mapped onto settings permissions ── + + +async def test_org_owner_is_confined_to_its_org(owners_edit_settings, client_for): + async with client_for("a@x.io") as (a, _), client_for("b@x.io") as (b, _): + alpha = await _org(a, "Alpha") + beta = await _org(b, "Beta") + assert (await a.put(_url(f"tenant/{alpha}/k"), json={"value": "a"})).status_code == 200 + assert (await a.put(_url(f"tenant/{beta}/k"), json={"value": "x"})).status_code == 403 + assert (await a.put(_url("system/k"), json={"value": "x"})).status_code == 403 + assert (await a.get(_url("resolve/k"), params={"tenant_id": beta})).status_code == 403 + + +async def test_platform_admin_inside_an_org_stays_platform(owners_edit_settings, client_for): + """With the resolver, ``admin`` is a platform role even while in an org.""" + async with client_for("root@x.io", role="admin") as (c, _): + await _org(c, "Mine") + assert (await c.put(_url("system/k"), json={"value": "s"})).status_code == 200 + async with client_for("elsewhere@x.io") as (other, _): + elsewhere = await _org(other, "Elsewhere") + assert (await c.put(_url(f"tenant/{elsewhere}/k"), json={"value": "t"})).status_code == 200