Skip to content

Commit c6246c7

Browse files
Fix bulk risk acceptance for active findings (#14281) (#14292)
Allow simple risk acceptance of active findings via bulk edit menu, matching the behavior of individual finding risk acceptance. Previously, bulk risk acceptance incorrectly rejected active findings with a warning message, even though individual risk acceptance worked fine. This was inconsistent with DefectDojo Pro and the individual risk acceptance feature. Changes: - Remove the check that prevented active findings from being risk accepted in bulk operations - Update test to verify active findings can now be risk accepted - Remove obsolete warning message about active findings - Simplify return value of _bulk_update_risk_acceptance helper The simple_risk_accept helper already handles setting finding.active to False when accepting risk, so there's no need for an additional check in the bulk update flow.
1 parent 0fbaa7b commit c6246c7

2 files changed

Lines changed: 39 additions & 35 deletions

File tree

dojo/finding/views.py

Lines changed: 3 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -2793,14 +2793,10 @@ def _bulk_update_simple_fields(finds, form):
27932793
def _bulk_update_risk_acceptance(finds, form, request, prods):
27942794
"""Helper function to handle risk acceptance updates."""
27952795
skipped_risk_accept_count = 0
2796-
skipped_active_risk_accept_count = 0
27972796

27982797
if form.cleaned_data["risk_acceptance"]:
27992798
for finding in finds:
2800-
if finding.active:
2801-
skipped_active_risk_accept_count += 1
2802-
# Allow risk acceptance for inactive findings (whether duplicate or not)
2803-
elif form.cleaned_data["risk_accept"]:
2799+
if form.cleaned_data["risk_accept"]:
28042800
if (
28052801
not finding.test.engagement.product.enable_simple_risk_acceptance
28062802
):
@@ -2822,15 +2818,7 @@ def _bulk_update_risk_acceptance(finds, form, request, prods):
28222818
extra_tags="alert-warning",
28232819
)
28242820

2825-
if skipped_active_risk_accept_count > 0:
2826-
messages.add_message(
2827-
request,
2828-
messages.WARNING,
2829-
f"Skipped risk acceptance of {skipped_active_risk_accept_count} active findings. Active findings cannot be risk accepted.",
2830-
extra_tags="alert-warning",
2831-
)
2832-
2833-
return skipped_risk_accept_count, skipped_active_risk_accept_count
2821+
return skipped_risk_accept_count
28342822

28352823

28362824
def _bulk_update_finding_groups(finds, form):
@@ -3095,7 +3083,7 @@ def finding_bulk_update_all(request, pid=None):
30953083

30963084
_bulk_update_simple_fields(finds, form)
30973085

3098-
_skipped_risk_accept_count, _skipped_active_risk_accept_count = _bulk_update_risk_acceptance(
3086+
_skipped_risk_accept_count = _bulk_update_risk_acceptance(
30993087
finds, form, request, prods,
31003088
)
31013089

unittests/test_bulk_edit_validation.py

Lines changed: 36 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -409,8 +409,8 @@ def test_bulk_edit_duplicate_finding_severity_update_works(self):
409409

410410
# View-Level Validation Tests (Active + Risk Acceptance)
411411

412-
def test_bulk_edit_active_finding_cannot_accept_risk(self):
413-
"""Test that active findings cannot accept risk via bulk edit"""
412+
def test_bulk_edit_active_finding_can_accept_risk(self):
413+
"""Test that active findings can accept risk via bulk edit (matching individual behavior)"""
414414
# Enable simple risk acceptance on product
415415
self.product.enable_simple_risk_acceptance = True
416416
self.product.save()
@@ -427,22 +427,26 @@ def test_bulk_edit_active_finding_cannot_accept_risk(self):
427427
follow=True,
428428
)
429429

