Fix paths

Author: kiblik

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,
@@ -1564,33 +1564,39 @@ def get_path(self, obj):
         path = "No proof has been supplied"
         if obj.filename() is not None:
             path = reverse(
-                "download_risk_acceptance", args=(obj.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
 
     def validate(self, 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)
+
+        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/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/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"),

dojo/engagement/views.py

@@ -1259,20 +1259,20 @@ def add_risk_acceptance(request, eid, fid=None):
                   })
 
 
-@user_is_authorized(Engagement, Permissions.Engagement_View, "eid")
-def view_risk_acceptance(request, eid, raid):
-    return view_edit_risk_acceptance(request, eid=eid, raid=raid, edit_mode=False)
+@user_is_authorized(Risk_Acceptance, Permissions.Engagement_View, "raid")
+def view_risk_acceptance(request, raid):
+    return view_edit_risk_acceptance(request, raid=raid, edit_mode=False)
 
 
-@user_is_authorized(Engagement, Permissions.Risk_Acceptance, "eid")
-def edit_risk_acceptance(request, eid, raid):
-    return view_edit_risk_acceptance(request, eid=eid, raid=raid, edit_mode=True)
+@user_is_authorized(Risk_Acceptance, Permissions.Risk_Acceptance, "raid")
+def edit_risk_acceptance(request, raid):
+    return view_edit_risk_acceptance(request, raid=raid, edit_mode=True)
 
 
 # will only be called by view_risk_acceptance and edit_risk_acceptance
-def view_edit_risk_acceptance(request, eid, raid, *, edit_mode=False):
+def view_edit_risk_acceptance(request, raid, *, edit_mode=False):
     risk_acceptance = get_object_or_404(Risk_Acceptance, pk=raid)
-    eng = get_object_or_404(Engagement, pk=eid)
+    eng = risk_acceptance.engagement
 
     if edit_mode and not eng.product.enable_full_risk_acceptance:
         raise PermissionDenied
@@ -1381,7 +1381,7 @@ def view_edit_risk_acceptance(request, eid, raid, *, edit_mode=False):
                     extra_tags="alert-success")
         if not errors:
             logger.debug("redirecting to return_url")
-            return redirect_to_return_url_or_else(request, reverse("view_risk_acceptance", args=(eid, raid)))
+            return redirect_to_return_url_or_else(request, reverse("view_risk_acceptance", args=(raid, )))
         logger.error("errors found")
 
     elif edit_mode:
@@ -1433,34 +1433,32 @@ def view_edit_risk_acceptance(request, eid, raid, *, edit_mode=False):
         })
 
 
-@user_is_authorized(Engagement, Permissions.Risk_Acceptance, "eid")
-def expire_risk_acceptance(request, eid, raid):
+@user_is_authorized(Risk_Acceptance, Permissions.Risk_Acceptance, "raid")
+def expire_risk_acceptance(request, raid):
     risk_acceptance = get_object_or_404(prefetch_for_expiration(Risk_Acceptance.objects.all()), pk=raid)
-    # Validate the engagement ID exists before moving forward
-    get_object_or_404(Engagement, pk=eid)
 
     ra_helper.expire_now(risk_acceptance)
 
-    return redirect_to_return_url_or_else(request, reverse("view_risk_acceptance", args=(eid, raid)))
+    return redirect_to_return_url_or_else(request, reverse("view_risk_acceptance", args=(raid, )))
 
 
-@user_is_authorized(Engagement, Permissions.Risk_Acceptance, "eid")
-def reinstate_risk_acceptance(request, eid, raid):
+@user_is_authorized(Risk_Acceptance, Permissions.Risk_Acceptance, "raid")
+def reinstate_risk_acceptance(request, raid):
     risk_acceptance = get_object_or_404(prefetch_for_expiration(Risk_Acceptance.objects.all()), pk=raid)
-    eng = get_object_or_404(Engagement, pk=eid)
+    eng = risk_acceptance.engagement
 
     if not eng.product.enable_full_risk_acceptance:
         raise PermissionDenied
 
     ra_helper.reinstate(risk_acceptance, risk_acceptance.expiration_date)
 
-    return redirect_to_return_url_or_else(request, reverse("view_risk_acceptance", args=(eid, raid)))
+    return redirect_to_return_url_or_else(request, reverse("view_risk_acceptance", args=(raid, )))
 
 
-@user_is_authorized(Engagement, Permissions.Risk_Acceptance, "eid")
-def delete_risk_acceptance(request, eid, raid):
+@user_is_authorized(Risk_Acceptance, Permissions.Risk_Acceptance, "raid")
+def delete_risk_acceptance(request, raid):
     risk_acceptance = get_object_or_404(Risk_Acceptance, pk=raid)
-    eng = get_object_or_404(Engagement, pk=eid)
+    eng = risk_acceptance.engagement
 
     ra_helper.delete(eng, risk_acceptance)
 
