From 808151ff2b7f64ff64138323a5827b7d572103a4 Mon Sep 17 00:00:00 2001 From: mdtro <20070360+mdtro@users.noreply.github.com> Date: Tue, 14 Jan 2025 14:25:40 -0600 Subject: [PATCH] fix: only return necessary information in audit logs --- .../api/serializers/models/auditlogentry.py | 5 +- .../api/serializers/test_auditlogentry.py | 71 +++++++++++++++++++ 2 files changed, 75 insertions(+), 1 deletion(-) diff --git a/src/sentry/api/serializers/models/auditlogentry.py b/src/sentry/api/serializers/models/auditlogentry.py index db128311df679e..40e7afb45755d1 100644 --- a/src/sentry/api/serializers/models/auditlogentry.py +++ b/src/sentry/api/serializers/models/auditlogentry.py @@ -16,7 +16,10 @@ def fix(data): if hasattr(data["teams"][0], "id"): data["teams"] = [t.id for t in data["teams"]] - return data + # Only return fields in the `data` field that are actually used by the frontend + # Do not add fields to this list that contain sensitive information + required_fields = {"id", "slug", "old_slug", "new_slug", "name"} + return {k: v for k, v in data.items() if k in required_fields} def override_actor_id(user): diff --git a/tests/sentry/api/serializers/test_auditlogentry.py b/tests/sentry/api/serializers/test_auditlogentry.py index 764767cff16a9d..8c9213a3803f72 100644 --- a/tests/sentry/api/serializers/test_auditlogentry.py +++ b/tests/sentry/api/serializers/test_auditlogentry.py @@ -25,6 +25,77 @@ def test_simple(self): assert result["event"] == "team.create" assert result["actor"]["username"] == self.user.username assert result["dateCreated"] == datetime + assert result["data"] == {"slug": "New Team"} + + def test_data_field_filtering(self): + """Test that only allowed fields are present in the data field""" + log = AuditLogEntry.objects.create( + organization_id=self.organization.id, + event=audit_log.get_event_id("PROJECT_EDIT"), + actor=self.user, + datetime=timezone.now(), + data={ + "id": "123", + "slug": "project-slug", + "old_slug": "old-slug", + "new_slug": "new-slug", + "name": "Project Name", + "sensitive_field": "should_not_appear", + "another_field": "should_not_appear", + }, + ) + + serializer = AuditLogEntrySerializer() + result = serialize(log, serializer=serializer) + + # Check that only allowed fields are present + allowed_fields = {"id", "slug", "old_slug", "new_slug", "name"} + assert set(result["data"].keys()) == allowed_fields + assert "sensitive_field" not in result["data"] + assert "another_field" not in result["data"] + + def test_data_field_empty_when_no_required_fields(self): + """Test that data field is empty when none of the required fields are present""" + log = AuditLogEntry.objects.create( + organization_id=self.organization.id, + event=audit_log.get_event_id("PROJECT_EDIT"), + actor=self.user, + datetime=timezone.now(), + data={ + "unrequired_field1": "value1", + "unrequired_field2": "value2", + }, + ) + + serializer = AuditLogEntrySerializer() + result = serialize(log, serializer=serializer) + + assert result["data"] == {} + + def test_teams_conversion(self): + """Test that teams are properly converted from objects to IDs""" + + class MockTeam: + def __init__(self, id): + self.id = id + + log = AuditLogEntry.objects.create( + organization_id=self.organization.id, + event=audit_log.get_event_id("PROJECT_EDIT"), + actor=self.user, + datetime=timezone.now(), + data={ + "id": "123", + "slug": "project-slug", + "teams": [MockTeam(1), MockTeam(2)], + }, + ) + + serializer = AuditLogEntrySerializer() + result = serialize(log, serializer=serializer) + + assert result["data"]["teams"] == [1, 2] + assert isinstance(result["data"]["teams"][0], int) def test_scim_logname(self): uuid_prefix = "681d6e"