From c1e133dbadf19e5b81e34c28e5dc9a5f9453bc81 Mon Sep 17 00:00:00 2001 From: Anto Subash Date: Thu, 1 Oct 2026 12:03:43 +0200 Subject: [PATCH] fix(settings): confine settings scopes to the caller's own on multi-tenant hosts (#368) The settings routes checked settings.view/edit/delete only, so on a multi-tenant host any holder could write the host-wide system scope and any tenant's or user's scope by naming it in the URL. Add a settings.system permission and settings/scope_guard.py. With multi_tenant on, a caller reaches its own tenant scope 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. Without a tenant resolver, a user whose record carries a tenant_id is never a platform admin, even with the admin role. Single-tenant hosts are unchanged. --- CHANGELOG.md | 11 + docs/framework/multi-tenancy.md | 11 + modules/settings/settings/constants.py | 7 +- modules/settings/settings/endpoints/api.py | 50 ++-- .../settings/settings/endpoints/module_api.py | 12 +- modules/settings/settings/endpoints/views.py | 7 +- modules/settings/settings/scope_guard.py | 132 ++++++++++ .../tests/test_settings_tenant_scope.py | 230 ++++++++++++++++++ 8 files changed, 438 insertions(+), 22 deletions(-) create mode 100644 modules/settings/settings/scope_guard.py create mode 100644 modules/settings/tests/test_settings_tenant_scope.py 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 9ce0f2cb..316bd82f 100644 --- a/docs/framework/multi-tenancy.md +++ b/docs/framework/multi-tenancy.md @@ -137,6 +137,17 @@ then the session's choice validated against a membership (#363). Without one it `tenant_id` claim, and for **anonymous** requests only, the configured `tenant_header`. An authenticated user can never pick a tenant by header. +## 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 8032a101..c252e98d 100644 --- a/modules/settings/settings/constants.py +++ b/modules/settings/settings/constants.py @@ -85,7 +85,12 @@ PERM_CREATE: Final = "settings.create" PERM_EDIT: Final = "settings.edit" PERM_DELETE: Final = "settings.delete" -ALL_PERMISSIONS: Final = (PERM_VIEW, PERM_CREATE, PERM_EDIT, PERM_DELETE) +# Host-wide settings on a multi-tenant host: the system scope, other tenants' +# and users' scopes, and the cross-scope admin screens (GH #368). Never granted +# by a tenant role, and withheld from a principal whose identity is bound to a +# tenant — see ``settings.scope_guard``. +PERM_SYSTEM: Final = "settings.system" +ALL_PERMISSIONS: Final = (PERM_VIEW, PERM_CREATE, PERM_EDIT, PERM_DELETE, PERM_SYSTEM) # ── Database ───────────────────────────────────────────────────────── DB_SCHEMA: Final = MODULE_PACKAGE diff --git a/modules/settings/settings/endpoints/api.py b/modules/settings/settings/endpoints/api.py index 2e60cee6..ed6d1f4f 100644 --- a/modules/settings/settings/endpoints/api.py +++ b/modules/settings/settings/endpoints/api.py @@ -33,6 +33,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 router = APIRouter() @@ -47,6 +54,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) @@ -55,7 +69,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), @@ -69,7 +83,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), @@ -85,7 +101,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: @@ -95,7 +111,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, @@ -104,7 +120,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: @@ -112,7 +128,7 @@ async def delete_system_setting( raise _not_found() -@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, @@ -124,7 +140,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, @@ -134,7 +150,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, @@ -144,7 +162,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, @@ -156,7 +174,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, @@ -166,7 +184,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, @@ -179,7 +197,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, service: SettingService = Depends(get_setting_service), @@ -187,7 +207,7 @@ async def create_setting( return await service.create(data) -@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: @@ -197,7 +217,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, @@ -209,7 +229,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 cf3c6331..9b3d3556 100644 --- a/modules/settings/settings/endpoints/views.py +++ b/modules/settings/settings/endpoints/views.py @@ -52,6 +52,7 @@ ) from settings.contracts.schemas import SettingCreate, SettingUpdate from settings.deps import get_setting_service +from settings.scope_guard import require_platform from settings.service import SettingService _PAGE_BROWSE = "Settings/Browse" @@ -70,8 +71,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..c67c6836 --- /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`` and, on a host without a +tenant resolver, has no tenant on its own identity: there the only admin role +is the global ``admin``, so a user record bound to ``acme`` is acme's admin, +not the host's. With a resolver (the ``tenants`` module) the request's tenant +is a choice the user made, and tenant roles arrive as ``tenant:``, so +``admin`` stays a platform role even while working inside an organisation. + +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..17ca0b97 --- /dev/null +++ b/modules/settings/tests/test_settings_tenant_scope.py @@ -0,0 +1,230 @@ +"""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", tenant_id="acme") as (c, uid)``.""" + + @asynccontextmanager + async def factory( + email: str, *, role: str = "user", tenant_id: str | None = None + ) -> 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, + tenant_id=tenant_id, + ) + 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 + + +# ── Legacy path: no tenant resolver, tenant from users_user.tenant_id ── + + +@pytest.fixture +def legacy_app(app): + """The host as it runs without the ``tenants`` module.""" + app.state.tenant_resolver = None + return app + + +async def test_tenant_bound_admin_cannot_write_the_system_scope(legacy_app, client_for): + """The issue's first repro: acme's admin changed a host-wide setting.""" + async with client_for("acme-admin@x.io", role="admin", tenant_id="acme") as (c, _): + 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_bound_admin_cannot_write_another_tenant(legacy_app, client_for): + """The issue's second repro: acme's admin wrote into globex's scope.""" + async with client_for("acme-admin@x.io", role="admin", tenant_id="acme") as (c, _): + assert (await c.put(_url("tenant/globex/k"), json={"value": "x"})).status_code == 403 + assert (await c.get(_url("tenant/globex/k"))).status_code == 403 + assert (await c.delete(_url("tenant/globex/k"))).status_code == 403 + + +async def test_tenant_bound_admin_manages_its_own_tenant(legacy_app, client_for): + async with client_for("acme-admin@x.io", role="admin", tenant_id="acme") as (c, _): + assert (await c.put(_url("tenant/acme/k"), json={"value": "x"})).status_code == 200 + assert (await c.get(_url("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("tenant/acme/k"))).status_code == 204 + + +async def test_user_scope_is_limited_to_the_caller(legacy_app, client_for): + async with client_for("acme-admin@x.io", role="admin", tenant_id="acme") as (c, uid): + 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(legacy_app, client_for): + async with client_for("acme-admin@x.io", role="admin", tenant_id="acme") as (c, uid): + await c.put(_url("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(legacy_app, 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", role="admin", tenant_id="acme") as (c, _): + 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(legacy_app, client_for, path): + async with client_for("acme-admin@x.io", role="admin", tenant_id="acme") as (c, _): + 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( + legacy_app, client_for, method, path +): + """The screens' actions and the module-settings API write the system scope.""" + async with client_for("acme-admin@x.io", role="admin", tenant_id="acme") as (c, _): + 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(legacy_app, authenticated_client): + c = authenticated_client + assert (await c.put(_url("system/k"), json={"value": "s"})).status_code == 200 + assert (await c.put(_url("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(legacy_app, client_for): + legacy_app.state.sm.settings.multi_tenant = False + async with client_for("acme-admin@x.io", role="admin", tenant_id="acme") as (c, _): + assert (await c.put(_url("system/k"), json={"value": "s"})).status_code == 200 + assert (await c.put(_url("tenant/globex/k"), json={"value": "t"})).status_code == 200 + + +# ── tenants module: membership roles mapped onto settings permissions ── + + +@pytest.fixture +def owners_edit_settings(app): + """A host letting org owners manage settings — where #368 still bites.""" + 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_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 + assert (await c.put(_url("tenant/elsewhere/k"), json={"value": "t"})).status_code == 200