feat(RiskAcc-Eng): Clean RiskAcc-Eng mapping + Allow correct handling by API

docker-compose.yml

@@ -127,6 +127,7 @@ services:
       POSTGRES_PASSWORD: ${DD_DATABASE_PASSWORD:-defectdojo}
     volumes:
       - defectdojo_postgres:/var/lib/postgresql/data
+      - ./psql_bck:/psql_bck
   redis:
     # Pinning to this version due to licensing constraints
     image: redis:7.2.9-alpine@sha256:fce236b99c58ef7196c4e243e43f533b404d5c17239cae4e6e262b729a1952b3

dojo/api_v2/serializers.py

@@ -31,7 +31,6 @@
     save_vulnerability_ids,
     save_vulnerability_ids_template,
 )
-from dojo.finding.queries import get_authorized_findings
 from dojo.group.utils import get_auth_group_name
 from dojo.importers.auto_create_context import AutoCreateContextManager
 from dojo.importers.base_importer import BaseImporter
@@ -117,6 +116,7 @@
     LargeScanSizeProductAnnouncement,
     ScanTypeProductAnnouncement,
 )
+from dojo.risk_acceptance.helper import validate_findings_engagement
 from dojo.tools.factory import (
     get_choices_sorted,
     requires_file,
@@ -1535,68 +1535,68 @@ def create(self, validated_data):
         return instance
 
     def update(self, instance, validated_data):
-        # Determine findings to risk accept, and findings to unaccept risk
-        existing_findings = Finding.objects.filter(risk_acceptance=self.instance.id)
-        new_findings_ids = [x.id for x in validated_data.get("accepted_findings", [])]
-        new_findings = Finding.objects.filter(id__in=new_findings_ids)
-        findings_to_add = set(new_findings) - set(existing_findings)
-        findings_to_remove = set(existing_findings) - set(new_findings)
-        findings_to_add = Finding.objects.filter(id__in=[x.id for x in findings_to_add])
-        findings_to_remove = Finding.objects.filter(id__in=[x.id for x in findings_to_remove])
+        if "accepted_findings" in validated_data:
+            # Determine findings to risk accept, and findings to unaccept risk
+            existing_findings = Finding.objects.filter(risk_acceptance=self.instance.id)
+            new_findings_ids = [x.id for x in validated_data.get("accepted_findings", [])]
+            new_findings = Finding.objects.filter(id__in=new_findings_ids)
+            findings_to_add = set(new_findings) - set(existing_findings)
+            findings_to_remove = set(existing_findings) - set(new_findings)
+            findings_to_add = Finding.objects.filter(id__in=[x.id for x in findings_to_add])
+            findings_to_remove = Finding.objects.filter(id__in=[x.id for x in findings_to_remove])
+        else:
+            findings_to_remove = findings_to_add = []
+
         # Make the update in the database
         instance = super().update(instance, validated_data)
-        user = getattr(self.context.get("request", None), "user", None)
-        # Add the new findings
-        ra_helper.add_findings_to_risk_acceptance(user, instance, findings_to_add)
-        # Remove the ones that were not present in the payload
-        for finding in findings_to_remove:
-            ra_helper.remove_finding_from_risk_acceptance(user, instance, finding)
+
+        if findings_to_add or findings_to_remove:
+            user = getattr(self.context.get("request", None), "user", None)
+            # Add the new findings
+            ra_helper.add_findings_to_risk_acceptance(user, instance, findings_to_add)
+            # Remove the ones that were not present in the payload
+            for finding in findings_to_remove:
+                ra_helper.remove_finding_from_risk_acceptance(user, instance, finding)
         return instance
 
     @extend_schema_field(serializers.CharField())
     def get_path(self, obj):
-        engagement = Engagement.objects.filter(
-            risk_acceptance__id__in=[obj.id],
-        ).first()
         path = "No proof has been supplied"
-        if engagement and obj.filename() is not None:
+        if obj.filename() is not None:
             path = reverse(
-                "download_risk_acceptance", args=(engagement.id, obj.id),
+                "download_risk_acceptance", args=(obj.id, ),
             )
             request = self.context.get("request")
             if request:
                 path = request.build_absolute_uri(path)
         return path
 
-    @extend_schema_field(serializers.IntegerField())
-    def get_engagement(self, obj):
-        engagement = Engagement.objects.filter(
-            risk_acceptance__id__in=[obj.id],
-        ).first()
-        return EngagementSerializer(read_only=True).to_representation(
-            engagement,
-        )
-
     def validate(self, data):
-        def validate_findings_have_same_engagement(finding_objects: list[Finding]):
-            engagements = finding_objects.values_list("test__engagement__id", flat=True).distinct().count()
-            if engagements > 1:
-                msg = "You are not permitted to add findings from multiple engagements"
-                raise PermissionDenied(msg)
-
-        findings = data.get("accepted_findings", [])
-        findings_ids = [x.id for x in findings]
-        finding_objects = Finding.objects.filter(id__in=findings_ids)
-        authed_findings = get_authorized_findings(Permissions.Finding_Edit).filter(id__in=findings_ids)
-        if len(findings) != len(authed_findings):
-            msg = "You are not permitted to add one or more selected findings to this risk acceptance"
-            raise PermissionDenied(msg)
-        if self.context["request"].method == "POST":
-            validate_findings_have_same_engagement(finding_objects)
-        elif self.context["request"].method in {"PATCH", "PUT"}:
-            existing_findings = Finding.objects.filter(risk_acceptance=self.instance.id)
-            existing_and_new_findings = existing_findings | finding_objects
-            validate_findings_have_same_engagement(existing_and_new_findings)
+
+        findings = data.get("accepted_findings", self.instance.accepted_findings.all() if self.instance else None)
+        engagement = data.get("engagement", self.instance.engagement if self.instance else None)
+        validate_findings_engagement(engagement, findings)
+        return data
+
+        # def validate_findings_have_same_engagement(finding_objects: list[Finding]):  # TODO: check
+        #     engagements = finding_objects.values_list("test__engagement__id", flat=True).distinct().count()
+        #     if engagements > 1:
+        #         msg = "You are not permitted to add findings from multiple engagements"  # TODO: same is missing for UI
+        #         raise PermissionDenied(msg)
+
+        # findings = data.get("accepted_findings", [])
+        # findings_ids = [x.id for x in findings]
+        # finding_objects = Finding.objects.filter(id__in=findings_ids)
+        # authed_findings = get_authorized_findings(Permissions.Finding_Edit).filter(id__in=findings_ids)
+        # if len(findings) != len(authed_findings):
+        #     msg = "You are not permitted to add one or more selected findings to this risk acceptance"
+        #     raise PermissionDenied(msg)
+        # if self.context["request"].method == "POST":
+        #     validate_findings_have_same_engagement(finding_objects)
+        # elif self.context["request"].method in {"PATCH", "PUT"}:
+        #     existing_findings = Finding.objects.filter(risk_acceptance=self.instance.id)
+        #     existing_and_new_findings = existing_findings | finding_objects
+        #     validate_findings_have_same_engagement(existing_and_new_findings)
         return data
 
     class Meta:

dojo/api_v2/views.py

@@ -420,7 +420,7 @@ def destroy(self, request, *args, **kwargs):
     def get_queryset(self):
         return (
             get_authorized_engagements(Permissions.Engagement_View)
-            .prefetch_related("notes", "risk_acceptance", "files")
+            .prefetch_related("notes", "risk_acceptance_set", "files")
             .distinct()
         )
 
@@ -704,7 +704,7 @@ def get_queryset(self):
         return (
             get_authorized_risk_acceptances(Permissions.Risk_Acceptance)
             .prefetch_related(
-                "notes", "engagement_set", "owner", "accepted_findings",
+                "notes", "engagement", "owner", "accepted_findings",
             )
             .distinct()
         )

dojo/authorization/authorization.py

@@ -23,6 +23,7 @@
     Product_Type,
     Product_Type_Group,
     Product_Type_Member,
+    Risk_Acceptance,
     Stub_Finding,
     Test,
 )
