diff --git a/docsgpt/alembic/versions/0034_auth_events_actor_target.py b/docsgpt/alembic/versions/0034_auth_events_actor_target.py new file mode 100644 index 00000000..ffe10aac --- /dev/null +++ b/docsgpt/alembic/versions/0034_auth_events_actor_target.py @@ -0,0 +1,94 @@ +"""0034 auth events actor/target — who did it, and to whom. + +``auth_events.user_id`` was overloaded: admin mutations file the row under the +*target* user and hide the acting admin in ``metadata->>'by'``, team events file +it under the *actor*, and quota events switch between the two depending on +scope. The practical consequence is that "show me everything admin X did" is +unanswerable — the first question any audit review asks. + +This adds two explicit columns: + +* ``actor_id`` — who performed the action (never NULL going forward). +* ``target_id`` — the user the action was performed on; NULL when the event is + not about a user (a team or an instance-wide policy change). + +``user_id`` is deliberately left alone. It stays the per-user feed key +(``list_recent``) and every historical row keeps its meaning, so this migration +is additive and the backfill never rewrites it. + +Backfill rules, applied only to rows that predate the columns: + +* ``actor_id`` = the first present of ``metadata->>'by'``, + ``metadata->>'granted_by'``, ``metadata->>'revoked_by'``, else ``user_id``. +* ``target_id`` = ``user_id``, except for ``team.*`` events, which were always + filed under the actor and have no single user target. + +Also adds the indexes the global admin feed needs. Before this the only index +was ``(user_id, created_at DESC)``, so the cross-user feed — which orders by +``created_at DESC`` with no user predicate — degraded to a sequential scan plus +a sort on every page. + +Idempotent both ways. + +Revision ID: 0034_auth_events_actor_target +Revises: 0033_quotas +""" + +from typing import Sequence, Union + +from alembic import op + + +revision: str = "0034_auth_events_actor_target" +down_revision: Union[str, None] = "0033_quotas" +branch_labels: Union[str, Sequence[str], None] = None +depends_on: Union[str, Sequence[str], None] = None + + +def upgrade() -> None: + op.execute("ALTER TABLE auth_events ADD COLUMN IF NOT EXISTS actor_id TEXT;") + op.execute("ALTER TABLE auth_events ADD COLUMN IF NOT EXISTS target_id TEXT;") + + # Backfill. ``actor_id IS NULL`` scopes this to pre-migration rows, so a + # re-run (or a downgrade/upgrade cycle) never clobbers written values. + op.execute( + """ + UPDATE auth_events + SET actor_id = COALESCE( + metadata->>'by', + metadata->>'granted_by', + metadata->>'revoked_by', + user_id + ), + target_id = CASE + WHEN event LIKE 'team.%' THEN NULL + ELSE user_id + END + WHERE actor_id IS NULL; + """ + ) + + # Backfilled every row above, so the NOT NULL is safe to assert now. New + # rows always carry an actor (the repository derives one from ``user_id``). + op.execute("ALTER TABLE auth_events ALTER COLUMN actor_id SET NOT NULL;") + + op.execute( + "CREATE INDEX IF NOT EXISTS auth_events_created_idx " + "ON auth_events (created_at DESC);" + ) + op.execute( + "CREATE INDEX IF NOT EXISTS auth_events_event_created_idx " + "ON auth_events (event, created_at DESC);" + ) + op.execute( + "CREATE INDEX IF NOT EXISTS auth_events_actor_idx " + "ON auth_events (actor_id, created_at DESC);" + ) + + +def downgrade() -> None: + op.execute("DROP INDEX IF EXISTS auth_events_actor_idx;") + op.execute("DROP INDEX IF EXISTS auth_events_event_created_idx;") + op.execute("DROP INDEX IF EXISTS auth_events_created_idx;") + op.execute("ALTER TABLE auth_events DROP COLUMN IF EXISTS target_id;") + op.execute("ALTER TABLE auth_events DROP COLUMN IF EXISTS actor_id;") diff --git a/docsgpt/api/admin/quotas.py b/docsgpt/api/admin/quotas.py index 0235f36b..83d2ec52 100644 --- a/docsgpt/api/admin/quotas.py +++ b/docsgpt/api/admin/quotas.py @@ -119,6 +119,10 @@ def _audit(conn, event: str, scope: str, subject_id: Optional[str], detail: dict ip=request.remote_addr, user_agent=request.headers.get("User-Agent"), metadata={"by": actor, "via": "admin_api", "scope": scope, "subject_id": subject_id, **detail}, + actor_id=actor or "unknown", + # Only a user-scoped policy targets a user; instance and team policies + # change configuration, not an account. + target_id=subject_id if scope == "user" else None, ) diff --git a/docsgpt/api/admin/routes.py b/docsgpt/api/admin/routes.py index 3d178904..d0dbf2d1 100644 --- a/docsgpt/api/admin/routes.py +++ b/docsgpt/api/admin/routes.py @@ -171,6 +171,8 @@ class AdminUserResource(Resource): ip=request.remote_addr, user_agent=request.headers.get("User-Agent"), metadata={"by": actor, "via": "admin_api"}, + actor_id=actor, + target_id=user_id, ) if not active: # Best-effort live-session revocation (mirrors SCIM deactivation). @@ -203,6 +205,8 @@ class AdminUserRoleResource(Resource): "granted_by": actor, "via": "admin_api", }, + actor_id=actor, + target_id=user_id, ) return make_response( jsonify({"success": True, "granted": inserted, "role": ROLE_ADMIN}), 200 @@ -238,6 +242,8 @@ class AdminUserRoleResource(Resource): "revoked_by": actor, "via": "admin_api", }, + actor_id=actor, + target_id=user_id, ) return make_response(jsonify({"success": True, "revoked": removed}), 200) @@ -261,6 +267,8 @@ class AdminUserSessionsResource(Resource): ip=request.remote_addr, user_agent=request.headers.get("User-Agent"), metadata={"token_id": token_id, "by": _actor(), "via": "admin_sessions_revoked"}, + actor_id=_actor(), + target_id=user_id, ) AuthEventsRepository(conn).insert( user_id, @@ -273,6 +281,8 @@ class AdminUserSessionsResource(Resource): "persisted": ok, "personal_access_tokens_revoked": len(revoked_token_ids), }, + actor_id=_actor(), + target_id=user_id, ) return make_response(jsonify({"success": True, "revoked": ok}), 200) diff --git a/docsgpt/api/oidc/routes.py b/docsgpt/api/oidc/routes.py index 90ca72b6..5d86333a 100644 --- a/docsgpt/api/oidc/routes.py +++ b/docsgpt/api/oidc/routes.py @@ -26,7 +26,10 @@ from docsgpt.api.oidc import denylist, provider from docsgpt.auth import handle_auth from docsgpt.cache import get_redis_instance from docsgpt.core.settings import settings -from docsgpt.storage.db.repositories.auth_events import AuthEventsRepository +from docsgpt.storage.db.repositories.auth_events import ( + SYSTEM_ACTOR_OIDC, + AuthEventsRepository, +) from docsgpt.storage.db.repositories.user_roles import UserRolesRepository from docsgpt.storage.db.repositories.users import UsersRepository from docsgpt.storage.db.session import db_readonly, db_session @@ -129,6 +132,9 @@ def _reconcile_oidc_admin(user_id: str, groups: list[str] | None) -> None: ip=request.remote_addr, user_agent=request.headers.get("User-Agent"), metadata={"role": "admin", "source": "oidc_group"}, + # The provider's group membership drove this, not the user. + actor_id=SYSTEM_ACTOR_OIDC, + target_id=user_id, ) except Exception: logger.error("OIDC admin reconcile failed for %s", user_id, exc_info=True) diff --git a/docsgpt/api/pat/routes.py b/docsgpt/api/pat/routes.py index b8a9e00d..a8c0f0d6 100644 --- a/docsgpt/api/pat/routes.py +++ b/docsgpt/api/pat/routes.py @@ -307,6 +307,8 @@ class AdminToken(Resource): ip=request.remote_addr, user_agent=request.headers.get("User-Agent"), metadata={"token_id": token_id, "by": actor, "via": "admin_api"}, + actor_id=actor, + target_id=row["user_id"], ) if not revoked: return _error("Token not found", 404) diff --git a/docsgpt/api/scim/routes.py b/docsgpt/api/scim/routes.py index cb9338c7..e2393ab0 100644 --- a/docsgpt/api/scim/routes.py +++ b/docsgpt/api/scim/routes.py @@ -20,7 +20,10 @@ from sqlalchemy import Connection from docsgpt.api.oidc.denylist import deny_user from docsgpt.core.settings import settings -from docsgpt.storage.db.repositories.auth_events import AuthEventsRepository +from docsgpt.storage.db.repositories.auth_events import ( + SYSTEM_ACTOR_SCIM, + AuthEventsRepository, +) from docsgpt.storage.db.repositories.users import UsersRepository from docsgpt.storage.db.session import db_readonly, db_session @@ -201,7 +204,13 @@ def _audit(conn: Connection, user_id: str, event: str) -> None: """Best-effort audit insert in a savepoint; failure never fails the request.""" try: with conn.begin_nested(): - AuthEventsRepository(conn).insert(user_id, event, metadata={"via": "scim"}) + AuthEventsRepository(conn).insert( + user_id, + event, + metadata={"via": "scim"}, + actor_id=SYSTEM_ACTOR_SCIM, + target_id=user_id, + ) except Exception: logger.error("SCIM audit insert failed for user %s event %s", user_id, event, exc_info=True) diff --git a/docsgpt/api/user/teams/routes.py b/docsgpt/api/user/teams/routes.py index ca3de7de..df51ec0e 100644 --- a/docsgpt/api/user/teams/routes.py +++ b/docsgpt/api/user/teams/routes.py @@ -62,13 +62,26 @@ def _current_user() -> str | None: return token.get("sub") if isinstance(token, dict) else None +# Metadata keys naming the user a team event acted on, most specific first. +# Lets ``_audit`` fill ``target_id`` without every call site repeating it. +_TARGET_METADATA_KEYS = ("target_user", "target_user_id", "new_owner") + + def _audit(conn, actor: str | None, event: str, **metadata) -> None: """Append a team management event to the audit trail (best-effort). Runs inside the action's transaction so the audit row commits atomically with the change. Never raises into the request path — an audit failure must not fail the operation. + + Team events are filed under the acting user. When the action names another + member (add, role change, removal, ownership transfer) that member becomes + the row's ``target_id``; otherwise the event has no user target. """ + detail = {k: v for k, v in metadata.items() if v is not None} + target = next( + (detail[key] for key in _TARGET_METADATA_KEYS if detail.get(key)), None + ) try: # SAVEPOINT: a failed audit insert poisons the surrounding txn, so # nest it — on failure only the audit rolls back, not the action. @@ -78,7 +91,9 @@ def _audit(conn, actor: str | None, event: str, **metadata) -> None: event=event, ip=request.remote_addr, user_agent=request.headers.get("User-Agent"), - metadata={k: v for k, v in metadata.items() if v is not None}, + metadata=detail, + actor_id=actor or "unknown", + target_id=target, ) except Exception: logger.warning("team audit insert failed for event=%s", event, exc_info=True) diff --git a/docsgpt/storage/db/models.py b/docsgpt/storage/db/models.py index 1582e32d..b3bd9a1f 100644 --- a/docsgpt/storage/db/models.py +++ b/docsgpt/storage/db/models.py @@ -68,6 +68,11 @@ auth_events_table = Table( metadata, Column("id", UUID(as_uuid=True), primary_key=True, server_default=func.gen_random_uuid()), Column("user_id", Text, nullable=False), + # Who performed the action, and (when the action is about a user) whom it + # was performed on. ``user_id`` predates both and is kept as the per-user + # feed key; see migration 0034. + Column("actor_id", Text, nullable=False), + Column("target_id", Text), Column("event", Text, nullable=False), Column("ip", Text), Column("user_agent", Text), diff --git a/docsgpt/storage/db/repositories/auth_events.py b/docsgpt/storage/db/repositories/auth_events.py index 834b31e7..b87e6ae7 100644 --- a/docsgpt/storage/db/repositories/auth_events.py +++ b/docsgpt/storage/db/repositories/auth_events.py @@ -3,15 +3,33 @@ from __future__ import annotations import json -from typing import Optional +from typing import Any, Optional, Sequence from sqlalchemy import Connection, text from docsgpt.storage.db.base_repository import row_to_dict +# Actors that are not a person. Used where an automated system performs the +# action on a user's behalf, so the feed never reads as if the user did it. +SYSTEM_ACTOR_SCIM = "system:scim" +SYSTEM_ACTOR_OIDC = "system:oidc" + + class AuthEventsRepository: - """Append-only audit trail of login / logout / provisioning events.""" + """Append-only audit trail of identity, access and data-plane events. + + Every row carries three attribution columns: + + * ``actor_id`` — who performed the action. Never NULL. + * ``target_id`` — the user the action was performed on, or NULL when the + event is not about a user (a team change, an instance-wide policy). + * ``user_id`` — the legacy subject column, kept as the key for the + per-user feed (:meth:`list_recent`). Defaults to the target, falling + back to the actor for actor-only events. + + See migration ``0034_auth_events_actor_target``. + """ def __init__(self, conn: Connection) -> None: self._conn = conn @@ -23,18 +41,45 @@ class AuthEventsRepository: ip: Optional[str] = None, user_agent: Optional[str] = None, metadata: Optional[dict] = None, + *, + actor_id: Optional[str] = None, + target_id: Optional[str] = "", ) -> dict: - """Record one auth event and return the inserted row.""" + """Record one audit event and return the inserted row. + + Args: + user_id: The row's subject, kept for the per-user feed. + event: Event name (``oidc_login``, ``source.deleted``, ...). + ip: Client IP, when the event came from a request. + user_agent: Client ``User-Agent``, when the event came from a request. + metadata: Free-form JSON detail shown in the audit drill-down. + actor_id: Who performed the action. Defaults to ``user_id``, which + is correct for self-service events (login, PAT create). + target_id: The user acted upon. Defaults to ``user_id``; pass + ``None`` explicitly for events with no user target. + + Returns: + The inserted row as a dict. + """ + # Sentinel default: ``None`` is a meaningful value here ("no target"), + # so it cannot double as "caller did not say". + resolved_target = user_id if target_id == "" else target_id + resolved_actor = actor_id or user_id result = self._conn.execute( text( """ - INSERT INTO auth_events (user_id, event, ip, user_agent, metadata) - VALUES (:user_id, :event, :ip, :user_agent, CAST(:metadata AS jsonb)) + INSERT INTO auth_events + (user_id, actor_id, target_id, event, ip, user_agent, metadata) + VALUES + (:user_id, :actor_id, :target_id, :event, :ip, :user_agent, + CAST(:metadata AS jsonb)) RETURNING * """ ), { "user_id": user_id, + "actor_id": resolved_actor, + "target_id": resolved_target, "event": event, "ip": ip, "user_agent": user_agent, @@ -59,18 +104,45 @@ class AuthEventsRepository: return [row_to_dict(row) for row in result.fetchall()] @staticmethod - def _filter_clauses(event, user_id, since) -> tuple[str, dict]: + def _filter_clauses( + event: Optional[str], + user_id: Optional[str], + since: Any, + *, + events: Optional[Sequence[str]] = None, + actor_id: Optional[str] = None, + until: Any = None, + search: Optional[str] = None, + ) -> tuple[str, dict]: clauses: list[str] = [] params: dict = {} if event: clauses.append("event = :event") params["event"] = event + if events: + clauses.append("event = ANY(:events)") + params["events"] = list(events) if user_id: - clauses.append("user_id = :user_id") + clauses.append("(user_id = :user_id OR target_id = :user_id)") params["user_id"] = user_id + if actor_id: + clauses.append("actor_id = :actor_id") + params["actor_id"] = actor_id if since is not None: clauses.append("created_at >= :since") params["since"] = since + if until is not None: + clauses.append("created_at <= :until") + params["until"] = until + if search: + # Substring match across the human-meaningful columns plus the + # serialized metadata, so "ci-deploy-key" finds the PAT it named. + clauses.append( + "(user_id ILIKE :search OR actor_id ILIKE :search " + "OR target_id ILIKE :search OR event ILIKE :search " + "OR ip ILIKE :search OR metadata::text ILIKE :search)" + ) + params["search"] = f"%{search}%" where = ("WHERE " + " AND ".join(clauses)) if clauses else "" return where, params @@ -78,13 +150,25 @@ class AuthEventsRepository: self, *, event: Optional[str] = None, + events: Optional[Sequence[str]] = None, user_id: Optional[str] = None, + actor_id: Optional[str] = None, since=None, + until=None, + search: Optional[str] = None, limit: int = 50, offset: int = 0, ) -> list[dict]: - """Global audit feed (admin), newest first; optional event/user/since filters.""" - where, params = self._filter_clauses(event, user_id, since) + """Global audit feed (admin), newest first; see :meth:`_filter_clauses`.""" + where, params = self._filter_clauses( + event, + user_id, + since, + events=events, + actor_id=actor_id, + until=until, + search=search, + ) params.update({"limit": int(limit), "offset": int(offset)}) result = self._conn.execute( text( @@ -96,13 +180,40 @@ class AuthEventsRepository: return [row_to_dict(row) for row in result.fetchall()] def count_all( - self, *, event: Optional[str] = None, user_id: Optional[str] = None, since=None + self, + *, + event: Optional[str] = None, + events: Optional[Sequence[str]] = None, + user_id: Optional[str] = None, + actor_id: Optional[str] = None, + since=None, + until=None, + search: Optional[str] = None, ) -> int: """Total matching the same filters as :meth:`list_all` (for pagination).""" - where, params = self._filter_clauses(event, user_id, since) + where, params = self._filter_clauses( + event, + user_id, + since, + events=events, + actor_id=actor_id, + until=until, + search=search, + ) return int( self._conn.execute( text(f"SELECT count(*) FROM auth_events {where}"), params ).scalar() or 0 ) + + def event_names(self) -> list[str]: + """Distinct event names present in the table, alphabetically. + + Feeds the admin filter's event picker, so operators pick from what the + instance actually recorded instead of typing a name from memory. + """ + result = self._conn.execute( + text("SELECT DISTINCT event FROM auth_events ORDER BY event") + ) + return [row[0] for row in result.fetchall()] diff --git a/tests/storage/db/repositories/test_auth_events.py b/tests/storage/db/repositories/test_auth_events.py new file mode 100644 index 00000000..d76fea9b --- /dev/null +++ b/tests/storage/db/repositories/test_auth_events.py @@ -0,0 +1,85 @@ +"""Repository tests for ``auth_events`` (actor/target attribution + feed filters).""" + +from __future__ import annotations + +from datetime import datetime, timedelta, timezone + +import pytest + +from docsgpt.storage.db.repositories.auth_events import AuthEventsRepository + + +pytestmark = pytest.mark.integration + + +@pytest.fixture() +def repo(pg_conn): + return AuthEventsRepository(pg_conn) + + +class TestInsertAttribution: + def test_self_service_event_is_its_own_actor(self, repo): + row = repo.insert("u1", "oidc_login") + assert row["actor_id"] == "u1" + assert row["target_id"] == "u1" + assert row["user_id"] == "u1" + + def test_admin_action_separates_actor_from_target(self, repo): + row = repo.insert( + "victim", "admin_user_deactivated", actor_id="admin-1", target_id="victim" + ) + assert row["actor_id"] == "admin-1" + assert row["target_id"] == "victim" + # ``user_id`` stays the subject, so the per-user feed is unchanged. + assert row["user_id"] == "victim" + + def test_actor_only_event_has_no_target(self, repo): + row = repo.insert("admin-1", "team.create", actor_id="admin-1", target_id=None) + assert row["actor_id"] == "admin-1" + assert row["target_id"] is None + + def test_positional_user_id_still_supported(self, repo): + """Existing call sites pass only ``user_id``; it must keep working.""" + row = repo.insert("u2", "pat_created", metadata={"token_id": "t"}) + assert (row["actor_id"], row["target_id"]) == ("u2", "u2") + + +class TestFeedFilters: + def _seed(self, repo): + repo.insert("a", "oidc_login") + repo.insert("b", "oidc_login_denied") + repo.insert("b", "admin_user_deactivated", actor_id="admin-9", target_id="b") + + def test_filter_by_actor(self, repo): + self._seed(repo) + rows = repo.list_all(actor_id="admin-9") + assert [r["event"] for r in rows] == ["admin_user_deactivated"] + assert repo.count_all(actor_id="admin-9") == 1 + + def test_filter_by_multiple_events(self, repo): + self._seed(repo) + rows = repo.list_all(events=["oidc_login", "oidc_login_denied"]) + assert {r["event"] for r in rows} == {"oidc_login", "oidc_login_denied"} + assert repo.count_all(events=["oidc_login", "oidc_login_denied"]) == 2 + + def test_single_event_filter_still_supported(self, repo): + self._seed(repo) + assert repo.count_all(event="oidc_login") == 1 + + def test_filter_by_until(self, repo): + self._seed(repo) + past = datetime.now(timezone.utc) - timedelta(days=1) + assert repo.count_all(until=past) == 0 + assert repo.count_all(until=datetime.now(timezone.utc) + timedelta(days=1)) == 3 + + def test_search_matches_user_actor_and_metadata(self, repo): + repo.insert("someone", "pat_created", metadata={"name": "ci-deploy-key"}) + assert repo.count_all(search="ci-deploy") == 1 + assert repo.count_all(search="someone") == 1 + assert repo.count_all(search="nothing-here") == 0 + + def test_event_names_catalogue(self, repo): + self._seed(repo) + names = repo.event_names() + assert names == sorted(names) + assert "oidc_login" in names and "admin_user_deactivated" in names diff --git a/tests/storage/db/test_migration_0034.py b/tests/storage/db/test_migration_0034.py new file mode 100644 index 00000000..0bcd5e36 --- /dev/null +++ b/tests/storage/db/test_migration_0034.py @@ -0,0 +1,127 @@ +"""Migration round-trip test for 0034_auth_events_actor_target.""" + +from __future__ import annotations + +import os +import subprocess +import sys +from pathlib import Path + +import pytest +from sqlalchemy import text + + +pytestmark = pytest.mark.integration + + +def _alembic_ini() -> Path: + return Path(__file__).resolve().parents[3] / "docsgpt" / "alembic.ini" + + +def _run_alembic(url: str, *args: str) -> None: + subprocess.check_call( + [sys.executable, "-m", "alembic", "-c", str(_alembic_ini()), *args], + timeout=60, + env={**os.environ, "POSTGRES_URI": url}, + ) + + +def _alembic_heads(url: str) -> list[str]: + out = subprocess.check_output( + [sys.executable, "-m", "alembic", "-c", str(_alembic_ini()), "heads"], + timeout=60, + env={**os.environ, "POSTGRES_URI": url}, + text=True, + ) + return [line for line in out.splitlines() if line.strip()] + + +def _alembic_version(conn) -> str: + return conn.execute(text("SELECT version_num FROM alembic_version")).scalar() + + +def _column_exists(conn, table: str, column: str) -> bool: + row = conn.execute( + text( + "SELECT 1 FROM information_schema.columns " + "WHERE table_name = :t AND column_name = :c AND table_schema = 'public'" + ), + {"t": table, "c": column}, + ).fetchone() + return row is not None + + +def _index_exists(conn, name: str) -> bool: + row = conn.execute( + text("SELECT 1 FROM pg_indexes WHERE schemaname = 'public' AND indexname = :n"), + {"n": name}, + ).fetchone() + return row is not None + + +_0034 = "0034_auth_events_actor_target" +_0033 = "0033_quotas" + + +class TestMigration0034RoundTrip: + def test_single_head(self, pg_engine): + url = pg_engine.url.render_as_string(hide_password=False) + assert len(_alembic_heads(url)) == 1 + + def test_head_has_actor_target_columns(self, pg_engine): + with pg_engine.connect() as conn: + assert _alembic_version(conn) >= _0034 + assert _column_exists(conn, "auth_events", "actor_id") + assert _column_exists(conn, "auth_events", "target_id") + + def test_head_has_feed_indexes(self, pg_engine): + """The global feed orders by created_at with no user filter.""" + with pg_engine.connect() as conn: + assert _index_exists(conn, "auth_events_created_idx") + assert _index_exists(conn, "auth_events_event_created_idx") + assert _index_exists(conn, "auth_events_actor_idx") + + def test_downgrade_drops_then_upgrade_restores(self, pg_engine): + url = pg_engine.url.render_as_string(hide_password=False) + _run_alembic(url, "downgrade", _0033) + with pg_engine.connect() as conn: + assert _alembic_version(conn) == _0033 + assert not _column_exists(conn, "auth_events", "actor_id") + assert not _column_exists(conn, "auth_events", "target_id") + assert not _index_exists(conn, "auth_events_created_idx") + _run_alembic(url, "upgrade", "head") + with pg_engine.connect() as conn: + assert _alembic_version(conn) >= _0034 + assert _column_exists(conn, "auth_events", "actor_id") + + def test_backfill_reads_actor_out_of_metadata(self, pg_engine): + """An admin action filed under its target keeps the acting admin.""" + url = pg_engine.url.render_as_string(hide_password=False) + _run_alembic(url, "downgrade", _0033) + with pg_engine.begin() as conn: + conn.execute( + text( + "INSERT INTO auth_events (user_id, event, metadata) VALUES " + "('victim', 'admin_user_deactivated', '{\"by\": \"admin-1\"}'::jsonb), " + "('grantee', 'role_granted', '{\"granted_by\": \"admin-2\"}'::jsonb), " + "('someone', 'oidc_login', '{}'::jsonb), " + "('actor-3', 'team.create', '{\"team_id\": \"t1\"}'::jsonb)" + ) + ) + _run_alembic(url, "upgrade", "head") + with pg_engine.connect() as conn: + rows = dict( + conn.execute( + text( + "SELECT event, actor_id || '|' || COALESCE(target_id, '-') " + "FROM auth_events WHERE user_id IN " + "('victim', 'grantee', 'someone', 'actor-3')" + ) + ).fetchall() + ) + assert rows["admin_user_deactivated"] == "admin-1|victim" + assert rows["role_granted"] == "admin-2|grantee" + # Self-service events: the user is both actor and target. + assert rows["oidc_login"] == "someone|someone" + # Team events were always filed under the actor; they have no user target. + assert rows["team.create"] == "actor-3|-"