From e9d45b3148b8f8dc51378492374113ef8de044a3 Mon Sep 17 00:00:00 2001 From: Samiat Date: Mon, 22 Jun 2026 09:47:34 +0100 Subject: [PATCH 1/3] fix(jira): close deleted findings --- dojo/finding/helper.py | 37 ++++++- dojo/jira/helper.py | 79 ++++++++++++++ dojo/jira/services.py | 27 +++++ unittests/test_jira_helper.py | 195 +++++++++++++++++++++++++++++++++- 4 files changed, 334 insertions(+), 4 deletions(-) diff --git a/dojo/finding/helper.py b/dojo/finding/helper.py index d83d032b176..cd74f9526ea 100644 --- a/dojo/finding/helper.py +++ b/dojo/finding/helper.py @@ -532,6 +532,12 @@ def post_process_findings_batch( @receiver(pre_delete, sender=Finding) def finding_pre_delete(sender, instance, **kwargs): logger.debug("finding pre_delete: %d", instance.id) + if ( + instance.has_jira_issue + and not getattr(instance, "_skip_jira_close_on_delete", False) + and jira_services.is_delete_sync_allowed(instance) + ): + jira_services.close_issue_for_deleted_finding(instance) # this shouldn't be necessary as Django should remove any Many-To-Many entries automatically, might be a bug in Django? # https://code.djangoproject.com/ticket/154 instance.found_by.clear() @@ -562,7 +568,8 @@ def finding_delete(instance, **kwargs): if settings.DUPLICATE_CLUSTER_CASCADE_DELETE: duplicate_cluster.order_by("-id").delete() else: - reconfigure_duplicate_cluster(instance, duplicate_cluster) + new_original = reconfigure_duplicate_cluster(instance, duplicate_cluster) + _reassign_jira_issue_to_new_original(instance, new_original) else: logger.debug("no duplicate cluster found for finding: %d, so no need to reconfigure", instance.id) @@ -579,6 +586,28 @@ def finding_post_delete(sender, instance, **kwargs): logger.debug("finding post_delete, sender: %s instance: %s", to_str_typed(sender), to_str_typed(instance)) +def _reassign_jira_issue_to_new_original(deleted_finding, new_original): + if not new_original or new_original.has_jira_issue or not jira_services.is_delete_sync_allowed(deleted_finding): + return False + + jira_issue = jira_services.get_issue(deleted_finding) + if not jira_issue: + return False + + jira_instance = jira_services.get_instance(deleted_finding) + jira_services.add_simple_comment( + jira_instance, + jira_issue, + ( + f"DefectDojo finding {deleted_finding.id} was deleted. " + f"This Jira issue was reassigned to finding {new_original.id}." + ), + ) + jira_services.reassign_issue_to_finding(jira_issue, new_original) + deleted_finding._skip_jira_close_on_delete = True + return True + + # can't use model to id here due to the queryset # @dojo_async_task # @app.task @@ -586,12 +615,12 @@ def reconfigure_duplicate_cluster(original, cluster_outside): # when a finding is deleted, and is an original of a duplicate cluster, we have to chose a new original for the cluster # only look for a new original if there is one outside this test if original is None or cluster_outside is None or len(cluster_outside) == 0: - return + return None if settings.DUPLICATE_CLUSTER_CASCADE_DELETE: # Don't delete here — the caller (async_delete_crawl_task or finding_delete) # handles deletion of outside-scope duplicates efficiently via bulk_delete_findings. - return + return None logger.debug("reconfigure_duplicate_cluster: cluster_outside: %s", cluster_outside) # set new original to first finding in cluster (ordered by id) new_original = cluster_outside.order_by("id").first() @@ -610,6 +639,8 @@ def reconfigure_duplicate_cluster(original, cluster_outside): # Re-point remaining duplicates to the new original in a single query cluster_outside.exclude(id=new_original.id).update(duplicate_finding=new_original) + return new_original + return None def prepare_duplicates_for_delete(obj, *, preview_only=False): diff --git a/dojo/jira/helper.py b/dojo/jira/helper.py index 9ce434801ea..93c344cf2f9 100644 --- a/dojo/jira/helper.py +++ b/dojo/jira/helper.py @@ -159,6 +159,10 @@ def is_keep_in_sync_with_jira(obj: Finding | Finding_Group, prefetched_jira_inst return False +def is_delete_sync_allowed(finding): + return bool(is_keep_in_sync_with_jira(finding) or is_push_all_issues(finding)) + + # checks if a finding can be pushed to JIRA # optionally provides a form with the new data for the finding # any finding that already has a JIRA issue can be pushed again to JIRA @@ -1273,6 +1277,81 @@ def push_status_to_jira(obj, jira_instance, jira, issue, *, save=False): return updated +def close_jira_issue_for_deleted_finding(finding) -> tuple[bool | None, str]: + logger.debug("closing linked Jira issue before deleting finding %d", finding.id) + + if not is_jira_enabled(): + return False, "JIRA integration is not enabled." + + if not finding.has_jira_issue: + return False, f"Finding {finding.id} has no linked JIRA issue." + + if not is_delete_sync_allowed(finding): + return False, f"Finding {finding.id} is not configured to sync deleted findings to JIRA." + + if not is_jira_configured_and_enabled(finding): + message = ( + f"Finding {finding.id} cannot close its linked JIRA issue " + "because JIRA is not configured or enabled." + ) + logger.debug(message) + return False, message + + jira_instance = get_jira_instance(finding) + if not jira_instance: + message = ( + f"Finding {finding.id} cannot close its linked JIRA issue " + "because the JIRA instance is not available." + ) + logger.warning(message) + return False, message + + jira_issue = get_jira_issue(finding) + if not jira_issue: + return False, f"Finding {finding.id} has no local JIRA issue record." + + try: + JIRAError.log_to_tempfile = False + jira = get_jira_connection(jira_instance) + if not jira: + message = ( + f"Finding {finding.id} cannot close its linked JIRA issue " + "because the JIRA connection could not be established." + ) + logger.warning(message) + return False, message + issue = jira.issue(jira_issue.jira_id) + except Exception as e: + message = f"The following jira instance could not be connected: {jira_instance} - {e}" + logger.exception(message) + log_jira_alert(message, finding) + return False, message + + if not issue_from_jira_is_active(issue): + logger.debug("Jira issue %s is already resolved", jira_issue.jira_key) + return False, f"Jira issue {jira_issue.jira_key} is already resolved." + + updated = jira_transition(jira, issue, jira_instance.close_status_key) + if updated: + add_simple_jira_comment( + jira_instance, + jira_issue, + f"DefectDojo finding {finding.id} was deleted. This Jira issue was closed automatically.", + ) + jira_issue.jira_change = timezone.now() + jira_issue.save(update_fields=["jira_change"]) + return True, f"Jira issue {jira_issue.jira_key} closed successfully." + + return updated, f"Jira issue {jira_issue.jira_key} was not closed." + + +def reassign_jira_issue_to_finding(jira_issue, finding): + jira_issue.finding = finding + jira_issue.finding_group = None + jira_issue.engagement = None + jira_issue.save(update_fields=["finding", "finding_group", "engagement"]) + + # gets the metadata for the provided issue type in the provided jira project def get_issuetype_fields( jira, diff --git a/dojo/jira/services.py b/dojo/jira/services.py index 54dec073be1..a2899666b90 100644 --- a/dojo/jira/services.py +++ b/dojo/jira/services.py @@ -137,6 +137,24 @@ def push_status(obj, jira_instance, jira, issue, *, save=False): return _get_helper().push_status_to_jira(obj, jira_instance, jira, issue, save=save) +def close_issue_for_deleted_finding(finding): + """ + Close the linked Jira issue before a finding is deleted. + + Wraps: jira_helper.close_jira_issue_for_deleted_finding + """ + return _get_helper().close_jira_issue_for_deleted_finding(finding) + + +def reassign_issue_to_finding(jira_issue, finding): + """ + Reassign a local Jira issue record to another finding. + + Wraps: jira_helper.reassign_jira_issue_to_finding + """ + return _get_helper().reassign_jira_issue_to_finding(jira_issue, finding) + + def update_issue(obj, *args, **kwargs): """ Update a Jira issue. @@ -339,6 +357,15 @@ def is_keep_in_sync(obj, prefetched_jira_instance=None): return _get_helper().is_keep_in_sync_with_jira(obj, prefetched_jira_instance=prefetched_jira_instance) +def is_delete_sync_allowed(finding): + """ + Check if deleting a finding should update its linked Jira issue. + + Wraps: jira_helper.is_delete_sync_allowed + """ + return _get_helper().is_delete_sync_allowed(finding) + + def is_push(instance, push_to_jira_parameter=None): """ Check if Jira push should happen. diff --git a/unittests/test_jira_helper.py b/unittests/test_jira_helper.py index ba2019f0765..76f955dfb50 100644 --- a/unittests/test_jira_helper.py +++ b/unittests/test_jira_helper.py @@ -1,7 +1,8 @@ import logging from unittest import TestCase -from unittest.mock import Mock +from unittest.mock import Mock, patch +import dojo.finding.helper as finding_helper import dojo.jira.helper as jira_helper logger = logging.getLogger(__name__) @@ -30,6 +31,198 @@ def test_issue_from_jira_is_active_defaults_to_active_on_missing_attribute(self) """AttributeError anywhere in the fields.status.statusCategory.key chain defaults to active.""" self.assertTrue(jira_helper.issue_from_jira_is_active(Mock(spec=[]))) + @patch("dojo.jira.helper.jira_transition", return_value=True) + @patch("dojo.jira.helper.get_jira_connection") + @patch("dojo.jira.helper.get_jira_issue") + @patch("dojo.jira.helper.get_jira_instance") + @patch("dojo.jira.helper.is_jira_configured_and_enabled", return_value=True) + @patch("dojo.jira.helper.is_jira_enabled", return_value=True) + def test_close_jira_issue_for_deleted_finding_closes_active_issue( + self, + is_jira_enabled, + is_jira_configured_and_enabled, + get_jira_instance, + get_jira_issue, + get_jira_connection, + jira_transition, + ): + finding = Mock(id=1) + finding.has_jira_issue = True + jira_instance = Mock(close_status_key=41) + jira_issue = Mock(jira_id="10001", jira_key="DD-1") + jira = Mock() + issue = self._make_issue("new") + get_jira_instance.return_value = jira_instance + get_jira_issue.return_value = jira_issue + get_jira_connection.return_value = jira + jira.issue.return_value = issue + + with ( + patch("dojo.jira.helper.is_delete_sync_allowed", return_value=True) as is_delete_sync_allowed, + patch("dojo.jira.helper.add_simple_jira_comment", return_value=True) as add_simple_jira_comment, + ): + updated, message = jira_helper.close_jira_issue_for_deleted_finding(finding) + + self.assertTrue(updated) + self.assertEqual("Jira issue DD-1 closed successfully.", message) + is_jira_enabled.assert_called_once_with() + is_delete_sync_allowed.assert_called_once_with(finding) + is_jira_configured_and_enabled.assert_called_once_with(finding) + jira.issue.assert_called_once_with("10001") + jira_transition.assert_called_once_with(jira, issue, 41) + add_simple_jira_comment.assert_called_once_with( + jira_instance, + jira_issue, + "DefectDojo finding 1 was deleted. This Jira issue was closed automatically.", + ) + jira_issue.save.assert_called_once_with(update_fields=["jira_change"]) + + def test_close_jira_issue_for_deleted_finding_skips_when_sync_disabled(self): + finding = Mock(id=1) + finding.has_jira_issue = True + + with ( + patch("dojo.jira.helper.is_jira_enabled", return_value=True) as is_jira_enabled, + patch("dojo.jira.helper.is_delete_sync_allowed", return_value=False) as is_delete_sync_allowed, + patch("dojo.jira.helper.is_jira_configured_and_enabled") as is_jira_configured_and_enabled, + ): + updated, message = jira_helper.close_jira_issue_for_deleted_finding(finding) + + self.assertFalse(updated) + self.assertEqual("Finding 1 is not configured to sync deleted findings to JIRA.", message) + is_jira_enabled.assert_called_once_with() + is_delete_sync_allowed.assert_called_once_with(finding) + is_jira_configured_and_enabled.assert_not_called() + + def test_reassign_jira_issue_to_finding_moves_local_link(self): + jira_issue = Mock() + finding = Mock() + + jira_helper.reassign_jira_issue_to_finding(jira_issue, finding) + + self.assertEqual(finding, jira_issue.finding) + self.assertIsNone(jira_issue.finding_group) + self.assertIsNone(jira_issue.engagement) + jira_issue.save.assert_called_once_with( + update_fields=["finding", "finding_group", "engagement"], + ) + + def test_reassign_jira_issue_to_new_original_moves_local_link_and_comments(self): + deleted_finding = Mock(id=1) + new_original = Mock(id=2) + new_original.has_jira_issue = False + jira_issue = Mock() + jira_instance = Mock() + + with ( + patch( + "dojo.finding.helper.jira_services.is_delete_sync_allowed", + return_value=True, + ) as is_delete_sync_allowed, + patch("dojo.finding.helper.jira_services.get_issue", return_value=jira_issue) as get_issue, + patch("dojo.finding.helper.jira_services.get_instance", return_value=jira_instance) as get_instance, + patch("dojo.finding.helper.jira_services.add_simple_comment", return_value=True) as add_simple_comment, + patch("dojo.finding.helper.jira_services.reassign_issue_to_finding") as reassign_issue_to_finding, + ): + reassigned = finding_helper._reassign_jira_issue_to_new_original(deleted_finding, new_original) + + self.assertTrue(reassigned) + is_delete_sync_allowed.assert_called_once_with(deleted_finding) + get_issue.assert_called_once_with(deleted_finding) + get_instance.assert_called_once_with(deleted_finding) + add_simple_comment.assert_called_once_with( + jira_instance, + jira_issue, + "DefectDojo finding 1 was deleted. This Jira issue was reassigned to finding 2.", + ) + reassign_issue_to_finding.assert_called_once_with(jira_issue, new_original) + self.assertTrue(deleted_finding._skip_jira_close_on_delete) + + def test_reassign_jira_issue_to_new_original_skips_when_new_original_has_jira_issue(self): + deleted_finding = Mock(id=1) + new_original = Mock(id=2) + new_original.has_jira_issue = True + + with ( + patch("dojo.finding.helper.jira_services.get_issue") as get_issue, + patch("dojo.finding.helper.jira_services.reassign_issue_to_finding") as reassign_issue_to_finding, + ): + reassigned = finding_helper._reassign_jira_issue_to_new_original(deleted_finding, new_original) + + self.assertFalse(reassigned) + get_issue.assert_not_called() + reassign_issue_to_finding.assert_not_called() + + @patch("dojo.finding.helper.delete_related_files") + @patch("dojo.finding.helper.delete_related_notes") + @patch("dojo.finding.helper.jira_services.close_issue_for_deleted_finding") + def test_finding_pre_delete_closes_linked_jira_issue_before_cleanup( + self, + close_issue_for_deleted_finding, + delete_related_notes, + delete_related_files, + ): + finding = Mock(id=1) + finding.has_jira_issue = True + finding._skip_jira_close_on_delete = False + + with patch( + "dojo.finding.helper.jira_services.is_delete_sync_allowed", + return_value=True, + ) as is_delete_sync_allowed: + finding_helper.finding_pre_delete(sender=Mock(), instance=finding) + + is_delete_sync_allowed.assert_called_once_with(finding) + close_issue_for_deleted_finding.assert_called_once_with(finding) + finding.found_by.clear.assert_called_once_with() + delete_related_notes.assert_called_once_with(finding) + delete_related_files.assert_called_once_with(finding) + + @patch("dojo.finding.helper.delete_related_files") + @patch("dojo.finding.helper.delete_related_notes") + @patch("dojo.finding.helper.jira_services.close_issue_for_deleted_finding") + def test_finding_pre_delete_skips_jira_close_when_sync_disabled( + self, + close_issue_for_deleted_finding, + delete_related_notes, + delete_related_files, + ): + finding = Mock(id=1) + finding.has_jira_issue = True + finding._skip_jira_close_on_delete = False + + with patch( + "dojo.finding.helper.jira_services.is_delete_sync_allowed", + return_value=False, + ) as is_delete_sync_allowed: + finding_helper.finding_pre_delete(sender=Mock(), instance=finding) + + is_delete_sync_allowed.assert_called_once_with(finding) + close_issue_for_deleted_finding.assert_not_called() + finding.found_by.clear.assert_called_once_with() + delete_related_notes.assert_called_once_with(finding) + delete_related_files.assert_called_once_with(finding) + + @patch("dojo.finding.helper.delete_related_files") + @patch("dojo.finding.helper.delete_related_notes") + @patch("dojo.finding.helper.jira_services.close_issue_for_deleted_finding") + def test_finding_pre_delete_skips_jira_close_after_reassigning_issue( + self, + close_issue_for_deleted_finding, + delete_related_notes, + delete_related_files, + ): + finding = Mock(id=1) + finding.has_jira_issue = True + finding._skip_jira_close_on_delete = True + + finding_helper.finding_pre_delete(sender=Mock(), instance=finding) + + close_issue_for_deleted_finding.assert_not_called() + finding.found_by.clear.assert_called_once_with() + delete_related_notes.assert_called_once_with(finding) + delete_related_files.assert_called_once_with(finding) + class JIRAComponentFieldTest(TestCase): From b980bb2756b5b2e43ba575d50fe1d44b45add89c Mon Sep 17 00:00:00 2001 From: Samiat Date: Wed, 24 Jun 2026 14:40:38 +0100 Subject: [PATCH 2/3] fix ci --- dojo/api_v2/views.py | 11 +++++++++-- dojo/finding/helper.py | 10 ++++++++++ unittests/test_jira_helper.py | 2 ++ 3 files changed, 21 insertions(+), 2 deletions(-) diff --git a/dojo/api_v2/views.py b/dojo/api_v2/views.py index 3f3070f2fcb..0bb1460c25b 100644 --- a/dojo/api_v2/views.py +++ b/dojo/api_v2/views.py @@ -26,8 +26,6 @@ from dojo.api_v2 import ( mixins as dojo_mixins, -) -from dojo.api_v2 import ( prefetch, serializers, ) @@ -88,6 +86,15 @@ labels = get_labels() +def get_request_boolean(request, name): + value = request.query_params.get(name) if name in request.query_params else request.data.get(name) + + if value is None: + return None + + return drf_serializers.BooleanField(required=False).run_validation(value) + + def schema_with_prefetch() -> dict: return { "list": extend_schema( diff --git a/dojo/finding/helper.py b/dojo/finding/helper.py index cd74f9526ea..78f942f4b84 100644 --- a/dojo/finding/helper.py +++ b/dojo/finding/helper.py @@ -608,6 +608,16 @@ def _reassign_jira_issue_to_new_original(deleted_finding, new_original): return True +def get_push_to_jira_on_delete(finding): + push_to_jira = getattr(finding, "_push_to_jira_on_delete", None) + return push_to_jira if isinstance(push_to_jira, bool) else None + + +def set_push_to_jira_on_delete(finding, push_to_jira): + if push_to_jira is not None: + finding._push_to_jira_on_delete = push_to_jira + + # can't use model to id here due to the queryset # @dojo_async_task # @app.task diff --git a/unittests/test_jira_helper.py b/unittests/test_jira_helper.py index 76f955dfb50..aecd4a34714 100644 --- a/unittests/test_jira_helper.py +++ b/unittests/test_jira_helper.py @@ -4,6 +4,8 @@ import dojo.finding.helper as finding_helper import dojo.jira.helper as jira_helper +from dojo.api_v2 import views as api_views +from dojo.finding import views as finding_views logger = logging.getLogger(__name__) From 472fd5efb920396243158a030370d026969e3032 Mon Sep 17 00:00:00 2001 From: Samiat Date: Thu, 25 Jun 2026 06:25:16 +0100 Subject: [PATCH 3/3] fix ruff --- docs/content/en/open_source/upgrading/3.1.md | 8 +- dojo/api_v2/views.py | 3 + dojo/finding/api/views.py | 23 +- dojo/finding/helper.py | 65 ++-- dojo/finding/models.py | 5 +- dojo/finding/ui/forms.py | 7 +- dojo/finding/ui/views.py | 5 +- dojo/jira/helper.py | 85 ++++-- dojo/jira/services.py | 17 +- .../templates/dojo/findings_list_snippet.html | 6 + dojo/templates/dojo/view_finding.html | 6 + dojo/templates/dojo/view_test.html | 6 + unittests/dojo_test_case.py | 7 +- unittests/test_jira_helper.py | 281 ++++++++++++------ 14 files changed, 368 insertions(+), 156 deletions(-) diff --git a/docs/content/en/open_source/upgrading/3.1.md b/docs/content/en/open_source/upgrading/3.1.md index bc1265d40c2..ee9087cd74d 100644 --- a/docs/content/en/open_source/upgrading/3.1.md +++ b/docs/content/en/open_source/upgrading/3.1.md @@ -2,6 +2,10 @@ title: 'Upgrading to DefectDojo Version 3.1.x' toc_hide: true weight: -20260615 -description: No special instructions. +description: JIRA sync behavior changed for deleted findings. --- -There are no special instructions for upgrading to 3.1.x. Check the [Release Notes](https://github.com/DefectDojo/django-DefectDojo/releases/tag/3.1.0) for the contents of the release. +## JIRA sync when deleting findings + +Direct finding deletes now honor the explicit `push_to_jira` choice before falling back to keep-in-sync or push-all settings. The UI checkbox is two-state, so users who want deleted findings to close or reassign linked JIRA issues must tick the option. In the API, omitting `push_to_jira` still uses the automatic keep-in-sync or push-all fallback, while passing `push_to_jira=false` skips the JIRA close/reassign action. + +Cascade deletes, such as deleting a Test or Engagement that contains findings, do not close linked JIRA issues automatically. Delete findings directly first when you want DefectDojo to close or reassign their linked JIRA issues. diff --git a/dojo/api_v2/views.py b/dojo/api_v2/views.py index 0bb1460c25b..8e975c04341 100644 --- a/dojo/api_v2/views.py +++ b/dojo/api_v2/views.py @@ -19,6 +19,7 @@ ) from drf_spectacular.views import SpectacularAPIView from rest_framework import mixins, status, viewsets +from rest_framework import serializers as drf_serializers from rest_framework.decorators import action from rest_framework.parsers import MultiPartParser from rest_framework.permissions import DjangoModelPermissions, IsAuthenticated @@ -26,6 +27,8 @@ from dojo.api_v2 import ( mixins as dojo_mixins, +) +from dojo.api_v2 import ( prefetch, serializers, ) diff --git a/dojo/finding/api/views.py b/dojo/finding/api/views.py index d40f72fd165..3a5b6340dd4 100644 --- a/dojo/finding/api/views.py +++ b/dojo/finding/api/views.py @@ -18,6 +18,7 @@ ) from rest_framework import mixins, status, viewsets from rest_framework.decorators import action +from rest_framework.exceptions import ValidationError as DRFValidationError from rest_framework.parsers import MultiPartParser from rest_framework.permissions import IsAuthenticated from rest_framework.response import Response @@ -32,7 +33,7 @@ from dojo.api_v2 import ( serializers as api_v2_serializers, ) -from dojo.api_v2.views import DojoModelViewSet, report_generate +from dojo.api_v2.views import DojoModelViewSet, get_request_boolean, report_generate from dojo.authorization import api_permissions as permissions from dojo.finding.api.filters import ApiFindingFilter, ApiTemplateFindingFilter from dojo.finding.api.serializer import ( @@ -128,6 +129,17 @@ def get_queryset(self): ), ], ), + destroy=extend_schema( + parameters=[ + OpenApiParameter( + "push_to_jira", + OpenApiTypes.BOOL, + OpenApiParameter.QUERY, + required=False, + description="Close or reassign the linked JIRA issue when deleting this finding.", + ), + ], + ), ) class FindingViewSet( prefetch.PrefetchListMixin, @@ -159,6 +171,15 @@ def perform_update(self, serializer): serializer.save(push_to_jira=push_to_jira) + def destroy(self, request, *args, **kwargs): + instance = self.get_object() + try: + push_to_jira = get_request_boolean(request, "push_to_jira") + except DRFValidationError as error: + raise DRFValidationError({"push_to_jira": error.detail}) from error + instance.delete(push_to_jira=push_to_jira) + return Response(status=status.HTTP_204_NO_CONTENT) + def get_queryset(self): if settings.V3_FEATURE_LOCATIONS: findings = get_authorized_findings( diff --git a/dojo/finding/helper.py b/dojo/finding/helper.py index 78f942f4b84..6e8334937d2 100644 --- a/dojo/finding/helper.py +++ b/dojo/finding/helper.py @@ -68,6 +68,7 @@ WAS_ACCEPTED_FINDINGS_QUERY = Q(risk_acceptance__isnull=False) & Q(risk_acceptance__expiration_date_handled__isnull=False) CLOSED_FINDINGS_QUERY = Q(is_mitigated=True) UNDER_REVIEW_QUERY = Q(under_review=True) +DELETE_JIRA_SYNC_UNSET = object() # this signal is triggered just before a finding is getting saved @@ -532,12 +533,6 @@ def post_process_findings_batch( @receiver(pre_delete, sender=Finding) def finding_pre_delete(sender, instance, **kwargs): logger.debug("finding pre_delete: %d", instance.id) - if ( - instance.has_jira_issue - and not getattr(instance, "_skip_jira_close_on_delete", False) - and jira_services.is_delete_sync_allowed(instance) - ): - jira_services.close_issue_for_deleted_finding(instance) # this shouldn't be necessary as Django should remove any Many-To-Many entries automatically, might be a bug in Django? # https://code.djangoproject.com/ticket/154 instance.found_by.clear() @@ -545,7 +540,7 @@ def finding_pre_delete(sender, instance, **kwargs): delete_related_files(instance) -def finding_delete(instance, **kwargs): +def finding_delete(instance, *, push_to_jira=DELETE_JIRA_SYNC_UNSET, **kwargs): logger.debug("finding delete, instance: %s", instance.id) # the idea is that the engagement/test pre delete already prepared all the duplicates inside @@ -563,16 +558,31 @@ def finding_delete(instance, **kwargs): # but django still calls delete() in this case return + jira_sync_requested = push_to_jira is None or isinstance(push_to_jira, bool) + jira_issue_reassigned = False duplicate_cluster = instance.original_finding.all() if duplicate_cluster: if settings.DUPLICATE_CLUSTER_CASCADE_DELETE: duplicate_cluster.order_by("-id").delete() else: new_original = reconfigure_duplicate_cluster(instance, duplicate_cluster) - _reassign_jira_issue_to_new_original(instance, new_original) + if jira_sync_requested: + jira_issue_reassigned = _reassign_jira_issue_to_new_original( + instance, + new_original, + push_to_jira=push_to_jira, + ) else: logger.debug("no duplicate cluster found for finding: %d, so no need to reconfigure", instance.id) + if ( + jira_sync_requested + and not jira_issue_reassigned + and instance.has_jira_issue + and jira_services.is_delete_sync_allowed(instance, push_to_jira=push_to_jira) + ): + jira_services.close_issue_for_deleted_finding(instance, push_to_jira=push_to_jira) + # this shouldn't be necessary as Django should remove any Many-To-Many entries automatically, might be a bug in Django? # https://code.djangoproject.com/ticket/154 logger.debug("finding delete: clearing found by") @@ -586,8 +596,12 @@ def finding_post_delete(sender, instance, **kwargs): logger.debug("finding post_delete, sender: %s instance: %s", to_str_typed(sender), to_str_typed(instance)) -def _reassign_jira_issue_to_new_original(deleted_finding, new_original): - if not new_original or new_original.has_jira_issue or not jira_services.is_delete_sync_allowed(deleted_finding): +def _reassign_jira_issue_to_new_original(deleted_finding, new_original, *, push_to_jira=None): + if ( + not new_original + or new_original.has_jira_issue + or not jira_services.is_delete_sync_allowed(deleted_finding, push_to_jira=push_to_jira) + ): return False jira_issue = jira_services.get_issue(deleted_finding) @@ -595,29 +609,24 @@ def _reassign_jira_issue_to_new_original(deleted_finding, new_original): return False jira_instance = jira_services.get_instance(deleted_finding) - jira_services.add_simple_comment( - jira_instance, - jira_issue, - ( - f"DefectDojo finding {deleted_finding.id} was deleted. " - f"This Jira issue was reassigned to finding {new_original.id}." - ), + if not jira_instance: + return False + + jira_id = jira_issue.jira_id + jira_instance_id = jira_instance.id + comment = ( + f"DefectDojo finding {deleted_finding.id} was deleted. " + f"This Jira issue was reassigned to finding {new_original.id}." ) jira_services.reassign_issue_to_finding(jira_issue, new_original) - deleted_finding._skip_jira_close_on_delete = True + jira_services.add_simple_comment_async( + jira_id, + jira_instance_id, + comment, + ) return True -def get_push_to_jira_on_delete(finding): - push_to_jira = getattr(finding, "_push_to_jira_on_delete", None) - return push_to_jira if isinstance(push_to_jira, bool) else None - - -def set_push_to_jira_on_delete(finding, push_to_jira): - if push_to_jira is not None: - finding._push_to_jira_on_delete = push_to_jira - - # can't use model to id here due to the queryset # @dojo_async_task # @app.task diff --git a/dojo/finding/models.py b/dojo/finding/models.py index 36f3554137d..58a1a9bc212 100644 --- a/dojo/finding/models.py +++ b/dojo/finding/models.py @@ -34,6 +34,7 @@ logger = logging.getLogger(__name__) deduplicationLogger = logging.getLogger("dojo.specific-loggers.deduplication") +DELETE_JIRA_SYNC_UNSET = object() class Finding(BaseModel): @@ -648,10 +649,10 @@ def copy(self, test=None): return copy - def delete(self, *args, product_grading_option=True, **kwargs): + def delete(self, *args, product_grading_option=True, push_to_jira=DELETE_JIRA_SYNC_UNSET, **kwargs): logger.debug("%d finding delete", self.id) from dojo.finding import helper as finding_helper # noqa: PLC0415 -- lazy import, avoids circular dependency - finding_helper.finding_delete(self) + finding_helper.finding_delete(self, push_to_jira=push_to_jira) super().delete(*args, **kwargs) if product_grading_option: from dojo.models import ( # noqa: PLC0415 -- lazy import, avoids circular dependency diff --git a/dojo/finding/ui/forms.py b/dojo/finding/ui/forms.py index 79f3e317059..ff02f5b4a79 100644 --- a/dojo/finding/ui/forms.py +++ b/dojo/finding/ui/forms.py @@ -1056,10 +1056,15 @@ class Meta: class DeleteFindingForm(forms.ModelForm): id = forms.IntegerField(required=True, widget=forms.widgets.HiddenInput()) + push_to_jira = forms.BooleanField( + required=False, + label="Push to JIRA", + help_text="Checking this will close or reassign the linked JIRA issue when this finding is deleted.", + ) class Meta: model = Finding - fields = ["id"] + fields = ["id", "push_to_jira"] class CopyFindingForm(forms.Form): diff --git a/dojo/finding/ui/views.py b/dojo/finding/ui/views.py index e4ed5327e1c..2ad43489a92 100644 --- a/dojo/finding/ui/views.py +++ b/dojo/finding/ui/views.py @@ -1086,7 +1086,7 @@ def get_finding(self, finding_id: int): def process_form(self, request: HttpRequest, finding: Finding, context: dict): if context["form"].is_valid(): product = finding.test.engagement.product - finding.delete() + finding.delete(push_to_jira=context["form"].cleaned_data.get("push_to_jira")) # Update the grade of the product async dojo_dispatch_task(calculate_grade, product.id) # Add a message to the request that the finding was successfully deleted @@ -2475,8 +2475,9 @@ def _bulk_delete_findings(request, pid, form, finding_to_update, finds, total_fi skipped_find_count = total_find_count - finds.count() deleted_find_count = finds.count() + push_to_jira = form.cleaned_data.get("push_to_jira") for find in finds: - find.delete() + find.delete(push_to_jira=push_to_jira) if skipped_find_count > 0: add_error_message_to_response( diff --git a/dojo/jira/helper.py b/dojo/jira/helper.py index 93c344cf2f9..5574e3b82fb 100644 --- a/dojo/jira/helper.py +++ b/dojo/jira/helper.py @@ -159,7 +159,9 @@ def is_keep_in_sync_with_jira(obj: Finding | Finding_Group, prefetched_jira_inst return False -def is_delete_sync_allowed(finding): +def is_delete_sync_allowed(finding, push_to_jira=None): + if push_to_jira is not None: + return is_push_to_jira(finding, push_to_jira_parameter=push_to_jira) return bool(is_keep_in_sync_with_jira(finding) or is_push_all_issues(finding)) @@ -1277,8 +1279,8 @@ def push_status_to_jira(obj, jira_instance, jira, issue, *, save=False): return updated -def close_jira_issue_for_deleted_finding(finding) -> tuple[bool | None, str]: - logger.debug("closing linked Jira issue before deleting finding %d", finding.id) +def close_jira_issue_for_deleted_finding(finding, push_to_jira=None) -> tuple[bool | None, str]: + logger.debug("queueing linked Jira issue close for deleted finding %d", finding.id) if not is_jira_enabled(): return False, "JIRA integration is not enabled." @@ -1286,7 +1288,7 @@ def close_jira_issue_for_deleted_finding(finding) -> tuple[bool | None, str]: if not finding.has_jira_issue: return False, f"Finding {finding.id} has no linked JIRA issue." - if not is_delete_sync_allowed(finding): + if not is_delete_sync_allowed(finding, push_to_jira=push_to_jira): return False, f"Finding {finding.id} is not configured to sync deleted findings to JIRA." if not is_jira_configured_and_enabled(finding): @@ -1297,52 +1299,57 @@ def close_jira_issue_for_deleted_finding(finding) -> tuple[bool | None, str]: logger.debug(message) return False, message - jira_instance = get_jira_instance(finding) - if not jira_instance: - message = ( - f"Finding {finding.id} cannot close its linked JIRA issue " - "because the JIRA instance is not available." - ) - logger.warning(message) - return False, message - jira_issue = get_jira_issue(finding) if not jira_issue: return False, f"Finding {finding.id} has no local JIRA issue record." + jira_project = get_jira_project(jira_issue) + if not jira_project or not jira_project.jira_instance: + return False, f"Finding {finding.id} has no JIRA instance for its linked issue." + + dojo_dispatch_task( + close_deleted_finding_jira_issue, + jira_issue.jira_id, + jira_project.jira_instance.id, + finding.id, + ) + return True, f"Jira issue {jira_issue.jira_key} close queued." + + +@app.task +def close_deleted_finding_jira_issue(jira_id, jira_instance_id, finding_id, **_kwargs) -> tuple[bool | None, str]: + jira_instance = get_object_or_none(JIRA_Instance, id=jira_instance_id) + if not jira_instance: + message = f"JIRA instance {jira_instance_id} is not available for issue {jira_id}." + logger.warning(message) + return False, message + try: JIRAError.log_to_tempfile = False jira = get_jira_connection(jira_instance) if not jira: - message = ( - f"Finding {finding.id} cannot close its linked JIRA issue " - "because the JIRA connection could not be established." - ) + message = f"JIRA connection could not be established for issue {jira_id}." logger.warning(message) return False, message - issue = jira.issue(jira_issue.jira_id) + issue = jira.issue(jira_id) except Exception as e: message = f"The following jira instance could not be connected: {jira_instance} - {e}" logger.exception(message) - log_jira_alert(message, finding) return False, message if not issue_from_jira_is_active(issue): - logger.debug("Jira issue %s is already resolved", jira_issue.jira_key) - return False, f"Jira issue {jira_issue.jira_key} is already resolved." + logger.debug("Jira issue %s is already resolved", jira_id) + return False, f"Jira issue {jira_id} is already resolved." updated = jira_transition(jira, issue, jira_instance.close_status_key) if updated: - add_simple_jira_comment( - jira_instance, - jira_issue, - f"DefectDojo finding {finding.id} was deleted. This Jira issue was closed automatically.", + jira.add_comment( + jira_id, + f"DefectDojo finding {finding_id} was deleted. This Jira issue was closed automatically.", ) - jira_issue.jira_change = timezone.now() - jira_issue.save(update_fields=["jira_change"]) - return True, f"Jira issue {jira_issue.jira_key} closed successfully." + return True, f"Jira issue {jira_id} closed successfully." - return updated, f"Jira issue {jira_issue.jira_key} was not closed." + return updated, f"Jira issue {jira_id} was not closed." def reassign_jira_issue_to_finding(jira_issue, finding): @@ -1745,6 +1752,26 @@ def add_simple_jira_comment(jira_instance, jira_issue, comment): return True +def add_simple_jira_comment_async(jira_id, jira_instance_id, comment): + return dojo_dispatch_task(add_simple_jira_comment_by_id, jira_id, jira_instance_id, comment) + + +@app.task +def add_simple_jira_comment_by_id(jira_id, jira_instance_id, comment, **_kwargs): + jira_instance = get_object_or_none(JIRA_Instance, id=jira_instance_id) + if not jira_instance: + logger.warning("JIRA instance %s is not available for issue %s", jira_instance_id, jira_id) + return False + + try: + jira = get_jira_connection(jira_instance) + jira.add_comment(jira_id, comment) + except Exception as e: + log_jira_generic_alert("Jira Add Comment Error", str(e)) + return False + return True + + def jira_already_linked(finding, jira_issue_key, jira_id) -> Finding | None: jira_issues = JIRA_Issue.objects.filter(jira_id=jira_id, jira_key=jira_issue_key).exclude(engagement__isnull=False) jira_issues = jira_issues.exclude(finding=finding) diff --git a/dojo/jira/services.py b/dojo/jira/services.py index a2899666b90..d6e7418eb2d 100644 --- a/dojo/jira/services.py +++ b/dojo/jira/services.py @@ -47,6 +47,15 @@ def add_simple_comment(jira_instance, jira_issue, comment): return _get_helper().add_simple_jira_comment(jira_instance, jira_issue, comment) +def add_simple_comment_async(jira_id, jira_instance_id, comment): + """ + Add a simple text comment to a Jira issue from durable IDs. + + Wraps: jira_helper.add_simple_jira_comment_async + """ + return _get_helper().add_simple_jira_comment_async(jira_id, jira_instance_id, comment) + + def add_comment_internal(jira_issue_id, note_id, *, force_push=False, **kwargs): """ Internal add comment by IDs. @@ -137,13 +146,13 @@ def push_status(obj, jira_instance, jira, issue, *, save=False): return _get_helper().push_status_to_jira(obj, jira_instance, jira, issue, save=save) -def close_issue_for_deleted_finding(finding): +def close_issue_for_deleted_finding(finding, push_to_jira=None): """ Close the linked Jira issue before a finding is deleted. Wraps: jira_helper.close_jira_issue_for_deleted_finding """ - return _get_helper().close_jira_issue_for_deleted_finding(finding) + return _get_helper().close_jira_issue_for_deleted_finding(finding, push_to_jira=push_to_jira) def reassign_issue_to_finding(jira_issue, finding): @@ -357,13 +366,13 @@ def is_keep_in_sync(obj, prefetched_jira_instance=None): return _get_helper().is_keep_in_sync_with_jira(obj, prefetched_jira_instance=prefetched_jira_instance) -def is_delete_sync_allowed(finding): +def is_delete_sync_allowed(finding, push_to_jira=None): """ Check if deleting a finding should update its linked Jira issue. Wraps: jira_helper.is_delete_sync_allowed """ - return _get_helper().is_delete_sync_allowed(finding) + return _get_helper().is_delete_sync_allowed(finding, push_to_jira=push_to_jira) def is_push(instance, push_to_jira_parameter=None): diff --git a/dojo/templates/dojo/findings_list_snippet.html b/dojo/templates/dojo/findings_list_snippet.html index eb8d3db7edb..51790689da6 100644 --- a/dojo/templates/dojo/findings_list_snippet.html +++ b/dojo/templates/dojo/findings_list_snippet.html @@ -529,6 +529,12 @@

{% csrf_token %} + {% if system_settings.enable_jira and finding.has_jira_issue %} + + {% endif %} {% trans "Delete" %} diff --git a/dojo/templates/dojo/view_finding.html b/dojo/templates/dojo/view_finding.html index 19d94fe2942..54ef15ea939 100755 --- a/dojo/templates/dojo/view_finding.html +++ b/dojo/templates/dojo/view_finding.html @@ -181,6 +181,12 @@

{% csrf_token %} + {% if system_settings.enable_jira and finding.has_jira_issue %} + + {% endif %}
{% endif %} diff --git a/dojo/templates/dojo/view_test.html b/dojo/templates/dojo/view_test.html index 1472d6c42e1..a1348a74b3d 100644 --- a/dojo/templates/dojo/view_test.html +++ b/dojo/templates/dojo/view_test.html @@ -1129,6 +1129,12 @@

{% csrf_token %} + {% if system_settings.enable_jira and finding.has_jira_issue %} + + {% endif %}