mirror of
https://github.com/tiennm99/DocsGPT.git
synced 2026-10-14 16:16:48 +00:00
feat(audit): record data-plane events, not just identity ones
Logins, role grants and provisioning were audited from the first release; creating and deleting sources, agents, agent keys and conversations were not. An operator reviewing the trail could see who signed in but not who deleted the source they were asking about. Adds docsgpt/api/audit.py: one helper that records an event inside the caller's transaction, in a savepoint, swallowing failures -- an audit write must never be able to fail the action it describes -- and tolerating the absence of a Flask request so a Celery task can record too. Events added: source.created (upload and wiki), source.deleted, source.reingested, agent.created, agent.updated, agent.deleted, agent.key_regenerated, conversation.deleted, conversation.deleted_all. agent.updated records field names only, never values, which can carry prompts and credentials. The same module carries the event -> category map (identity / access / config / data) that the admin activity feed filters on.
This commit is contained in:
1 parent
cb5343397f
commit
57b9034f2d
7 files changed
+469
-1
No files matched your search
@@ -0,0 +1,106 @@
|
||||
"""Unit tests for the shared request-scoped audit helper."""
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
from unittest.mock import MagicMock, Mock, patch
|
||||
|
||||
import pytest
|
||||
|
||||
from docsgpt.api.audit import EVENT_CATEGORIES, category_for, record_event
|
||||
|
||||
|
||||
@pytest.fixture
|
||||
def app():
|
||||
from docsgpt.app import app as flask_app
|
||||
|
||||
flask_app.config["TESTING"] = True
|
||||
return flask_app
|
||||
|
||||
|
||||
@pytest.mark.unit
|
||||
class TestRecordEvent:
|
||||
def test_writes_actor_target_and_request_context(self, app):
|
||||
repo = Mock()
|
||||
conn = MagicMock()
|
||||
with app.test_request_context(
|
||||
"/api/anything",
|
||||
environ_base={"REMOTE_ADDR": "203.0.113.7"},
|
||||
headers={"User-Agent": "curl/8"},
|
||||
):
|
||||
with patch("docsgpt.api.audit.AuthEventsRepository", return_value=repo):
|
||||
record_event(
|
||||
conn, "source.deleted", actor="u1", target=None, source_id="s1"
|
||||
)
|
||||
kwargs = repo.insert.call_args.kwargs
|
||||
assert kwargs["event"] == "source.deleted"
|
||||
assert kwargs["actor_id"] == "u1"
|
||||
assert kwargs["target_id"] is None
|
||||
assert kwargs["ip"] == "203.0.113.7"
|
||||
assert kwargs["user_agent"] == "curl/8"
|
||||
assert kwargs["metadata"] == {"source_id": "s1"}
|
||||
|
||||
def test_drops_none_metadata_values(self, app):
|
||||
repo = Mock()
|
||||
with app.test_request_context("/api/anything"):
|
||||
with patch("docsgpt.api.audit.AuthEventsRepository", return_value=repo):
|
||||
record_event(MagicMock(), "agent.created", actor="u1", name=None, id="a1")
|
||||
assert repo.insert.call_args.kwargs["metadata"] == {"id": "a1"}
|
||||
|
||||
def test_insert_failure_never_raises(self, app):
|
||||
"""An audit write must not be able to fail the action it records."""
|
||||
conn = Mock()
|
||||
conn.begin_nested.side_effect = RuntimeError("boom")
|
||||
with app.test_request_context("/api/anything"):
|
||||
record_event(conn, "source.deleted", actor="u1")
|
||||
|
||||
def test_runs_in_a_savepoint(self, app):
|
||||
"""A poisoned audit insert must roll back alone, not the action."""
|
||||
repo = Mock()
|
||||
conn = MagicMock()
|
||||
with app.test_request_context("/api/anything"):
|
||||
with patch("docsgpt.api.audit.AuthEventsRepository", return_value=repo):
|
||||
record_event(conn, "agent.deleted", actor="u1")
|
||||
conn.begin_nested.assert_called_once()
|
||||
|
||||
def test_works_without_a_request_context(self, app):
|
||||
"""Celery tasks have no request; the row still records."""
|
||||
repo = Mock()
|
||||
with patch("docsgpt.api.audit.AuthEventsRepository", return_value=repo):
|
||||
record_event(MagicMock(), "source.created", actor="u1")
|
||||
kwargs = repo.insert.call_args.kwargs
|
||||
assert kwargs["ip"] is None and kwargs["user_agent"] is None
|
||||
|
||||
def test_anonymous_actor_falls_back(self, app):
|
||||
repo = Mock()
|
||||
with app.test_request_context("/api/anything"):
|
||||
with patch("docsgpt.api.audit.AuthEventsRepository", return_value=repo):
|
||||
record_event(MagicMock(), "agent.created", actor=None)
|
||||
assert repo.insert.call_args.kwargs["actor_id"] == "unknown"
|
||||
|
||||
|
||||
@pytest.mark.unit
|
||||
class TestCategories:
|
||||
@pytest.mark.parametrize(
|
||||
"event,expected",
|
||||
[
|
||||
("oidc_login", "identity"),
|
||||
("oidc_login_denied", "identity"),
|
||||
("pat_created", "identity"),
|
||||
("scim_created", "identity"),
|
||||
("role_granted", "access"),
|
||||
("admin_user_deactivated", "access"),
|
||||
("team.member_add", "access"),
|
||||
("quota_policy_set", "config"),
|
||||
("source.deleted", "data"),
|
||||
("agent.created", "data"),
|
||||
("conversation.deleted", "data"),
|
||||
],
|
||||
)
|
||||
def test_known_events(self, event, expected):
|
||||
assert category_for(event) == expected
|
||||
|
||||
def test_unknown_event_is_other(self):
|
||||
assert category_for("something_new") == "other"
|
||||
|
||||
def test_every_category_is_declared(self):
|
||||
assert "other" in EVENT_CATEGORIES
|
||||
@@ -0,0 +1,142 @@
|
||||
"""The data-plane routes must leave an audit trail.
|
||||
|
||||
Identity events were audited from the start; source/agent/conversation
|
||||
mutations were not, so "who deleted that source" had no answer. These pin the
|
||||
hooks at the route layer — the repositories are mocked, what is asserted is
|
||||
that the audit row is written with the acting user and the resource id.
|
||||
"""
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
from contextlib import ExitStack, contextmanager
|
||||
from unittest.mock import MagicMock, Mock, patch
|
||||
|
||||
import pytest
|
||||
|
||||
|
||||
@pytest.fixture
|
||||
def client():
|
||||
from docsgpt.app import app as flask_app
|
||||
|
||||
flask_app.config["TESTING"] = True
|
||||
return flask_app.test_client()
|
||||
|
||||
|
||||
@contextmanager
|
||||
def _authed(module: str, **patches):
|
||||
"""Authenticate as ``u1`` and patch names inside ``module``."""
|
||||
recorded: list[tuple] = []
|
||||
|
||||
def _record(_conn, event, *, actor, target=None, **metadata):
|
||||
recorded.append((event, actor, target, metadata))
|
||||
|
||||
@contextmanager
|
||||
def _conn():
|
||||
yield MagicMock()
|
||||
|
||||
with ExitStack() as stack:
|
||||
stack.enter_context(
|
||||
patch("docsgpt.app.handle_auth", return_value={"sub": "u1"})
|
||||
)
|
||||
stack.enter_context(patch("docsgpt.app.resolve_roles", return_value=["user"]))
|
||||
stack.enter_context(patch(f"{module}.db_session", _conn))
|
||||
stack.enter_context(patch(f"{module}.db_readonly", _conn))
|
||||
stack.enter_context(patch(f"{module}.record_event", _record))
|
||||
for name, value in patches.items():
|
||||
stack.enter_context(patch(f"{module}.{name}", value))
|
||||
yield recorded
|
||||
|
||||
|
||||
_SOURCES = "docsgpt.api.user.sources.routes"
|
||||
_AGENTS = "docsgpt.api.user.agents.routes"
|
||||
_CONVERSATIONS = "docsgpt.api.user.conversations.routes"
|
||||
|
||||
|
||||
@pytest.mark.unit
|
||||
class TestSourceAudit:
|
||||
def test_delete_records_source_deleted(self, client):
|
||||
repo = Mock()
|
||||
repo.get_any.return_value = {"id": "src-1", "name": "Handbook"}
|
||||
storage = Mock()
|
||||
storage.file_exists.return_value = False
|
||||
with _authed(
|
||||
_SOURCES,
|
||||
SourcesRepository=Mock(return_value=repo),
|
||||
StorageCreator=Mock(get_storage=Mock(return_value=storage)),
|
||||
) as recorded:
|
||||
resp = client.get("/api/delete_old?source_id=src-1")
|
||||
assert resp.status_code == 200
|
||||
assert recorded[0][:3] == ("source.deleted", "u1", None)
|
||||
assert recorded[0][3]["source_id"] == "src-1"
|
||||
assert recorded[0][3]["name"] == "Handbook"
|
||||
|
||||
def test_nothing_recorded_when_the_source_is_missing(self, client):
|
||||
repo = Mock()
|
||||
repo.get_any.return_value = None
|
||||
with _authed(_SOURCES, SourcesRepository=Mock(return_value=repo)) as recorded:
|
||||
resp = client.get("/api/delete_old?source_id=nope")
|
||||
assert resp.status_code == 404
|
||||
assert recorded == []
|
||||
|
||||
|
||||
@pytest.mark.unit
|
||||
class TestAgentAudit:
|
||||
def test_delete_records_agent_deleted(self, client):
|
||||
repo = Mock()
|
||||
repo.get_any.return_value = {
|
||||
"id": "agent-1",
|
||||
"name": "Support bot",
|
||||
"agent_type": "classic",
|
||||
}
|
||||
with _authed(
|
||||
_AGENTS,
|
||||
AgentsRepository=Mock(return_value=repo),
|
||||
WorkflowsRepository=Mock(),
|
||||
UsersRepository=Mock(),
|
||||
) as recorded:
|
||||
resp = client.delete("/api/delete_agent?id=agent-1")
|
||||
assert resp.status_code == 200
|
||||
assert recorded[0][:3] == ("agent.deleted", "u1", None)
|
||||
assert recorded[0][3]["agent_id"] == "agent-1"
|
||||
assert recorded[0][3]["name"] == "Support bot"
|
||||
|
||||
def test_missing_agent_records_nothing(self, client):
|
||||
repo = Mock()
|
||||
repo.get_any.return_value = None
|
||||
with _authed(_AGENTS, AgentsRepository=Mock(return_value=repo)) as recorded:
|
||||
assert client.delete("/api/delete_agent?id=x").status_code == 404
|
||||
assert recorded == []
|
||||
|
||||
|
||||
@pytest.mark.unit
|
||||
class TestConversationAudit:
|
||||
def test_delete_records_conversation_deleted(self, client):
|
||||
repo = Mock()
|
||||
repo.get_any.return_value = {"id": "conv-1"}
|
||||
with _authed(
|
||||
_CONVERSATIONS, ConversationsRepository=Mock(return_value=repo)
|
||||
) as recorded:
|
||||
resp = client.post("/api/delete_conversation?id=conv-1")
|
||||
assert resp.status_code == 200
|
||||
assert recorded[0][:3] == ("conversation.deleted", "u1", None)
|
||||
assert recorded[0][3]["conversation_id"] == "conv-1"
|
||||
|
||||
def test_delete_all_records_the_count(self, client):
|
||||
repo = Mock()
|
||||
repo.delete_all_for_user.return_value = 7
|
||||
with _authed(
|
||||
_CONVERSATIONS, ConversationsRepository=Mock(return_value=repo)
|
||||
) as recorded:
|
||||
resp = client.get("/api/delete_all_conversations")
|
||||
assert resp.status_code == 200
|
||||
assert recorded[0][0] == "conversation.deleted_all"
|
||||
assert recorded[0][3]["deleted"] == 7
|
||||
|
||||
def test_unknown_conversation_records_nothing(self, client):
|
||||
repo = Mock()
|
||||
repo.get_any.return_value = None
|
||||
with _authed(
|
||||
_CONVERSATIONS, ConversationsRepository=Mock(return_value=repo)
|
||||
) as recorded:
|
||||
assert client.post("/api/delete_conversation?id=x").status_code == 200
|
||||
assert recorded == []
|
||||
Reference in new issue
Block a user