Merge pull request #11511 from DefectDojo/bugfix

Merge `bugfix` -> `dev` for release 2.42.0

Author: Maffooch

dojo/finding/views.py

@@ -1438,25 +1438,7 @@ def reopen_finding(request, fid):
         status.save()
     # Clear the risk acceptance, if present
     ra_helper.risk_unaccept(request.user, finding)
-
-    # Manage the jira status changes
-    push_to_jira = False
-    # Determine if the finding is in a group. if so, not push to jira
-    finding_in_group = finding.has_finding_group
-    # Check if there is a jira issue that needs to be updated
-    jira_issue_exists = finding.has_jira_issue or (finding.finding_group and finding.finding_group.has_jira_issue)
-    # Only push if the finding is not in a group
-    if jira_issue_exists:
-        # Determine if any automatic sync should occur
-        push_to_jira = jira_helper.is_push_all_issues(finding) \
-            or jira_helper.get_jira_instance(finding).finding_jira_sync
-    # Save the finding
-    finding.save(push_to_jira=(push_to_jira and not finding_in_group))
-
-    # we only push the group after saving the finding to make sure
-    # the updated data of the finding is pushed as part of the group
-    if push_to_jira and finding_in_group:
-        jira_helper.push_to_jira(finding.finding_group)
+    jira_helper.save_and_push_to_jira(finding)
 
     reopen_external_issue(finding, "re-opened by defectdojo", "github")
 

dojo/fixtures/dojo_testdata.json