@@ -1472,13 +1470,10 @@ def delete_risk_acceptance(request, eid, raid):
     return HttpResponseRedirect(reverse("view_engagement", args=(eng.id, )))
 
 
-@user_is_authorized(Engagement, Permissions.Engagement_View, "eid")
-def download_risk_acceptance(request, eid, raid):
+@user_is_authorized(Risk_Acceptance, Permissions.Risk_Acceptance, "raid")
+def download_risk_acceptance(request, raid):
     mimetypes.init()
     risk_acceptance = get_object_or_404(Risk_Acceptance, pk=raid)
-    # Ensure the risk acceptance is under the supplied engagement
-    if not Engagement.objects.filter(risk_acceptance=risk_acceptance, id=eid).exists():
-        raise PermissionDenied
     response = StreamingHttpResponse(
         FileIterWrapper(
             (Path(settings.MEDIA_ROOT) / "risk_acceptance.path.name").open(mode="rb")))

dojo/risk_acceptance/helper.py

@@ -2,7 +2,7 @@
 from contextlib import suppress
 
 from dateutil.relativedelta import relativedelta
-from django.core.exceptions import PermissionDenied
+from django.core.exceptions import PermissionDenied, ValidationError
 from django.urls import reverse
 from django.utils import timezone
 
@@ -50,7 +50,7 @@ def expire_now(risk_acceptance):
     create_notification(event="risk_acceptance_expiration", title=title, risk_acceptance=risk_acceptance, accepted_findings=accepted_findings,
                          reactivated_findings=reactivated_findings, engagement=risk_acceptance.engagement,
                          product=risk_acceptance.engagement.product,
-                         url=reverse("view_risk_acceptance", args=(risk_acceptance.engagement.id, risk_acceptance.id)))
+                         url=reverse("view_risk_acceptance", args=(risk_acceptance.id, )))
 
 
 def reinstate(risk_acceptance, old_expiration_date):
@@ -184,7 +184,7 @@ def expiration_handler(*args, **kwargs):
             create_notification(event="risk_acceptance_expiration", title=notification_title, risk_acceptance=risk_acceptance,
                                 accepted_findings=risk_acceptance.accepted_findings.all(), engagement=risk_acceptance.engagement,
                                 product=risk_acceptance.engagement.product,
-                                url=reverse("view_risk_acceptance", args=(risk_acceptance.engagement.id, risk_acceptance.id)))
+                                url=reverse("view_risk_acceptance", args=(risk_acceptance.id, )))
 
             post_jira_comments(risk_acceptance, risk_acceptance.accepted_findings.all(), expiration_warning_message_creator, heads_up_days)
 
@@ -197,45 +197,45 @@ def get_view_risk_acceptance(risk_acceptance: Risk_Acceptance) -> str:
     # Suppressing this error because it does not happen under most circumstances that a risk acceptance does not have engagement
     with suppress(AttributeError):
         get_full_url(
-            reverse("view_risk_acceptance", args=(risk_acceptance.engagement.id, risk_acceptance.id)),
+            reverse("view_risk_acceptance", args=(risk_acceptance.id, )),
         )
     return ""
 
 
 def expiration_message_creator(risk_acceptance, heads_up_days=0):
     return "Risk acceptance [({})|{}] with {} findings has expired".format(
         escape_for_jira(risk_acceptance.name),
-        get_full_url(reverse("view_risk_acceptance", args=(risk_acceptance.engagement.id, risk_acceptance.id))),
+        get_full_url(reverse("view_risk_acceptance", args=(risk_acceptance.id,))),
         len(risk_acceptance.accepted_findings.all()))
 
 
 def expiration_warning_message_creator(risk_acceptance, heads_up_days=0):
     return "Risk acceptance [({})|{}] with {} findings will expire in {} days".format(
         escape_for_jira(risk_acceptance.name),
-        get_full_url(reverse("view_risk_acceptance", args=(risk_acceptance.engagement.id, risk_acceptance.id))),
+        get_full_url(reverse("view_risk_acceptance", args=(risk_acceptance.id, ))),
         len(risk_acceptance.accepted_findings.all()), heads_up_days)
 
 
 def reinstation_message_creator(risk_acceptance, heads_up_days=0):
     return "Risk acceptance [({})|{}] with {} findings has been reinstated (expires on {})".format(
         escape_for_jira(risk_acceptance.name),
-        get_full_url(reverse("view_risk_acceptance", args=(risk_acceptance.engagement.id, risk_acceptance.id))),
+        get_full_url(reverse("view_risk_acceptance", args=(risk_acceptance.id, ))),
         len(risk_acceptance.accepted_findings.all()), timezone.localtime(risk_acceptance.expiration_date).strftime("%b %d, %Y"))
 
 
 def accepted_message_creator(risk_acceptance, heads_up_days=0):
     if risk_acceptance:
         return "Finding has been added to risk acceptance [({})|{}] with {} findings (expires on {})".format(
             escape_for_jira(risk_acceptance.name),
-            get_full_url(reverse("view_risk_acceptance", args=(risk_acceptance.engagement.id, risk_acceptance.id))),
+            get_full_url(reverse("view_risk_acceptance", args=(risk_acceptance.id, ))),
             len(risk_acceptance.accepted_findings.all()), timezone.localtime(risk_acceptance.expiration_date).strftime("%b %d, %Y"))
     return "Finding has been risk accepted"
 
 
 def unaccepted_message_creator(risk_acceptance, heads_up_days=0):
     if risk_acceptance:
         return "finding was unaccepted/deleted from risk acceptance [({})|{}]".format(escape_for_jira(risk_acceptance.name),
-            get_full_url(reverse("view_risk_acceptance", args=(risk_acceptance.engagement.id, risk_acceptance.id))))
+            get_full_url(reverse("view_risk_acceptance", args=(risk_acceptance.id, ))))
     return "Finding is no longer risk accepted"
 
 
