settings: on a multi-tenant host any tenant's admin can edit system-scope settings and any other tenant's tenant-scope settings #368

Summary

The settings module's API guards every route with the settings permissions only (settings.view, settings.edit, settings.delete). On a host with multi_tenant=True that means an admin user belonging to tenant acme can read and write the host-wide system scope, which is where every module's DB-backed configuration lives, and can read and write tenant scope for any scope_id they type into the URL, including globex. Neither the permission check nor the service compares the caller's tenant with the scope being addressed.

Observed

settings/endpoints/api.py (0.0.26):

_EDIT = [Depends(RequiresPermission(PERM_EDIT))]        # line 47

@router.put(API_SYSTEM_PATH, response_model=SettingOut, dependencies=_EDIT)   # line 98
async def upsert_system_setting(key, data, service=Depends(get_setting_service)):
    return await service.upsert_scoped(SettingScope.SYSTEM, SYSTEM_SCOPE_ID, key, data)

@router.put(API_TENANT_PATH, response_model=SettingOut, dependencies=_EDIT)   # line 126
async def upsert_tenant_setting(scope_id, key, data, service=Depends(get_setting_service)):
    return await service.upsert_scoped(SettingScope.TENANT, scope_id, key, data)

API_TENANT_PATH is /tenant/{scope_id}/{key} (settings/constants.py:55), so scope_id comes straight from the caller.

Reproduction on a host booted with SM_MULTI_TENANT=true SM_TENANT_HEADER=X-Tenant-ID, a user with the admin role and users_user.tenant_id='acme':

# as acme's admin (session cookie)
curl -b cj -X PUT http://localhost:8011/api/settings/system/sm_records.admin_header_tenant \
     -H 'Content-Type: application/json' -d '{"value": true, "value_type": "bool"}'
-> 200: a host-wide records setting changed by one tenant's admin

curl -b cj -X PUT http://localhost:8011/api/settings/tenant/globex/some.key \
     -H 'Content-Type: application/json' -d '{"value": "x", "value_type": "string"}'
-> 200: a setting in another tenant's scope written

Observed while reviewing the records module's tenancy work (antosubash/smpy_modules#37): the acme admin turned on records' admin_header_tenant for the whole host.

Expected

On a multi-tenant host, system-scope writes need a host-level role that is not granted per tenant (or at least a distinct permission such as settings.system), and tenant-scope routes must refuse a scope_id that differs from the caller's own tenant unless the caller holds that host-level role.

Why a module cannot work around it

The settings module owns the routes and the Setting table. A module can only choose not to register DB-backed settings, which loses the admin UI for all of them.

Proposed API

• RequiresTenantScope() dependency on the tenant routes comparing scope_id with request.state.tenant_id.
• A separate settings.system permission (or a host_admin role) for the system scope, not implied by a tenant's admin role.
• The settings UI hides the system scope from users who lack it.