fix tests, cleanup

Author: valentijnscholten

dojo/api_v2/serializers.py

@@ -118,7 +118,8 @@
     requires_tool_type,
 )
 from dojo.user.utils import get_configuration_permissions_codenames
-from dojo.utils import is_scan_file_too_large, tag_validator
+from dojo.utils import is_scan_file_too_large
+from dojo.validators import tag_validator
 
 logger = logging.getLogger(__name__)
 deduplicationLogger = logging.getLogger("dojo.specific-loggers.deduplication")
@@ -1840,8 +1841,6 @@ def validate(self, data):
         # doing it here instead of in update because update doesn't know if the value changed
         self.process_risk_acceptance(data)
 
-        # cvss3_validator(data.get("cvssv3"), exception_class=RestFrameworkValidationError)
-
         return data
 
     def validate_severity(self, value: str) -> str:
@@ -1971,8 +1970,6 @@ def validate(self, data):
             msg = "Active findings cannot be risk accepted."
             raise serializers.ValidationError(msg)
 
-        # cvss3_validator(data.get("cvssv3"), exception_class=RestFrameworkValidationError)
-
         return data
 
     def validate_severity(self, value: str) -> str:
@@ -1999,7 +1996,6 @@ class Meta:
         exclude = ("cve",)
 
     def create(self, validated_data):
-        # cvss3_validator(validated_data.get("cvssv3"), exception_class=RestFrameworkValidationError)
 
         to_be_tagged, validated_data = self._pop_tags(validated_data)
 
@@ -2028,8 +2024,6 @@ def create(self, validated_data):
         return new_finding_template
 
     def update(self, instance, validated_data):
-        # cvss3_validator(validated_data.get("cvssv3"), exception_class=RestFrameworkValidationError)
-
         # Save vulnerability ids and pop them
         if "vulnerability_id_template_set" in validated_data:
             vulnerability_id_set = validated_data.pop(

dojo/forms.py

@@ -110,8 +110,8 @@
     get_system_setting,
     is_finding_groups_enabled,
     is_scan_file_too_large,
-    tag_validator,
 )
+from dojo.validators import tag_validator
 from dojo.widgets import TableCheckboxWidget
 
 logger = logging.getLogger(__name__)

dojo/utils.py

@@ -25,7 +25,6 @@
 from django.conf import settings
 from django.contrib import messages
 from django.contrib.auth.signals import user_logged_in, user_logged_out, user_login_failed
-from django.core.exceptions import ValidationError
 from django.core.paginator import Paginator
 from django.db.models import Case, Count, IntegerField, Q, Sum, Value, When
 from django.db.models.query import QuerySet
