feat(settings): tenant self-service settings, validate TENANT scope_id (#382) [stack 10/11]

docs/framework/multi-tenancy.md

@@ -239,6 +239,15 @@ overrides with no tenant bound — deliberately cross-tenant (the table is not
 `MultiTenantMixin`), so keep it that way. No tenant role holds
 `feature_flags.manage`; the tenant-override admin screens are platform-only.
 
+## Settings
+
+`settings` keeps its explicit `(scope, scope_id, key)` rows — no mixin. A key
+declared `tenant_overridable` can be changed by a tenant owner/admin for their
+active tenant (`/api/settings/tenant/current/{key}`, `settings.tenant.edit`);
+the platform routes take a tenant id from the URL and 404 an unknown one. Every
+write publishes a per-(tenant, key) notice on the `settings.values`
+invalidation channel. See [settings](/modules/settings#tenant-overridable-keys).
+
 Screens that take a tenant id from the URL can vet it without importing
 `tenants`: the module publishes `app.state.tenant_exists`, and
 `await simple_module_core.tenancy.tenant_exists(app, tenant_id)` answers

docs/modules/settings.md

@@ -72,11 +72,52 @@ class OrdersModule(ModuleBase):
 
 The browse UI uses these definitions to render meaningful empty states for unset keys.
 
+### Tenant-overridable keys
+
+A definition with `tenant_overridable=True` can be changed by a tenant for
+itself (#382). Tenant owners and admins hold `settings.tenant.edit` (mapped onto
+`tenant:owner` / `tenant:admin`; members get nothing) and write through
+`/api/settings/tenant/current/{key}`, which acts on `request.state.tenant_id`
+only — the tenant never comes from the URL. Keys without the flag answer 422
+there; a request acting for no tenant gets 403. The `tenants` module renders
+these keys at `/tenants/settings`.
+
+```python
+async def check_logo(request, tenant_id: str, value: str) -> None:
+    if value and not await tenant_owns_file(request.app, tenant_id, value):
+        raise LookupError("unknown file")  # -> 404; ValueError -> 422
+
+
+registry.add(
+    SettingDefinition(
+        key="orders.checkout_note",
+        tenant_overridable=True,
+        check=check_logo,
+    )
+)
+```
+
+`check` runs before **every** TENANT-scope write of the key — the self-service
+route and the platform routes alike, with the tenant being written (not the
+caller's). The declared `value_type` wins over the one a tenant sends.
+
+Runtime reads need nothing new: `SettingsDep` is already bound to the active
+tenant, so `await settings.get(key)` resolves **tenant → system → default**.
+
+### Cache invalidation
+
+Every SYSTEM / TENANT write publishes on the `settings.values`
+[invalidation](/framework/invalidation) channel after commit, keyed per
+(tenant, key): `"<tenant_id>|<key>"`, or `"|<key>"` for a system write (every
+tenant inheriting it is affected). `settings.contracts.invalidation` builds and
+parses the key. Settings caches nothing itself; a module that caches a resolved
+per-tenant value subscribes and forgets.
+
 ## Routes
 
 ### Generic K/V API (`/api/settings/...`)
 
-All write endpoints require `settings.edit` / `settings.create` / `settings.delete`; reads need `settings.view`.
+All write endpoints require `settings.edit` / `settings.create` / `settings.delete`; reads need `settings.view`. These are platform-operator permissions: no tenant role holds them, and the holder may write any key at system scope or at any tenant's scope. A TENANT `scope_id` must name a real tenant when a tenant-owning module is installed (`simple_module_core.tenancy.tenant_exists`): 404 on the `/tenant/{scope_id}/…` GET/PUT, 422 for a `POST /` body. `DELETE` stays unvalidated so a deleted tenant's leftovers can be cleared. A key declaring `SettingDefinition.clear_via` (a file set through an upload route, which reaps the stored file) is protected inside `SettingService.delete` / `delete_scoped`, so it holds for every scope (SYSTEM, TENANT, USER) and every caller: a generic delete raises `ManagedKeyError`, which the API maps to 422 naming that route (`clear_via` may be a `{SettingScope: route}` mapping when system and tenant rows clear differently; the 422 names the route for the row's scope, via `clear_route`) and the Inertia store screen shows as a toast. Two deliberate ways through: the owner of the key passes `as_owner=True` after taking responsibility for the file (branding's tenant clear route reaps it), and a TENANT row whose tenant no longer exists may be cleared by a platform operator. The guard needs the registry, which `get_setting_service` supplies; a bare `SettingService(db)` guards nothing.
 
 | Method + path | Purpose |
 |---|---|
@@ -85,6 +126,8 @@ All write endpoints require `settings.edit` / `settings.create` / `settings.dele
 | `GET / PUT / DELETE /api/settings/system/{key}` | system-scope CRUD |
 | `GET / PUT / DELETE /api/settings/tenant/{scope_id}/{key}` | tenant-scope CRUD |
 | `GET / PUT / DELETE /api/settings/user/{scope_id}/{key}` | user-scope CRUD |
+| `GET /api/settings/tenant/current` | the active tenant's overridable keys: `inherited`, own `value`, `effective` (`settings.tenant.edit`) |
+| `GET / PUT / DELETE /api/settings/tenant/current/{key}` | the active tenant's override of an overridable key (`settings.tenant.edit`) |
 | `POST /api/settings/` | create with explicit `scope` + `scope_id` (`SettingCreate`) |
 | `GET / PUT / DELETE /api/settings/{setting_id}` | by-id CRUD |
 

docs/modules/tenants.md

@@ -34,6 +34,7 @@ scoped to them.
 |---|---|---|
 | `GET /tenants/` | signed in | My organisations: switch, create |
 | `GET /tenants/members` | `tenants.members.view` | Members and invitations of the active tenant |
+| `GET /tenants/settings` | `settings.tenant.edit` | The active tenant's overrides of `tenant_overridable` settings |
 | `GET /tenants/invitations/accept?token=` | signed in | Accept an invitation |
 | `GET /admin/tenants/` | `tenants.platform.view` | Platform list of all tenants |
 | `GET/POST /api/tenants/` | signed in | List mine / create |
@@ -47,6 +48,12 @@ scoped to them.
 Tenant-level routes act on the *active* tenant (`/current`), never on an id
 from the URL.
 
+Organisation settings use `settings.tenant.edit`, which the `settings` module
+owns and maps onto owner and admin (#382); the page writes through
+`/api/settings/tenant/current/{key}`. The former `tenants.settings.manage`
+permission — declared, owner-only, and checked by nothing — is retired, so
+there is one permission for the one surface.
+
 ## Configuration
 
 DB-backed (Settings screen):

modules/settings/settings/_announce.py

@@ -0,0 +1,36 @@
+"""Publish a per-(tenant, key) invalidation after a settings write commits.
+
+Kept out of ``service.py`` (and its 300-line budget). See
+``settings.contracts.invalidation`` for the key format.
+"""
+
+from __future__ import annotations
+
+from typing import TYPE_CHECKING
+
+from simple_module_db.callbacks import register_on_commit
+
+from settings.constants import INVALIDATION_CHANNEL, SCOPE_SYSTEM, SCOPE_TENANT
+from settings.contracts.invalidation import invalidation_key
+
+if TYPE_CHECKING:
+    from simple_module_core.invalidation import InvalidationBus
+    from sqlalchemy.ext.asyncio import AsyncSession
+
+
+def announce(
+    db: AsyncSession, bus: InvalidationBus | None, scope: str, scope_id: str, key: str
+) -> None:
+    """Queue the notice for this session's commit; user-scope writes have no consumer.
+
+    After commit, not inline: a consumer that drops its entry before the row is
+    durable re-reads the *old* value and caches it for a full TTL.
+    """
+    if bus is None or scope not in (SCOPE_SYSTEM, SCOPE_TENANT):
+        return
+    wire_key = invalidation_key(scope_id if scope == SCOPE_TENANT else None, key)
+
+    async def publish() -> None:
+        await bus.publish(INVALIDATION_CHANNEL, key=wire_key)
+
+    register_on_commit(db, publish)

modules/settings/settings/_listing.py

@@ -0,0 +1,121 @@
+"""Read-side listing queries of :class:`SettingService`, split out for size."""
+
+from __future__ import annotations
+
+from simple_module_db import LIKE_ESCAPE_CHAR, like_contains_pattern
+from sqlalchemy import func, select
+from sqlalchemy.ext.asyncio import AsyncSession
+
+from settings._row_masking import out
+from settings.constants import (
+    ALL_SCOPES,
+    DEFAULT_PER_PAGE,
+    SCOPE_ALL,
+    SYSTEM_SCOPE_ID,
+)
+from settings.contracts.schemas import SettingOut, SettingScope
+from settings.models import Setting
+
+
+class SettingListing:
+    """Mixin: the ``list_*`` / ``count_by_scope`` queries. Needs ``self.db``."""
+
+    db: AsyncSession
+
+    # ── Listing ─────────────────────────────────────────────────────
+
+    async def list_all(self) -> list[SettingOut]:
+        result = await self.db.execute(
+            select(Setting).order_by(Setting.scope, Setting.scope_id, Setting.key)
+        )
+        return [out(row) for row in result.scalars()]
+
+    async def list_filtered(
+        self,
+        scope: SettingScope | None = None,
+        q: str | None = None,
+        page: int = 1,
+        per_page: int = DEFAULT_PER_PAGE,
+    ) -> tuple[list[SettingOut], int]:
+        """One page of rows plus the unpaged total for the same filters.
+
+        The browse screen used to receive every row and filter in the browser,
+        which made the payload, the render and find-in-page all scale with the
+        whole table instead of with what was asked for. ``q`` matches the key
+        only — the search box says "Search keys…", and quietly matching values
+        would surface rows whose key has nothing to do with the query.
+        """
+        conditions = self._filter_conditions(scope, q)
+        total = await self.db.scalar(select(func.count()).select_from(Setting).where(*conditions))
+        stmt = (
+            select(Setting)
+            .where(*conditions)
+            .order_by(Setting.scope, Setting.scope_id, Setting.key)
+            .offset(max(page - 1, 0) * per_page)
+            .limit(per_page)
+        )
+        result = await self.db.execute(stmt)
+        return [out(row) for row in result.scalars()], int(total or 0)
+
+    async def count_by_scope(self, q: str | None = None) -> dict[str, int]:
+        """Per-scope tallies for the filter tabs, plus ``all``.
+
+        Every scope is named even at zero: a tab that disappears when its count
+        drops to nothing moves the other tabs under the cursor mid-search.
+        The scope filter itself is deliberately not applied — the tabs describe
+        what each of them *would* show, so selecting one must not zero the rest.
+        """
+        conditions = self._filter_conditions(None, q)
+        stmt = select(Setting.scope, func.count()).where(*conditions).group_by(Setting.scope)
+        result = await self.db.execute(stmt)
+        tallies = {str(scope): int(count) for scope, count in result.all()}
+        counts = {name: tallies.get(name, 0) for name in ALL_SCOPES}
+        return {SCOPE_ALL: sum(counts.values()), **counts}
+
+    @staticmethod
+    def _filter_conditions(scope: SettingScope | None, q: str | None) -> list:
+        conditions = []
+        if scope is not None:
+            conditions.append(Setting.scope == scope.value)
+        needle = (q or "").strip()
+        if needle:
+            # Setting keys are full of underscores, and `_` is a LIKE wildcard:
+            # unescaped, a search for "smtp_host" also matches "smtpXhost", and
+            # a stray "%" matches the entire table. ``ilike`` is emulated by
+            # SQLAlchemy on SQLite (lower() on both sides), so one expression
+            # is case-insensitive on both databases.
+            conditions.append(
+                Setting.key.ilike(like_contains_pattern(needle), escape=LIKE_ESCAPE_CHAR)
+            )
+        return conditions
+
+    async def list_by_scope(
+        self, scope: SettingScope, scope_id: str = SYSTEM_SCOPE_ID
+    ) -> list[SettingOut]:
+        result = await self.db.execute(self._scope_stmt(scope, scope_id))
+        return [out(row) for row in result.scalars()]
+
+    async def list_by_scope_unmasked(
+        self, scope: SettingScope, scope_id: str = SYSTEM_SCOPE_ID
+    ) -> list[SettingOut]:
+        """The same rows with their real values, for code that *applies* them.
+
+        The masking in :func:`_out` is for the screens. Hydration is not a
+        screen: ``SettingsStore`` feeds these values back into the live module
+        settings objects at boot, so a masked read writes a row of dots over the
+        real secret — a mailer that cannot authenticate, and a
+        ``reset_password_token_secret`` that no longer verifies the tokens it
+        signed. This is the one read that must see through the mask, and it is
+        spelled out rather than reached by passing a flag so that every caller
+        of it is one grep away.
+        """
+        result = await self.db.execute(self._scope_stmt(scope, scope_id))
+        return [SettingOut.model_validate(row) for row in result.scalars()]
+
+    @staticmethod
+    def _scope_stmt(scope: SettingScope, scope_id: str):
+        return (
+            select(Setting)
+            .where(Setting.scope == scope.value, Setting.scope_id == scope_id)
+            .order_by(Setting.key)
+        )

modules/settings/settings/_managed_keys.py

@@ -0,0 +1,59 @@
+"""Rows of a key set by upload are not deleted by a bare row delete.
+
+A key declaring ``SettingDefinition.clear_via`` is a file the owner's upload
+route stores and reaps. Deleting the row any other way leaves the stored file
+behind, so :meth:`SettingService.delete` / ``delete_scoped`` refuse it with
+:class:`ManagedKeyError` **for every scope** — the rule lives in the service so
+a route (or a module calling the service directly) cannot forget it.
+
+Two ways through, both deliberate:
+
+* the owner passes ``as_owner=True`` after taking responsibility for the file
+  (branding's clear routes reap it);
+* a TENANT row whose tenant no longer exists has no live owner left to clear
+  it, so a platform operator may delete it by hand. SYSTEM and USER rows get no
+  such exception: a SYSTEM row always has a live owner (the platform's own
+  upload route), and a USER row of a managed key is cleared through the owner
+  too. The guard is only as wide as the registry the service was built with;
+  a service built without one (``SettingService(db)``) guards nothing.
+"""
+
+from __future__ import annotations
+
+from collections.abc import Awaitable, Callable
+
+from settings.constants import ERR_MANAGED_KEY_DELETE
+from settings.contracts.registry import SettingsRegistry, clear_route
+from settings.contracts.schemas import SettingScope
+
+TenantIsLive = Callable[[str], Awaitable[bool]]
+
+
+class ManagedKeyError(Exception):
+    """The key is cleared through ``clear_via``, not by deleting its row."""
+
+    def __init__(self, key: str, clear_via: str) -> None:
+        self.key = key
+        self.clear_via = clear_via
+        super().__init__(ERR_MANAGED_KEY_DELETE.format(clear_via=clear_via))
+
+
+async def ensure_deletable(
+    registry: SettingsRegistry | None,
+    tenant_is_live: TenantIsLive | None,
+    scope: str,
+    scope_id: str,
+    key: str,
+) -> None:
+    """Raise :class:`ManagedKeyError` unless the row may be deleted generically."""
+    definition = registry.get(key) if registry is not None else None
+    if definition is None or not definition.clear_via:
+        return
+    orphaned = (
+        scope == SettingScope.TENANT.value
+        and tenant_is_live is not None
+        and not await tenant_is_live(scope_id)
+    )
+    if orphaned:
+        return
+    raise ManagedKeyError(key, clear_route(definition, scope))

modules/settings/settings/_module_settings_props.py

@@ -9,9 +9,10 @@
 
 from typing import Any
 
+from fastapi import FastAPI
 from fastapi.encoders import jsonable_encoder
 
-from settings._module_settings import ModuleSettingsView
+from settings._module_settings import ModuleSettingsView, _package_of
 
 
 def serialize(views: list[ModuleSettingsView]) -> list[dict[str, Any]]:
@@ -54,3 +55,24 @@ def serialize(views: list[ModuleSettingsView]) -> list[dict[str, Any]]:
         }
         for v in views
     ]
+
+
+def testable_packages(app: FastAPI) -> dict[str, list[str]]:
+    """Package -> the names of the health checks its module registered.
+
+    "Test connection" is just that module's health checks run on demand —
+    reusing the registry means settings never learns what an SMTP or an S3
+    connection is. The names come back with the packages so the button can say
+    what it is about to dial ("Test mailer connection") instead of the useless
+    "Test connection" a bare package list can produce.
+    """
+    checks_by_owner: dict[str, list[str]] = {}
+    for check in app.state.sm.health_registry.all_checks:
+        if check.module:
+            checks_by_owner.setdefault(check.module, []).append(check.name)
+
+    return {
+        _package_of(mod): sorted(checks_by_owner[mod.meta.name])
+        for mod in getattr(app.state.sm, "modules", ())
+        if mod.meta.name in checks_by_owner
+    }

modules/settings/settings/_unique_write.py

@@ -0,0 +1,29 @@
+"""Race-safe inserts for the (scope, scope_id, key) unique key.
+
+A read-then-insert cannot be made safe by reading harder: two requests can both
+see "no row" and both insert. The unique constraint is the arbiter, so the
+insert runs in a savepoint — a loser's ``IntegrityError`` rolls back only the
+savepoint, leaving the request's transaction (and its session) usable.
+"""
+
+from __future__ import annotations
+
+from sqlalchemy.exc import IntegrityError
+from sqlalchemy.ext.asyncio import AsyncSession
+
+from settings.models import Setting
+
+
+class DuplicateSettingError(Exception):
+    """A row with this (scope, scope_id, key) already exists."""
+
+
+async def insert_if_free(db: AsyncSession, entity: Setting) -> bool:
+    """Insert ``entity``; ``False`` when its (scope, scope_id, key) is already taken."""
+    try:
+        async with db.begin_nested():
+            db.add(entity)
+            await db.flush()
+    except IntegrityError:
+        return False
+    return True