@@ -92,6 +93,11 @@ def user_has_permission(user, obj, permission):
         and permission in Permissions.get_engagement_permissions()
     ):
         return user_has_permission(user, obj.product, permission)
+    if (
+        isinstance(obj, Risk_Acceptance)
+        and permission in Permissions.get_engagement_permissions()
+    ):
+        return user_has_permission(user, obj.engagement, permission)
     if (
         isinstance(obj, Test)
         and permission in Permissions.get_test_permissions()

dojo/db_migrations/0231_add_engagement_risk_acceptance.py

@@ -0,0 +1,19 @@
+# Generated by Django 5.1.8 on 2025-05-01 12:54
+
+import django.db.models.deletion
+from django.db import migrations, models
+
+
+class Migration(migrations.Migration):
+
+    dependencies = [
+        ('dojo', '0230_alter_jira_instance_accepted_mapping_resolution_and_more'),
+    ]
+
+    operations = [
+        migrations.AddField(
+            model_name='risk_acceptance',
+            name='engagement',
+            field=models.ForeignKey(blank=True, editable=False, null=True, on_delete=django.db.models.deletion.CASCADE, to='dojo.engagement'),
+        ),
+    ]

dojo/db_migrations/0232_set_risk_acceptance_engagement.py

@@ -0,0 +1,28 @@
+# Generated by Django 5.1.8 on 2025-05-01 12:59
+
+import django.db.models.deletion
+from django.db import migrations, models
+import logging
+
+logger = logging.getLogger(__name__)
+
+def set_engagement_based_on_findings(apps, schema_editor):
+    Engagement = apps.get_model('dojo', 'Engagement')
+    RiskAcceptance = apps.get_model('dojo', 'Risk_Acceptance')
+    through_model = Engagement.risk_acceptance.through
+
+    for rel in through_model.objects.all():
+        ra = RiskAcceptance.objects.get(pk=rel.risk_acceptance_id)
+        ra.engagement_id = rel.engagement_id
+        ra.save()
+
+
+class Migration(migrations.Migration):
+
+    dependencies = [
+        ('dojo', '0231_add_engagement_risk_acceptance'),
+    ]
+
+    operations = [
+        migrations.RunPython(set_engagement_based_on_findings, migrations.RunPython.noop),
+    ]

dojo/db_migrations/0233_alter_risk_acceptance_engagement.py

@@ -0,0 +1,23 @@
+# Generated by Django 5.1.8 on 2025-05-01 12:59
+
+import django.db.models.deletion
+from django.db import migrations, models
+
+
+class Migration(migrations.Migration):
+
+    dependencies = [
+        ('dojo', '0232_set_risk_acceptance_engagement'),
+    ]
+
+    operations = [
+        migrations.AlterField(
+            model_name='risk_acceptance',
+            name='engagement',
+            field=models.ForeignKey(on_delete=django.db.models.deletion.CASCADE, to='dojo.engagement'),
+        ),
+        migrations.RemoveField(
+            model_name='engagement',
+            name='risk_acceptance',
+        ),
+    ]

dojo/engagement/urls.py

@@ -36,17 +36,17 @@
         views.add_risk_acceptance, name="add_risk_acceptance"),
     re_path(r"^engagement/(?P<eid>\d+)/risk_acceptance/add/(?P<fid>\d+)$",
         views.add_risk_acceptance, name="add_risk_acceptance"),
