From 6e4477a9fa12bf2389829eebe979754413779f72 Mon Sep 17 00:00:00 2001 From: Cody Maffucci <46459665+Maffooch@users.noreply.github.com> Date: Mon, 9 Feb 2026 15:07:48 -0700 Subject: [PATCH 1/6] Add engagement, finding, and test note permissions --- dojo/api_v2/permissions.py | 27 +++++++++++++++++++++++++++ 1 file changed, 27 insertions(+) diff --git a/dojo/api_v2/permissions.py b/dojo/api_v2/permissions.py index 2fdd1eb48f9..e1d046e8e41 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_View, + "delete_permission": Permissions.Engagement_View, + "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_View, + "delete_permission": Permissions.Finding_View, + "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_View, + "delete_permission": Permissions.Test_View, + "post_permission": Permissions.Test_View, + } + + class UserHasTestImportPermission(permissions.BasePermission): def has_permission(self, request, view): return check_post_permission( From 0195675118b070f7a165dd8eb1526f8bcd177a20 Mon Sep 17 00:00:00 2001 From: Cody Maffucci <46459665+Maffooch@users.noreply.github.com> Date: Mon, 9 Feb 2026 15:08:14 -0700 Subject: [PATCH 2/6] Update note permissions in Engagement, Finding, and Test viewsets --- dojo/api_v2/views.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/dojo/api_v2/views.py b/dojo/api_v2/views.py index 15ba41e3d01..4c8aebbc180 100644 --- a/dojo/api_v2/views.py +++ b/dojo/api_v2/views.py @@ -504,7 +504,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": @@ -1096,7 +1096,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": @@ -1226,7 +1226,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 +2160,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": From 64960fb02fa240628606177b7e85bfdfc1133b18 Mon Sep 17 00:00:00 2001 From: Cody Maffucci <46459665+Maffooch@users.noreply.github.com> Date: Mon, 9 Feb 2026 15:08:27 -0700 Subject: [PATCH 3/6] Add note history tracking to Engagement, Finding, and Test viewsets --- dojo/api_v2/views.py | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/dojo/api_v2/views.py b/dojo/api_v2/views.py index 4c8aebbc180..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, @@ -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( @@ -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"]) @@ -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( From 577859fc92f2ea66be6ec7af736d5dc861869cf4 Mon Sep 17 00:00:00 2001 From: Cody Maffucci <46459665+Maffooch@users.noreply.github.com> Date: Mon, 9 Feb 2026 15:53:15 -0700 Subject: [PATCH 4/6] add tests --- unittests/test_rest_framework.py | 92 ++++++++++++++++++-------------- 1 file changed, 53 insertions(+), 39 deletions(-) diff --git a/unittests/test_rest_framework.py b/unittests/test_rest_framework.py index 3354dce111a..a783c87c894 100644 --- a/unittests/test_rest_framework.py +++ b/unittests/test_rest_framework.py @@ -856,6 +856,56 @@ 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", "file": b"empty"}), + ("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 + 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.assertIn(response.status_code, [200, 201], response.content[:1000]) + @versioned_fixtures class AppAnalysisTest(BaseClass.BaseClassTest): @@ -1487,7 +1537,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 +1567,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 +1740,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 +2429,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): From 53ee84d60ad80d0dd77bffe9c08f3a5426145c0f Mon Sep 17 00:00:00 2001 From: Cody Maffucci <46459665+Maffooch@users.noreply.github.com> Date: Mon, 9 Feb 2026 21:33:58 -0700 Subject: [PATCH 5/6] Correct test memory issue --- unittests/test_rest_framework.py | 19 +++++++++++++------ 1 file changed, 13 insertions(+), 6 deletions(-) diff --git a/unittests/test_rest_framework.py b/unittests/test_rest_framework.py index a783c87c894..7732a44d711 100644 --- a/unittests/test_rest_framework.py +++ b/unittests/test_rest_framework.py @@ -869,7 +869,7 @@ def test_notes_can_be_added_by_users_with_read_only_permissions(self): @parameterized.expand( [ - ("files", {"title": "test", "file": b"empty"}), + ("files", {"title": "test"}), ("tags", {"tags": ["apple", "banana", "cherry"]}), ], ) @@ -898,12 +898,19 @@ def test_related_objects(self, related_object_path, payload): # 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, + { + "file": SimpleUploadedFile( + name="test_file.txt", + content=b"empty", + content_type="text/plain", + ), + **payload, + }, ) - response = self.client.post(relative_url, payload) + else: + response = self.client.post(relative_url, payload) self.assertIn(response.status_code, [200, 201], response.content[:1000]) From 02562cf7f62298bd1348143d7c6e6334d8933a48 Mon Sep 17 00:00:00 2001 From: Cody Maffucci <46459665+Maffooch@users.noreply.github.com> Date: Thu, 12 Feb 2026 17:33:58 -0700 Subject: [PATCH 6/6] Apply suggestions from code review --- dojo/api_v2/permissions.py | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/dojo/api_v2/permissions.py b/dojo/api_v2/permissions.py index e1d046e8e41..7fff1bdd1e2 100644 --- a/dojo/api_v2/permissions.py +++ b/dojo/api_v2/permissions.py @@ -404,8 +404,8 @@ class UserHasEngagementRelatedObjectPermission(BaseRelatedObjectPermission): class UserHasEngagementNotePermission(BaseRelatedObjectPermission): permission_map = { "get_permission": Permissions.Engagement_View, - "put_permission": Permissions.Engagement_View, - "delete_permission": Permissions.Engagement_View, + "put_permission": Permissions.Engagement_Edit, + "delete_permission": Permissions.Engagement_Edit, "post_permission": Permissions.Engagement_View, } @@ -465,8 +465,8 @@ class UserHasFindingRelatedObjectPermission(BaseRelatedObjectPermission): class UserHasFindingNotePermission(BaseRelatedObjectPermission): permission_map = { "get_permission": Permissions.Finding_View, - "put_permission": Permissions.Finding_View, - "delete_permission": Permissions.Finding_View, + "put_permission": Permissions.Finding_Edit, + "delete_permission": Permissions.Finding_Edit, "post_permission": Permissions.Finding_View, } @@ -854,8 +854,8 @@ class UserHasTestRelatedObjectPermission(BaseRelatedObjectPermission): class UserHasTestNotePermission(BaseRelatedObjectPermission): permission_map = { "get_permission": Permissions.Test_View, - "put_permission": Permissions.Test_View, - "delete_permission": Permissions.Test_View, + "put_permission": Permissions.Test_Edit, + "delete_permission": Permissions.Test_Edit, "post_permission": Permissions.Test_View, }