Skip to content

Commit f243b6d

Browse files
rossopsrossops
authored
Merge pull request #13453 from valentijnscholten/top-10-metrics
Fix incorrect (inflated) numbers in top 10 metrics
2 parents b0b04b9 + f9688ad commit f243b6d

3 files changed

Lines changed: 143 additions & 74 deletions

File tree

‎dojo/metrics/utils.py‎

Lines changed: 70 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,6 @@
2727
from dojo.finding.helper import ACCEPTED_FINDINGS_QUERY, CLOSED_FINDINGS_QUERY, OPEN_FINDINGS_QUERY
2828
from dojo.finding.queries import get_authorized_findings
2929
from dojo.models import Endpoint_Status, Finding, Product_Type
30-
from dojo.product.queries import get_authorized_products
3130
from dojo.utils import (
3231
get_system_setting,
3332
queryset_check,
@@ -107,23 +106,49 @@ def finding_queries(
107106
monthly_counts = query_counts_for_period(MetricsPeriod.MONTH, months_between)
108107
weekly_counts = query_counts_for_period(MetricsPeriod.WEEK, weeks_between)
109108

110-
top_ten = get_authorized_products(Permissions.Product_View)
109+
# Build Top 10 from all authorized Findings (not date-limited) to avoid empty lists due to date window
110+
findings_for_top_ten = all_authorized_findings
111+
if len(prod_type) > 0:
112+
findings_for_top_ten = findings_for_top_ten.filter(
113+
test__engagement__product__prod_type__in=prod_type,
114+
)
111115
if get_system_setting("enforce_verified_status", True) or get_system_setting("enforce_verified_status_metrics", True):
112-
top_ten = top_ten.filter(engagement__test__finding__verified=True)
113-
114-
top_ten = top_ten.filter(engagement__test__finding__false_p=False,
115-
engagement__test__finding__duplicate=False,
116-
engagement__test__finding__out_of_scope=False,
117-
engagement__test__finding__mitigated__isnull=True,
118-
engagement__test__finding__severity__in=("Critical", "High", "Medium", "Low"),
119-
prod_type__in=prod_type)
120-
121-
top_ten = severity_count(
122-
top_ten, "annotate", "engagement__test__finding__severity",
123-
).order_by(
116+
findings_for_top_ten = findings_for_top_ten.filter(verified=True)
117+
118+
findings_for_top_ten = findings_for_top_ten.filter(
119+
false_p=False,
120+
duplicate=False,
121+
out_of_scope=False,
122+
mitigated__isnull=True,
123+
active=True,
124+
risk_accepted=False,
125+
severity__in=("Critical", "High", "Medium", "Low"),
126+
)
127+
128+
# Group by product id/name and count findings by severity
129+
top_ten = findings_for_top_ten.values(
130+
product_id=F("test__engagement__product__id"),
131+
product_name=F("test__engagement__product__name"),
132+
)
133+
top_ten = severity_count(top_ten, "annotate", "severity").order_by(
124134
"-critical", "-high", "-medium", "-low",
125135
)[:10]
126136

137+
# Remap keys to match template expectations (id/name)
138+
top_ten = [
139+
{
140+
"id": row.get("product_id"),
141+
"name": row.get("product_name"),
142+
"critical": row.get("critical"),
143+
"high": row.get("high"),
144+
"medium": row.get("medium"),
145+
"low": row.get("low"),
146+
"info": row.get("info"),
147+
"total": row.get("total"),
148+
}
149+
for row in top_ten
150+
]
151+
127152
return {
128153
"all": filtered_findings,
129154
"closed": closed_filtered_findings,
@@ -217,19 +242,39 @@ def endpoint_queries(
217242
monthly_counts = query_counts_for_period(MetricsPeriod.MONTH, months_between)
218243
weekly_counts = query_counts_for_period(MetricsPeriod.WEEK, weeks_between)
219244

220-
top_ten = get_authorized_products(Permissions.Product_View)
221-
top_ten = top_ten.filter(engagement__test__finding__status_finding__mitigated=False,
222-
engagement__test__finding__status_finding__false_positive=False,
223-
engagement__test__finding__status_finding__out_of_scope=False,
224-
engagement__test__finding__status_finding__risk_accepted=False,
225-
engagement__test__finding__severity__in=("Critical", "High", "Medium", "Low"),
226-
prod_type__in=prod_type)
227-
228-
top_ten = severity_count(
229-
top_ten, "annotate", "engagement__test__finding__severity",
230-
).order_by(
245+
# Build Top 10 from Findings related to the open Endpoint_Status queryset
246+
findings_for_top_ten = findings_queryset(endpoints_qs).filter(
247+
false_p=False,
248+
duplicate=False,
249+
out_of_scope=False,
250+
risk_accepted=False,
251+
severity__in=("Critical", "High", "Medium", "Low"),
252+
)
253+
if len(prod_type) > 0:
254+
findings_for_top_ten = findings_for_top_ten.filter(
255+
test__engagement__product__prod_type__in=prod_type,
256+
)
257+
258+
top_ten = findings_for_top_ten.values(
259+
product_id=F("test__engagement__product__id"),
260+
product_name=F("test__engagement__product__name"),
261+
)
262+
top_ten = severity_count(top_ten, "annotate", "severity").order_by(
231263
"-critical", "-high", "-medium", "-low",
232264
)[:10]
265+
top_ten = [
266+
{
267+
"id": row.get("product_id"),
268+
"name": row.get("product_name"),
269+
"critical": row.get("critical"),
270+
"high": row.get("high"),
271+
"medium": row.get("medium"),
272+
"low": row.get("low"),
273+
"info": row.get("info"),
274+
"total": row.get("total"),
275+
}
276+
for row in top_ten
277+
]
233278

234279
return {
235280
"all": endpoints,

‎dojo/metrics/views.py‎

Lines changed: 59 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,7 @@
3333
identify_view,
3434
severity_count,
3535
)
36-
from dojo.models import Dojo_User, Finding, Product, Product_Type, Risk_Acceptance
36+
from dojo.models import Dojo_User, Finding, Product_Type, Risk_Acceptance
3737
from dojo.product.queries import get_authorized_products
3838
from dojo.product_type.queries import get_authorized_product_types
3939
from dojo.utils import (
@@ -355,15 +355,20 @@ def product_type_counts(request):
355355
"reporter").order_by(
356356
"numerical_severity")
357357

358-
top_ten = Product.objects.filter(engagement__test__finding__date__lte=end_date,
359-
engagement__test__finding__verified=True,
360-
engagement__test__finding__false_p=False,
361-
engagement__test__finding__duplicate=False,
362-
engagement__test__finding__out_of_scope=False,
363-
engagement__test__finding__mitigated__isnull=True,
364-
engagement__test__finding__severity__in=(
365-
"Critical", "High", "Medium", "Low"),
366-
prod_type=pt)
358+
# Build Top 10 from Findings for this product type
359+
top_ten = Finding.objects.filter(
360+
date__lte=end_date,
361+
verified=True,
362+
false_p=False,
363+
duplicate=False,
364+
out_of_scope=False,
365+
mitigated__isnull=True,
366+
severity__in=("Critical", "High", "Medium", "Low"),
367+
test__engagement__product__prod_type=pt,
368+
).values(
369+
name=F("test__engagement__product__name"),
370+
)
371+
top_ten = severity_count(top_ten, "annotate", "severity").order_by("-critical", "-high", "-medium", "-low")[:10]
367372
else:
368373
overall_in_pt = Finding.objects.filter(date__lt=end_date,
369374
false_p=False,
@@ -400,16 +405,20 @@ def product_type_counts(request):
400405
"reporter").order_by(
401406
"numerical_severity")
402407

403-
top_ten = Product.objects.filter(engagement__test__finding__date__lte=end_date,
404-
engagement__test__finding__false_p=False,
405-
engagement__test__finding__duplicate=False,
406-
engagement__test__finding__out_of_scope=False,
407-
engagement__test__finding__mitigated__isnull=True,
408-
engagement__test__finding__severity__in=(
409-
"Critical", "High", "Medium", "Low"),
410-
prod_type=pt)
411-
412-
top_ten = severity_count(top_ten, "annotate", "engagement__test__finding__severity").order_by("-critical", "-high", "-medium", "-low")[:10]
408+
top_ten = Finding.objects.filter(
409+
date__lte=end_date,
410+
false_p=False,
411+
duplicate=False,
412+
out_of_scope=False,
413+
mitigated__isnull=True,
414+
severity__in=("Critical", "High", "Medium", "Low"),
415+
test__engagement__product__prod_type=pt,
416+
).values(
417+
name=F("test__engagement__product__name"),
418+
)
419+
top_ten = severity_count(top_ten, "annotate", "severity").order_by("-critical", "-high", "-medium", "-low")[:10]
420+
421+
# top_ten already annotated above using Findings-based grouping
413422

414423
cip = {"S0": 0,
415424
"S1": 0,
@@ -557,15 +566,21 @@ def product_tag_counts(request):
557566
"reporter").order_by(
558567
"numerical_severity")
559568

560-
top_ten = Product.objects.filter(engagement__test__finding__date__lte=end_date,
561-
engagement__test__finding__verified=True,
562-
engagement__test__finding__false_p=False,
563-
engagement__test__finding__duplicate=False,
564-
engagement__test__finding__out_of_scope=False,
565-
engagement__test__finding__mitigated__isnull=True,
566-
engagement__test__finding__severity__in=(
567-
"Critical", "High", "Medium", "Low"),
568-
tags__name=pt, engagement__product__in=prods)
569+
# Build Top 10 from Findings for this product tag
570+
top_ten = Finding.objects.filter(
571+
date__lte=end_date,
572+
verified=True,
573+
false_p=False,
574+
duplicate=False,
575+
out_of_scope=False,
576+
mitigated__isnull=True,
577+
severity__in=("Critical", "High", "Medium", "Low"),
578+
test__engagement__product__tags__name=pt,
579+
test__engagement__product__in=prods,
580+
).values(
581+
name=F("test__engagement__product__name"),
582+
)
583+
top_ten = severity_count(top_ten, "annotate", "severity").order_by("-critical", "-high", "-medium", "-low")[:10]
569584
else:
570585
overall_in_pt = Finding.objects.filter(date__lt=end_date,
571586
false_p=False,
@@ -605,16 +620,21 @@ def product_tag_counts(request):
605620
"reporter").order_by(
606621
"numerical_severity")
607622

608-
top_ten = Product.objects.filter(engagement__test__finding__date__lte=end_date,
609-
engagement__test__finding__false_p=False,
610-
engagement__test__finding__duplicate=False,
611-
engagement__test__finding__out_of_scope=False,
612-
engagement__test__finding__mitigated__isnull=True,
613-
engagement__test__finding__severity__in=(
614-
"Critical", "High", "Medium", "Low"),
615-
tags__name=pt, engagement__product__in=prods)
616-
617-
top_ten = severity_count(top_ten, "annotate", "engagement__test__finding__severity").order_by("-critical", "-high", "-medium", "-low")[:10]
623+
top_ten = Finding.objects.filter(
624+
date__lte=end_date,
625+
false_p=False,
626+
duplicate=False,
627+
out_of_scope=False,
628+
mitigated__isnull=True,
629+
severity__in=("Critical", "High", "Medium", "Low"),
630+
test__engagement__product__tags__name=pt,
631+
test__engagement__product__in=prods,
632+
).values(
633+
name=F("test__engagement__product__name"),
634+
)
635+
top_ten = severity_count(top_ten, "annotate", "severity").order_by("-critical", "-high", "-medium", "-low")[:10]
636+
637+
# top_ten already annotated above using Findings-based grouping
618638

619639
cip = {"S0": 0,
620640
"S1": 0,

‎unittests/test_metrics_queries.py‎

Lines changed: 14 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -80,7 +80,7 @@ def test_finding_queries(self, mock_timezone):
8080
mock_timezone.return_value = mock_datetime
8181

8282
# Queries over Finding
83-
with self.assertNumQueries(28):
83+
with self.assertNumQueries(29):
8484
product_types = []
8585
finding_queries = utils.finding_queries(
8686
product_types,
@@ -113,10 +113,12 @@ def test_finding_queries(self, mock_timezone):
113113
finding_queries["accepted_count"],
114114
{"total": 3, "critical": 0, "high": 3, "medium": 0, "low": 0, "info": 0},
115115
)
116-
self.assertSequenceEqual(
117-
finding_queries["top_ten"].values(),
118-
[],
119-
)
116+
self.assertIsInstance(finding_queries["top_ten"], list)
117+
for row in finding_queries["top_ten"]:
118+
self.assertSetEqual(
119+
set(row.keys()),
120+
{"id", "name", "critical", "high", "medium", "low", "info", "total"},
121+
)
120122
self.assertEqual(
121123
finding_queries["monthly_counts"],
122124
{
@@ -192,7 +194,7 @@ def test_endpoint_queries(self, mock_now):
192194
mock_now.return_value = fake_now
193195

194196
# Queries over Finding and Endpoint_Status
195-
with self.assertNumQueries(44):
197+
with self.assertNumQueries(45):
196198
product_types = Product_Type.objects.all()
197199
endpoint_queries = utils.endpoint_queries(
198200
product_types,
@@ -245,10 +247,12 @@ def test_endpoint_queries(self, mock_now):
245247
list(endpoint_queries["accepted_count"].values()),
246248
[1, 0, 0, 0, 0, 1],
247249
)
248-
self.assertSequenceEqual(
249-
endpoint_queries["top_ten"].values(),
250-
[],
251-
)
250+
self.assertIsInstance(endpoint_queries["top_ten"], list)
251+
for row in endpoint_queries["top_ten"]:
252+
self.assertSetEqual(
253+
set(row.keys()),
254+
{"id", "name", "critical", "high", "medium", "low", "info", "total"},
255+
)
252256
self.assertEqual(
253257
list(endpoint_queries["monthly_counts"].values()),
254258
[

0 commit comments

Comments
 (0)