From be8f6ab6dc1126328f12ce6f9bafdbf22ab10328 Mon Sep 17 00:00:00 2001 From: Anto Subash Date: Thu, 1 Oct 2026 12:43:27 +0200 Subject: [PATCH 1/2] feat(dashboard): platform-only stats behind dashboard.view (#374) Claude-Session: https://claude.ai/code/session_01F8RiTBUJQnZmSq56qReZeV --- modules/dashboard/README.md | 1 + modules/dashboard/dashboard/constants.py | 4 + modules/dashboard/dashboard/endpoints/api.py | 10 ++- .../dashboard/dashboard/endpoints/views.py | 25 +++--- modules/dashboard/dashboard/module.py | 8 ++ modules/dashboard/dashboard/pages/Home.tsx | 20 +++++ .../tests/test_dashboard_permission.py | 76 +++++++++++++++++++ 7 files changed, 132 insertions(+), 12 deletions(-) create mode 100644 modules/dashboard/dashboard/constants.py create mode 100644 modules/dashboard/tests/test_dashboard_permission.py diff --git a/modules/dashboard/README.md b/modules/dashboard/README.md index bac6b1e9..894aca2a 100644 --- a/modules/dashboard/README.md +++ b/modules/dashboard/README.md @@ -13,6 +13,7 @@ pip install simple_module_dashboard ## What it provides - `/dashboard` Inertia view, a single entry point for logged-in users. +- Platform-wide stats gated by the `dashboard.view` permission (held by `admin` through `*`, mapped to no tenant role): the counts span every tenant, so `/api/dashboard/stats` answers 403 without it and `/dashboard` renders only a welcome for viewers who lack it. - Global sidebar renderer — aggregates `register_menu_items()` calls from all modules into one tree. - Breadcrumb + page-title provider used by downstream module pages. diff --git a/modules/dashboard/dashboard/constants.py b/modules/dashboard/dashboard/constants.py new file mode 100644 index 00000000..71181ed2 --- /dev/null +++ b/modules/dashboard/dashboard/constants.py @@ -0,0 +1,4 @@ +"""Stable identifiers for the dashboard module.""" + +PERM_VIEW = "dashboard.view" +PERM_GROUP = "Dashboard" diff --git a/modules/dashboard/dashboard/endpoints/api.py b/modules/dashboard/dashboard/endpoints/api.py index 39256c29..ff9dc2ab 100644 --- a/modules/dashboard/dashboard/endpoints/api.py +++ b/modules/dashboard/dashboard/endpoints/api.py @@ -4,14 +4,20 @@ from fastapi import APIRouter, Depends, Request from simple_module_db.deps import get_db +from simple_module_hosting.permissions import RequiresPermission from sqlalchemy.ext.asyncio import AsyncSession +from dashboard.constants import PERM_VIEW from dashboard.stats import fetch_dashboard_stats router = APIRouter() -@router.get("/stats") +@router.get("/stats", dependencies=[Depends(RequiresPermission(PERM_VIEW))]) async def dashboard_stats(request: Request, db: AsyncSession = Depends(get_db)) -> dict: - """Return dashboard statistics including user counts and system info.""" + """Platform-wide statistics: user counts across every tenant and system info. + + Gated by ``dashboard.view``, which no tenant role holds — a tenant member + must not learn how many accounts the whole install has. + """ return await fetch_dashboard_stats(db, request.app) diff --git a/modules/dashboard/dashboard/endpoints/views.py b/modules/dashboard/dashboard/endpoints/views.py index 82461ef6..732bbd4e 100644 --- a/modules/dashboard/dashboard/endpoints/views.py +++ b/modules/dashboard/dashboard/endpoints/views.py @@ -9,14 +9,16 @@ import asyncio from fastapi import APIRouter, Depends, HTTPException, Request -from simple_module_core.permissions import is_admin +from simple_module_core.permissions import grants, is_admin from simple_module_db.deps import get_db from simple_module_hosting.i18n_deps import TranslatorDep from simple_module_hosting.inertia_deps import InertiaDep +from simple_module_hosting.permissions import resolved_permissions_for from simple_module_inertia import InertiaResponse from sqlalchemy.ext.asyncio import AsyncSession from starlette.responses import RedirectResponse +from dashboard.constants import PERM_VIEW from dashboard.stats import fetch_dashboard_stats router = APIRouter() @@ -47,15 +49,18 @@ async def dashboard( t: TranslatorDep, db: AsyncSession = Depends(get_db), ) -> InertiaResponse: - """Authenticated dashboard — requires login (enforced by AuthMiddleware).""" - stats = await fetch_dashboard_stats(db, request.app) - return await inertia.render( - _PAGE_HOME, - { - "welcome": t.t("dashboard.home.welcome_message"), - **stats, - }, - ) + """Authenticated dashboard — requires login (enforced by AuthMiddleware). + + This is every signed-in user's post-login landing page, so the route stays + open; only the platform-wide stats are gated on ``dashboard.view``. A viewer + without it gets the welcome and no numbers (``can_view_stats`` false). + """ + props: dict = {"welcome": t.t("dashboard.home.welcome_message")} + can_view = grants(resolved_permissions_for(request), PERM_VIEW) + props["can_view_stats"] = can_view + if can_view: + props.update(await fetch_dashboard_stats(db, request.app)) + return await inertia.render(_PAGE_HOME, props) @admin_router.get("/", response_model=None) diff --git a/modules/dashboard/dashboard/module.py b/modules/dashboard/dashboard/module.py index 00eb499a..d436f600 100644 --- a/modules/dashboard/dashboard/module.py +++ b/modules/dashboard/dashboard/module.py @@ -8,6 +8,9 @@ from fastapi import APIRouter from simple_module_core.menu import MenuItem, MenuRegistry, MenuSection from simple_module_core.module import ModuleBase, ModuleMeta +from simple_module_core.permissions import PermissionRegistry + +from dashboard.constants import PERM_GROUP, PERM_VIEW _MODULE_USERS = "Users" _URL_DASHBOARD = "/dashboard/" @@ -39,6 +42,11 @@ def register_admin_routes(self, admin_router: APIRouter) -> None: admin_router.include_router(doctor_views) + def register_permissions(self, registry: PermissionRegistry) -> None: + # Platform-wide: the stats span every tenant (User is not tenant-scoped), + # so no tenant role is mapped to this. ``admin`` holds it through ``*``. + registry.add_group(PERM_GROUP, [PERM_VIEW]) + def register_menu_items(self, registry: MenuRegistry) -> None: registry.add( MenuItem( diff --git a/modules/dashboard/dashboard/pages/Home.tsx b/modules/dashboard/dashboard/pages/Home.tsx index 988dcf0a..92e56bc3 100644 --- a/modules/dashboard/dashboard/pages/Home.tsx +++ b/modules/dashboard/dashboard/pages/Home.tsx @@ -35,6 +35,9 @@ interface SystemInfo { } interface Props { + welcome: string; + /** False for a viewer without `dashboard.view`: the stats props are absent. */ + can_view_stats: boolean; total_users: number; active_users_7d: number; users_created_this_month: number; @@ -48,6 +51,23 @@ function Home() { const { menus } = page.props as unknown as SharedProps; const { t } = useT(); + // The stats span every tenant and need `dashboard.view`; anyone else lands + // here after login too, so they get the welcome and no numbers. + if (!props.can_view_stats) { + return ( + <> + + + + +

{props.welcome}

+ + + + + ); + } + const { health_checks: healthChecks } = props.system_info; const unhealthy = healthChecks.filter((c) => c.status !== 'healthy').length; const moduleTarget = moduleTargetResolver(menus, props.system_info.modules); diff --git a/modules/dashboard/tests/test_dashboard_permission.py b/modules/dashboard/tests/test_dashboard_permission.py new file mode 100644 index 00000000..81eb4ad8 --- /dev/null +++ b/modules/dashboard/tests/test_dashboard_permission.py @@ -0,0 +1,76 @@ +"""The dashboard is platform-only: its stats span every tenant (#374). + +``User`` is not tenant-scoped, so ``total_users`` counts the whole install. The +stats therefore need ``dashboard.view``, which only the platform ``admin`` +(wildcard) holds and no tenant role is mapped to. +""" + +from __future__ import annotations + +import pytest +from dashboard.constants import PERM_VIEW +from dashboard.stats import invalidate_stats_cache +from simple_module_core.tenancy import TenantRole, tenant_role + +_STATS = "/api/dashboard/stats" +_INERTIA = {"X-Inertia": "true", "Accept": "application/json"} + + +@pytest.fixture(autouse=True) +def _clear_stats_cache(): + invalidate_stats_cache() + yield + invalidate_stats_cache() + + +def test_permission_is_registered_and_mapped_to_no_tenant_role(app): + registry = app.state.sm.permissions + assert registry.has(PERM_VIEW) + for role in TenantRole: + assert PERM_VIEW not in registry.role_map[tenant_role(role)] + + +async def test_anonymous_is_refused(client): + resp = await client.get(_STATS, follow_redirects=False) + assert resp.status_code in (302, 401, 403) + + +@pytest.mark.parametrize("role", list(TenantRole)) +async def test_tenant_member_is_forbidden(tenant_client, role): + async with tenant_client(role) as m: + resp = await m.client.get(_STATS) + assert resp.status_code == 403 + assert PERM_VIEW in resp.json()["detail"] + + +async def test_admin_sees_the_stats(authenticated_client): + resp = await authenticated_client.get(_STATS) + assert resp.status_code == 200 + assert resp.json()["total_users"] >= 1 + + +async def test_a_warm_cache_does_not_open_the_api_to_a_tenant_member( + authenticated_client, tenant_client +): + assert (await authenticated_client.get(_STATS)).status_code == 200 # warms the cache + async with tenant_client() as m: + assert (await m.client.get(_STATS)).status_code == 403 + + +async def test_home_page_for_admin_carries_the_stats(authenticated_client): + props = (await authenticated_client.get("/dashboard/", headers=_INERTIA)).json()["props"] + assert props["can_view_stats"] is True + assert props["total_users"] >= 1 + assert "system_info" in props + + +async def test_home_page_for_a_tenant_member_renders_without_stats(tenant_client): + """``/dashboard/`` is the post-login landing page: it must not 403 or leak.""" + async with tenant_client("member") as m: + resp = await m.client.get("/dashboard/", headers=_INERTIA) + assert resp.status_code == 200 + props = resp.json()["props"] + assert props["can_view_stats"] is False + assert props["welcome"] + for leaked in ("total_users", "active_users_7d", "module_count", "system_info"): + assert leaked not in props From ae79cae2adbb9588c034f3199652c285af93d49c Mon Sep 17 00:00:00 2001 From: Anto Subash Date: Thu, 1 Oct 2026 14:51:08 +0200 Subject: [PATCH 2/2] fix(dashboard): keep stats for ordinary users on single-tenant installs (review of #374) Gating the stats on dashboard.view took them away from every non-admin on every install. With multi_tenant off the install is the only tenant, so the module now maps dashboard.view onto the platform user role at startup. With multi_tenant on it stays unmapped: every tenant member holds user. Claude-Session: https://claude.ai/code/session_01F8RiTBUJQnZmSq56qReZeV --- docs/modules/dashboard.md | 13 ++++- modules/dashboard/README.md | 2 +- modules/dashboard/dashboard/module.py | 14 ++++- .../tests/test_dashboard_permission.py | 6 +++ .../tests/test_dashboard_single_tenant.py | 51 +++++++++++++++++++ 5 files changed, 82 insertions(+), 4 deletions(-) create mode 100644 modules/dashboard/tests/test_dashboard_single_tenant.py diff --git a/docs/modules/dashboard.md b/docs/modules/dashboard.md index bf8569f7..849907a2 100644 --- a/docs/modules/dashboard.md +++ b/docs/modules/dashboard.md @@ -19,7 +19,7 @@ It's intentionally simple — a place for new installs to land that proves the p | Method + path | Inertia component | Permission | |---|---|---| -| `GET /dashboard/` | `Dashboard/Home` | authenticated user (any role) | +| `GET /dashboard/` | `Dashboard/Home` | authenticated user (any role); the stats need `dashboard.view` | | `GET /admin/doctor/` | `Dashboard/Doctor` | `admin` role | `/admin/doctor/` is a browser mirror of `make doctor` — it shows the same module list, static checks, dev-server and environment info from the stats payload. It is served from the module's `admin_view_prefix` (`/admin/doctor`) rather than its `view_prefix`, because dashboard owns both a public-facing screen and an admin one and a module gets only one view router. @@ -30,7 +30,16 @@ The route is guarded by an admin dependency, and its menu entry carries the matc | Method + path | Returns | Permission | |---|---|---| -| `GET /api/dashboard/stats` | `dict` (see below) | authenticated user (any role) | +| `GET /api/dashboard/stats` | `dict` (see below) | `dashboard.view` | + +The stats count the whole install (`User` is not tenant-scoped), so they sit +behind `dashboard.view`. `admin` holds it through `*`. On a single-tenant +install (`multi_tenant` off) the module also maps it onto the platform `user` +role, so every signed-in user keeps the numbers they always had. With +`multi_tenant` on it is mapped to no other role — every tenant member holds +`user`, and one tenant must not learn the size of the install — so ordinary +users see only the welcome on `/dashboard/`, and the API answers 403. Grant it +to a role in the role editor to change that. `/api/dashboard/stats` is what the page itself calls; you can hit it from your own UI or scripts. Response shape: diff --git a/modules/dashboard/README.md b/modules/dashboard/README.md index 894aca2a..5c60bcdf 100644 --- a/modules/dashboard/README.md +++ b/modules/dashboard/README.md @@ -13,7 +13,7 @@ pip install simple_module_dashboard ## What it provides - `/dashboard` Inertia view, a single entry point for logged-in users. -- Platform-wide stats gated by the `dashboard.view` permission (held by `admin` through `*`, mapped to no tenant role): the counts span every tenant, so `/api/dashboard/stats` answers 403 without it and `/dashboard` renders only a welcome for viewers who lack it. +- Platform-wide stats gated by the `dashboard.view` permission (held by `admin` through `*`; mapped onto the platform `user` role only when `multi_tenant` is off, and to no tenant role): the counts span every tenant, so `/api/dashboard/stats` answers 403 without it and `/dashboard` renders only a welcome for viewers who lack it. - Global sidebar renderer — aggregates `register_menu_items()` calls from all modules into one tree. - Breadcrumb + page-title provider used by downstream module pages. diff --git a/modules/dashboard/dashboard/module.py b/modules/dashboard/dashboard/module.py index d436f600..c851aa4b 100644 --- a/modules/dashboard/dashboard/module.py +++ b/modules/dashboard/dashboard/module.py @@ -5,7 +5,7 @@ import importlib.resources from pathlib import Path -from fastapi import APIRouter +from fastapi import APIRouter, FastAPI from simple_module_core.menu import MenuItem, MenuRegistry, MenuSection from simple_module_core.module import ModuleBase, ModuleMeta from simple_module_core.permissions import PermissionRegistry @@ -47,6 +47,18 @@ def register_permissions(self, registry: PermissionRegistry) -> None: # so no tenant role is mapped to this. ``admin`` holds it through ``*``. registry.add_group(PERM_GROUP, [PERM_VIEW]) + async def on_startup(self, app: FastAPI) -> None: + # A single-tenant install has one tenant, so "every tenant" is just the + # install and ordinary users keep the stats they always had. With + # multi_tenant on every tenant member also holds ``user``, so mapping + # it there would hand each tenant the install-wide counts. Done here + # rather than in register_permissions, which cannot see the settings. + if getattr(app.state.sm.settings, "multi_tenant", False): + return + from users.constants import USER_ROLE_NAME + + app.state.sm.permissions.map_role(USER_ROLE_NAME, [PERM_VIEW]) + def register_menu_items(self, registry: MenuRegistry) -> None: registry.add( MenuItem( diff --git a/modules/dashboard/tests/test_dashboard_permission.py b/modules/dashboard/tests/test_dashboard_permission.py index 81eb4ad8..bd53ed52 100644 --- a/modules/dashboard/tests/test_dashboard_permission.py +++ b/modules/dashboard/tests/test_dashboard_permission.py @@ -30,6 +30,12 @@ def test_permission_is_registered_and_mapped_to_no_tenant_role(app): assert PERM_VIEW not in registry.role_map[tenant_role(role)] +def test_user_role_is_not_mapped_when_multi_tenant(app): + """Every tenant member holds ``user``; mapping it would leak the counts.""" + assert app.state.sm.settings.multi_tenant + assert PERM_VIEW not in app.state.sm.permissions.role_map.get("user", []) + + async def test_anonymous_is_refused(client): resp = await client.get(_STATS, follow_redirects=False) assert resp.status_code in (302, 401, 403) diff --git a/modules/dashboard/tests/test_dashboard_single_tenant.py b/modules/dashboard/tests/test_dashboard_single_tenant.py new file mode 100644 index 00000000..eac5f0fc --- /dev/null +++ b/modules/dashboard/tests/test_dashboard_single_tenant.py @@ -0,0 +1,51 @@ +"""Single-tenant installs keep the stats for ordinary users (review of #374). + +With ``multi_tenant`` off there is one tenant — the install — so the counts +leak nothing, and every signed-in ``user`` had them before ``dashboard.view`` +existed. With it on, every tenant member also holds ``user``, so the mapping +must not be made there (``test_dashboard_permission.py``). +""" + +from __future__ import annotations + +import pytest +from dashboard.constants import PERM_VIEW +from dashboard.stats import invalidate_stats_cache +from simple_module_hosting.settings import Settings +from simple_module_test.tenant_client import session_client + +_STATS = "/api/dashboard/stats" +_INERTIA = {"X-Inertia": "true", "Accept": "application/json"} + + +@pytest.fixture(autouse=True) +def _clear_stats_cache(): + invalidate_stats_cache() + yield + invalidate_stats_cache() + + +@pytest.fixture +def settings(settings: Settings) -> Settings: + return settings.model_copy(update={"multi_tenant": False, "tenant_header": ""}) + + +def test_user_role_is_mapped_to_dashboard_view(app): + assert PERM_VIEW in app.state.sm.permissions.role_map["user"] + + +async def _plain_user(app) -> str: + from users.bootstrap import create_standard_user + + async with app.state.sm.db.session_factory() as session: + result = await create_standard_user(session, email="plain@example.com", password="x" * 12) + await session.commit() + return str(result.user.id) + + +async def test_ordinary_user_sees_the_stats(app): + async with session_client(app, {"user_id": await _plain_user(app)}) as c: + assert (await c.get(_STATS)).status_code == 200 + props = (await c.get("/dashboard/", headers=_INERTIA)).json()["props"] + assert props["can_view_stats"] is True + assert props["total_users"] >= 1