From f8b7dc4594ef51429476cfb291194f26765880de Mon Sep 17 00:00:00 2001 From: Fabian Schindler Date: Wed, 8 Jan 2025 15:12:41 +0100 Subject: [PATCH 1/6] feat(tempest): add audit log events for tempest client ids Add audit log events TEMPEST_CLIENT_ID_ADD/TEMPEST_CLIENT_ID_REMOVE for when tempes client ids are created/deleted. --- src/sentry/audit_log/register.py | 17 +++++++++++++++++ .../tempest/endpoints/tempest_credentials.py | 13 +++++++++++-- .../endpoints/tempest_credentials_details.py | 15 +++++++++++++-- src/sentry/tempest/models.py | 9 +++++++++ 4 files changed, 50 insertions(+), 4 deletions(-) diff --git a/src/sentry/audit_log/register.py b/src/sentry/audit_log/register.py index 4c5d60716e70eb..b1ec58180b01c2 100644 --- a/src/sentry/audit_log/register.py +++ b/src/sentry/audit_log/register.py @@ -612,3 +612,20 @@ api_name="data-secrecy.reinstated", ) ) + +default_manager.add( + AuditLogEvent( + event_id=1150, + name="TEMPEST_CLIENT_ID_ADD", + api_name="tempest-client-id.create", + template="added tempest client id {client_id}", + ) +) +default_manager.add( + AuditLogEvent( + event_id=1151, + name="TEMPEST_CLIENT_ID_REMOVE", + api_name="tempest-client-id.remove", + template="removed tempest client id {client_id}", + ) +) diff --git a/src/sentry/tempest/endpoints/tempest_credentials.py b/src/sentry/tempest/endpoints/tempest_credentials.py index 34c05de20e5665..ce5743e17427fe 100644 --- a/src/sentry/tempest/endpoints/tempest_credentials.py +++ b/src/sentry/tempest/endpoints/tempest_credentials.py @@ -3,7 +3,7 @@ from rest_framework.request import Request from rest_framework.response import Response -from sentry import features +from sentry import audit_log, features from sentry.api.api_owners import ApiOwner from sentry.api.api_publish_status import ApiPublishStatus from sentry.api.base import region_silo_endpoint @@ -50,9 +50,18 @@ def post(self, request: Request, project: Project) -> Response: serializer = DRFTempestCredentialsSerializer(data=request.data) serializer.is_valid(raise_exception=True) try: - serializer.save(created_by_id=request.user.id, project=project) + credentials = serializer.save(created_by_id=request.user.id, project=project) except IntegrityError: return Response( {"detail": "A credential with this client ID already exists."}, status=400 ) + + self.create_audit_entry( + request, + organization=project.organization, + target_object=credentials.id, + event=audit_log.get_event_id("TEMPEST_CLIENT_ID_ADD"), + data=credentials.get_audit_log_data(), + ) + return Response(serializer.data, status=201) diff --git a/src/sentry/tempest/endpoints/tempest_credentials_details.py b/src/sentry/tempest/endpoints/tempest_credentials_details.py index abebd916ae8071..bfd84c1d6c413f 100644 --- a/src/sentry/tempest/endpoints/tempest_credentials_details.py +++ b/src/sentry/tempest/endpoints/tempest_credentials_details.py @@ -2,7 +2,7 @@ from rest_framework.request import Request from rest_framework.response import Response -from sentry import features +from sentry import audit_log, features from sentry.api.api_owners import ApiOwner from sentry.api.api_publish_status import ApiPublishStatus from sentry.api.base import region_silo_endpoint @@ -30,5 +30,16 @@ def delete(self, request: Request, project: Project, tempest_credentials_id: int if not self.has_feature(request, project): raise NotFound - TempestCredentials.objects.filter(project=project, id=tempest_credentials_id).delete() + credentials = TempestCredentials.objects.filter(project=project, id=tempest_credentials_id) + data = credentials.get_audit_log_data() + credentials.delete() + + self.create_audit_entry( + request, + organization=project.organization, + target_object=credentials.id, + event=audit_log.get_event_id("TEMPEST_CLIENT_ID_REMOVE"), + data=data, + ) + return Response(status=204) diff --git a/src/sentry/tempest/models.py b/src/sentry/tempest/models.py index 63e019ea5a59e7..3fd3cab63ea602 100644 --- a/src/sentry/tempest/models.py +++ b/src/sentry/tempest/models.py @@ -42,3 +42,12 @@ class Meta: name="sentry_tempestcredentials_client_project_uniq", ) ] + + def get_audit_log_data(self) -> dict: + return { + "project_id": self.project.id, + "message": self.message, + "message_type": self.message_type, + "client_id": self.client_id, + "client_secret": self.client_secret, + } From d3a8295be0dbebd6804b49743c0612a996f4cfd5 Mon Sep 17 00:00:00 2001 From: Fabian Schindler Date: Wed, 8 Jan 2025 15:23:40 +0100 Subject: [PATCH 2/6] fix(tempest): event ids for tempest client id audit log events --- src/sentry/audit_log/register.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/sentry/audit_log/register.py b/src/sentry/audit_log/register.py index b1ec58180b01c2..18369b660857e7 100644 --- a/src/sentry/audit_log/register.py +++ b/src/sentry/audit_log/register.py @@ -615,7 +615,7 @@ default_manager.add( AuditLogEvent( - event_id=1150, + event_id=1152, name="TEMPEST_CLIENT_ID_ADD", api_name="tempest-client-id.create", template="added tempest client id {client_id}", @@ -623,7 +623,7 @@ ) default_manager.add( AuditLogEvent( - event_id=1151, + event_id=1153, name="TEMPEST_CLIENT_ID_REMOVE", api_name="tempest-client-id.remove", template="removed tempest client id {client_id}", From 0a82345d60d4b7549cbef63c7888f500680f6cf4 Mon Sep 17 00:00:00 2001 From: Fabian Schindler Date: Wed, 8 Jan 2025 16:43:10 +0100 Subject: [PATCH 3/6] fix(tempest): audit log template fix and exclude fields from audit log data --- src/sentry/audit_log/register.py | 8 ++++---- src/sentry/tempest/models.py | 3 --- 2 files changed, 4 insertions(+), 7 deletions(-) diff --git a/src/sentry/audit_log/register.py b/src/sentry/audit_log/register.py index 18369b660857e7..2283d9a349fcc4 100644 --- a/src/sentry/audit_log/register.py +++ b/src/sentry/audit_log/register.py @@ -617,15 +617,15 @@ AuditLogEvent( event_id=1152, name="TEMPEST_CLIENT_ID_ADD", - api_name="tempest-client-id.create", - template="added tempest client id {client_id}", + api_name="playstation-client-id.create", + template="added playstation client id {client_id}", ) ) default_manager.add( AuditLogEvent( event_id=1153, name="TEMPEST_CLIENT_ID_REMOVE", - api_name="tempest-client-id.remove", - template="removed tempest client id {client_id}", + api_name="playstation-client-id.remove", + template="removed playstation client id {client_id}", ) ) diff --git a/src/sentry/tempest/models.py b/src/sentry/tempest/models.py index 3fd3cab63ea602..0f221bef564ea5 100644 --- a/src/sentry/tempest/models.py +++ b/src/sentry/tempest/models.py @@ -46,8 +46,5 @@ class Meta: def get_audit_log_data(self) -> dict: return { "project_id": self.project.id, - "message": self.message, - "message_type": self.message_type, "client_id": self.client_id, - "client_secret": self.client_secret, } From cac9fb353137c049831bb8723f232e870d901875 Mon Sep 17 00:00:00 2001 From: Fabian Schindler Date: Wed, 8 Jan 2025 16:55:55 +0100 Subject: [PATCH 4/6] test(tempest): add test to ensure audit logs are created for create/delete --- .../sentry/tempest/endpoints/test_tempest_credentials.py | 9 ++++++++- .../endpoints/test_tempest_credentials_details.py | 8 +++++++- 2 files changed, 15 insertions(+), 2 deletions(-) diff --git a/tests/sentry/tempest/endpoints/test_tempest_credentials.py b/tests/sentry/tempest/endpoints/test_tempest_credentials.py index d193702d52a225..44479e911ac80a 100644 --- a/tests/sentry/tempest/endpoints/test_tempest_credentials.py +++ b/tests/sentry/tempest/endpoints/test_tempest_credentials.py @@ -1,3 +1,5 @@ +from unittest.mock import patch + from sentry.tempest.models import TempestCredentials from sentry.testutils.cases import APITestCase from sentry.testutils.helpers.features import Feature @@ -38,7 +40,10 @@ def test_client_secret_is_obfuscated(self): def test_unauthenticated_user_cant_access_endpoint(self): self.get_error_response(self.project.organization.slug, self.project.slug) - def test_create_tempest_credentials(self): + @patch( + "sentry.tempest.endpoints.tempest_credentials.TempestCredentialsDetailsEndpoint.create_audit_entry" + ) + def test_create_tempest_credentials(self, create_audit_entry): with Feature({"organizations:tempest-access": True}): self.login_as(self.user) response = self.get_success_response( @@ -54,6 +59,8 @@ def test_create_tempest_credentials(self): assert creds_obj.project == self.project assert creds_obj.created_by_id == self.user.id + create_audit_entry.assert_called() + def test_create_tempest_credentials_without_feature_flag(self): self.login_as(self.user) response = self.get_error_response( diff --git a/tests/sentry/tempest/endpoints/test_tempest_credentials_details.py b/tests/sentry/tempest/endpoints/test_tempest_credentials_details.py index 953c8e445b379b..2ff39e47324c1f 100644 --- a/tests/sentry/tempest/endpoints/test_tempest_credentials_details.py +++ b/tests/sentry/tempest/endpoints/test_tempest_credentials_details.py @@ -1,3 +1,5 @@ +from unittest.mock import patch + from sentry.tempest.models import TempestCredentials from sentry.testutils.cases import APITestCase from sentry.testutils.helpers.features import Feature @@ -29,7 +31,10 @@ def test_cant_access_endpoint_if_user_is_not_authenticated(self): ) assert response.status_code == 401 - def test_delete_tempest_credentials_as_org_admin(self): + @patch( + "sentry.tempest.endpoints.tempest_credentials_details.TempestCredentialsDetailsEndpoint.create_audit_entry" + ) + def test_delete_tempest_credentials_as_org_admin(self, create_audit_entry): with Feature({"organizations:tempest-access": True}): self.login_as(self.user) response = self.get_response( @@ -41,6 +46,7 @@ def test_delete_tempest_credentials_as_org_admin(self): assert response.status_code == 204 assert not TempestCredentials.objects.filter(id=self.tempest_credentials.id).exists() + create_audit_entry.assert_called() def test_non_admin_cant_delete_credentials(self): non_admin_user = self.create_user() From 79964790893971c892f291b720afa577c5ad0f9d Mon Sep 17 00:00:00 2001 From: Fabian Schindler Date: Thu, 9 Jan 2025 08:52:17 +0100 Subject: [PATCH 5/6] fix(tempest): raise NotFound error when tempest credentials to delete do not exist --- src/sentry/tempest/endpoints/tempest_credentials_details.py | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/sentry/tempest/endpoints/tempest_credentials_details.py b/src/sentry/tempest/endpoints/tempest_credentials_details.py index bfd84c1d6c413f..3e5a1282c22b97 100644 --- a/src/sentry/tempest/endpoints/tempest_credentials_details.py +++ b/src/sentry/tempest/endpoints/tempest_credentials_details.py @@ -30,7 +30,11 @@ def delete(self, request: Request, project: Project, tempest_credentials_id: int if not self.has_feature(request, project): raise NotFound - credentials = TempestCredentials.objects.filter(project=project, id=tempest_credentials_id) + try: + credentials = TempestCredentials.objects.get(project=project, id=tempest_credentials_id) + except TempestCredentials.DoesNotExist: + raise NotFound + data = credentials.get_audit_log_data() credentials.delete() From f7b03439f8e6adaa4b6d0e2d3c2ae65e80cea5c7 Mon Sep 17 00:00:00 2001 From: Fabian Schindler Date: Thu, 9 Jan 2025 09:33:25 +0100 Subject: [PATCH 6/6] test(tempest): fix path for mocked method --- tests/sentry/tempest/endpoints/test_tempest_credentials.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/sentry/tempest/endpoints/test_tempest_credentials.py b/tests/sentry/tempest/endpoints/test_tempest_credentials.py index 44479e911ac80a..4986724e252ee2 100644 --- a/tests/sentry/tempest/endpoints/test_tempest_credentials.py +++ b/tests/sentry/tempest/endpoints/test_tempest_credentials.py @@ -41,7 +41,7 @@ def test_unauthenticated_user_cant_access_endpoint(self): self.get_error_response(self.project.organization.slug, self.project.slug) @patch( - "sentry.tempest.endpoints.tempest_credentials.TempestCredentialsDetailsEndpoint.create_audit_entry" + "sentry.tempest.endpoints.tempest_credentials.TempestCredentialsEndpoint.create_audit_entry" ) def test_create_tempest_credentials(self, create_audit_entry): with Feature({"organizations:tempest-access": True}):