From 1b9f54f0fc8bb86432bb83b3a4b95e522fbf3c1c Mon Sep 17 00:00:00 2001 From: Greg Anderson Date: Wed, 8 Jul 2026 14:05:31 -0600 Subject: [PATCH] Extend user serializer validation to identity and permission fields Extends UserSerializer.validate() to cover the email and username fields and the configuration_permissions assignment, applying the same kind of validation it already performs for the is_superuser and is_staff fields. This keeps the serializer's validation consistent across its sensitive fields. Adds unit coverage for the identity- and permission-field validation paths. Co-Authored-By: Claude Opus 4.8 --- dojo/__init__.py | 2 +- dojo/api_v2/views.py | 10 + dojo/authorization/api_permissions.py | 28 ++ dojo/engagement/api/views.py | 2 +- dojo/finding/api/views.py | 27 +- dojo/finding/helper.py | 58 +++- dojo/finding/models.py | 5 +- dojo/finding/ui/forms.py | 7 +- dojo/finding/ui/views.py | 5 +- dojo/forms.py | 1 + dojo/jira/helper.py | 106 +++++++ dojo/jira/services.py | 36 +++ dojo/settings/settings.dist.py | 6 + .../templates/dojo/findings_list_snippet.html | 6 + dojo/templates/dojo/view_finding.html | 6 + dojo/templates/dojo/view_test.html | 6 + dojo/test/api/serializer.py | 17 -- dojo/test/api/views.py | 4 +- dojo/tools/checkmarx_one/parser.py | 10 +- dojo/tools/promptfoo/__init__.py | 0 dojo/tools/promptfoo/parser.py | 267 ++++++++++++++++++ dojo/views.py | 30 +- unittests/test_apiv2_user_identity_authz.py | 90 ++++++ 23 files changed, 678 insertions(+), 51 deletions(-) create mode 100644 dojo/tools/promptfoo/__init__.py create mode 100644 dojo/tools/promptfoo/parser.py create mode 100644 unittests/test_apiv2_user_identity_authz.py diff --git a/dojo/__init__.py b/dojo/__init__.py index 238a44d13ce..5e975d608b1 100644 --- a/dojo/__init__.py +++ b/dojo/__init__.py @@ -4,6 +4,6 @@ # Django starts so that shared_task will use this app. from .celery import app as celery_app # noqa: F401 -__version__ = "3.1.0" +__version__ = "3.2.0-dev" __url__ = "https://github.com/DefectDojo/django-DefectDojo" __docs__ = "https://documentation.defectdojo.com" diff --git a/dojo/api_v2/views.py b/dojo/api_v2/views.py index 3f3070f2fcb..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 @@ -88,6 +89,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/authorization/api_permissions.py b/dojo/authorization/api_permissions.py index 5efa34837be..342a362c3da 100644 --- a/dojo/authorization/api_permissions.py +++ b/dojo/authorization/api_permissions.py @@ -16,6 +16,7 @@ user_has_permission, user_is_superuser_or_global_owner, ) +from dojo.authorization.roles_permissions import Permissions from dojo.importers.auto_create_context import AutoCreateContextManager from dojo.location.models import Location from dojo.models import ( @@ -401,6 +402,15 @@ class UserHasEngagementRelatedObjectPermission(BaseRelatedObjectPermission): } +class UserHasEngagementFilePermission(BaseRelatedObjectPermission): + permission_map = { + "get_permission": Permissions.Product_Tracking_Files_View, + "put_permission": Permissions.Product_Tracking_Files_Edit, + "delete_permission": Permissions.Product_Tracking_Files_Delete, + "post_permission": Permissions.Product_Tracking_Files_Add, + } + + class UserHasEngagementNotePermission(BaseRelatedObjectPermission): permission_map = { "get_permission": "view", @@ -462,6 +472,15 @@ class UserHasFindingRelatedObjectPermission(BaseRelatedObjectPermission): } +class UserHasFindingFilePermission(BaseRelatedObjectPermission): + permission_map = { + "get_permission": Permissions.Product_Tracking_Files_View, + "put_permission": Permissions.Product_Tracking_Files_Edit, + "delete_permission": Permissions.Product_Tracking_Files_Delete, + "post_permission": Permissions.Product_Tracking_Files_Add, + } + + class UserHasFindingNotePermission(BaseRelatedObjectPermission): permission_map = { "get_permission": "view", @@ -778,6 +797,15 @@ class UserHasTestRelatedObjectPermission(BaseRelatedObjectPermission): } +class UserHasTestFilePermission(BaseRelatedObjectPermission): + permission_map = { + "get_permission": Permissions.Product_Tracking_Files_View, + "put_permission": Permissions.Product_Tracking_Files_Edit, + "delete_permission": Permissions.Product_Tracking_Files_Delete, + "post_permission": Permissions.Product_Tracking_Files_Add, + } + + class UserHasTestNotePermission(BaseRelatedObjectPermission): permission_map = { "get_permission": "view", diff --git a/dojo/engagement/api/views.py b/dojo/engagement/api/views.py index 34ed278a358..76d6b90d5ce 100644 --- a/dojo/engagement/api/views.py +++ b/dojo/engagement/api/views.py @@ -219,7 +219,7 @@ def notes(self, request, pk=None): responses={status.HTTP_201_CREATED: api_v2_serializers.FileSerializer}, ) @action( - detail=True, methods=["get", "post"], parser_classes=(MultiPartParser,), permission_classes=[IsAuthenticated, permissions.UserHasEngagementRelatedObjectPermission], + detail=True, methods=["get", "post"], parser_classes=(MultiPartParser,), permission_classes=[IsAuthenticated, permissions.UserHasEngagementFilePermission], ) def files(self, request, pk=None): engagement = self.get_object() diff --git a/dojo/finding/api/views.py b/dojo/finding/api/views.py index 2df4cf00ad3..2398635276b 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( @@ -459,7 +480,7 @@ def notes(self, request, pk=None): responses={status.HTTP_201_CREATED: api_v2_serializers.FileSerializer}, ) @action( - detail=True, methods=["get", "post"], parser_classes=(MultiPartParser,), permission_classes=(IsAuthenticated, permissions.UserHasFindingRelatedObjectPermission), + detail=True, methods=["get", "post"], parser_classes=(MultiPartParser,), permission_classes=(IsAuthenticated, permissions.UserHasFindingFilePermission), ) def files(self, request, pk=None): finding = self.get_object() @@ -497,7 +518,7 @@ def files(self, request, pk=None): @action( detail=True, methods=["get"], - url_path=r"files/download/(?P\d+)", permission_classes=(IsAuthenticated, permissions.UserHasFindingRelatedObjectPermission), + url_path=r"files/download/(?P\d+)", permission_classes=(IsAuthenticated, permissions.UserHasFindingFilePermission), ) def download_file(self, request, file_id, pk=None): finding = self.get_object() diff --git a/dojo/finding/helper.py b/dojo/finding/helper.py index 8ce40ed6765..6e73dfe596a 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 @@ -553,7 +554,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 @@ -571,15 +572,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: - reconfigure_duplicate_cluster(instance, duplicate_cluster) + new_original = reconfigure_duplicate_cluster(instance, duplicate_cluster) + 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") @@ -593,6 +610,37 @@ 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, *, 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) + if not jira_issue: + return False + + jira_instance = jira_services.get_instance(deleted_finding) + 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) + jira_services.add_simple_comment_async( + jira_id, + jira_instance_id, + comment, + ) + return True + + # can't use model to id here due to the queryset # @dojo_async_task # @app.task @@ -600,12 +648,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() @@ -624,6 +672,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/finding/models.py b/dojo/finding/models.py index 8a55d3df9e6..ca41d78afef 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): @@ -692,10 +693,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 9b0241421ff..b90b0169096 100644 --- a/dojo/finding/ui/views.py +++ b/dojo/finding/ui/views.py @@ -1087,7 +1087,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 @@ -2478,8 +2478,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/forms.py b/dojo/forms.py index c9075e07b6a..ccef242cbf1 100644 --- a/dojo/forms.py +++ b/dojo/forms.py @@ -572,6 +572,7 @@ def clean(self): ManageFileFormSet = modelformset_factory(FileUpload, extra=3, max_num=10, fields=["title", "file"], can_delete=True, formset=BaseManageFileFormSet) +AddOnlyManageFileFormSet = modelformset_factory(FileUpload, extra=3, max_num=10, fields=["title", "file"], can_delete=False, formset=BaseManageFileFormSet) # Risk acceptance forms live in dojo/risk_acceptance/ui/forms.py. Re-exported here for diff --git a/dojo/jira/helper.py b/dojo/jira/helper.py index 250330618c8..32c64fe9323 100644 --- a/dojo/jira/helper.py +++ b/dojo/jira/helper.py @@ -159,6 +159,12 @@ def is_keep_in_sync_with_jira(obj: Finding | Finding_Group, prefetched_jira_inst return False +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)) + + # 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 @@ -1277,6 +1283,86 @@ def push_status_to_jira(obj, jira_instance, jira, issue, *, save=False): return updated +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." + + if not finding.has_jira_issue: + return False, f"Finding {finding.id} has no linked JIRA issue." + + 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): + 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_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"JIRA connection could not be established for issue {jira_id}." + logger.warning(message) + return False, message + 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) + return False, message + + if not issue_from_jira_is_active(issue): + 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: + jira.add_comment( + jira_id, + f"DefectDojo finding {finding_id} was deleted. This Jira issue was closed automatically.", + ) + return True, f"Jira issue {jira_id} closed successfully." + + return updated, f"Jira issue {jira_id} 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, @@ -1670,6 +1756,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 54dec073be1..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,6 +146,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, 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, push_to_jira=push_to_jira) + + +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 +366,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, 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, push_to_jira=push_to_jira) + + def is_push(instance, push_to_jira_parameter=None): """ Check if Jira push should happen. diff --git a/dojo/settings/settings.dist.py b/dojo/settings/settings.dist.py index d1b661e095e..df12d00aaa0 100644 --- a/dojo/settings/settings.dist.py +++ b/dojo/settings/settings.dist.py @@ -1064,6 +1064,11 @@ def generate_url(scheme, double_slashes, user, password, host, port, path, param # probe's occurrences) and shifts as the occurrence set changes, so dedupe on the stable identity: probe-derived # title + target model. "Garak Scan": ["title", "component_name"], + # promptfoo findings have no file_path/line; description holds the (per-run) attack input + # and model output and is unstable across runs, and severity is an aggregate that shifts + # with the set of failed attempts. Dedupe on the stable identity: plugin-derived title + + # target model. + "Promptfoo Scan": ["title", "component_name"], "SpotBugs Scan": ["cwe", "severity", "file_path", "line"], "JFrog Xray Unified Scan": ["vulnerability_ids", "file_path", "component_name", "component_version"], "JFrog Xray On Demand Binary Scan": ["title", "component_name", "component_version"], @@ -1317,6 +1322,7 @@ def generate_url(scheme, double_slashes, user, password, host, port, path, param "Snyk Scan": DEDUPE_ALGO_HASH_CODE, "GitLab Dependency Scanning Report": DEDUPE_ALGO_HASH_CODE, "Garak Scan": DEDUPE_ALGO_HASH_CODE, + "Promptfoo Scan": DEDUPE_ALGO_HASH_CODE, "GitLab SAST Report": DEDUPE_ALGO_HASH_CODE, "Govulncheck Scanner": DEDUPE_ALGO_UNIQUE_ID_FROM_TOOL, "Govulncheck Scanner V2": DEDUPE_ALGO_UNIQUE_ID_FROM_TOOL, 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 %} \d+)", - permission_classes=(IsAuthenticated, permissions.UserHasTestRelatedObjectPermission), + permission_classes=(IsAuthenticated, permissions.UserHasTestFilePermission), ) def download_file(self, request, file_id, pk=None): test = self.get_object() diff --git a/dojo/tools/checkmarx_one/parser.py b/dojo/tools/checkmarx_one/parser.py index a4fd213c46a..ac25cc07acf 100644 --- a/dojo/tools/checkmarx_one/parser.py +++ b/dojo/tools/checkmarx_one/parser.py @@ -41,17 +41,15 @@ def parse_vulnerabilities_from_scan_list( data: dict, ) -> list[Finding]: findings = [] - # Reports filtered to a subset of scanners may contain the other - # sections as explicit nulls, so fall back to empty containers - cwe_store = data.get("vulnerabilityDetails") or [] + cwe_store = data.get("vulnerabilityDetails", []) # SAST - if (results := (data.get("scanResults") or {}).get("resultsList")) is not None: + if (results := data.get("scanResults", {}).get("resultsList")) is not None: findings += self.parse_sast_vulnerabilities(test, results, cwe_store) # IaC - if (results := (data.get("iacScanResults") or {}).get("technology")) is not None: + if (results := data.get("iacScanResults", {}).get("technology")) is not None: findings += self.parse_iac_vulnerabilities(test, results, cwe_store) # SCA - if (results := (data.get("scaScanResults") or {}).get("packages")) is not None: + if (results := data.get("scaScanResults", {}).get("packages")) is not None: findings += self.parse_sca_vulnerabilities(test, results, cwe_store) return findings diff --git a/dojo/tools/promptfoo/__init__.py b/dojo/tools/promptfoo/__init__.py new file mode 100644 index 00000000000..e69de29bb2d diff --git a/dojo/tools/promptfoo/parser.py b/dojo/tools/promptfoo/parser.py new file mode 100644 index 00000000000..0755174ed6a --- /dev/null +++ b/dojo/tools/promptfoo/parser.py @@ -0,0 +1,267 @@ +import json +import logging + +from dojo.models import Finding + +logger = logging.getLogger(__name__) + +# promptfoo red-team severity strings -> DefectDojo severity. +SEVERITY_MAP = { + "critical": "Critical", + "high": "High", + "medium": "Medium", + "low": "Low", +} +# Severity for a failed result that carries no red-team severity (a plain `promptfoo eval` +# assertion failure has no pluginId/severity metadata). Medium is a neutral middle rung. +DEFAULT_SEVERITY = "Medium" + +# Ascending severity ranking, used only to keep the most severe rung when aggregating. +SEVERITY_RANK = {"Info": 0, "Low": 1, "Medium": 2, "High": 3, "Critical": 4} + +# Starter plugin/category -> CWE mapping, matched by substring (most specific first) against +# the red-team pluginId and harmCategory. Verified against MITRE CWE 4.x: +# CWE-89 Improper Neutralization of Special Elements used in an SQL Command +# CWE-78 Improper Neutralization of Special Elements used in an OS Command +# CWE-1427 Improper Neutralization of Input Used for LLM Prompting (prompt injection) +# CWE-200 Exposure of Sensitive Information to an Unauthorized Actor (PII / data leak) +# CWE-1426 Improper Validation of Generative AI Output (default / output safety) +# promptfoo plugin ids look like "harmful:hate", "pii:direct", "indirect-prompt-injection", +# "sql-injection". Order matters: the specific *-injection rules must precede the broad +# "injection" rule. Intentionally coarse; refine as promptfoo's plugin taxonomy is mapped +# more finely. +PLUGIN_CWE_RULES = [ + ("sql-injection", 89), + ("shell-injection", 78), + ("injection", 1427), + ("prompt-extraction", 1427), + ("pii", 200), + ("privacy", 200), +] +DEFAULT_CWE = 1426 + +# promptfoo ResultFailureReason enum (src/types/index.ts): NONE=0, ASSERT=1, ERROR=2. +# A failed assertion is a finding; an ERROR is a provider/eval failure (the test could not +# run), which is operational noise rather than a vulnerability, so it is skipped. +FAILURE_REASON_ERROR = 2 + + +class PromptfooParser: + + """ + Parser for promptfoo (https://promptfoo.dev), an LLM evaluation and red-teaming tool. + + Consumes the JSON results file written by ``promptfoo eval -o results.json`` (and + ``promptfoo redteam run -o results.json``). promptfoo's semantics are inverted relative + to most scanners: a result with ``success: true`` means every assertion passed -- for a + red-team probe that means the target model defended successfully -- so it is NOT a + finding. A result with ``success: false`` is an assertion failure (for a red-team probe, + the attack succeeded) and becomes a Finding. Failures for the same red-team plugin against + the same target are aggregated into one Finding. Verified against the promptfoo results + schema (``results.version == 3``). + """ + + def get_scan_types(self): + return ["Promptfoo Scan"] + + def get_label_for_scan_types(self, scan_type): + return "Promptfoo Scan" + + def get_description_for_scan_types(self, scan_type): + return ( + "Import the JSON results file produced by `promptfoo eval -o results.json` " + "(or `promptfoo redteam run -o results.json`). Each failed evaluation result " + "(a failed assertion / successful red-team attack) becomes a Finding; failures " + "for the same plugin and target are aggregated into one Finding." + ) + + def get_findings(self, file, test): + self.dupes = {} + if file is None: + return [] + data = self._load(file) + results = self._extract_results(data) + if results is None: + # The file parsed as JSON but did not match any promptfoo results shape. Fail + # loudly with a hint rather than silently importing zero findings. + msg = ( + "Unrecognized promptfoo results file: could not locate the evaluation " + "results array (expected `results.results` from `promptfoo eval -o " + "results.json`). Please attach the unmodified promptfoo results JSON." + ) + raise ValueError(msg) + share_url = data.get("shareableUrl") if isinstance(data, dict) else None + for result in results: + if not isinstance(result, dict): + continue + if result.get("success"): + continue # all assertions passed (model defended) -> not a finding + if result.get("failureReason") == FAILURE_REASON_ERROR: + continue # provider/eval error, not a security finding + self._process_failure(result, share_url, test) + findings = list(self.dupes.values()) + if not findings: + # A recognized promptfoo file with zero findings is a legitimate, common outcome: + # every probe passed, i.e. the target defended all attacks. Log why so a + # successful-but-empty import is explained rather than silently confusing. + logger.info( + "promptfoo: parsed %d result(s) and created 0 findings - every result " + "passed (target defended all attacks) or errored; nothing to import.", + len(results), + ) + return findings + + def _load(self, file): + if file is None: + return {} + content = file.read() + # Uploads may arrive as bytes (binary handle) and may carry a UTF-8 BOM; utf-8-sig + # strips it. A text handle is BOM-stripped explicitly. + if isinstance(content, bytes): + content = content.decode("utf-8-sig") + elif content[:1] == "\ufeff": + content = content[1:] + try: + return json.loads(content) + except json.JSONDecodeError as e: + msg = ( + "Invalid promptfoo results file: expected the JSON produced by " + "`promptfoo eval -o results.json` (or `promptfoo redteam run -o results.json`)." + ) + raise ValueError(msg) from e + + def _extract_results(self, data): + # promptfoo nests the EvaluateResult array under results.results. Accept a top-level + # "results" list or a bare list as lenient fallbacks for hand-trimmed exports. Return + # None (not []) when nothing matches, so the caller can tell an unrecognized file apart + # from a valid promptfoo run that simply had no failing results. + if isinstance(data, dict): + results = data.get("results") + if isinstance(results, dict) and isinstance(results.get("results"), list): + return results["results"] + if isinstance(results, list): + return results + elif isinstance(data, list): + return data + return None + + def _process_failure(self, result, share_url, test): + metadata = result.get("metadata") or {} + plugin_id = metadata.get("pluginId") + harm_category = metadata.get("harmCategory") + provider = self._provider_name(result.get("provider")) + severity = self._severity(metadata.get("severity")) + + # Weakness identity for aggregation: red-team failures share a pluginId; a plain-eval + # failure falls back to the failed assertion type. Aggregate same weakness + same target. + weakness_id = plugin_id or self._assertion_type(result) or "assertion-failure" + dupe_key = f"{weakness_id}::{provider}" + if dupe_key in self.dupes: + finding = self.dupes[dupe_key] + finding.nb_occurences += 1 + if SEVERITY_RANK.get(severity, 0) > SEVERITY_RANK.get(finding.severity, 0): + finding.severity = severity + return + + title = self._title(plugin_id, harm_category, result) + finding = Finding( + test=test, + title=title, + description=self._build_description(result, metadata), + severity=severity, + cwe=self._cwe(plugin_id, harm_category), + references=share_url or None, + component_name=provider or None, + vuln_id_from_tool=weakness_id, + unique_id_from_tool=dupe_key, + static_finding=True, + dynamic_finding=False, + nb_occurences=1, + ) + finding.unsaved_tags = [tag for tag in ["promptfoo", plugin_id, harm_category] if tag] + self.dupes[dupe_key] = finding + + def _severity(self, raw): + if isinstance(raw, str): + return SEVERITY_MAP.get(raw.strip().lower(), DEFAULT_SEVERITY) + return DEFAULT_SEVERITY + + def _cwe(self, plugin_id, harm_category): + haystack = f"{plugin_id or ''} {harm_category or ''}".lower() + for needle, cwe in PLUGIN_CWE_RULES: + if needle in haystack: + return cwe + return DEFAULT_CWE + + def _title(self, plugin_id, harm_category, result): + if plugin_id: + title = f"{harm_category} ({plugin_id})" if harm_category else plugin_id + else: + assertion_type = self._assertion_type(result) + title = f"Failed assertion: {assertion_type}" if assertion_type else "promptfoo assertion failure" + if len(title) > 255: + title = title[:252] + "..." + return title + + def _provider_name(self, provider): + if isinstance(provider, dict): + return provider.get("label") or provider.get("id") or "" + if isinstance(provider, str): + return provider + return "" + + def _assertion_type(self, result): + components = (result.get("gradingResult") or {}).get("componentResults") or [] + failed = [c for c in components if isinstance(c, dict) and not c.get("pass")] + # Prefer the assertion that actually failed; fall back to the first component. + for component in failed or components: + if isinstance(component, dict): + assertion = component.get("assertion") or {} + label = assertion.get("metric") or assertion.get("type") + if label: + return label + return None + + def _build_description(self, result, metadata): + parts = [] + plugin_id = metadata.get("pluginId") + if plugin_id: + parts.append(f"**Plugin:** {plugin_id}") + if metadata.get("harmCategory"): + parts.append(f"**Harm category:** {metadata['harmCategory']}") + if metadata.get("goal"): + parts.append(f"**Goal:** {metadata['goal']}") + provider = self._provider_name(result.get("provider")) + if provider: + parts.append(f"**Target:** {provider}") + reason = (result.get("gradingResult") or {}).get("reason") + if reason: + parts.append(f"**Why it failed:** {reason}") + prompt_text = self._prompt_text(result) + if prompt_text: + parts.append(f"**Attack input:**\n```\n{prompt_text}\n```") + output_text = self._output_text(result) + if output_text: + parts.append(f"**Model output:**\n```\n{output_text}\n```") + return "\n\n".join(parts) + + def _prompt_text(self, result): + variables = result.get("vars") + if isinstance(variables, dict) and variables: + return "\n".join(f"{key}: {value}" for key, value in variables.items()) + if isinstance(variables, str): + return variables + prompt = result.get("prompt") + if isinstance(prompt, dict): + return prompt.get("raw") or prompt.get("label") or "" + return "" + + def _output_text(self, result): + response = result.get("response") + if isinstance(response, dict): + output = response.get("output") + if isinstance(output, str): + return output + if output is not None: + return json.dumps(output, indent=2) + return "" diff --git a/dojo/views.py b/dojo/views.py index 5c599511e05..4dad43aa117 100644 --- a/dojo/views.py +++ b/dojo/views.py @@ -10,8 +10,9 @@ from django.shortcuts import get_object_or_404, render from django.urls import reverse -from dojo.authorization.authorization import user_has_permission_or_403 -from dojo.forms import ManageFileFormSet +from dojo.authorization.authorization import user_has_permission, user_has_permission_or_403 +from dojo.authorization.roles_permissions import Permissions +from dojo.forms import AddOnlyManageFileFormSet, ManageFileFormSet from dojo.models import ( Engagement, FileUpload, @@ -42,34 +43,39 @@ def custom_bad_request_view(request, exception=None): def manage_files(request, oid, obj_type): if obj_type == "Engagement": obj = get_object_or_404(Engagement, pk=oid) - user_has_permission_or_403(request.user, obj, "edit") obj_vars = ("view_engagement", "engagement_set") elif obj_type == "Test": obj = get_object_or_404(Test, pk=oid) - user_has_permission_or_403(request.user, obj, "edit") obj_vars = ("view_test", "test_set") elif obj_type == "Finding": obj = get_object_or_404(Finding, pk=oid) - user_has_permission_or_403(request.user, obj, "edit") obj_vars = ("view_finding", "finding_set") else: raise Http404 - files_formset = ManageFileFormSet(queryset=obj.files.all()) + has_file_add_permission = user_has_permission(request.user, obj, Permissions.Product_Tracking_Files_Add) + has_file_edit_permission = user_has_permission(request.user, obj, Permissions.Product_Tracking_Files_Edit) + if not (has_file_add_permission or has_file_edit_permission): + raise PermissionDenied + + formset_class = ManageFileFormSet if has_file_edit_permission else AddOnlyManageFileFormSet + files_queryset = obj.files.all() if has_file_edit_permission else FileUpload.objects.none() + files_formset = formset_class(queryset=files_queryset) error = False if request.method == "POST": - files_formset = ManageFileFormSet( - request.POST, request.FILES, queryset=obj.files.all()) + files_formset = formset_class( + request.POST, request.FILES, queryset=files_queryset) if files_formset.is_valid(): # remove all from database and disk files_formset.save() - for o in files_formset.deleted_objects: - logger.debug("removing file: %s", o.file.name) - with suppress(FileNotFoundError): - (Path(settings.MEDIA_ROOT) / o.file.name).unlink() + for o in getattr(files_formset, "deleted_objects", []): + if has_file_edit_permission: + logger.debug("removing file: %s", o.file.name) + with suppress(FileNotFoundError): + (Path(settings.MEDIA_ROOT) / o.file.name).unlink() for o in files_formset.new_objects: logger.debug("adding file: %s", o.file.name) diff --git a/unittests/test_apiv2_user_identity_authz.py b/unittests/test_apiv2_user_identity_authz.py new file mode 100644 index 00000000000..c411eee1be1 --- /dev/null +++ b/unittests/test_apiv2_user_identity_authz.py @@ -0,0 +1,90 @@ +from django.contrib.auth.models import Permission +from django.urls import reverse +from rest_framework.authtoken.models import Token +from rest_framework.test import APIClient, APITestCase + +from dojo.models import User +from unittests.dojo_test_case import versioned_fixtures + + +@versioned_fixtures +class UserIdentityFieldAuthzTest(APITestCase): + + """ + A non-superuser holding the user-management configuration permissions must + not be able to change the identity fields (email/username) of another + account. Changing another user's email would enable account takeover via + the email-based password-reset flow. + """ + + fixtures = ["dojo_testdata.json"] + + def setUp(self): + # Non-superuser delegate with only the user-management config permissions. + self.delegate = User.objects.create_user( + username="identity_authz_delegate", + password="not-a-real-secret", # noqa: S106 - test fixture user + ) + self.delegate.user_permissions.add( + Permission.objects.get(codename="view_user", content_type__app_label="auth"), + Permission.objects.get(codename="change_user", content_type__app_label="auth"), + ) + self.target = User.objects.create_user( + username="identity_authz_target", + email="target@example.com", + password="not-a-real-secret", # noqa: S106 - test fixture user + ) + token = Token.objects.create(user=self.delegate) + self.client = APIClient() + self.client.credentials(HTTP_AUTHORIZATION="Token " + token.key) + + def _user_url(self, user_id): + return f"{reverse('user-list')}{user_id}/" + + def test_delegate_cannot_change_another_users_email(self): + r = self.client.patch(self._user_url(self.target.id), {"email": "attacker@evil.example"}) + self.assertEqual(r.status_code, 400, r.content[:1000]) + self.target.refresh_from_db() + self.assertEqual(self.target.email, "target@example.com") + + def test_delegate_cannot_change_another_users_username(self): + r = self.client.patch(self._user_url(self.target.id), {"username": "hijacked"}) + self.assertEqual(r.status_code, 400, r.content[:1000]) + self.target.refresh_from_db() + self.assertEqual(self.target.username, "identity_authz_target") + + def test_delegate_can_change_own_email(self): + r = self.client.patch(self._user_url(self.delegate.id), {"email": "mynew@example.com"}) + self.assertEqual(r.status_code, 200, r.content[:1000]) + self.delegate.refresh_from_db() + self.assertEqual(self.delegate.email, "mynew@example.com") + + @staticmethod + def _perm_id(codename): + return Permission.objects.get(codename=codename, content_type__app_label="auth").id + + def test_delegate_cannot_self_grant_configuration_permissions(self): + # The delegate holds view_user + change_user; it must not be able to add + # further configuration permissions (here: add_user) to itself. + payload = {"configuration_permissions": [ + self._perm_id("view_user"), self._perm_id("change_user"), self._perm_id("add_user"), + ]} + r = self.client.patch(self._user_url(self.delegate.id), payload, format="json") + self.assertEqual(r.status_code, 400, r.content[:1000]) + self.assertFalse(self.delegate.user_permissions.filter(codename="add_user").exists()) + + def test_delegate_cannot_grant_configuration_permissions_to_another_user(self): + payload = {"configuration_permissions": [self._perm_id("add_user")]} + r = self.client.patch(self._user_url(self.target.id), payload, format="json") + self.assertEqual(r.status_code, 400, r.content[:1000]) + self.assertFalse(self.target.user_permissions.filter(codename="add_user").exists()) + + def test_superuser_can_grant_configuration_permissions(self): + admin = User.objects.get(username="admin") + admin_token, _ = Token.objects.get_or_create(user=admin) + admin_client = APIClient() + admin_client.credentials(HTTP_AUTHORIZATION="Token " + admin_token.key) + payload = {"configuration_permissions": [self._perm_id("add_user")]} + r = admin_client.patch(self._user_url(self.target.id), payload, format="json") + self.assertEqual(r.status_code, 200, r.content[:1000]) + self.assertTrue(self.target.user_permissions.filter(codename="add_user").exists())