@@ -2655,20 +2654,3 @@ def generate_file_response_from_file_path(
     response["Content-Disposition"] = f'attachment; filename="{full_file_name}"'
     response["Content-Length"] = file_size
     return response
-
-
-def tag_validator(value: str | list[str], exception_class: Callable = ValidationError) -> None:
-    TAG_PATTERN = re.compile(r'[ ,\'"]')
-    error_messages = []
-
-    if isinstance(value, list):
-        error_messages.extend(f"Invalid tag: '{tag}'. Tags should not contain spaces, commas, or quotes." for tag in value if TAG_PATTERN.search(tag))
-    elif isinstance(value, str):
-        if TAG_PATTERN.search(value):
-            error_messages.append(f"Invalid tag: '{value}'. Tags should not contain spaces, commas, or quotes.")
-    else:
-        error_messages.append(f"Value must be a string or list of strings: {value} - {type(value)}.")
-
-    if error_messages:
-        logger.debug(f"Tag validation failed: {error_messages}")
-        raise exception_class(error_messages)

dojo/validators.py

@@ -1,4 +1,5 @@
 import logging
+import re
 from collections.abc import Callable
 
 import cvss.parser
@@ -8,6 +9,23 @@
 logger = logging.getLogger(__name__)
 
 
+def tag_validator(value: str | list[str], exception_class: Callable = ValidationError) -> None:
+    TAG_PATTERN = re.compile(r'[ ,\'"]')
+    error_messages = []
+
+    if isinstance(value, list):
+        error_messages.extend(f"Invalid tag: '{tag}'. Tags should not contain spaces, commas, or quotes." for tag in value if TAG_PATTERN.search(tag))
+    elif isinstance(value, str):
+        if TAG_PATTERN.search(value):
+            error_messages.append(f"Invalid tag: '{value}'. Tags should not contain spaces, commas, or quotes.")
+    else:
+        error_messages.append(f"Value must be a string or list of strings: {value} - {type(value)}.")
+
+    if error_messages:
+        logger.debug(f"Tag validation failed: {error_messages}")
+        raise exception_class(error_messages)
+
+
 def cvss3_validator(value: str | list[str], exception_class: Callable = ValidationError) -> None:
     logger.error("cvss3_validator called with value: %s", value)
     cvss_vectors = cvss.parser.parse_cvss_from_text(value)
@@ -31,5 +49,5 @@ def cvss3_validator(value: str | list[str], exception_class: Callable = Validati
 
     # Explicitly raise an error if no CVSS vectors are found,
     # to avoid 'NoneType' errors during severity processing later.
-    msg = "No CVSS vectors found by cvss.parse_cvss_from_text()"
+    msg = "No valid CVSS vectors found by cvss.parse_cvss_from_text()"
     raise exception_class(msg)

tests/finding_test.py

@@ -112,7 +112,6 @@ def test_edit_finding(self):
         driver.find_element(By.ID, "dropdownMenu1").click()
         # Click on `Edit Finding`
         driver.find_element(By.LINK_TEXT, "Edit Finding").click()
-        # Change: 'Severity' and 'cvssv3'
         # finding Severity
         Select(driver.find_element(By.ID, "id_severity")).select_by_visible_text("Critical")
         # finding Vulnerability Ids
@@ -165,6 +164,8 @@ def _edit_finding_cvssv3_and_assert(
             self.assertEqual(str(expected_cvssv3_score), driver.find_element(By.ID, "id_cvssv3_score").get_attribute("value"))
         else:
             self.assertTrue(self.is_error_message_present(text=error_message))
+            self.assertEqual(expected_cvssv3_value, driver.find_element(By.ID, "id_cvssv3").get_attribute("value"))
+            self.assertEqual(str(expected_cvssv3_score), driver.find_element(By.ID, "id_cvssv3_score").get_attribute("value"))
 
     # See https://github.com/DefectDojo/django-DefectDojo/issues/8264
     # Capturing current behavior which might not be the desired one yet
@@ -184,8 +185,9 @@ def test_edit_finding_cvssv3_valid_vector_no_prefix(self):
             cvssv3_value="AV:N/AC:L/PR:L/UI:N/S:U/C:H/I:H/A:H",
             cvssv3_score="2",
             expected_cvssv3_value="AV:N/AC:L/PR:L/UI:N/S:U/C:H/I:H/A:H",
-            expected_cvssv3_score="2.0",
-            expect_success=True,
+            expected_cvssv3_score="2",
+            expect_success=False,
+            error_message="No valid CVSS vectors found by cvss.parse_cvss_from_text()",
         )
 
     @on_exception_html_source_logger
@@ -194,29 +196,53 @@ def test_edit_finding_cvssv3_valid_vector_with_trailing_slash(self):
             cvssv3_value="CVSS:3.0/AV:N/AC:L/PR:L/UI:N/S:U/C:H/I:H/A:H/",
             cvssv3_score="3",
             expected_cvssv3_value="CVSS:3.0/AV:N/AC:L/PR:L/UI:N/S:U/C:H/I:H/A:H/",
-            expected_cvssv3_score="3.0",
-            expect_success=True,
+            expected_cvssv3_score="3",
+            expect_success=False,
+            error_message="No valid CVSS vectors found by cvss.parse_cvss_from_text()",
         )
 
     @on_exception_html_source_logger
-    def test_edit_finding_cvssv3_with_v2_vector(self):
+    def test_edit_finding_cvssv3_with_v2_vector_invalid_due_to_prefix(self):
         self._edit_finding_cvssv3_and_assert(
             cvssv3_value="CVSS:2.0/AV:N/AC:L/Au:N/C:P/I:P/A:P",
             cvssv3_score="4",
             expected_cvssv3_value="CVSS:2.0/AV:N/AC:L/Au:N/C:P/I:P/A:P",
-            expected_cvssv3_score="4.0",
-            expect_success=True,
+            expected_cvssv3_score="4",
+            expect_success=False,
+            error_message="No valid CVSS vectors found by cvss.parse_cvss_from_text()",
+        )
+
+    @on_exception_html_source_logger
+    def test_edit_finding_cvssv3_with_v2_vector(self):
+        self._edit_finding_cvssv3_and_assert(
+            cvssv3_value="AV:N/AC:L/Au:N/C:P/I:P/A:P",
+            cvssv3_score="4",
+            expected_cvssv3_value="AV:N/AC:L/Au:N/C:P/I:P/A:P",
+            expected_cvssv3_score="4",
+            expect_success=False,
+            error_message="Unsupported CVSS(2) version detected.",
+        )
+
+    @on_exception_html_source_logger
+    def test_edit_finding_cvssv3_with_v4_vector(self):
+        self._edit_finding_cvssv3_and_assert(
+            cvssv3_value="CVSS:4.0/AV:N/AC:L/AT:N/PR:L/UI:N/S:U/C:H/I:H/A:H",
+            cvssv3_score="5",
+            expected_cvssv3_value="CVSS:4.0/AV:N/AC:L/AT:N/PR:L/UI:N/S:U/C:H/I:H/A:H",
+            expected_cvssv3_score="5",
+            expect_success=False,
+            error_message="No valid CVSS vectors found by cvss.parse_cvss_from_text()",
         )
 
     @on_exception_html_source_logger
     def test_edit_finding_cvssv3_with_rubbish(self):
         self._edit_finding_cvssv3_and_assert(
             cvssv3_value="happy little vector",
-            cvssv3_score="4",
-            expected_cvssv3_value=None,
-            expected_cvssv3_score=None,
+            cvssv3_score="5",
+            expected_cvssv3_value="happy little vector",
+            expected_cvssv3_score="5",
             expect_success=False,
-            error_message="CVSS must be entered in format: 'AV:N/AC:L/PR:N/UI:N/S:C/C:H/I:H/A:H'",
+            error_message="No valid CVSS vectors found by cvss.parse_cvss_from_text()",
         )
 
     def test_add_image(self):
@@ -611,6 +637,8 @@ def add_finding_tests_to_suite(suite, *, jira=False, github=False, block_executi
     suite.addTest(FindingTest("test_edit_finding_cvssv3_valid_vector_no_prefix"))
     suite.addTest(FindingTest("test_edit_finding_cvssv3_valid_vector_with_trailing_slash"))
     suite.addTest(FindingTest("test_edit_finding_cvssv3_with_v2_vector"))
+    suite.addTest(FindingTest("test_edit_finding_cvssv3_with_v2_vector_invalid_due_to_prefix"))
+    suite.addTest(FindingTest("test_edit_finding_cvssv3_with_v4_vector"))
     suite.addTest(FindingTest("test_edit_finding_cvssv3_with_rubbish"))
     suite.addTest(FindingTest("test_add_note_to_finding"))
     suite.addTest(FindingTest("test_add_image"))

unittests/test_finding_model.py

@@ -318,6 +318,7 @@ def test_get_references_with_links_markdown(self):
 
     # See https://github.com/DefectDojo/django-DefectDojo/issues/8264
     # Capturing current behavior which might not be the desired one yet
+    # This test saves vectors without any validation. This is capturing current behavior.
     def test_cvssv3(self):
         """Tests if the CVSSv3 score is calculated correctly"""
         user, _ = User.objects.get_or_create(username="admin")

unittests/test_rest_framework.py

@@ -1281,48 +1281,85 @@ def test_severity_validation(self):
         self.assertEqual(result.status_code, status.HTTP_400_BAD_REQUEST, "Severity just got set to something invalid")
         self.assertEqual(result.json()["severity"], ["Severity must be one of the following: ['Info', 'Low', 'Medium', 'High', 'Critical']"])
 
-    # See https://github.com/DefectDojo/django-DefectDojo/issues/8264
     def test_cvss3_validation(self):
         with self.subTest(i=0):
             self.assertEqual(None, Finding.objects.get(id=2).cvssv3)
-            result = self.client.patch(self.url + "2/", data={"cvssv3": "CVSS:3.0/AV:N/AC:L/PR:L/UI:N/S:U/C:H/I:H/A:H"})
+            result = self.client.patch(self.url + "2/", data={"cvssv3": "CVSS:3.0/AV:N/AC:L/PR:L/UI:N/S:U/C:H/I:H/A:H", "cvssv3_score": 3})
             self.assertEqual(result.status_code, status.HTTP_200_OK)
-            self.assertEqual("CVSS:3.0/AV:N/AC:L/PR:L/UI:N/S:U/C:H/I:H/A:H", Finding.objects.get(id=2).cvssv3)
+            finding = Finding.objects.get(id=2)
+            # valid so vector must be set and score calculated
+            self.assertEqual("CVSS:3.0/AV:N/AC:L/PR:L/UI:N/S:U/C:H/I:H/A:H", finding.cvssv3)
+            self.assertEqual(8.8, finding.cvssv3_score)
 
         with self.subTest(i=1):
             # extra slash makes it invalid
-            result = self.client.patch(self.url + "3/", data={"cvssv3": "CVSS:3.0/AV:N/AC:L/PR:L/UI:N/S:U/C:H/I:H/A:H/"})
+            result = self.client.patch(self.url + "3/", data={"cvssv3": "CVSS:3.0/AV:N/AC:L/PR:L/UI:N/S:U/C:H/I:H/A:H/", "cvssv3_score": 3})
             self.assertEqual(result.status_code, status.HTTP_400_BAD_REQUEST)
-            self.assertEqual(result.json()["cvssv3"], ["No CVSS vectors found by cvss.parse_cvss_from_text()"])
-            self.assertEqual(None, Finding.objects.get(id=3).cvssv3)
+            finding = Finding.objects.get(id=3)
+            self.assertEqual(result.json()["cvssv3"], ["No valid CVSS vectors found by cvss.parse_cvss_from_text()"])
+            # invalid vector, so no calculated score and no score stored
+            self.assertEqual(None, finding.cvssv3)
+            self.assertEqual(None, finding.cvssv3_score)
 
         with self.subTest(i=2):
             # no CVSS version prefix makes it invalid
-            result = self.client.patch(self.url + "3/", data={"cvssv3": "AV:N/AC:L/PR:L/UI:N/S:U/C:H/I:H/A:H"})
+            result = self.client.patch(self.url + "3/", data={"cvssv3": "AV:N/AC:L/PR:L/UI:N/S:U/C:H/I:H/A:H", "cvssv3_score": 4})
             self.assertEqual(result.status_code, status.HTTP_400_BAD_REQUEST)
-            self.assertEqual(result.json()["cvssv3"], ["No CVSS vectors found by cvss.parse_cvss_from_text()"])
-            self.assertEqual(None, Finding.objects.get(id=3).cvssv3)
+            finding = Finding.objects.get(id=3)
+            self.assertEqual(result.json()["cvssv3"], ["No valid CVSS vectors found by cvss.parse_cvss_from_text()"])
+            # invalid vector, so no calculated score and no score stored
+            self.assertEqual(None, finding.cvssv3)
+            self.assertEqual(None, finding.cvssv3_score)
 
         with self.subTest(i=3):
             # CVSS4 version makes it invalid
-            result = self.client.patch(self.url + "3/", data={"cvssv3": "CVSS:4.0/AV:N/AC:L/PR:L/UI:N/S:U/C:H/I:H/A:H"})
+            result = self.client.patch(self.url + "3/", data={"cvssv3": "CVSS:4.0/AV:N/AC:L/PR:L/UI:N/S:U/C:H/I:H/A:H", "cvssv3_score": 5})
             self.assertEqual(result.status_code, status.HTTP_400_BAD_REQUEST)
-            self.assertEqual(result.json()["cvssv3"], ["No CVSS vectors found by cvss.parse_cvss_from_text()"])
-            self.assertEqual(None, Finding.objects.get(id=3).cvssv3)
+            self.assertEqual(result.json()["cvssv3"], ["No valid CVSS vectors found by cvss.parse_cvss_from_text()"])
+            finding = Finding.objects.get(id=3)
+            # invalid vector, so no calculated score and no score stored
+            self.assertEqual(None, finding.cvssv3)
+            self.assertEqual(None, finding.cvssv3_score)
 
         with self.subTest(i=4):
             # CVSS2 style vector makes not supported
-            result = self.client.patch(self.url + "3/", data={"cvssv3": "AV:N/AC:L/Au:N/C:P/I:P/A:P"})
+            result = self.client.patch(self.url + "3/", data={"cvssv3": "AV:N/AC:L/Au:N/C:P/I:P/A:P", "cvssv3_score": 6})
             self.assertEqual(result.status_code, status.HTTP_400_BAD_REQUEST)
             self.assertEqual(result.json()["cvssv3"], ["Unsupported CVSS(2) version detected."])
-            self.assertEqual(None, Finding.objects.get(id=3).cvssv3)
+            finding = Finding.objects.get(id=3)
+            # invalid vector, so no calculated score and no score stored
+            self.assertEqual(None, finding.cvssv3)
+            self.assertEqual(None, finding.cvssv3_score)
 
         with self.subTest(i=5):
             # CVSS2 prefix makes it invalid
-            result = self.client.patch(self.url + "3/", data={"cvssv3": "CVSS:2.0/AV:N/AC:L/Au:N/C:P/I:P/A:P"})
+            result = self.client.patch(self.url + "3/", data={"cvssv3": "CVSS:2.0/AV:N/AC:L/Au:N/C:P/I:P/A:P", "cvssv3_score": 7})
             self.assertEqual(result.status_code, status.HTTP_400_BAD_REQUEST)
-            self.assertEqual(result.json()["cvssv3"], ["No CVSS vectors found by cvss.parse_cvss_from_text()"])
-            self.assertEqual(None, Finding.objects.get(id=3).cvssv3)
+            self.assertEqual(result.json()["cvssv3"], ["No valid CVSS vectors found by cvss.parse_cvss_from_text()"])
+            finding = Finding.objects.get(id=3)
+            # invalid vector, so no calculated score and no score stored
+            self.assertEqual(None, finding.cvssv3)
+            self.assertEqual(None, finding.cvssv3_score)
+
+        with self.subTest(i=6):
+            # try to put rubbish in there
+            result = self.client.patch(self.url + "4/", data={"cvssv3": "happy little vector", "cvssv3_score": 3})
+            self.assertEqual(result.status_code, status.HTTP_400_BAD_REQUEST)
+            self.assertEqual(result.json()["cvssv3"], ["No valid CVSS vectors found by cvss.parse_cvss_from_text()"])
+            finding = Finding.objects.get(id=4)
+            # invalid vector, so no calculated score and no score stored
+            self.assertEqual(None, finding.cvssv3)
+            self.assertEqual(None, finding.cvssv3_score)
+
+        with self.subTest(i=7):
+            # CVSS4 prefix makes it invalid
+            result = self.client.patch(self.url + "3/", data={"cvssv3": "CVSS:4.0/AV:N/AC:L/AT:N/PR:L/UI:N/S:U/C:H/I:H/A:H", "cvssv3_score": 7})
+            self.assertEqual(result.status_code, status.HTTP_400_BAD_REQUEST)
+            self.assertEqual(result.json()["cvssv3"], ["No valid CVSS vectors found by cvss.parse_cvss_from_text()"])
+            finding = Finding.objects.get(id=3)
+            # invalid vector, so no calculated score and no score stored
+            self.assertEqual(None, finding.cvssv3)
+            self.assertEqual(None, finding.cvssv3_score)
 
 
 class FindingMetadataTest(BaseClass.BaseClassTest):