diff --git a/api/audit/constants.py b/api/audit/constants.py index eddb96ef2b76..768121dfba11 100644 --- a/api/audit/constants.py +++ b/api/audit/constants.py @@ -81,3 +81,5 @@ "Phased rollout created for feature: %s by release pipeline: %s (stage: %s)" ) PHASED_ROLLOUT_STATE_UPDATED_MESSAGE = "Phased rollout split changed from '%s%%' to '%s%%' for feature '%s' by release pipeline '%s' (stage: '%s')" +PROJECT_CREATED_MESSAGE = "New Project created: %s" +PROJECT_DELETED_MESSAGE = "Project deleted: %s" diff --git a/api/audit/related_object_type.py b/api/audit/related_object_type.py index 53f6d0a9fbe3..5d5ce4a0f2ce 100644 --- a/api/audit/related_object_type.py +++ b/api/audit/related_object_type.py @@ -15,3 +15,4 @@ class RelatedObjectType(enum.Enum): WAREHOUSE_CONNECTION = "Warehouse connection" EXPERIMENT = "Experiment" METRIC = "Metric" + PROJECT = "project" diff --git a/api/projects/views.py b/api/projects/views.py index 8274dd79cd5f..93f081a2832d 100644 --- a/api/projects/views.py +++ b/api/projects/views.py @@ -1,11 +1,14 @@ # -*- coding: utf-8 -*- from __future__ import unicode_literals +import typing + from common.projects.permissions import ( TAG_SUPPORTED_PERMISSIONS, VIEW_PROJECT, ) from django.conf import settings +from django.db import transaction from django.utils.decorators import method_decorator from drf_spectacular.utils import extend_schema from rest_framework import status, viewsets @@ -16,6 +19,9 @@ from rest_framework.request import Request from rest_framework.response import Response +from audit.constants import PROJECT_CREATED_MESSAGE, PROJECT_DELETED_MESSAGE +from audit.models import AuditLog +from audit.related_object_type import RelatedObjectType from environments.dynamodb.migrator import IdentityMigrator from environments.identities.models import Identity from environments.serializers import EnvironmentSerializerLight @@ -106,10 +112,40 @@ def get_queryset(self): # type: ignore[no-untyped-def] def perform_create(self, serializer): # type: ignore[no-untyped-def] project = serializer.save() + is_master_api_key_user = getattr( + self.request.user, "is_master_api_key_user", False + ) if getattr(self.request.user, "is_master_api_key_user", False) is False: UserProjectPermission.objects.create( # type: ignore[misc] user=self.request.user, project=project, admin=True ) + AuditLog.objects.create( + project=project, + author=None if is_master_api_key_user else self.request.user, + master_api_key=getattr(self.request.user, "key", None) + if is_master_api_key_user + else None, + related_object_id=project.id, + related_object_type=RelatedObjectType.PROJECT.name, + log=PROJECT_CREATED_MESSAGE % project.name, + ) + + def perform_destroy(self, instance: typing.Any) -> None: + with transaction.atomic(): + is_master_api_key_user = getattr( + self.request.user, "is_master_api_key_user", False + ) + AuditLog.objects.create( + project=None, + author=None if is_master_api_key_user else self.request.user, + master_api_key=getattr(self.request.user, "key", None) + if is_master_api_key_user + else None, + related_object_id=instance.id, + related_object_type=RelatedObjectType.PROJECT.name, + log=PROJECT_DELETED_MESSAGE % instance.name, + ) + instance.delete() @action( detail=False, diff --git a/api/tests/integration/audit/test_audit_logs.py b/api/tests/integration/audit/test_audit_logs.py index edbc5ef5dbf0..abb996c36c4c 100644 --- a/api/tests/integration/audit/test_audit_logs.py +++ b/api/tests/integration/audit/test_audit_logs.py @@ -46,7 +46,7 @@ def test_list_audit_logs__with_project_filter__makes_expected_queries( # type: # Then assert res.status_code == status.HTTP_200_OK - assert res.json()["count"] == 3 + assert res.json()["count"] == 4 def test_retrieve_audit_log__environment_change__includes_change_details( @@ -363,7 +363,7 @@ def test_retrieve_audit_log__segment_override_created_and_deleted__includes_chan get_audit_logs_response_2 = admin_client.get(get_audit_logs_url) assert get_audit_logs_response_2.status_code == status.HTTP_200_OK results = get_audit_logs_response_2.json()["results"] - assert len(results) == 5 + assert len(results) == 6 # and the first one in the list should be for the deletion of the segment override delete_override_audit_log_id = results[0]["id"] @@ -421,9 +421,9 @@ def test_retrieve_audit_log__segment_override_created_for_feature_value__include # and we should only have one audit log in the list related to the segment override # (since the FeatureState hasn't changed) - # 1 for creating the feature + 1 for creating the environment + 1 for creating the segment - # + 1 for the segment override = 4 - assert len(results) == 4 + # 1 for creating the project + 1 for creating the feature + 1 for creating the environment + 1 for creating the segment + # + 1 for the segment override = 5 + assert len(results) == 5 # the first audit log in the list (i.e. most recent) should be the one that we want audit_log_id = results[0]["id"] diff --git a/api/tests/unit/projects/test_unit_projects_views.py b/api/tests/unit/projects/test_unit_projects_views.py index cff455eef308..75feb270d536 100644 --- a/api/tests/unit/projects/test_unit_projects_views.py +++ b/api/tests/unit/projects/test_unit_projects_views.py @@ -17,6 +17,9 @@ from rest_framework.test import APIClient from task_processor.task_run_method import TaskRunMethod +from audit.constants import PROJECT_CREATED_MESSAGE, PROJECT_DELETED_MESSAGE +from audit.models import AuditLog +from audit.related_object_type import RelatedObjectType from environments.dynamodb.types import ProjectIdentityMigrationStatus from environments.identities.models import Identity from features.models import Feature, FeatureSegment @@ -1069,3 +1072,48 @@ def test_list_projects__default_enforce_feature_owners__returns_false( assert len(response.json()) > 0 assert "enforce_feature_owners" in response.json()[0] assert response.json()[0]["enforce_feature_owners"] is False + + +def test_create_project__valid_request__creates_audit_log( + admin_client: APIClient, organisation: Organisation +) -> None: + # Given + url = reverse("api-v1:projects:project-list") + project_name = "New Audit Log Project" + data = {"name": project_name, "organisation": organisation.id} + initial_audit_log_count = AuditLog.objects.count() + + # When + response = admin_client.post(url, data=data) + + # Then + assert response.status_code == status.HTTP_201_CREATED + assert AuditLog.objects.count() == initial_audit_log_count + 1 + + # Verify the audit log details + audit_log = AuditLog.objects.order_by("-created_date").first() + assert audit_log.related_object_type == RelatedObjectType.PROJECT.name + assert audit_log.log == PROJECT_CREATED_MESSAGE % project_name + assert audit_log.project_id == response.data["id"] + + +def test_delete_project__valid_request__creates_audit_log( + admin_client: APIClient, project: Project, organisation: Organisation +) -> None: + # Given + url = reverse("api-v1:projects:project-detail", args=[project.id]) + project_name = project.name + initial_audit_log_count = AuditLog.objects.count() + + # When + response = admin_client.delete(url) + + # Then + assert response.status_code == status.HTTP_204_NO_CONTENT + assert AuditLog.objects.count() == initial_audit_log_count + 1 + + # Verify the audit log details + audit_log = AuditLog.objects.order_by("-created_date").first() + assert audit_log.related_object_type == RelatedObjectType.PROJECT.name + assert audit_log.log == PROJECT_DELETED_MESSAGE % project_name + assert audit_log.related_object_id == project.id