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/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..95511ef3 100644 --- a/modules/audit_log/audit_log/constants.py +++ b/modules/audit_log/audit_log/constants.py @@ -38,7 +38,15 @@ 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" 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..e2d48b73 100644 --- a/modules/audit_log/audit_log/export.py +++ b/modules/audit_log/audit_log/export.py @@ -26,7 +26,19 @@ 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", + "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" _ARROW = " → " @@ -88,6 +100,7 @@ def _row(entry: AuditEntryRead, *, entity_label: str, actor: str) -> list[str]: entity_label, actor, format_changes(entry.changes), + entry.tenant_id or "", ) ] 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..dd4ed7fc 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} @@ -143,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,6 +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 === 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/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 }: + + + + 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 @@ -179,3 +182,17 @@ async def distinct_entity_types(self) -> 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-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 7bf945b4..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,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 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"] == [] 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', }, 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"