fix tests, cleanup

Author: valentijnscholten

.gitignore

@@ -41,6 +41,7 @@ pip-delete-this-directory.txt
 .ruff_cache
 nosetests.xml
 coverage.xml
+selenium_page_source.html
 
 # Translations
 *.mo

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")
@@ -1838,8 +1839,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:
@@ -1969,8 +1968,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:
@@ -1997,7 +1994,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)
 
@@ -2026,8 +2022,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,11 +112,8 @@ 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")
-        # cvssv3
-        driver.find_element(By.ID, "id_cvssv3").send_keys("CVSS:3.0/AV:N/AC:L/PR:L/UI:N/S:U/C:H/I:H/A:H")
         # finding Vulnerability Ids
         driver.find_element(By.ID, "id_vulnerability_ids").send_keys("\nREF-3\nREF-4\n")
         # "Click" the Done button to Edit the finding
@@ -131,6 +128,123 @@ def test_edit_finding(self):
         self.assertTrue(self.is_text_present_on_page(text="REF-4"))
         self.assertTrue(self.is_text_present_on_page(text="Additional Vulnerability Ids"))
 
+    def _edit_finding_cvssv3_and_assert(
+        self,
+        cvssv3_value,
+        cvssv3_score,
+        expected_cvssv3_value,
+        expected_cvssv3_score,
+        expect_success=True,  # noqa: FBT002
+        success_message="Finding saved successfully",
+        error_message=None,
+    ):
+        driver = self.driver
+        # Navigate to All Finding page
+        self.goto_all_findings_list(driver)
+        # Select and click on the particular finding to edit
+        driver.find_element(By.LINK_TEXT, "App Vulnerable to XSS").click()
+        # Click on the 'dropdownMenu1 button'
+        driver.find_element(By.ID, "dropdownMenu1").click()
+        # Click on `Edit Finding`
+        driver.find_element(By.LINK_TEXT, "Edit Finding").click()
+        # Set cvssv3 value and score
+        driver.find_element(By.ID, "id_cvssv3").clear()
+        driver.find_element(By.ID, "id_cvssv3").send_keys(cvssv3_value)
+        driver.find_element(By.ID, "id_cvssv3_score").clear()
+        driver.find_element(By.ID, "id_cvssv3_score").send_keys(str(cvssv3_score))
+        # Submit the form
+        driver.find_element(By.XPATH, "//input[@name='_Finished']").click()
+
+        if expect_success:
+            self.assertTrue(self.is_success_message_present(text=success_message))
+            # Go into edit mode again to check stored values
+            driver.find_element(By.ID, "dropdownMenu1").click()
+            driver.find_element(By.LINK_TEXT, "Edit Finding").click()
+            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"))
+        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
+    @on_exception_html_source_logger
+    def test_edit_finding_cvssv3_valid_vector(self):
+        self._edit_finding_cvssv3_and_assert(
+            cvssv3_value="CVSS:3.0/AV:N/AC:L/PR:L/UI:N/S:U/C:H/I:H/A:H",
+            cvssv3_score="1",
+            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="8.8",
+            expect_success=True,
+        )
+
+    @on_exception_html_source_logger
+    def test_edit_finding_cvssv3_valid_vector_no_prefix(self):
+        self._edit_finding_cvssv3_and_assert(
+            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",
+            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_valid_vector_with_trailing_slash(self):
+        self._edit_finding_cvssv3_and_assert(
+            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",
+            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_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",
+            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="5",
+            expected_cvssv3_value="happy little vector",
+            expected_cvssv3_score="5",
+            expect_success=False,
+            error_message="No valid CVSS vectors found by cvss.parse_cvss_from_text()",
+        )
+
     def test_add_image(self):
         # The Name of the Finding created by test_add_product_finding => 'App Vulnerable to XSS'
         # Test To Add Finding To product
@@ -519,6 +633,13 @@ def add_finding_tests_to_suite(suite, *, jira=False, github=False, block_executi
     suite.addTest(FindingTest("test_excel_export"))
     suite.addTest(FindingTest("test_list_components"))
     suite.addTest(FindingTest("test_edit_finding"))
+    suite.addTest(FindingTest("test_edit_finding_cvssv3_valid_vector"))
+    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"))
     suite.addTest(FindingTest("test_delete_image"))

unittests/test_finding_model.py

@@ -8,6 +8,7 @@
 
 
 class TestFindingModel(DojoTestCase):
+    fixtures = ["dojo_testdata.json"]
 
     def test_get_sast_source_file_path_with_link_no_file_path(self):
         finding = Finding()
@@ -315,6 +316,46 @@ def test_get_references_with_links_markdown(self):
         finding.references = "URL: [https://www.example.com](https://www.example.com)"
         self.assertEqual("URL: [https://www.example.com](https://www.example.com)", finding.get_references_with_links())
 
+    # 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")
+        product_type = self.create_product_type("test_product_type")
+        product = self.create_product(name="test_product", prod_type=product_type)
+        engagement = self.create_engagement("test_eng", product)
+        test = self.create_test(engagement=engagement, scan_type="ZAP Scan", title="test_test")
+        finding = Finding.objects.create(
+            test=test,
+            reporter=user,
+            title="test_finding",
+            severity="Critical",
+            date=datetime.now().date())
+        finding.cvssv3 = "CVSS:3.1/AV:N/AC:L/PR:N/UI:N/S:U/C:H/I:H/A:H"
+        finding.save()
+
+        self.assertEqual(finding.cvssv3, "CVSS:3.1/AV:N/AC:L/PR:N/UI:N/S:U/C:H/I:H/A:H")
+        self.assertEqual(finding.cvssv3_score, 9.8)
+        finding_id = finding.id
+
+        finding = Finding.objects.get(id=finding_id)
+        finding.cvssv3 = "AV:N/AC:L/PR:L/UI:N/S:U/C:H/I:H/A:H"
+        finding.save()
+
+        self.assertEqual(finding.cvssv3, "AV:N/AC:L/PR:L/UI:N/S:U/C:H/I:H/A:H")
+        # invalid vector, so score still 9.8 from previous save (and not 8.8)
+        self.assertEqual(finding.cvssv3_score, 9.8)
+
+        finding = Finding.objects.get(id=finding_id)
+        finding.cvssv3 = "happy little vector"
+        finding.save()
+
+        self.assertEqual(finding.cvssv3, "happy little vector")
+        # invalid vector, so score still 9.8 from previous save
+        self.assertEqual(finding.cvssv3_score, 9.8)
+
+        # we already have more cvssv3 test in test_rest_framework.py and finding_test.py
+        # the above here only shows that invalid vectors can be saved
+
 
 class TestFindingSLAExpiration(DojoTestCase):
     fixtures = ["dojo_testdata.json"]