@@ -370,3 +370,10 @@ def update_endpoint_statuses(finding: Finding, *, accept_risk: bool) -> None:
             status.risk_accepted = False
         status.last_modified = timezone.now()
         status.save()
+
+
+def validate_findings_engagement(engagement, findings):
+    invalid = [f.id for f in findings if f.test.engagement.id != engagement.id]
+    if invalid:
+        msg = f"Findings with IDs {invalid} do not belong to engagement {engagement.id}."
+        raise ValidationError(msg)

dojo/templates/dojo/custom_html_report_endpoint_list.html

@@ -86,7 +86,7 @@ <h5 class="report_findings_entry">
                                 <td>
                                     {% comment %} for some reason the font-awesome icons don't work with the report template{% endcomment %}
                                     {% for ra in finding.risk_acceptance_set.all|slice:":5" %}
-                                        <a href="{% url 'view_risk_acceptance' finding.test.engagement.id ra.id %}"
+                                        <a href="{% url 'view_risk_acceptance' ra.id %}"
                                         class="{% if ra.is_expired %}lightgrey{% endif%}">acceptance</a>&nbsp;
                                     {% endfor %}
                                 </td>

dojo/templates/dojo/custom_html_report_finding_list.html

@@ -75,7 +75,7 @@ <h5 class="report_findings_entry">
                             <td>
                                 {% comment %} for some reason the font-awesome icons don't work with the report template{% endcomment %}
                                 {% for ra in finding.risk_acceptance_set.all|slice:":5" %}
-                                    <a href="{% url 'view_risk_acceptance' finding.test.engagement.id ra.id %}"
+                                    <a href="{% url 'view_risk_acceptance' ra.id %}"
                                     class="{% if ra.is_expired %}lightgrey{% endif%}">acceptance</a>&nbsp;
                                 {% endfor %}
                             </td>

dojo/templates/dojo/endpoint_pdf_report.html

@@ -148,7 +148,7 @@ <h5>
                                         <td>
                                             {% comment %} for some reason the font-awesome icons don't work with the report template{% endcomment %}
                                             {% for ra in finding.risk_acceptance_set.all|slice:":5" %}
-                                                <a href="{% url 'view_risk_acceptance' finding.test.engagement.id ra.id %}"
+                                                <a href="{% url 'view_risk_acceptance' ra.id %}"
                                                 class="{% if ra.is_expired %}lightgrey{% endif%}">acceptance</a>&nbsp;
                                             {% endfor %}
                                         </td>

dojo/templates/dojo/engagement_pdf_report.html

@@ -283,7 +283,7 @@ <h5>
                                         <td>
                                             {% comment %} for some reason the font-awesome icons don't work with the report template{% endcomment %}
                                             {% for ra in finding.risk_acceptance_set.all|slice:":5" %}
-                                                <a href="{% url 'view_risk_acceptance' finding.test.engagement.id ra.id %}"
+                                                <a href="{% url 'view_risk_acceptance' ra.id %}"
                                                 class="{% if ra.is_expired %}lightgrey{% endif%}">acceptance</a>&nbsp;
                                             {% endfor %}
                                         </td>

dojo/templates/dojo/finding_pdf_report.html

@@ -127,7 +127,7 @@ <h5>
                                         <td>
                                             {% comment %} for some reason the font-awesome icons don't work with the report template{% endcomment %}
                                             {% for ra in finding.risk_acceptance_set.all|slice:":5" %}
-                                                <a href="{% url 'view_risk_acceptance' finding.test.engagement.id ra.id %}"
+                                                <a href="{% url 'view_risk_acceptance' ra.id %}"
                                                 class="{% if ra.is_expired %}lightgrey{% endif%}">acceptance</a>&nbsp;
                                             {% endfor %}
                                         </td>