@@ -2158,8 +2158,8 @@
     "fields": {
       "configuration_name": "Happy little JIRA 2",
       "url": "https://defectdojo.atlassian.net/",
-      "username": "YOUR USERNAME",
-      "password": "YOU API TOKEN",
+      "username": "[YOUR USERNAME]",
+      "password": "[YOUR API TOKEN]",
       "default_issue_type": "Task",
       "epic_name_id": 10011,
       "open_status_key": 11,
@@ -2253,7 +2253,7 @@
       "component": "",
       "enable_engagement_epic_mapping": true,
       "jira_instance": 2,
-      "project_key": "key1"
+      "project_key": "NTEST"
     }
   },
   {

dojo/jira_link/helper.py

@@ -785,14 +785,15 @@ def failure_to_add_message(message: str, exception: Exception, object: Any) -> b
         JIRAError.log_to_tempfile = False
         jira = get_jira_connection(jira_instance)
     except Exception as e:
-        message = f"The following jira instance could not be connected: {jira_instance} - {e.text}"
+        message = f"The following jira instance could not be connected: {jira_instance} - {e}"
         return failure_to_add_message(message, e, obj)
     # Set the list of labels to set on the jira issue
     labels = get_labels(obj) + get_tags(obj)
     if labels:
         labels = list(dict.fromkeys(labels))  # de-dup
     # Determine what due date to set on the jira issue
     duedate = None
+
     if System_Settings.objects.get().enable_finding_sla:
         duedate = obj.sla_deadline()
     # Set the fields that will compose the jira issue
@@ -1104,6 +1105,7 @@ def get_issuetype_fields(
 
     issuetype_fields = None
     use_cloud_api = jira.deploymentType.lower() == "cloud" or jira._version < (9, 0, 0)
+
     try:
         if use_cloud_api:
             try:
@@ -1706,3 +1708,24 @@ def process_resolution_from_jira(finding, resolution_id, resolution_name, assign
     if status_changed:
         finding.save()
     return status_changed
+
+
+def save_and_push_to_jira(finding):
+    # Manage the jira status changes
+    push_to_jira = False
+    # Determine if the finding is in a group. if so, not push to jira yet
+    finding_in_group = finding.has_finding_group
+    # Check if there is a jira issue that needs to be updated
+    jira_issue_exists = finding.has_jira_issue or (finding.finding_group and finding.finding_group.has_jira_issue)
+    # Only push if the finding is not in a group
+    if jira_issue_exists:
+        # Determine if any automatic sync should occur
+        push_to_jira = is_push_all_issues(finding) \
+            or get_jira_instance(finding).finding_jira_sync
+    # Save the finding
+    finding.save(push_to_jira=(push_to_jira and not finding_in_group))
+
+    # we only push the group after saving the finding to make sure
+    # the updated data of the finding is pushed as part of the group
+    if push_to_jira and finding_in_group:
+        push_to_jira(finding.finding_group)

dojo/risk_acceptance/helper.py

@@ -22,23 +22,21 @@ def expire_now(risk_acceptance):
     reactivated_findings = []
     if risk_acceptance.reactivate_expired:
         for finding in risk_acceptance.accepted_findings.all():
-            if not finding.active:
-                logger.debug("%i:%s: unaccepting a.k.a reactivating finding.", finding.id, finding)
-                finding.active = True
-                finding.risk_accepted = False
+            if not finding.active:  # not sure why this is important
+                logger.debug("%i:%s: unaccepting/reactivating finding.", finding.id, finding)
+
                 # Update any endpoint statuses on each of the findings
                 update_endpoint_statuses(finding, accept_risk=False)
+                risk_unaccept(None, finding, post_comments=False)  # comments will be posted at end
 
                 if risk_acceptance.restart_sla_expired:
                     finding.sla_start_date = timezone.now().date()
+                    finding.save(dedupe_option=False)  # resave if changed after risk_unaccept
 
-                finding.save(dedupe_option=False)
                 reactivated_findings.append(finding)
-                # findings remain in this risk acceptance for reporting / metrics purposes
             else:
                 logger.debug("%i:%s already active, no changes made.", finding.id, finding)
 
-        # best effort JIRA integration, no status changes
         post_jira_comments(risk_acceptance, risk_acceptance.accepted_findings.all(), expiration_message_creator)
 
     risk_acceptance.expiration_date = timezone.now()
@@ -189,7 +187,7 @@ def expiration_handler(*args, **kwargs):
                                 product=risk_acceptance.engagement.product,
                                 url=reverse("view_risk_acceptance", args=(risk_acceptance.engagement.id, risk_acceptance.id)))
 
-            post_jira_comments(risk_acceptance, expiration_warning_message_creator, heads_up_days)
+            post_jira_comments(risk_acceptance, risk_acceptance.accepted_findings.all(), expiration_warning_message_creator, heads_up_days)
 
             risk_acceptance.expiration_date_warned = timezone.now()
             risk_acceptance.save()
@@ -243,20 +241,22 @@ def unaccepted_message_creator(risk_acceptance, heads_up_days=0):
 
 
 def post_jira_comment(finding, message_factory, heads_up_days=0):
-    if not finding or not finding.has_jira_issue:
+    if not finding or (not finding.has_jira_issue and not finding.has_jira_group_issue):
         return
-
     jira_project = jira_helper.get_jira_project(finding)
 
     if jira_project and jira_project.risk_acceptance_expiration_notification:
         jira_instance = jira_helper.get_jira_instance(finding)
-
         if jira_instance:
 
             jira_comment = message_factory(None, heads_up_days)
 
-            logger.debug("Creating JIRA comment for something risk acceptance related")
-            jira_helper.add_simple_jira_comment(jira_instance, finding.jira_issue, jira_comment)
+            jira_issue = None
+            if finding.has_jira_issue:
+                jira_issue = finding.jira_issue
+            elif finding.has_jira_group_issue:
+                jira_issue = finding.finding_group.jira_issue
+            jira_helper.add_simple_jira_comment(jira_instance, jira_issue, jira_comment)
 
 
 def post_jira_comments(risk_acceptance, findings, message_factory, heads_up_days=0):
@@ -270,11 +270,15 @@ def post_jira_comments(risk_acceptance, findings, message_factory, heads_up_days
 
         if jira_instance:
             jira_comment = message_factory(risk_acceptance, heads_up_days)
-
             for finding in findings:
+                jira_issue = None
                 if finding.has_jira_issue:
-                    logger.debug("Creating JIRA comment for something risk acceptance related")
-                    jira_helper.add_simple_jira_comment(jira_instance, finding.jira_issue, jira_comment)
+                    jira_issue = finding.jira_issue
+                elif finding.has_jira_group_issue:
+                    jira_issue = finding.finding_group.jira_issue
+
+                if jira_issue:
+                    jira_helper.add_simple_jira_comment(jira_instance, jira_issue, jira_comment)
 
 
 def get_expired_risk_acceptances_to_handle():
@@ -319,7 +323,7 @@ def simple_risk_accept(user: Dojo_User, finding: Finding, perform_save=True) ->
         ))
 
 
-def risk_unaccept(user: Dojo_User, finding: Finding, perform_save=True) -> None:
+def risk_unaccept(user: Dojo_User, finding: Finding, perform_save=True, post_comments=True) -> None:
     logger.debug("unaccepting finding %i:%s if it is currently risk accepted", finding.id, finding)
     if finding.risk_accepted:
         logger.debug("unaccepting finding %i:%s", finding.id, finding)
@@ -336,7 +340,12 @@ def risk_unaccept(user: Dojo_User, finding: Finding, perform_save=True) -> None:
 
         # post_jira_comment might reload from database so see unaccepted finding. but the comment
         # only contains some text so that's ok
-        post_jira_comment(finding, unaccepted_message_creator)
+        if post_comments:
+            post_jira_comment(finding, unaccepted_message_creator)
+
+        # Update the JIRA obect for this finding
+        jira_helper.save_and_push_to_jira(finding)
+
         # Add a note to reflect that the finding was removed from the risk acceptance
         if user is not None:
             finding.notes.add(Notes.objects.create(

unittests/test_jira_import_and_pushing_api.py

@@ -2,12 +2,14 @@
 import logging
 
 from crum import impersonate
+from django.urls import reverse
 from rest_framework.authtoken.models import Token
 from rest_framework.test import APIClient
 from vcr import VCR
 
+import dojo.risk_acceptance.helper as ra_helper
 from dojo.jira_link import helper as jira_helper
-from dojo.models import Finding, Finding_Group, JIRA_Instance, User
+from dojo.models import Finding, Finding_Group, JIRA_Instance, Risk_Acceptance, User
 
 from .dojo_test_case import DojoVCRAPITestCase, get_unit_tests_path, toggle_system_setting_boolean
 
@@ -68,6 +70,7 @@ def setUp(self):
         self.scans_path = "/scans/"
         self.zap_sample5_filename = self.scans_path + "zap/5_zap_sample_one.xml"
         self.npm_groups_sample_filename = self.scans_path + "npm_audit/many_vuln_with_groups.json"
+        self.client.force_login(self.get_test_admin())
 
     def test_import_no_push_to_jira(self):
         import0 = self.import_scan_with_params(self.zap_sample5_filename, verified=True)
@@ -281,6 +284,65 @@ def test_import_twice_push_to_jira(self):
         self.assert_jira_issue_count_in_test(test_id1, 0)
         self.assert_jira_group_issue_count_in_test(test_id, 0)
 
+    def add_risk_acceptance(self, eid, data_risk_accceptance, fid=None):
+        args = (eid, fid) if fid else (eid,)
+        response = self.client.post(reverse("add_risk_acceptance", args=args), data_risk_accceptance)
+        self.assertEqual(302, response.status_code, response.content[:1000])
+        return response
+
+    def test_import_grouped_reopen_expired_sla(self):
+        # steps
+        # import scan, make sure they are in grouped JIRA
+        # risk acceptance all the grouped findings, make sure they are closed in JIRA
+        # expire risk acceptance on all grouped findings, make sure they are open in JIRA
+        import0 = self.import_scan_with_params(self.npm_groups_sample_filename, scan_type="NPM Audit Scan", group_by="component_name+component_version", push_to_jira=True, verified=True)
+        test_id = import0["test"]
+        self.assert_jira_issue_count_in_test(test_id, 0)
+        self.assert_jira_group_issue_count_in_test(test_id, 3)
+        findings = self.get_test_findings_api(test_id)
+        finding_id = findings["results"][0]["id"]
+
+        ra_data = {
+            "name": "Accept: Unit test",
+            "accepted_findings": [],
+            "recommendation": "A",
+            "recommendation_details": "recommendation 1",
+            "decision": "A",
+            "decision_details": "it has been decided!",
+            "accepted_by": "pointy haired boss",
+            "owner": 1,
+            "expiration_date": "2024-12-31",
+            "reactivate_expired": True,
+            }
+
+        for finding in findings["results"]:
+            ra_data["accepted_findings"].append(finding["id"])
+
+        pre_jira_status = self.get_jira_issue_status(finding_id)
+
+        response = self.add_risk_acceptance(1, data_risk_accceptance=ra_data)
+        self.assertEqual("/engagement/1", response.url)
+
+        # We do this to update the JIRA
+        for finding in ra_data["accepted_findings"]:
+            self.patch_finding_api(finding, {"push_to_jira": True})
+
+        post_jira_status = self.get_jira_issue_status(finding_id)
+        self.assertNotEqual(pre_jira_status, post_jira_status)
+
+        pre_jira_status = post_jira_status
+        ra = Risk_Acceptance.objects.last()
+        ra_helper.expire_now(ra)
+        # We do this to update the JIRA
+        for finding in ra_data["accepted_findings"]:
+            self.patch_finding_api(finding, {"push_to_jira": True})
+
+        post_jira_status = self.get_jira_issue_status(finding_id)
+        self.assertNotEqual(pre_jira_status, post_jira_status)
+
+        # by asserting full cassette is played we know all calls to JIRA have been made as expected
+        self.assert_cassette_played()
+
     def test_import_with_groups_twice_push_to_jira(self):
         import0 = self.import_scan_with_params(self.npm_groups_sample_filename, scan_type="NPM Audit Scan", group_by="component_name+component_version", push_to_jira=True, verified=True)
         test_id = import0["test"]