diff --git a/dojo/api_v2/permissions.py b/dojo/api_v2/permissions.py index 2fdd1eb48f9..7fff1bdd1e2 100644 --- a/dojo/api_v2/permissions.py +++ b/dojo/api_v2/permissions.py @@ -401,6 +401,15 @@ class UserHasEngagementRelatedObjectPermission(BaseRelatedObjectPermission): } +class UserHasEngagementNotePermission(BaseRelatedObjectPermission): + permission_map = { + "get_permission": Permissions.Engagement_View, + "put_permission": Permissions.Engagement_Edit, + "delete_permission": Permissions.Engagement_Edit, + "post_permission": Permissions.Engagement_View, + } + + class UserHasRiskAcceptancePermission(permissions.BasePermission): def has_permission(self, request, view): # The previous implementation only checked for the object permission if the path was @@ -453,6 +462,15 @@ class UserHasFindingRelatedObjectPermission(BaseRelatedObjectPermission): } +class UserHasFindingNotePermission(BaseRelatedObjectPermission): + permission_map = { + "get_permission": Permissions.Finding_View, + "put_permission": Permissions.Finding_Edit, + "delete_permission": Permissions.Finding_Edit, + "post_permission": Permissions.Finding_View, + } + + class UserHasImportPermission(permissions.BasePermission): def has_permission(self, request, view): # permission check takes place before validation, so we don't have access to serializer.validated_data() @@ -833,6 +851,15 @@ class UserHasTestRelatedObjectPermission(BaseRelatedObjectPermission): } +class UserHasTestNotePermission(BaseRelatedObjectPermission): + permission_map = { + "get_permission": Permissions.Test_View, + "put_permission": Permissions.Test_Edit, + "delete_permission": Permissions.Test_Edit, + "post_permission": Permissions.Test_View, + } + + class UserHasTestImportPermission(permissions.BasePermission): def has_permission(self, request, view): return check_post_permission( diff --git a/dojo/api_v2/views.py b/dojo/api_v2/views.py index 15ba41e3d01..f8f3557dde2 100644 --- a/dojo/api_v2/views.py +++ b/dojo/api_v2/views.py @@ -121,6 +121,7 @@ Languages, Network_Locations, Note_Type, + NoteHistory, Notes, Notification_Webhooks, Notifications, @@ -504,7 +505,7 @@ def generate_report(self, request, pk=None): request=serializers.AddNewNoteOptionSerializer, responses={status.HTTP_201_CREATED: serializers.NoteSerializer}, ) - @action(detail=True, methods=["get", "post"], permission_classes=[IsAuthenticated, permissions.UserHasEngagementRelatedObjectPermission]) + @action(detail=True, methods=["get", "post"], permission_classes=[IsAuthenticated, permissions.UserHasEngagementNotePermission]) def notes(self, request, pk=None): engagement = self.get_object() if request.method == "POST": @@ -532,6 +533,10 @@ def notes(self, request, pk=None): note_type=note_type, ) note.save() + # Add an entry to the note history + history = NoteHistory.objects.create(data=note.entry, time=note.date, current_editor=note.author) + note.history.add(history) + # Now add the note to the object engagement.notes.add(note) # Determine if we need to send any notifications for user mentioned process_tag_notifications( @@ -1096,7 +1101,7 @@ def request_response(self, request, pk=None): request=serializers.AddNewNoteOptionSerializer, responses={status.HTTP_201_CREATED: serializers.NoteSerializer}, ) - @action(detail=True, methods=["get", "post"], permission_classes=(IsAuthenticated, permissions.UserHasFindingRelatedObjectPermission)) + @action(detail=True, methods=["get", "post"], permission_classes=(IsAuthenticated, permissions.UserHasFindingNotePermission)) def notes(self, request, pk=None): finding = self.get_object() if request.method == "POST": @@ -1125,6 +1130,10 @@ def notes(self, request, pk=None): note_type=note_type, ) note.save() + # Add an entry to the note history + history = NoteHistory.objects.create(data=note.entry, time=note.date, current_editor=note.author) + note.history.add(history) + # Now add the note to the object finding.last_reviewed = note.date finding.last_reviewed_by = author finding.save(update_fields=["last_reviewed", "last_reviewed_by", "updated"]) @@ -1226,7 +1235,7 @@ def download_file(self, request, file_id, pk=None): request=serializers.FindingNoteSerializer, responses={status.HTTP_204_NO_CONTENT: ""}, ) - @action(detail=True, methods=["patch"], permission_classes=(IsAuthenticated, permissions.UserHasFindingRelatedObjectPermission)) + @action(detail=True, methods=["patch"], permission_classes=(IsAuthenticated, permissions.UserHasFindingNotePermission)) def remove_note(self, request, pk=None): """Remove Note From Finding Note""" finding = self.get_object() @@ -2160,7 +2169,7 @@ def generate_report(self, request, pk=None): request=serializers.AddNewNoteOptionSerializer, responses={status.HTTP_201_CREATED: serializers.NoteSerializer}, ) - @action(detail=True, methods=["get", "post"], permission_classes=(IsAuthenticated, permissions.UserHasTestRelatedObjectPermission)) + @action(detail=True, methods=["get", "post"], permission_classes=(IsAuthenticated, permissions.UserHasTestNotePermission)) def notes(self, request, pk=None): test = self.get_object() if request.method == "POST": @@ -2188,6 +2197,10 @@ def notes(self, request, pk=None): note_type=note_type, ) note.save() + # Add an entry to the note history + history = NoteHistory.objects.create(data=note.entry, time=note.date, current_editor=note.author) + note.history.add(history) + # Now add the note to the object test.notes.add(note) # Determine if we need to send any notifications for user mentioned process_tag_notifications( diff --git a/unittests/test_rest_framework.py b/unittests/test_rest_framework.py index 3354dce111a..7732a44d711 100644 --- a/unittests/test_rest_framework.py +++ b/unittests/test_rest_framework.py @@ -856,6 +856,63 @@ def test_detail_configuration_not_authorized(self): response = self.client.get(relative_url) self.assertEqual(200, response.status_code, response.content[:1000]) + class RelatedObjectsTest(BaseClassTest): + def test_notes_can_be_added_by_users_with_read_only_permissions(self): + self.setUp_global_reader() + response = self.client.get(self.url, format="json") + self.assertEqual(200, response.status_code, response.content[:1000]) + engagement_id = response.data["results"][0]["id"] + # Attempt to add a note with reader permissions + relative_url = f"{self.url}{engagement_id}/notes/" + response = self.client.post(relative_url, {"entry": "string"}) + self.assertEqual(201, response.status_code, response.content[:1000]) + + @parameterized.expand( + [ + ("files", {"title": "test"}), + ("tags", {"tags": ["apple", "banana", "cherry"]}), + ], + ) + def test_related_objects(self, related_object_path, payload): + """ + Tests that BaseRelatedObjectPermission enforces the permissions not associated + with the base object. For example, even though a request to add a tag to an + engagement is a POST, we do not need engagement add permissions, but rather + engagement edit permissions since that is what is defined in the + UserHasEngagementRelatedObjectPermission class + """ + self.setUp_global_reader() + # Skip tags for engagement and tests + if related_object_path == "tags" and self.endpoint_model in {Engagement, Test}: + return + # Get an object + response = self.client.get(self.url, format="json") + self.assertEqual(200, response.status_code, response.content[:1000]) + object_id = response.data["results"][0]["id"] + # Attempt to add a related object + relative_url = f"{self.url}{object_id}/{related_object_path}/" + response = self.client.post(relative_url, payload) + self.assertEqual(403, response.status_code, response.content[:1000]) + # Now switch to a user with edit permissions (but not create) + self.setUp_global_writer() + # Retry adding the related object + if related_object_path == "files": + # Convert bytes to a mock uploaded file + response = self.client.post( + relative_url, + { + "file": SimpleUploadedFile( + name="test_file.txt", + content=b"empty", + content_type="text/plain", + ), + **payload, + }, + ) + else: + response = self.client.post(relative_url, payload) + self.assertIn(response.status_code, [200, 201], response.content[:1000]) + @versioned_fixtures class AppAnalysisTest(BaseClass.BaseClassTest): @@ -1487,7 +1544,7 @@ def test_update_object_not_authorized(self): @versioned_fixtures -class EngagementTest(BaseClass.BaseClassTest): +class EngagementTest(BaseClass.RelatedObjectsTest, BaseClass.BaseClassTest): fixtures = ["dojo_testdata.json"] def __init__(self, *args, **kwargs): @@ -1517,42 +1574,6 @@ def __init__(self, *args, **kwargs): self.deleted_objects = 23 BaseClass.RESTEndpointTest.__init__(self, *args, **kwargs) - @parameterized.expand( - [ - ("files", {"title": "test", "file": b"empty"}), - ("notes", {"entry": "string"}), - ], - ) - def test_related_objects(self, related_object_path, payload): - """ - Tests that BaseRelatedObjectPermission enforces the permissions not associated - with the base object. For example, even though a request to add a note to an - engagement is a POST, we do not need engagement add permissions, but rather - engagement edit permissions since that is what is defined in the - UserHasEngagementRelatedObjectPermission class - """ - self.setUp_global_reader() - # Get an engagement - response = self.client.get(self.url, format="json") - self.assertEqual(200, response.status_code, response.content[:1000]) - engagement_id = response.data["results"][0]["id"] - # Attempt to add a related object - relative_url = f"{self.url}{engagement_id}/{related_object_path}/" - response = self.client.post(relative_url, payload) - self.assertEqual(403, response.status_code, response.content[:1000]) - # Now switch to a user with edit permissions (but not create) - self.setUp_global_writer() - # Retry adding the related object - if related_object_path == "files": - # Convert bytes to a mock uploaded file - payload["file"] = SimpleUploadedFile( - name="test_file.txt", - content=payload["file"], # the b"empty" - content_type="text/plain", - ) - response = self.client.post(relative_url, payload) - self.assertEqual(201, response.status_code, response.content[:1000]) - @versioned_fixtures class RiskAcceptanceTest(BaseClass.BaseClassTest): @@ -1726,7 +1747,7 @@ def test_file_with_quoted_name(self): @versioned_fixtures -class FindingsTest(BaseClass.BaseClassTest): +class FindingsTest(BaseClass.RelatedObjectsTest, BaseClass.BaseClassTest): fixtures = ["dojo_testdata.json"] def __init__(self, *args, **kwargs): @@ -2415,7 +2436,7 @@ def test_severity_validation(self): @versioned_fixtures -class TestsTest(BaseClass.BaseClassTest): +class TestsTest(BaseClass.RelatedObjectsTest, BaseClass.BaseClassTest): fixtures = ["dojo_testdata.json"] def __init__(self, *args, **kwargs):