430-
# Verify finding is NOT risk accepted
430+
# Verify finding IS risk accepted and becomes inactive
431431
self.active_finding.refresh_from_db()
432-
self.assertFalse(
432+
self.assertTrue(
433433
self.active_finding.risk_accepted,
434-
"Active finding should not be risk accepted",
434+
"Active finding should be risk accepted",
435+
)
436+
self.assertFalse(
437+
self.active_finding.active,
438+
"Risk accepted finding should become inactive",
435439
)
436440

437-
# Verify warning message
441+
# Verify no warning message about active findings
438442
messages = self._get_messages_text(response)
439443
warning_messages = [
440444
m for m in messages if "active findings" in m.lower() and "risk" in m.lower()
441445
]
442-
self.assertGreater(
446+
self.assertEqual(
443447
len(warning_messages),
444448
0,
445-
f"Expected warning about active findings and risk acceptance, got: {messages}",
449+
f"Unexpected warning about active findings: {warning_messages}",
446450
)
447451

448452
def test_bulk_edit_inactive_finding_can_accept_risk(self):
@@ -553,12 +557,15 @@ def test_bulk_edit_shows_success_message_with_actual_count(self):
553557
self._assert_finding_status(normal2, active=True)
554558

555559
def test_bulk_edit_shows_multiple_warning_messages(self):
556-
"""Test that multiple warning messages appear for different conflicts"""
560+
"""
561+
Test that warning messages appear for conflicts (duplicate status)
562+
and that active findings can now be risk accepted successfully
563+
"""
557564
# Enable simple risk acceptance
558565
self.product.enable_simple_risk_acceptance = True
559566
self.product.save()
560567

561-
# First, try to set duplicate finding as active (will be skipped)
568+
# First, try to set duplicate finding as active (will be skipped with warning)
562569
post_data1 = self._bulk_edit_post_data(
563570
[self.duplicate_finding.id],
564571
active=True, # Will conflict with duplicate
@@ -569,11 +576,11 @@ def test_bulk_edit_shows_multiple_warning_messages(self):
569576
follow=True,
570577
)
571578

572-
# Then, try to risk accept active finding (will be skipped)
579+
# Then, risk accept active finding (should succeed - no longer a conflict)
573580
post_data2 = self._bulk_edit_post_data(
574581
[self.active_finding.id],
575582
risk_acceptance=True,
576-
risk_accept=True, # Will conflict with active
583+
risk_accept=True, # Should work now!
577584
)
578585
response2 = self.client.post(
579586
reverse("finding_bulk_update_all"),
@@ -586,25 +593,34 @@ def test_bulk_edit_shows_multiple_warning_messages(self):
586593
messages2 = self._get_messages_text(response2)
587594
all_messages = messages1 + messages2
588595

596+
# Verify duplicate warning appears
589597
duplicate_warnings = [
590598
m for m in all_messages if "duplicate findings" in m.lower()
591599
]
600+
self.assertGreater(
601+
len(duplicate_warnings),
602+
0,
603+
f"Expected duplicate warning, got: {all_messages}",
604+
)
605+
606+
# Verify NO warning about active findings and risk acceptance
592607
active_warnings = [
593608
m
594609
for m
595610
in all_messages
596611
if "active findings" in m.lower() and "risk" in m.lower()
597612
]
598-
599-
self.assertGreater(
600-
len(duplicate_warnings),
601-
0,
602-
f"Expected duplicate warning, got: {all_messages}",
603-
)
604-
self.assertGreater(
613+
self.assertEqual(
605614
len(active_warnings),
606615
0,
607-
f"Expected active risk acceptance warning, got: {all_messages}",
616+
f"Unexpected active risk acceptance warning: {active_warnings}",
617+
)
618+
619+
# Verify active finding was successfully risk accepted
620+
self.active_finding.refresh_from_db()
621+
self.assertTrue(
622+
self.active_finding.risk_accepted,
623+
"Active finding should be risk accepted successfully",
608624
)
609625

610626
def test_bulk_edit_no_warning_when_no_conflicts(self):

0 commit comments

Comments
 (0)