Quick verify in menu and keyboard shortcuts to verify/close findings (DefectDojo#14318)

* feature: quick verify finding

* keyboard shortcuts to verify/close finding

* address feedback

* sync to JIRA in verify_finding

* lint

---------

Co-authored-by: Filipe Pina <63779195+fopinappb@users.noreply.github.com>

Author: 2 people

dojo/api_v2/serializers.py

@@ -2994,6 +2994,11 @@ def validate(self, data):
         return data
 
 
+class FindingVerifySerializer(serializers.Serializer):
+    note = serializers.CharField(required=False, allow_blank=True)
+    note_type = serializers.PrimaryKeyRelatedField(required=False, allow_null=True, queryset=Note_Type.objects.all())
+
+
 class ReportGenerateOptionSerializer(serializers.Serializer):
     include_finding_notes = serializers.BooleanField(default=False)
     include_finding_images = serializers.BooleanField(default=False)

dojo/api_v2/views.py

@@ -1024,6 +1024,32 @@ def close(self, request, pk=None):
         serialized_finding = serializers.FindingCloseSerializer(finding, context={"request": request})
         return Response(serialized_finding.data)
 
+    @extend_schema(
+        methods=["POST"],
+        request=serializers.FindingVerifySerializer,
+        responses={status.HTTP_200_OK: serializers.FindingSerializer},
+    )
+    @action(detail=True, methods=["post"], permission_classes=(IsAuthenticated, permissions.UserHasFindingRelatedObjectPermission))
+    def verify(self, request, pk=None):
+        finding = self.get_object()
+
+        serializer = serializers.FindingVerifySerializer(data=request.data)
+        if not serializer.is_valid():
+            return Response(serializer.errors, status=status.HTTP_400_BAD_REQUEST)
+
+        # Remove prefetched tags to keep queryset state in sync
+        finding.tags._remove_prefetched_objects()
+
+        finding_helper.verify_finding(
+            finding=finding,
+            user=request.user,
+            note_entry=serializer.validated_data.get("note"),
+            note_type=serializer.validated_data.get("note_type"),
+        )
+
+        serialized_finding = serializers.FindingSerializer(finding, context={"request": request})
+        return Response(serialized_finding.data)
+
     @extend_schema(
         methods=["GET"],
         responses={status.HTTP_200_OK: serializers.TagSerializer},

dojo/finding/helper.py

@@ -1012,6 +1012,54 @@ def normalize_datetime(value):
     return value
 
 
+def _create_note_if_provided(
+    finding,
+    note_entry,
+    *,
+    user=None,
+    note_type=None,
+    note_date=None,
+):
+    """
+    Create a note for the finding when content is provided. Returns the note or None.
+    Note author defaults to finding.last_reviewed_by
+    """
+    if not note_entry:
+        return None
+
+    new_note = Notes.objects.create(
+        entry=note_entry,
+        author=user or finding.last_reviewed_by,
+        note_type=note_type,
+        date=note_date,
+    )
+    finding.notes.add(new_note)
+    return new_note
+
+
+def _save_finding_with_jira_sync(finding, *, new_note=None):
+    """Persist finding and apply JIRA sync behavior used by finding status actions."""
+    push_to_jira = False
+    finding_in_group = finding.has_finding_group
+    jira_issue_exists = finding.has_jira_issue or (
+        finding.finding_group and finding.finding_group.has_jira_issue
+    )
+    jira_instance = jira_helper.get_jira_instance(finding)
+    jira_project = jira_helper.get_jira_project(finding)
+
+    if jira_issue_exists:
+        push_to_jira = (
+            jira_helper.is_push_all_issues(finding)
+            or (jira_instance and jira_instance.finding_jira_sync)
+        )
+        if new_note and (getattr(jira_project, "push_notes", False) or push_to_jira) and not finding_in_group:
+            jira_helper.add_comment(finding, new_note, force_push=True)
+
+    finding.save(push_to_jira=(push_to_jira and not finding_in_group))
+    if push_to_jira and finding_in_group:
+        jira_helper.push_to_jira(finding.finding_group)
+
+
 def close_finding(
     *,
     finding,
@@ -1046,15 +1094,12 @@ def close_finding(
     finding.last_reviewed_by = user
 
     # Create note if provided
-    new_note = None
-    if note_entry:
-        new_note = Notes.objects.create(
-            entry=note_entry,
-            author=user,
-            note_type=note_type,
-            date=mitigated_date,
-        )
-        finding.notes.add(new_note)
+    new_note = _create_note_if_provided(
+        finding,
+        note_entry,
+        note_type=note_type,
+        note_date=mitigated_date,
+    )
 
     if settings.V3_FEATURE_LOCATIONS:
         # Related locations
@@ -1076,26 +1121,7 @@ def close_finding(
     # External issues (best effort)
     close_external_issue(finding.id, "Closed by defectdojo", "github")
 
-    # JIRA sync
-    push_to_jira = False
-    finding_in_group = finding.has_finding_group
-    jira_issue_exists = finding.has_jira_issue or (
-        finding.finding_group and finding.finding_group.has_jira_issue
-    )
-    jira_instance = jira_helper.get_jira_instance(finding)
-    jira_project = jira_helper.get_jira_project(finding)
-    if jira_issue_exists:
-        push_to_jira = (
-            jira_helper.is_push_all_issues(finding)
-            or (jira_instance and jira_instance.finding_jira_sync)
-        )
-        if new_note and (getattr(jira_project, "push_notes", False) or push_to_jira) and not finding_in_group:
-            jira_helper.add_comment(finding, new_note, force_push=True)
-
-    # Persist and push JIRA if applicable
-    finding.save(push_to_jira=(push_to_jira and not finding_in_group))
-    if push_to_jira and finding_in_group:
-        jira_helper.push_to_jira(finding.finding_group)
+    _save_finding_with_jira_sync(finding, new_note=new_note)
 
     # Notification
     create_notification(
@@ -1105,3 +1131,28 @@ def close_finding(
         description=f'The finding "{finding.title}" was closed by {user}',
         url=reverse("view_finding", args=(finding.id,)),
     )
+
+
+def verify_finding(
+    *,
+    finding,
+    user,
+    note_entry=None,
+    note_type=None,
+) -> None:
+    """Shared verify logic used by UI and API."""
+    verification_time = now()
+
+    finding.verified = True
+    finding.last_reviewed = verification_time
+    finding.last_reviewed_by = user
+    finding.last_status_update = verification_time
+
+    new_note = _create_note_if_provided(
+        finding,
+        note_entry,
+        note_type=note_type,
+        note_date=verification_time,
+    )
+
+    _save_finding_with_jira_sync(finding, new_note=new_note)

dojo/finding/urls.py

@@ -142,6 +142,8 @@
         name="choose_finding_template_options"),
     re_path(r"^finding/(?P<fid>\d+)/(?P<tid>\d+)/apply_template_to_finding$",
         views.apply_template_to_finding, name="apply_template_to_finding"),
+    re_path(r"^finding/(?P<fid>\d+)/verify$", views.verify_finding,
+        name="verify_finding"),
     re_path(r"^finding/(?P<fid>\d+)/close$", views.close_finding,
         name="close_finding"),
     re_path(r"^finding/(?P<fid>\d+)/defect_review$",

dojo/finding/views.py

@@ -1226,6 +1226,65 @@ def close_finding(request, fid):
     )
 
 
+@user_is_authorized(Finding, Permissions.Finding_Edit, "fid")
+def verify_finding(request, fid):
+    finding = get_object_or_404(Finding, id=fid)
+
+    if finding.verified:
+        messages.add_message(
+            request,
+            messages.INFO,
+            "Finding already verified.",
+            extra_tags="alert-info",
+        )
+        return redirect_to_return_url_or_else(
+            request,
+            reverse("view_finding", args=(finding.id,)),
+        )
+
+    form = NoteForm(data=request.POST or None)
+    form.fields["entry"].required = False
+    form.fields["entry"].label = _("Comment (optional)")
+
+    if request.method == "POST" and form.is_valid():
+        entry = form.cleaned_data.get("entry", "")
+        finding_helper.verify_finding(
+            finding=finding,
+            user=request.user,
+            note_entry=entry,
+        )
+
+        messages.add_message(
+            request,
+            messages.SUCCESS,
+            "Finding verified.",
+            extra_tags="alert-success",
+        )
+
+        return redirect_to_return_url_or_else(
+            request,
+            reverse("view_finding", args=(finding.id,)),
+        )
+
+    product_tab = Product_Tab(
+        finding.test.engagement.product,
+        title="Verify Finding",
+        tab="findings",
+    )
+
+    return render(
+        request,
+        "dojo/verify_finding.html",
+        {
+            "finding": finding,
+            "product_tab": product_tab,
+            "user": request.user,
+            "form": form,
+            "active_tab": "findings",
+        },
+    )
+
+
 @user_is_authorized(Finding, Permissions.Finding_Edit, "fid")
 def defect_finding_review(request, fid):
     finding = get_object_or_404(Finding, id=fid)

dojo/templates/dojo/verify_finding.html

@@ -0,0 +1,18 @@
+{% extends "base.html" %}
+{% load i18n %}
+
+{% block content %}
+    {{ block.super }}
+    <h3>{% trans "Verify Finding" %}</h3>
+    <h4>{{ finding.title }}</h4>
+    <p>{% trans "Use this form to mark the finding as verified. Adding a comment is optional." %}</p>
+    <form class="form-horizontal" action="{% url 'verify_finding' finding.id %}" method="post">
+        {% csrf_token %}
+        {% include "dojo/form_fields.html" with form=form %}
+        <div class="form-group">
+            <div class="col-sm-offset-2 col-sm-10">
+                <input class="btn btn-primary" type="submit" value="{% trans "Verify Finding" %}" aria-label="{% trans "Verify Finding" %}"/>
+            </div>
+        </div>
+    </form>
+{% endblock %}

dojo/templates/dojo/view_finding.html

@@ -126,6 +126,13 @@ <h3 class="pull-left finding-title">
                                         </a>
                                     </li>
                                 {% else %}
+                                {% if not finding.verified %}
+                                    <li role="presentation">
+                                        <a href="{% url 'verify_finding' finding.id %}">
+                                            <i class="fa-solid fa-circle-check"></i> Verify Finding
+                                        </a>
+                                    </li>
+                                {% endif %}
                                 <li role="presentation">
                                     <a href="{% url 'close_finding' finding.id %}">
                                         <i class="fa-solid fa-fire-extinguisher"></i> Close Finding
@@ -1186,7 +1193,7 @@ <h4>Credential
     </div>
 
     <div class="protip">
-       <i class="fa-solid fa-lightbulb"></i> <strong>ProTip!</strong> Type <kbd>e</kbd> to edit any finding, <kbd>p</kbd> and <kbd>n</kbd> to navigate to the previous or next finding.
+       <i class="fa-solid fa-lightbulb"></i> <strong>ProTip!</strong> Type <kbd>e</kbd> to edit any finding, <kbd>p</kbd> and <kbd>n</kbd> to navigate to the previous or next finding, <kbd>v</kbd> to verify, and <kbd>c</kbd> to close the finding.
     </div>
 </div>
 {% endblock %}
@@ -1199,6 +1206,9 @@ <h4>Credential
         var firstID = {% if findings_list.0 %}{{findings_list.0}}{% else %}null{% endif %};
         var currentID = {% if finding.id %}{{finding.id}}{% else %}null{% endif %};
         var lastID = {% if findings_list_lastElement %}{{findings_list_lastElement}}{% else %}null{% endif %};
+        var canEditFinding = {% if finding|has_object_permission:"Finding_Edit" %}true{% else %}false{% endif %};
+        var findingIsMitigated = {% if finding.mitigated %}true{% else %}false{% endif %};
+        var findingIsVerified = {% if finding.verified %}true{% else %}false{% endif %};
         if(currentID != firstID)
         {
             $('.PrevAndNext_Buttons').append('<a href="{% url 'view_finding' prev_finding_id %}" class="btn btn-primary">Previous Finding</a> ');
@@ -1278,6 +1288,34 @@ <h4>Credential
             window.location.assign('{% url 'view_finding' next_finding_id %}');
         });
 
+        $(document).on('keypress', null, 'v', function () {
+            if (!canEditFinding) {
+                alert('You do not have permission to verify this finding.');
+                return;
+            }
+            if (findingIsMitigated) {
+                alert('Finding is already closed and cannot be verified.');
+                return;
+            }
+            if (findingIsVerified) {
+                alert('Finding has already been verified.');
+                return;
+            }
+            window.location.assign('{% url 'verify_finding' finding.id %}');
+        });
+
+        $(document).on('keypress', null, 'c', function () {
+            if (!canEditFinding) {
+                alert('You do not have permission to close this finding.');
+                return;
+            }
+            if (findingIsMitigated) {
+                alert('Finding has already been closed.');
+                return;
+            }
+            window.location.assign('{% url 'close_finding' finding.id %}');
+        });
+
         $('a.delete-finding').on('click', function (e) {
             if (confirm('Are you sure you want to delete this finding?')) {
                 $("form#delete-finding-form").submit();

unittests/test_rest_framework.py

@@ -1011,7 +1011,38 @@ def test_close_finding_pushes_note_to_jira_when_configured(self):
             }
             response = self.client.post(self._close_url(finding.id), payload, format="json")
             self.assertEqual(200, response.status_code, response.content[:1000])
-            self.assertTrue(add_comment_mock.called)
+        self.assertTrue(add_comment_mock.called)
+
+
+@versioned_fixtures
+class FindingVerifyAPITest(DojoAPITestCase):
+    fixtures = ["dojo_testdata.json"]
+
+    def setUp(self):
+        testuser = User.objects.get(username="admin")
+        token = Token.objects.get(user=testuser)
+        self.client = APIClient()
+        self.client.credentials(HTTP_AUTHORIZATION=f"Token {token.key}")
+        self.admin = testuser
+
+    def _verify_url(self, finding_id: int) -> str:
+        return f"/api/v2/findings/{finding_id}/verify/"
+
+    def test_verify_finding_basic(self):
+        finding = Finding.objects.get(id=7)
+        response = self.client.post(self._verify_url(finding.id), {"note": "Marked verified"}, format="json")
+        self.assertEqual(200, response.status_code, response.content[:1000])
+
+        finding.refresh_from_db()
+        self.assertTrue(finding.verified)
+        self.assertEqual(finding.last_reviewed_by, self.admin)
+        self.assertTrue(finding.notes.filter(entry__icontains="Marked verified").exists())
+
+    def test_verify_finding_invalid_payload(self):
+        finding = Finding.objects.get(id=7)
+        # note_type specified but invalid id
+        response = self.client.post(self._verify_url(finding.id), {"note_type": 9999}, format="json")
+        self.assertEqual(400, response.status_code, response.content[:1000])
 
 
 @versioned_fixtures