closed finding metrics: use mitigated_date instead of created_date

Author: valentijnscholten

dojo/fixtures/unit_metrics_additional_data.json

@@ -175,7 +175,7 @@
       "description": "TEST finding",
       "mitigated_by": null,
       "reporter": 2,
-      "mitigated": null,
+      "mitigated": "2018-01-02T00:00:00Z",
       "active": false,
       "line": 100,
       "under_review": false,
@@ -416,7 +416,7 @@
       "description": "test finding",
       "mitigated_by": null,
       "reporter": 1,
-      "mitigated": null,
+      "mitigated": "2017-12-28T00:00:00Z",
       "active": true,
       "line": 123,
       "under_review": false,

dojo/metrics/utils.py

@@ -76,8 +76,13 @@ def finding_queries(
 
     # Filter by the date ranges supplied
     all_findings_within_date_range = all_authorized_findings.filter(date__range=[start_date, end_date])
-    # Get the list of closed and risk accepted findings
-    closed_filtered_findings = all_findings_within_date_range.filter(CLOSED_FINDINGS_QUERY)
+    # Get the list of closed findings filtered by mitigated date (not discovery date)
+    # This ensures findings closed within the date range are included even if discovered outside it
+    closed_filtered_findings = all_authorized_findings.filter(
+        CLOSED_FINDINGS_QUERY,
+        mitigated__range=[start_date, end_date],
+        mitigated__isnull=False,
+    )
     accepted_filtered_findings = all_findings_within_date_range.filter(ACCEPTED_FINDINGS_QUERY)
     active_filtered_findings = all_findings_within_date_range.filter(OPEN_FINDINGS_QUERY)
 

dojo/product/views.py

@@ -438,7 +438,8 @@ def finding_queries(request, prod):
     filters["new_verified"] = findings_qs.filter(finding_helper.VERIFIED_FINDINGS_QUERY).filter(date__range=[start_date, end_date]).order_by("date")
     filters["open"] = findings_qs.filter(finding_helper.OPEN_FINDINGS_QUERY).filter(date__range=[start_date, end_date]).order_by("date")
     filters["inactive"] = findings_qs.filter(finding_helper.INACTIVE_FINDINGS_QUERY).filter(date__range=[start_date, end_date]).order_by("date")
-    filters["closed"] = findings_qs.filter(finding_helper.CLOSED_FINDINGS_QUERY).filter(date__range=[start_date, end_date]).order_by("date")
+    # Filter closed findings by mitigated date (not discovery date) to show findings closed within the date range
+    filters["closed"] = findings_qs.filter(finding_helper.CLOSED_FINDINGS_QUERY).filter(mitigated__range=[start_date, end_date], mitigated__isnull=False).order_by("mitigated")
     filters["false_positive"] = findings_qs.filter(finding_helper.FALSE_POSITIVE_FINDINGS_QUERY).filter(date__range=[start_date, end_date]).order_by("date")
     filters["out_of_scope"] = findings_qs.filter(finding_helper.OUT_OF_SCOPE_FINDINGS_QUERY).filter(date__range=[start_date, end_date]).order_by("date")
     filters["all"] = findings_qs.order_by("date")
@@ -610,7 +611,8 @@ def view_product_metrics(request, pid):
 
     all_findings = list(filters.get("all", []).values("id", "date", "severity"))
     open_findings = list(filters.get("open", []).values("id", "date", "mitigated", "severity"))
-    closed_findings = list(filters.get("closed", []).values("id", "date", "severity"))
+    # Include mitigated date for closed findings to group by when they were closed, not discovered
+    closed_findings = list(filters.get("closed", []).values("id", "date", "mitigated", "severity"))
     accepted_findings = list(filters.get("accepted", []).values("id", "date", "severity"))
 
     """
@@ -681,11 +683,29 @@ def view_product_metrics(request, pid):
             if open_objs_by_severity.get(finding.get("severity")) is not None:
                 open_objs_by_severity[finding.get("severity")] += 1
 
-        # Close findings
+        # Close findings - group by mitigated date, not discovery date
         elif closed_findings_dict.get(finding.get("id", None)):
-            if unix_timestamp in open_close_weekly:
+            # Find the closed finding to get its mitigated date
+            closed_finding = next((f for f in closed_findings if f.get("id") == finding.get("id")), None)
+            if closed_finding and closed_finding.get("mitigated"):
+                # Use mitigated date for grouping closed findings
+                mitigated_date = closed_finding.get("mitigated")
+                mitigated_date_only = mitigated_date.date() if isinstance(mitigated_date, datetime) else mitigated_date
+                iso_cal = mitigated_date_only.isocalendar()
+                mitigated_week_start = iso_to_gregorian(iso_cal[0], iso_cal[1], 1)
+                mitigated_html_date = mitigated_week_start.strftime("<span class='small'>%m/%d<br/>%Y</span>")
+                mitigated_unix_timestamp = (tcalendar.timegm(mitigated_week_start.timetuple()) * 1000)
+
+                if mitigated_unix_timestamp in open_close_weekly:
+                    open_close_weekly[mitigated_unix_timestamp]["closed"] += 1
+                else:
+                    open_close_weekly[mitigated_unix_timestamp] = {"closed": 1, "open": 0, "accepted": 0}
+                    open_close_weekly[mitigated_unix_timestamp]["week"] = mitigated_html_date
+            elif unix_timestamp in open_close_weekly:
+                # Fallback to discovery date if mitigated date is not available
                 open_close_weekly[unix_timestamp]["closed"] += 1
             else:
+                # Fallback to discovery date if mitigated date is not available
                 open_close_weekly[unix_timestamp] = {"closed": 1, "open": 0, "accepted": 0}
                 open_close_weekly[unix_timestamp]["week"] = html_date
             # Optimization: count severity level on server side

unittests/test_metrics_queries.py

@@ -8,7 +8,7 @@
 from django.urls import reverse
 
 from dojo.metrics import utils
-from dojo.models import Product_Type, User
+from dojo.models import Engagement, Finding, Product, Product_Type, Test, User
 
 from .dojo_test_case import DojoTestCase
 
@@ -31,12 +31,12 @@ def add(*args, **kwargs):
 FINDING_8 = {"id": 240, "date": date(2018, 1, 1), "severity": "High", "active": True, "verified": False, "false_p": False, "duplicate": True, "duplicate_finding_id": 2, "out_of_scope": False, "risk_accepted": False, "under_review": False, "is_mitigated": False, "mitigated": None, "mitigated_by_id": None, "reporter_id": 1, "numerical_severity": "S0"}
 FINDING_9 = {"id": 241, "date": date(2018, 1, 1), "severity": "High", "active": False, "verified": False, "false_p": False, "duplicate": True, "duplicate_finding_id": 2, "out_of_scope": False, "risk_accepted": True, "under_review": False, "is_mitigated": False, "mitigated": None, "mitigated_by_id": None, "reporter_id": 1, "numerical_severity": "S0"}
 FINDING_10 = {"id": 242, "date": date(2018, 1, 1), "severity": "High", "active": False, "verified": False, "false_p": False, "duplicate": True, "duplicate_finding_id": 2, "out_of_scope": False, "risk_accepted": True, "under_review": False, "is_mitigated": False, "mitigated": None, "mitigated_by_id": None, "reporter_id": 1, "numerical_severity": "S0"}
-FINDING_11 = {"id": 243, "date": date(2017, 12, 31), "severity": "High", "active": False, "verified": False, "false_p": False, "duplicate": False, "duplicate_finding_id": None, "out_of_scope": False, "risk_accepted": True, "under_review": False, "is_mitigated": True, "mitigated": None, "mitigated_by_id": None, "reporter_id": 2, "numerical_severity": "S0"}
+FINDING_11 = {"id": 243, "date": date(2017, 12, 31), "severity": "High", "active": False, "verified": False, "false_p": False, "duplicate": False, "duplicate_finding_id": None, "out_of_scope": False, "risk_accepted": True, "under_review": False, "is_mitigated": True, "mitigated": datetime(2018, 1, 2, tzinfo=zoneinfo.ZoneInfo("UTC")), "mitigated_by_id": None, "reporter_id": 2, "numerical_severity": "S0"}
 FINDING_12 = {"id": 244, "date": date(2017, 12, 29), "severity": "Low", "active": True, "verified": True, "false_p": False, "duplicate": False, "duplicate_finding_id": None, "out_of_scope": False, "risk_accepted": False, "under_review": False, "is_mitigated": False, "mitigated": None, "mitigated_by_id": None, "reporter_id": 1, "numerical_severity": "S0"}
 FINDING_13 = {"id": 245, "date": date(2017, 12, 27), "severity": "Low", "active": False, "verified": False, "false_p": False, "duplicate": True, "duplicate_finding_id": 22, "out_of_scope": False, "risk_accepted": False, "under_review": False, "is_mitigated": False, "mitigated": None, "mitigated_by_id": None, "reporter_id": 1, "numerical_severity": "S0"}
 FINDING_14 = {"id": 246, "date": date(2018, 1, 2), "severity": "Low", "active": False, "verified": False, "false_p": False, "duplicate": True, "duplicate_finding_id": 22, "out_of_scope": False, "risk_accepted": False, "under_review": False, "is_mitigated": False, "mitigated": None, "mitigated_by_id": None, "reporter_id": 1, "numerical_severity": "S0"}
 FINDING_15 = {"id": 247, "date": date(2018, 1, 3), "severity": "Low", "active": False, "verified": False, "false_p": False, "duplicate": True, "duplicate_finding_id": None, "out_of_scope": False, "risk_accepted": False, "under_review": False, "is_mitigated": False, "mitigated": None, "mitigated_by_id": None, "reporter_id": 1, "numerical_severity": "S0"}
-FINDING_16 = {"id": 248, "date": date(2017, 12, 27), "severity": "Low", "active": True, "verified": True, "false_p": False, "duplicate": False, "duplicate_finding_id": None, "out_of_scope": False, "risk_accepted": False, "under_review": False, "is_mitigated": True, "mitigated": None, "mitigated_by_id": None, "reporter_id": 1, "numerical_severity": "S0"}
+FINDING_16 = {"id": 248, "date": date(2017, 12, 27), "severity": "Low", "active": True, "verified": True, "false_p": False, "duplicate": False, "duplicate_finding_id": None, "out_of_scope": False, "risk_accepted": False, "under_review": False, "is_mitigated": True, "mitigated": datetime(2017, 12, 28, tzinfo=zoneinfo.ZoneInfo("UTC")), "mitigated_by_id": None, "reporter_id": 1, "numerical_severity": "S0"}
 FINDING_17 = {"id": 249, "date": date(2018, 1, 4), "severity": "Low", "active": False, "verified": False, "false_p": False, "duplicate": True, "duplicate_finding_id": 224, "out_of_scope": False, "risk_accepted": False, "under_review": False, "is_mitigated": False, "mitigated": None, "mitigated_by_id": None, "reporter_id": 1, "numerical_severity": "S0"}
 
 
@@ -163,6 +163,96 @@ def test_finding_queries(self, mock_timezone):
             self.assertIsInstance(finding_queries["start_date"], datetime)
             self.assertIsInstance(finding_queries["end_date"], datetime)
 
+    @patch("django.utils.timezone.now")
+    def test_closed_findings_filtered_by_mitigated_date(self, mock_timezone):
+        """
+        Test that closed findings are filtered by mitigated date, not discovery date.
+
+        This test verifies the fix for issue #9735: findings discovered outside the date
+        range but closed within it should appear in closed metrics.
+        """
+        mock_datetime = datetime(2020, 12, 9, tzinfo=zoneinfo.ZoneInfo("UTC"))
+        mock_timezone.return_value = mock_datetime
+
+        # Get a test product and engagement
+        product = Product.objects.first()
+        if not product:
+            self.skipTest("No product available in test data")
+        engagement = Engagement.objects.filter(product=product).first()
+        if not engagement:
+            self.skipTest("No engagement available in test data")
+        test = Test.objects.filter(engagement=engagement).first()
+        if not test:
+            self.skipTest("No test available in test data")
+
+        # Create a finding discovered BEFORE the date range but closed WITHIN it
+        # Date range: 2017-12-26 to 2018-01-05
+        finding_discovered_before = Finding.objects.create(
+            title="Finding discovered before range, closed within range",
+            description="Test finding",
+            severity="High",
+            date=date(2017, 10, 1),  # Discovered before range
+            test=test,
+            reporter=self.request.user,
+            active=False,
+            is_mitigated=True,
+            mitigated=datetime(2018, 1, 2, tzinfo=zoneinfo.ZoneInfo("UTC")),  # Closed within range
+        )
+
+        # Create a finding discovered WITHIN the date range but closed AFTER it
+        finding_closed_after = Finding.objects.create(
+            title="Finding discovered within range, closed after range",
+            description="Test finding",
+            severity="Medium",
+            date=date(2017, 12, 30),  # Discovered within range
+            test=test,
+            reporter=self.request.user,
+            active=False,
+            is_mitigated=True,
+            mitigated=datetime(2018, 2, 1, tzinfo=zoneinfo.ZoneInfo("UTC")),  # Closed after range
+        )
+
+        # Create a finding discovered and closed WITHIN the date range
+        finding_both_within = Finding.objects.create(
+            title="Finding discovered and closed within range",
+            description="Test finding",
+            severity="Low",
+            date=date(2017, 12, 30),  # Discovered within range
+            test=test,
+            reporter=self.request.user,
+            active=False,
+            is_mitigated=True,
+            mitigated=datetime(2018, 1, 3, tzinfo=zoneinfo.ZoneInfo("UTC")),  # Closed within range
+        )
+
+        try:
+            product_types = []
+            finding_queries = utils.finding_queries(
+                product_types,
+                self.request,
+            )
+
+            closed_findings = finding_queries["closed"]
+            closed_ids = list(closed_findings.values_list("id", flat=True))
+
+            # The finding discovered before but closed within should appear
+            self.assertIn(finding_discovered_before.id, closed_ids,
+                         "Finding discovered before range but closed within range should appear in closed metrics")
+
+            # The finding discovered within but closed after should NOT appear
+            self.assertNotIn(finding_closed_after.id, closed_ids,
+                            "Finding discovered within range but closed after range should NOT appear in closed metrics")
+
+            # The finding discovered and closed within should appear
+            self.assertIn(finding_both_within.id, closed_ids,
+                         "Finding discovered and closed within range should appear in closed metrics")
+
+        finally:
+            # Clean up test findings
+            finding_discovered_before.delete()
+            finding_closed_after.delete()
+            finding_both_within.delete()
+
 
 class EndpointQueriesTest(DojoTestCase):
     fixtures = ["dojo_testdata.json"]