-    re_path(r"^engagement/(?P<eid>\d+)/risk_acceptance/(?P<raid>\d+)$",
+    re_path(r"^engagement/risk_acceptance/(?P<raid>\d+)$",
         views.view_risk_acceptance, name="view_risk_acceptance"),
-    re_path(r"^engagement/(?P<eid>\d+)/risk_acceptance/(?P<raid>\d+)/edit$",
+    re_path(r"^engagement/risk_acceptance/(?P<raid>\d+)/edit$",
         views.edit_risk_acceptance, name="edit_risk_acceptance"),
-    re_path(r"^engagement/(?P<eid>\d+)/risk_acceptance/(?P<raid>\d+)/expire$",
+    re_path(r"^engagement/risk_acceptance/(?P<raid>\d+)/expire$",
         views.expire_risk_acceptance, name="expire_risk_acceptance"),
-    re_path(r"^engagement/(?P<eid>\d+)/risk_acceptance/(?P<raid>\d+)/reinstate$",
+    re_path(r"^engagement/risk_acceptance/(?P<raid>\d+)/reinstate$",
         views.reinstate_risk_acceptance, name="reinstate_risk_acceptance"),
-    re_path(r"^engagement/(?P<eid>\d+)/risk_acceptance/(?P<raid>\d+)/delete$",
+    re_path(r"^engagement/risk_acceptance/(?P<raid>\d+)/delete$",
         views.delete_risk_acceptance, name="delete_risk_acceptance"),
-    re_path(r"^engagement/(?P<eid>\d+)/risk_acceptance/(?P<raid>\d+)/download$",
+    re_path(r"^engagement/risk_acceptance/(?P<raid>\d+)/download$",
         views.download_risk_acceptance, name="download_risk_acceptance"),
     re_path(r"^engagement/(?P<eid>\d+)/threatmodel$", views.view_threatmodel,
         name="view_threatmodel"),