From 7bb003cf2c9cfe6fba8f5881e9c23d59667f2b4f Mon Sep 17 00:00:00 2001 From: Anto Subash Date: Thu, 1 Oct 2026 13:22:00 +0200 Subject: [PATCH 1/4] feat(audit_log): stamp entries with the writing tenant and add tenant column/filter (#372) AuditEntry gets a nullable, indexed tenant_id (NULL = platform action) stamped from current_tenant_id in the capture callback, plus a (tenant_id, created_at) index. Deliberately not MultiTenantMixin: platform writes have no tenant and strict mode would raise inside the flush. /admin/audit-log stays platform-wide with a tenant column and filter on browse, API and CSV export. Claude-Session: https://claude.ai/code/session_01F8RiTBUJQnZmSq56qReZeV --- .../a7c2e91d4b58_audit_log_entry_tenant_id.py | 40 ++++++ modules/audit_log/audit_log/capture.py | 5 + modules/audit_log/audit_log/constants.py | 4 + .../audit_log/audit_log/contracts/schemas.py | 1 + modules/audit_log/audit_log/endpoints/api.py | 4 + .../audit_log/audit_log/endpoints/views.py | 6 + modules/audit_log/audit_log/export.py | 12 +- modules/audit_log/audit_log/filters.py | 9 ++ modules/audit_log/audit_log/locales/en.json | 8 +- modules/audit_log/audit_log/models.py | 6 + modules/audit_log/audit_log/pages/Browse.tsx | 20 ++- .../pages/components/BrowseEmpty.tsx | 1 + .../pages/components/EntriesTable.tsx | 12 +- .../audit_log/pages/components/FilterBar.tsx | 37 +++++- modules/audit_log/audit_log/service.py | 17 +++ modules/audit_log/tests/test_export_csv.py | 2 +- modules/audit_log/tests/test_tenancy.py | 121 ++++++++++++++++++ packages/i18n/src/generated-resources.ts | 4 + packages/i18n/src/keys.generated.ts | 4 + 19 files changed, 305 insertions(+), 8 deletions(-) create mode 100644 host/migrations/versions/a7c2e91d4b58_audit_log_entry_tenant_id.py create mode 100644 modules/audit_log/tests/test_tenancy.py diff --git a/host/migrations/versions/a7c2e91d4b58_audit_log_entry_tenant_id.py b/host/migrations/versions/a7c2e91d4b58_audit_log_entry_tenant_id.py new file mode 100644 index 00000000..b7924248 --- /dev/null +++ b/host/migrations/versions/a7c2e91d4b58_audit_log_entry_tenant_id.py @@ -0,0 +1,40 @@ +"""audit_log_audit_entry: add nullable tenant_id + +NULL means a platform action (no tenant bound when the row was written), which +is also what every pre-existing row is. The table is deliberately not +``MultiTenantMixin``: platform writes have no tenant and strict mode would +raise inside the flush. See GH #372. + +Revision ID: a7c2e91d4b58 +Revises: 70786227af4c +Create Date: 2026-10-01 12:00:00.000000 +""" + +from collections.abc import Sequence + +import sqlalchemy as sa +from alembic import op + +# revision identifiers, used by Alembic. +revision: str = "a7c2e91d4b58" +down_revision: str | None = "70786227af4c" +branch_labels: str | Sequence[str] | None = None +depends_on: str | Sequence[str] | None = None + +_TABLE = "audit_log_audit_entry" +_SINGLE = "ix_audit_log_audit_entry_tenant_id" +_COMPOSITE = "ix_audit_entry_tenant_created" + + +def upgrade() -> None: + with op.batch_alter_table(_TABLE) as batch: + batch.add_column(sa.Column("tenant_id", sa.String(length=50), nullable=True)) + batch.create_index(_SINGLE, ["tenant_id"], unique=False) + batch.create_index(_COMPOSITE, ["tenant_id", "created_at"], unique=False) + + +def downgrade() -> None: + with op.batch_alter_table(_TABLE) as batch: + batch.drop_index(_COMPOSITE) + batch.drop_index(_SINGLE) + batch.drop_column("tenant_id") diff --git a/modules/audit_log/audit_log/capture.py b/modules/audit_log/audit_log/capture.py index 2a79d546..094d8bc3 100644 --- a/modules/audit_log/audit_log/capture.py +++ b/modules/audit_log/audit_log/capture.py @@ -4,6 +4,7 @@ import logging +from simple_module_db import current_tenant_id from simple_module_db.audit import AuditRecord from sqlalchemy.orm import Session @@ -13,6 +14,9 @@ def audit_callback(session: Session, records: list[AuditRecord]) -> None: + # NULL = a platform action (no tenant bound). Deliberately not + # MultiTenantMixin: strict mode would raise inside the flush for those. + tenant_id = current_tenant_id.get() try: for record in records: entry = AuditEntry( @@ -22,6 +26,7 @@ def audit_callback(session: Session, records: list[AuditRecord]) -> None: changes=record.changes, user_id=record.user_id, correlation_id=record.correlation_id, + tenant_id=tenant_id, ) session.add(entry) except Exception: diff --git a/modules/audit_log/audit_log/constants.py b/modules/audit_log/audit_log/constants.py index 04d7ab87..d3954437 100644 --- a/modules/audit_log/audit_log/constants.py +++ b/modules/audit_log/audit_log/constants.py @@ -42,3 +42,7 @@ PAGE_BROWSE: Final = f"{MODULE_NAME}/Browse" STATUS_OK: Final = 200 + +TENANT_ID_MAX_LENGTH = 50 +# Filter value meaning "entries with no tenant" (platform actions). +PLATFORM_TENANT_FILTER = "__platform__" diff --git a/modules/audit_log/audit_log/contracts/schemas.py b/modules/audit_log/audit_log/contracts/schemas.py index febb9334..787f339b 100644 --- a/modules/audit_log/audit_log/contracts/schemas.py +++ b/modules/audit_log/audit_log/contracts/schemas.py @@ -19,6 +19,7 @@ class AuditEntryRead(SQLModel): changes: list[dict] user_id: str | None = None correlation_id: str | None = None + tenant_id: str | None = None created_at: datetime diff --git a/modules/audit_log/audit_log/endpoints/api.py b/modules/audit_log/audit_log/endpoints/api.py index a2f45a30..a2d53d3a 100644 --- a/modules/audit_log/audit_log/endpoints/api.py +++ b/modules/audit_log/audit_log/endpoints/api.py @@ -30,6 +30,7 @@ async def list_audit_entries( action: str | None = Query(default=None), user_id: str | None = Query(default=None), correlation_id: str | None = Query(default=None), + tenant_id: str | None = Query(default=None), from_date: datetime | None = Query(default=None), to_date: datetime | None = Query(default=None), page: int = Query(default=1, ge=1), @@ -41,6 +42,7 @@ async def list_audit_entries( action=action, user_id=user_id, correlation_id=correlation_id, + tenant_id=tenant_id, from_date=from_date, to_date=to_date, page=page, @@ -58,6 +60,7 @@ async def export_audit_entries( action: str | None = Query(default=None), user_id: str | None = Query(default=None), correlation_id: str | None = Query(default=None), + tenant_id: str | None = Query(default=None), from_date: datetime | None = Query(default=None), to_date: datetime | None = Query(default=None), ) -> StreamingResponse: @@ -80,6 +83,7 @@ async def export_audit_entries( correlation_id=correlation_id, from_date=from_date, to_date=to_date, + tenant_id=tenant_id, ) return StreamingResponse( diff --git a/modules/audit_log/audit_log/endpoints/views.py b/modules/audit_log/audit_log/endpoints/views.py index 6025b13e..0c546f68 100644 --- a/modules/audit_log/audit_log/endpoints/views.py +++ b/modules/audit_log/audit_log/endpoints/views.py @@ -20,6 +20,7 @@ MAX_PAGE_SIZE, PAGE_BROWSE, PERM_VIEW, + PLATFORM_TENANT_FILTER, ) from audit_log.deps import AuditLogServiceDep from audit_log.filters import EntryFilters @@ -77,6 +78,7 @@ async def browse( action: str | None = Query(default=None), user_id: str | None = Query(default=None), correlation_id: str | None = Query(default=None), + tenant_id: str | None = Query(default=None), from_date: datetime | None = Query(default=None), to_date: datetime | None = Query(default=None), page: str | None = Query(default=None), @@ -97,6 +99,7 @@ async def browse( correlation_id=correlation_id, from_date=from_date, to_date=to_date, + tenant_id=tenant_id or None, ) result = await service.list_filtered(filters, page=page_int, page_size=page_size_int) @@ -152,6 +155,8 @@ async def browse( "page": result.page, "page_size": result.page_size, "entity_types": entity_types, + "tenant_ids": await service.distinct_tenant_ids(), + "platform_tenant_value": PLATFORM_TENANT_FILTER, "export_url": f"{API_PREFIX}/export.csv", "filters": { "entity_type": entity_type, @@ -161,6 +166,7 @@ async def browse( # what was typed into it. "user_id": actor_term or None, "correlation_id": correlation_id, + "tenant_id": tenant_id or None, "from_date": from_date.date().isoformat() if from_date else None, "to_date": to_date.date().isoformat() if to_date else None, }, diff --git a/modules/audit_log/audit_log/export.py b/modules/audit_log/audit_log/export.py index e6ac67ff..df03cb47 100644 --- a/modules/audit_log/audit_log/export.py +++ b/modules/audit_log/audit_log/export.py @@ -26,7 +26,16 @@ from audit_log.resolve import resolve_actors, resolve_entity_labels from audit_log.service import AuditLogService -CSV_COLUMNS = ("time", "action", "entity_type", "entity_id", "entity_label", "actor", "changes") +CSV_COLUMNS = ( + "time", + "action", + "entity_type", + "entity_id", + "entity_label", + "actor", + "tenant_id", + "changes", +) CSV_MEDIA_TYPE = "text/csv; charset=utf-8" CSV_FILENAME = "audit-log.csv" _ARROW = " → " @@ -87,6 +96,7 @@ def _row(entry: AuditEntryRead, *, entity_label: str, actor: str) -> list[str]: entry.entity_id, entity_label, actor, + entry.tenant_id or "", format_changes(entry.changes), ) ] diff --git a/modules/audit_log/audit_log/filters.py b/modules/audit_log/audit_log/filters.py index 673c6447..98f7d002 100644 --- a/modules/audit_log/audit_log/filters.py +++ b/modules/audit_log/audit_log/filters.py @@ -13,6 +13,7 @@ from datetime import datetime, time from typing import Any +from audit_log.constants import PLATFORM_TENANT_FILTER from audit_log.models import AuditEntry @@ -47,6 +48,8 @@ class EntryFilters: correlation_id: str | None = None from_date: datetime | None = None to_date: datetime | None = None + # A tenant id, or PLATFORM_TENANT_FILTER for entries with no tenant. + tenant_id: str | None = None @classmethod def for_date_only_range( @@ -59,6 +62,7 @@ def for_date_only_range( correlation_id: str | None = None, from_date: datetime | None = None, to_date: datetime | None = None, + tenant_id: str | None = None, ) -> EntryFilters: """Filters for the screen's controls, whose Date range is date-only. @@ -79,6 +83,7 @@ def for_date_only_range( correlation_id=correlation_id, from_date=from_date, to_date=end_of_day(to_date), + tenant_id=tenant_id, ) def conditions(self) -> list[Any]: @@ -88,6 +93,10 @@ def conditions(self) -> list[Any]: conditions.append(AuditEntry.entity_type == self.entity_type) if self.entity_id: conditions.append(AuditEntry.entity_id == self.entity_id) + if self.tenant_id == PLATFORM_TENANT_FILTER: + conditions.append(AuditEntry.tenant_id.is_(None)) + elif self.tenant_id: + conditions.append(AuditEntry.tenant_id == self.tenant_id) if self.action: conditions.append(AuditEntry.action == self.action) if self.user_id: diff --git a/modules/audit_log/audit_log/locales/en.json b/modules/audit_log/audit_log/locales/en.json index eb6fa17e..1becc13f 100644 --- a/modules/audit_log/audit_log/locales/en.json +++ b/modules/audit_log/audit_log/locales/en.json @@ -23,14 +23,18 @@ "date_range_any": "Any date", "date_range_reset": "Clear dates", "apply": "Apply", - "clear": "Clear" + "clear": "Clear", + "tenant_label": "Tenant", + "tenant_all": "All tenants", + "tenant_platform": "Platform" }, "table": { "timestamp": "Time", "action": "Action", "entity": "Entity", "user": "Actor", - "changes": "Changes" + "changes": "Changes", + "tenant": "Tenant" }, "actions": { "created": "created", diff --git a/modules/audit_log/audit_log/models.py b/modules/audit_log/audit_log/models.py index 10baad0c..7230bfc9 100644 --- a/modules/audit_log/audit_log/models.py +++ b/modules/audit_log/audit_log/models.py @@ -16,6 +16,7 @@ ENTITY_TYPE_MAX_LENGTH, MODULE_PACKAGE, TABLE_AUDIT_ENTRY, + TENANT_ID_MAX_LENGTH, USER_ID_MAX_LENGTH, ) @@ -31,6 +32,7 @@ class AuditEntry(Base, table=True): # ty: ignore[unsupported-base] Index("ix_audit_entry_entity_id", "entity_id"), Index("ix_audit_entry_user_id", "user_id"), Index("ix_audit_entry_created_at", "created_at"), + Index("ix_audit_entry_tenant_created", "tenant_id", "created_at"), ) id: uuid.UUID = Field(default_factory=uuid.uuid4, primary_key=True) @@ -40,6 +42,10 @@ class AuditEntry(Base, table=True): # ty: ignore[unsupported-base] changes: dict | list = Field(default_factory=list, sa_column=Column(JSON)) user_id: str | None = Field(default=None, max_length=USER_ID_MAX_LENGTH) correlation_id: str | None = Field(default=None, max_length=CORRELATION_ID_MAX_LENGTH) + # Not MultiTenantMixin on purpose: platform writes have no tenant and strict + # mode would raise inside the flush. NULL means a platform action; the admin + # screens read across tenants and filter on this column. + tenant_id: str | None = Field(default=None, max_length=TENANT_ID_MAX_LENGTH, index=True) created_at: datetime = Field( default_factory=lambda: datetime.now(UTC), sa_type=DateTime(timezone=True), diff --git a/modules/audit_log/audit_log/pages/Browse.tsx b/modules/audit_log/audit_log/pages/Browse.tsx index 7bb725a5..6ff6befc 100644 --- a/modules/audit_log/audit_log/pages/Browse.tsx +++ b/modules/audit_log/audit_log/pages/Browse.tsx @@ -25,6 +25,8 @@ interface Props { page: number; page_size: number; entity_types: EntityTypeOption[]; + tenant_ids: string[]; + platform_tenant_value: string; /** Where the CSV lives; the current filters are appended to it. */ export_url: string; /** `correlation_id` is set only by the per-row "Related" pivot — it has no @@ -35,6 +37,7 @@ interface Props { const CLEARED: FilterState = { entityType: ALL, action: ALL, + tenantId: ALL, userId: '', fromDate: '', toDate: '', @@ -51,6 +54,7 @@ function queryFor( const p: Record = {}; if (next.entityType && next.entityType !== ALL) p.entity_type = next.entityType; if (next.action && next.action !== ALL) p.action = next.action; + if (next.tenantId && next.tenantId !== ALL) p.tenant_id = next.tenantId; if (next.userId) p.user_id = next.userId; if (correlationId) p.correlation_id = correlationId; if (next.fromDate) p.from_date = next.fromDate; @@ -61,7 +65,17 @@ function queryFor( } function Browse() { - const { items, total, page, page_size, entity_types, export_url, filters } = usePage<{ + const { + items, + total, + page, + page_size, + entity_types, + tenant_ids, + platform_tenant_value, + export_url, + filters, + } = usePage<{ props: Props; }>().props as unknown as Props; const { t } = useT(); @@ -69,6 +83,7 @@ function Browse() { const [state, setState] = useState({ entityType: filters.entity_type ?? ALL, action: filters.action ?? ALL, + tenantId: filters.tenant_id ?? ALL, userId: filters.user_id ?? '', fromDate: filters.from_date ?? '', toDate: filters.to_date ?? '', @@ -105,6 +120,7 @@ function Browse() { { entityType: filters.entity_type ?? ALL, action: filters.action ?? ALL, + tenantId: filters.tenant_id ?? ALL, userId: filters.user_id ?? '', fromDate: filters.from_date ?? '', toDate: filters.to_date ?? '', @@ -132,6 +148,8 @@ function Browse() { navigate(state)} onClear={handleClear} diff --git a/modules/audit_log/audit_log/pages/components/BrowseEmpty.tsx b/modules/audit_log/audit_log/pages/components/BrowseEmpty.tsx index 78b09194..ed780a44 100644 --- a/modules/audit_log/audit_log/pages/components/BrowseEmpty.tsx +++ b/modules/audit_log/audit_log/pages/components/BrowseEmpty.tsx @@ -72,6 +72,7 @@ export function BrowseEmpty({ applied, entityTypes, onClear }: BrowseEmptyProps) `${t(keys.audit_log.filters.entity_type_label)}: ${typeLabel(entityTypes, applied.entity_type)}`, applied.action && `${t(keys.audit_log.filters.action_label)}: ${actionLabel(t, applied.action)}`, + applied.tenant_id && `${t(keys.audit_log.filters.tenant_label)}: ${applied.tenant_id}`, applied.user_id && `${t(keys.audit_log.filters.user_label)}: ${applied.user_id}`, applied.correlation_id && `${t(keys.audit_log.correlation.view_related)}: ${applied.correlation_id}`, diff --git a/modules/audit_log/audit_log/pages/components/EntriesTable.tsx b/modules/audit_log/audit_log/pages/components/EntriesTable.tsx index 6bd89908..ea33b1f6 100644 --- a/modules/audit_log/audit_log/pages/components/EntriesTable.tsx +++ b/modules/audit_log/audit_log/pages/components/EntriesTable.tsx @@ -29,6 +29,8 @@ export interface AuditEntryRead { * for another (see `AuditLink.label_permission`). */ entity: EntityRef; correlation_id: string | null; + /** Tenant that wrote the entry; null for a platform action. */ + tenant_id: string | null; created_at: string; } @@ -52,7 +54,7 @@ interface Props { onCorrelationSelect: (id: string) => void; } -/** The audit table itself: five columns, one row per entry. +/** The audit table itself: six columns, one row per entry. * * Split out of `Browse` so the page keeps its filter/pagination/navigation * logic in one screenful and the row rendering in another — the two change for @@ -71,6 +73,9 @@ export function EntriesTable({ items, correlationId, onCorrelationSelect }: Prop + @@ -107,6 +112,11 @@ export function EntriesTable({ items, correlationId, onCorrelationSelect }: Prop + {/* `TableCell` is `whitespace-nowrap` by default, which made one long value push the table wider than the card and cut every updated row mid-value. */} diff --git a/modules/audit_log/audit_log/pages/components/FilterBar.tsx b/modules/audit_log/audit_log/pages/components/FilterBar.tsx index 111387b6..6716a08d 100644 --- a/modules/audit_log/audit_log/pages/components/FilterBar.tsx +++ b/modules/audit_log/audit_log/pages/components/FilterBar.tsx @@ -27,6 +27,7 @@ export interface EntityTypeOption { export interface FilterState { entityType: string; action: string; + tenantId: string; userId: string; fromDate: string; toDate: string; @@ -38,6 +39,7 @@ export interface FilterState { export interface AppliedFilters { entity_type: string | null; action: string | null; + tenant_id: string | null; user_id: string | null; correlation_id: string | null; from_date: string | null; @@ -47,6 +49,10 @@ export interface AppliedFilters { interface FilterBarProps { state: FilterState; entity_types: EntityTypeOption[]; + /** Tenant ids that have entries; the screen is platform-wide. */ + tenant_ids: string[]; + /** Filter value that selects entries with no tenant (platform actions). */ + platform_tenant_value: string; onChange: (next: FilterState) => void; onSubmit: () => void; onClear: () => void; @@ -73,14 +79,22 @@ function Field({ ); } -export function FilterBar({ state, entity_types, onChange, onSubmit, onClear }: FilterBarProps) { +export function FilterBar({ + state, + entity_types, + tenant_ids, + platform_tenant_value, + onChange, + onSubmit, + onClear, +}: FilterBarProps) { const { t } = useT(); const set = (patch: Partial) => onChange({ ...state, ...patch }); return (
{ e.preventDefault(); onSubmit(); @@ -118,6 +132,25 @@ export function FilterBar({ state, entity_types, onChange, onSubmit, onClear }: + + + + list[str]: provider = DatabaseProvider(self.db.bind.dialect.name) result = await self.db.execute(_distinct_stmt_for_dialect(provider)) return list(result.scalars()) + + async def distinct_tenant_ids(self) -> list[str]: + """Every tenant id that has written an entry — feeds the tenant filter. + + Platform entries (NULL) are not listed; the screen offers them as a + fixed "Platform" option. + """ + stmt = ( + select(AuditEntry.tenant_id) + .where(AuditEntry.tenant_id.isnot(None)) + .distinct() + .order_by(AuditEntry.tenant_id) + ) + return list((await self.db.execute(stmt)).scalars()) diff --git a/modules/audit_log/tests/test_export_csv.py b/modules/audit_log/tests/test_export_csv.py index 7bf945b4..66a82c8b 100644 --- a/modules/audit_log/tests/test_export_csv.py +++ b/modules/audit_log/tests/test_export_csv.py @@ -52,7 +52,7 @@ async def test_header_names_every_column(self, authenticated_client: httpx.Async assert resp.status_code == 200, resp.text header = resp.text.splitlines()[0] - assert header == "time,action,entity_type,entity_id,entity_label,actor,changes" + assert header == "time,action,entity_type,entity_id,entity_label,actor,tenant_id,changes" async def test_response_is_offered_as_a_download( self, authenticated_client: httpx.AsyncClient diff --git a/modules/audit_log/tests/test_tenancy.py b/modules/audit_log/tests/test_tenancy.py new file mode 100644 index 00000000..eb506bf0 --- /dev/null +++ b/modules/audit_log/tests/test_tenancy.py @@ -0,0 +1,121 @@ +"""AuditEntry carries the tenant of the write that produced it (#372). + +The table is not tenant-scoped: platform writes have no tenant (NULL), and the +admin screens read across tenants with a tenant filter. +""" + +from __future__ import annotations + +import csv +import io + +import pytest +from audit_log.capture import audit_callback +from audit_log.constants import PLATFORM_TENANT_FILTER +from audit_log.models import AuditEntry +from simple_module_core.tenancy import TenantRole +from simple_module_db import all_tenants, tenant_context +from simple_module_db.audit import AuditRecord +from sqlalchemy import select + +LIST_URL = "/api/audit_log/" +EXPORT_URL = "/api/audit_log/export.csv" +_INERTIA = {"X-Inertia": "true", "Accept": "application/json"} + + +def _record(entity_id: str) -> AuditRecord: + return AuditRecord( + entity_type="Widget", + entity_id=entity_id, + action="created", + changes=[], + user_id=None, + correlation_id=None, + ) + + +async def _entry(db_session, entity_id: str) -> AuditEntry: + with all_tenants(): + return ( + await db_session.execute(select(AuditEntry).where(AuditEntry.entity_id == entity_id)) + ).scalar_one() + + +async def test_capture_stamps_current_tenant(db_session): + with tenant_context("tenant-a"): + audit_callback(db_session.sync_session, [_record("w1")]) + await db_session.flush() + assert (await _entry(db_session, "w1")).tenant_id == "tenant-a" + + +async def test_platform_write_has_null_tenant(db_session): + audit_callback(db_session.sync_session, [_record("w2")]) + await db_session.flush() + assert (await _entry(db_session, "w2")).tenant_id is None + + with all_tenants(): + audit_callback(db_session.sync_session, [_record("w3")]) + await db_session.flush() + assert (await _entry(db_session, "w3")).tenant_id is None + + +@pytest.fixture +async def seeded(app): + async with app.state.sm.db.session_factory() as session: + for tenant, n in (("tenant-a", 2), ("tenant-b", 1), (None, 1)): + for i in range(n): + session.add( + AuditEntry( + entity_type="Widget", + entity_id=f"{tenant}-{i}", + action="created", + changes=[], + tenant_id=tenant, + ) + ) + await session.commit() + + +async def test_list_filter_by_tenant(authenticated_client, seeded): + resp = await authenticated_client.get(LIST_URL, params={"tenant_id": "tenant-a"}) + body = resp.json() + assert body["total"] == 2 + assert {i["tenant_id"] for i in body["items"]} == {"tenant-a"} + + # Platform entries (NULL) include the admin seeding, so only assert shape. + resp = await authenticated_client.get( + LIST_URL, params={"tenant_id": PLATFORM_TENANT_FILTER, "page_size": 200} + ) + items = resp.json()["items"] + assert {i["tenant_id"] for i in items} == {None} + assert "None-0" in {i["entity_id"] for i in items} + + resp = await authenticated_client.get(LIST_URL, params={"entity_type": "Widget"}) + assert resp.json()["total"] == 4 # unfiltered stays platform-wide + + +async def test_export_filters_and_lists_tenant(authenticated_client, seeded): + resp = await authenticated_client.get(EXPORT_URL, params={"tenant_id": "tenant-b"}) + rows = list(csv.DictReader(io.StringIO(resp.text))) + assert [r["tenant_id"] for r in rows] == ["tenant-b"] + + resp = await authenticated_client.get(EXPORT_URL) + tenants = {r["tenant_id"] for r in csv.DictReader(io.StringIO(resp.text))} + assert {"tenant-a", "tenant-b", ""} <= tenants + + +async def test_browse_view_exposes_tenant_ids(authenticated_client, seeded): + resp = await authenticated_client.get( + "/admin/audit-log/", params={"tenant_id": "tenant-a"}, headers=_INERTIA + ) + props = resp.json()["props"] + assert {"tenant-a", "tenant-b"} <= set(props["tenant_ids"]) + assert props["total"] == 2 + assert props["filters"]["tenant_id"] == "tenant-a" + + +@pytest.mark.parametrize("role", list(TenantRole)) +async def test_tenant_member_cannot_read_audit_log(tenant_client, role): + async with tenant_client(role) as m: + assert (await m.client.get(LIST_URL)).status_code == 403 + assert (await m.client.get(EXPORT_URL)).status_code == 403 diff --git a/packages/i18n/src/generated-resources.ts b/packages/i18n/src/generated-resources.ts index d170d116..32067c57 100644 --- a/packages/i18n/src/generated-resources.ts +++ b/packages/i18n/src/generated-resources.ts @@ -40,12 +40,16 @@ export default { 'audit_log.filters.date_range_reset': '', 'audit_log.filters.entity_type_all': '', 'audit_log.filters.entity_type_label': '', + 'audit_log.filters.tenant_all': '', + 'audit_log.filters.tenant_label': '', + 'audit_log.filters.tenant_platform': '', 'audit_log.filters.user_label': '', 'audit_log.filters.user_placeholder': '', 'audit_log.nav.audit_log': '', 'audit_log.table.action': '', 'audit_log.table.changes': '', 'audit_log.table.entity': '', + 'audit_log.table.tenant': '', 'audit_log.table.timestamp': '', 'audit_log.table.user': '', 'auth.errors.missing_permission': '', diff --git a/packages/i18n/src/keys.generated.ts b/packages/i18n/src/keys.generated.ts index f9c2a15a..9ec68d82 100644 --- a/packages/i18n/src/keys.generated.ts +++ b/packages/i18n/src/keys.generated.ts @@ -51,6 +51,9 @@ export const keys = { date_range_reset: 'audit_log.filters.date_range_reset', entity_type_all: 'audit_log.filters.entity_type_all', entity_type_label: 'audit_log.filters.entity_type_label', + tenant_all: 'audit_log.filters.tenant_all', + tenant_label: 'audit_log.filters.tenant_label', + tenant_platform: 'audit_log.filters.tenant_platform', user_label: 'audit_log.filters.user_label', user_placeholder: 'audit_log.filters.user_placeholder', }, @@ -61,6 +64,7 @@ export const keys = { action: 'audit_log.table.action', changes: 'audit_log.table.changes', entity: 'audit_log.table.entity', + tenant: 'audit_log.table.tenant', timestamp: 'audit_log.table.timestamp', user: 'audit_log.table.user', }, From 62fee387bf1a38508a7468c678fac64a73be0e87 Mon Sep 17 00:00:00 2001 From: Anto Subash Date: Thu, 1 Oct 2026 15:15:58 +0200 Subject: [PATCH 2/4] fix(audit_log): name the platform filter and keep CSV columns positional (review of #372) - BrowseEmpty showed the raw __platform__ sentinel in its filter summary; map it to audit_log.filters.tenant_platform (Browse passes the value). - The CSV export appends tenant_id after changes instead of inserting it before, so positional consumers keep finding every old column. - Document that a platform admin's actions are attributed to their active organisation when one is active. Claude-Session: https://claude.ai/code/session_01F8RiTBUJQnZmSq56qReZeV --- docs/framework/multi-tenancy.md | 8 +++ docs/modules/audit_log.md | 20 +++++++ modules/audit_log/audit_log/export.py | 7 ++- modules/audit_log/audit_log/pages/Browse.tsx | 7 ++- .../pages/components/BrowseEmpty.tsx | 17 +++++- .../audit_log/tests-js/BrowseEmpty.test.tsx | 52 +++++++++++++++++++ modules/audit_log/tests/test_export_csv.py | 2 +- 7 files changed, 107 insertions(+), 6 deletions(-) create mode 100644 modules/audit_log/tests-js/BrowseEmpty.test.tsx diff --git a/docs/framework/multi-tenancy.md b/docs/framework/multi-tenancy.md index f770f2b3..a4dc08fc 100644 --- a/docs/framework/multi-tenancy.md +++ b/docs/framework/multi-tenancy.md @@ -233,6 +233,14 @@ Screens that take a tenant id from the URL can vet it without importing `feature_flags` uses it to 404 on an unknown tenant when setting or listing overrides; clearing stays unvalidated so a stale override can still be removed. +## Audit log + +`audit_log` stamps every entry with the tenant bound when the write flushed, +`NULL` when none was (#372); the table is platform-wide, read with a tenant +filter. A platform admin's actions are attributed to their **active** +organisation when one is active — the write itself is scoped to it — and to the +platform only when nothing is bound. See [audit_log](/modules/audit_log#multi-tenancy). + ## Testing The `simple_module_test` plugin ships `tenant_client` (needs the `users` and diff --git a/docs/modules/audit_log.md b/docs/modules/audit_log.md index 1b2d048e..94e39a6a 100644 --- a/docs/modules/audit_log.md +++ b/docs/modules/audit_log.md @@ -105,9 +105,29 @@ from audit_log.contracts.schemas import AuditEntryRead, AuditEntryList | `user_id` | `str(255) \| None` | indexed; from the request's `current_user_id` | | `correlation_id` | `str(255) \| None` | request correlation id | | `created_at` | `datetime` | indexed; tz-aware, `server_default = now()` | +| `tenant_id` | `str(50) \| None` | indexed; the tenant bound when the write happened, `NULL` for platform writes | The table itself carries `__audit_exclude__ = True` so audit writes never re-enter the capture loop. +## Multi-tenancy + +Each entry records the tenant bound when its write flushed (#372). The table is +deliberately **not** `MultiTenantMixin`: the audit log is a platform screen over +every tenant's entries, filtered by a *Tenant* control whose *Platform* choice +selects the entries with no tenant (writes made with nothing bound, or inside +`all_tenants()`). + +The CSV export carries the tenant as its **last** column, after `changes`, so a +consumer that reads the file by position keeps working. + +**Attribution follows the active tenant, not the actor's role.** A platform +admin who has an organisation active when they act is acting *in* that +organisation: the write is scoped to it, so its entry carries that tenant id +and appears under the tenant's filter, not under *Platform*. Only work done +with no tenant bound (no organisation active, a CLI command, a platform job) +is recorded as a platform entry. Filter by user to see everything one admin +did across tenants. + ## Entity and actor names An entry stores a model class name and a primary key, which proves what happened and names nobody. The browse screen, the CSV export and the users edit page's activity card all resolve those ids at render time through the audit-link registry: each module supplies a batch `label_resolver` naming its own rows, one query per entity type per page. Nothing is stored — the row keeps the ids it recorded. diff --git a/modules/audit_log/audit_log/export.py b/modules/audit_log/audit_log/export.py index df03cb47..e2d48b73 100644 --- a/modules/audit_log/audit_log/export.py +++ b/modules/audit_log/audit_log/export.py @@ -33,8 +33,11 @@ "entity_id", "entity_label", "actor", - "tenant_id", "changes", + # Appended last, not slotted in beside ``actor``: consumers that read the + # file by position (a spreadsheet macro, ``cut -d,``) predate the column + # and must keep finding ``changes`` where it always was. + "tenant_id", ) CSV_MEDIA_TYPE = "text/csv; charset=utf-8" CSV_FILENAME = "audit-log.csv" @@ -96,8 +99,8 @@ def _row(entry: AuditEntryRead, *, entity_label: str, actor: str) -> list[str]: entry.entity_id, entity_label, actor, - entry.tenant_id or "", format_changes(entry.changes), + entry.tenant_id or "", ) ] diff --git a/modules/audit_log/audit_log/pages/Browse.tsx b/modules/audit_log/audit_log/pages/Browse.tsx index 6ff6befc..dd4ed7fc 100644 --- a/modules/audit_log/audit_log/pages/Browse.tsx +++ b/modules/audit_log/audit_log/pages/Browse.tsx @@ -161,7 +161,12 @@ function Browse() { {items.length === 0 ? ( - + ) : ( void; } @@ -52,7 +55,12 @@ interface BrowseEmptyProps { * filtered case therefore names the filters doing the excluding, so the reader * can see it is their query and not the record that is empty. */ -export function BrowseEmpty({ applied, entityTypes, onClear }: BrowseEmptyProps) { +export function BrowseEmpty({ + applied, + entityTypes, + platformTenantValue, + onClear, +}: BrowseEmptyProps) { const { t } = useT(); if (!hasActiveFilters(applied)) { @@ -72,7 +80,12 @@ export function BrowseEmpty({ applied, entityTypes, onClear }: BrowseEmptyProps) `${t(keys.audit_log.filters.entity_type_label)}: ${typeLabel(entityTypes, applied.entity_type)}`, applied.action && `${t(keys.audit_log.filters.action_label)}: ${actionLabel(t, applied.action)}`, - applied.tenant_id && `${t(keys.audit_log.filters.tenant_label)}: ${applied.tenant_id}`, + applied.tenant_id && + `${t(keys.audit_log.filters.tenant_label)}: ${ + applied.tenant_id === platformTenantValue + ? t(keys.audit_log.filters.tenant_platform) + : applied.tenant_id + }`, applied.user_id && `${t(keys.audit_log.filters.user_label)}: ${applied.user_id}`, applied.correlation_id && `${t(keys.audit_log.correlation.view_related)}: ${applied.correlation_id}`, diff --git a/modules/audit_log/tests-js/BrowseEmpty.test.tsx b/modules/audit_log/tests-js/BrowseEmpty.test.tsx new file mode 100644 index 00000000..2093ec07 --- /dev/null +++ b/modules/audit_log/tests-js/BrowseEmpty.test.tsx @@ -0,0 +1,52 @@ +import '@testing-library/jest-dom/vitest'; +import { configureI18n } from '@simple-module-py/i18n'; +import { render, screen } from '@testing-library/react'; +import { describe, expect, test } from 'vitest'; + +configureI18n({ + locale: 'en', + messages: { + 'audit_log.browse.no_match_title': 'No entries match these filters', + 'audit_log.browse.clear_filters': 'Clear filters', + 'audit_log.filters.tenant_label': 'Tenant', + 'audit_log.filters.tenant_platform': 'Platform', + }, +}); + +import { BrowseEmpty } from '../audit_log/pages/components/BrowseEmpty'; + +const NONE = { + entity_type: null, + action: null, + tenant_id: null, + user_id: null, + correlation_id: null, + from_date: null, + to_date: null, +}; + +function renderWith(tenantId: string) { + render( + {}} + />, + ); +} + +describe('BrowseEmpty tenant summary', () => { + test('the platform filter is named, never shown as its sentinel', () => { + renderWith('__platform__'); + + expect(screen.getByText('Tenant: Platform')).toBeInTheDocument(); + expect(screen.queryByText(/__platform__/)).not.toBeInTheDocument(); + }); + + test('a real tenant id is shown as is', () => { + renderWith('acme'); + + expect(screen.getByText('Tenant: acme')).toBeInTheDocument(); + }); +}); diff --git a/modules/audit_log/tests/test_export_csv.py b/modules/audit_log/tests/test_export_csv.py index 66a82c8b..63691310 100644 --- a/modules/audit_log/tests/test_export_csv.py +++ b/modules/audit_log/tests/test_export_csv.py @@ -52,7 +52,7 @@ async def test_header_names_every_column(self, authenticated_client: httpx.Async assert resp.status_code == 200, resp.text header = resp.text.splitlines()[0] - assert header == "time,action,entity_type,entity_id,entity_label,actor,tenant_id,changes" + assert header == "time,action,entity_type,entity_id,entity_label,actor,changes,tenant_id" async def test_response_is_offered_as_a_download( self, authenticated_client: httpx.AsyncClient From 00ff1f7c8b9e711f4a0f1be1c0e1c13a50182ccf Mon Sep 17 00:00:00 2001 From: Anto Subash Date: Thu, 1 Oct 2026 16:57:04 +0200 Subject: [PATCH 3/4] fix(audit_log): clamp huge ?page= instead of overflowing into a 500 (qa F28) Claude-Session: https://claude.ai/code/session_01F8RiTBUJQnZmSq56qReZeV --- modules/audit_log/audit_log/constants.py | 4 ++++ modules/audit_log/audit_log/service.py | 4 ++-- modules/audit_log/tests/test_huge_page.py | 21 +++++++++++++++++++++ 3 files changed, 27 insertions(+), 2 deletions(-) create mode 100644 modules/audit_log/tests/test_huge_page.py diff --git a/modules/audit_log/audit_log/constants.py b/modules/audit_log/audit_log/constants.py index d3954437..95511ef3 100644 --- a/modules/audit_log/audit_log/constants.py +++ b/modules/audit_log/audit_log/constants.py @@ -38,6 +38,10 @@ DEFAULT_PAGE_SIZE: Final = 50 MAX_PAGE_SIZE: Final = 200 +# Upper bound on ``page``: keeps OFFSET inside a 64-bit int (a ?page= of 10**20 +# overflowed the driver and returned 500). Anything past the end is clamped to +# the last page by the view. +MAX_PAGE: Final = 1_000_000 PAGE_BROWSE: Final = f"{MODULE_NAME}/Browse" diff --git a/modules/audit_log/audit_log/service.py b/modules/audit_log/audit_log/service.py index f61911ab..a4942bec 100644 --- a/modules/audit_log/audit_log/service.py +++ b/modules/audit_log/audit_log/service.py @@ -9,7 +9,7 @@ from sqlalchemy import Select, and_, func, literal_column, or_, select from sqlalchemy.ext.asyncio import AsyncSession -from audit_log.constants import DEFAULT_PAGE_SIZE, MAX_PAGE_SIZE +from audit_log.constants import DEFAULT_PAGE_SIZE, MAX_PAGE, MAX_PAGE_SIZE from audit_log.contracts.schemas import AuditEntryList, AuditEntryRead from audit_log.filters import EntryFilters from audit_log.models import AuditEntry @@ -108,7 +108,7 @@ async def list_filtered( page_size: int = DEFAULT_PAGE_SIZE, ) -> AuditEntryList: page_size = min(max(page_size, 1), MAX_PAGE_SIZE) - page = max(page, 1) + page = min(max(page, 1), MAX_PAGE) conditions = filters.conditions() # Count the same conditions directly rather than wrapping the row query diff --git a/modules/audit_log/tests/test_huge_page.py b/modules/audit_log/tests/test_huge_page.py new file mode 100644 index 00000000..b551fa56 --- /dev/null +++ b/modules/audit_log/tests/test_huge_page.py @@ -0,0 +1,21 @@ +"""qa F28: an absurd ``?page=`` must clamp, not overflow the driver into a 500.""" + +from __future__ import annotations + +HUGE = "99999999999999999999" + + +async def test_view_with_huge_page_clamps(authenticated_client) -> None: + resp = await authenticated_client.get( + "/admin/audit-log/", + params={"page": HUGE}, + headers={"X-Inertia": "true", "Accept": "application/json"}, + ) + assert resp.status_code == 200, resp.text + assert resp.json()["props"]["page"] == 1 + + +async def test_api_with_huge_page_does_not_500(authenticated_client) -> None: + resp = await authenticated_client.get("/api/audit_log/", params={"page": HUGE}) + assert resp.status_code == 200, resp.text + assert resp.json()["items"] == [] From 1caa633160dbcebdfffa6ab009d4888f96713fcf Mon Sep 17 00:00:00 2001 From: Anto Subash Date: Thu, 1 Oct 2026 19:41:01 +0200 Subject: [PATCH 4/4] test(e2e): expect the audit-log CSV's trailing tenant_id column (#372) Claude-Session: https://claude.ai/code/session_01F8RiTBUJQnZmSq56qReZeV --- tests/e2e/test_mutating_flows.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/e2e/test_mutating_flows.py b/tests/e2e/test_mutating_flows.py index 37216424..a8833c3c 100644 --- a/tests/e2e/test_mutating_flows.py +++ b/tests/e2e/test_mutating_flows.py @@ -137,4 +137,4 @@ def test_the_audit_log_export_downloads_a_csv_with_a_header_row( with Path(download.path()).open(encoding="utf-8") as handle: first_line = handle.readline().rstrip("\r\n") - assert first_line == "time,action,entity_type,entity_id,entity_label,actor,changes" + assert first_line == "time,action,entity_type,entity_id,entity_label,actor,changes,tenant_id"