fix: risk acceptance proof download throws 500 (#14478)

valentijnscholten · mtesauro · web-flow · commit ea86b72c2661 · 2026-03-12T18:37:20.000+01:00
* fix: use actual field value instead of string literal for risk acceptance proof download (#14467) The path was wrapped in quotes, causing Django to open a file literally named "risk_acceptance.path.name" instead of the uploaded proof file. Also adds a selenium test for the proof download flow. * fix: find proof download link directly on engagement page The risk acceptance download link ("Yes") is in the RA table on the engagement detail page. No need to navigate to the RA detail page first. * fix: rewrite risk acceptance selenium tests that were silently broken The existing risk acceptance tests were silently passing despite never actually creating a risk acceptance. Key fixes: - Navigate to Ad Hoc Engagement (where test findings live) instead of Beta Test - Make bootstrap-select hidden native <select> visible for Selenium interaction - Upload a proof file to enable download regression test for #14467 - Add test_download_risk_acceptance_proof to verify no 500 error on download - Tighten assertions so failures are no longer masked --------- Co-authored-by: Matt Tesauro <mtesauro@gmail.com>
diff --git a/dojo/engagement/views.py b/dojo/engagement/views.py
@@ -1580,7 +1580,7 @@ def download_risk_acceptance(request, eid, raid):
         raise PermissionDenied
     response = StreamingHttpResponse(
         FileIterWrapper(
-            (Path(settings.MEDIA_ROOT) / "risk_acceptance.path.name").open(mode="rb")))
+            (Path(settings.MEDIA_ROOT) / risk_acceptance.path.name).open(mode="rb")))
     response["Content-Disposition"] = f'attachment; filename="{risk_acceptance.filename()}"'
     mimetype, _encoding = mimetypes.guess_type(risk_acceptance.path.name)
     response["Content-Type"] = mimetype or "application/octet-stream"
diff --git a/tests/risk_acceptance_test.py b/tests/risk_acceptance_test.py
@@ -1,6 +1,8 @@
+import os
 import sys
 import time
 import unittest
+from pathlib import Path
 
 from base_test_class import BaseTestCase, on_exception_html_source_logger, set_suite_settings
 from product_test import ProductTest
@@ -9,36 +11,6 @@
 
 class RiskAcceptanceTest(BaseTestCase):
 
-    def _get_engagement_url(self, driver):
-        """Navigate to the Beta Test engagement via the all engagements list."""
-        self.goto_all_engagements_overview(driver)
-        time.sleep(1)
-        # Find the Beta Test engagement link in the table
-        eng_links = driver.find_elements(By.LINK_TEXT, "Beta Test")
-        if len(eng_links) > 0:
-            eng_links[0].click()
-            time.sleep(1)
-            return driver.current_url
-        # Fallback: try partial link text
-        eng_links = driver.find_elements(By.PARTIAL_LINK_TEXT, "Beta")
-        if len(eng_links) > 0:
-            eng_links[0].click()
-            time.sleep(1)
-            return driver.current_url
-        return None
-
-    def _get_engagement_id(self, driver):
-        """Get the engagement ID from the current engagement page URL."""
-        eng_url = self._get_engagement_url(driver)
-        if eng_url is None:
-            return None
-        # URL pattern: .../engagement/<id>
-        parts = eng_url.rstrip("/").split("/")
-        for i, part in enumerate(parts):
-            if part == "engagement" and i + 1 < len(parts):
-                return parts[i + 1]
-        return parts[-1]
-
     @on_exception_html_source_logger
     def test_enable_full_risk_acceptance(self):
         """Enable full risk acceptance on the QA Test product."""
@@ -48,7 +20,6 @@ def test_enable_full_risk_acceptance(self):
         driver.find_element(By.ID, "dropdownMenu1").click()
         driver.find_element(By.LINK_TEXT, "Edit").click()
         time.sleep(1)
-        # Enable full risk acceptance checkbox
         checkbox = driver.find_element(By.ID, "id_enable_full_risk_acceptance")
         if not checkbox.is_selected():
             checkbox.click()
@@ -61,80 +32,95 @@ def test_enable_full_risk_acceptance(self):
 
     @on_exception_html_source_logger
     def test_add_risk_acceptance(self):
-        """Add a risk acceptance to the engagement."""
+        """Add a risk acceptance with proof to the Ad Hoc Engagement."""
         driver = self.driver
-        eid = self._get_engagement_id(driver)
-        self.assertIsNotNone(eid, "Could not find Beta Test engagement")
-        driver.get(self.base_url + f"engagement/{eid}/risk_acceptance/add")
+        # Navigate to the Ad Hoc Engagement (where test_add_product_finding creates its finding)
+        self.goto_all_engagements_overview(driver)
+        time.sleep(1)
+        driver.find_element(By.LINK_TEXT, "Ad Hoc Engagement").click()
+        time.sleep(1)
+        # Click "Add Risk Acceptance" from the engagement page
+        driver.find_element(By.CSS_SELECTOR, "a[href*='risk_acceptance/add']").click()
         time.sleep(2)
         # Fill the risk acceptance form
         name_field = driver.find_element(By.ID, "id_name")
         name_field.clear()
         name_field.send_keys("Test Risk Acceptance")
-        # Select an accepted finding if available
-        findings_select = driver.find_elements(By.ID, "id_accepted_findings")
-        if len(findings_select) > 0:
-            options = findings_select[0].find_elements(By.TAG_NAME, "option")
-            if len(options) > 0:
-                options[0].click()
-        # Radio buttons for recommendation and decision - click Accept options
+        # Select an accepted finding - the native <select> is hidden by bootstrap-select
+        driver.execute_script("document.getElementById('id_accepted_findings').style.display = 'inline'")
+        findings_select = driver.find_element(By.ID, "id_accepted_findings")
+        options = findings_select.find_elements(By.TAG_NAME, "option")
+        self.assertTrue(len(options) > 0, "No findings available for risk acceptance")
+        options[0].click()
+        # Radio buttons for recommendation and decision
         rec_radios = driver.find_elements(By.NAME, "recommendation")
         if len(rec_radios) > 0:
             rec_radios[0].click()
         dec_radios = driver.find_elements(By.NAME, "decision")
         if len(dec_radios) > 0:
             dec_radios[0].click()
+        # Upload a proof file
+        proof_path = Path(os.path.realpath(__file__)).parent / "dedupe_scans" / "dedupe_path_1.json"
+        driver.find_element(By.ID, "id_path").send_keys(str(proof_path))
         # Owner is pre-filled (current user), submit
         driver.find_element(By.CSS_SELECTOR, "input.btn.btn-primary").click()
         time.sleep(1)
 
         self.assertTrue(
-            self.is_success_message_present(text="Risk acceptance saved")
-            or self.is_text_present_on_page(text="Risk Acceptance")
-            or self.is_text_present_on_page(text="Engagement"),
+            self.is_success_message_present(text="Risk acceptance saved"),
+            "Risk acceptance was not saved",
         )
 
     @on_exception_html_source_logger
     def test_view_risk_acceptance(self):
-        """View a risk acceptance from the engagement."""
+        """View a risk acceptance from the Ad Hoc Engagement."""
         driver = self.driver
-        eng_url = self._get_engagement_url(driver)
-        self.assertIsNotNone(eng_url, "Could not find Beta Test engagement")
+        self.goto_all_engagements_overview(driver)
         time.sleep(1)
-        # Look for risk acceptance links on the engagement page
+        driver.find_element(By.LINK_TEXT, "Ad Hoc Engagement").click()
+        time.sleep(1)
+        # Click on the risk acceptance name link
         risk_links = driver.find_elements(By.PARTIAL_LINK_TEXT, "Test Risk Acceptance")
-        if len(risk_links) > 0:
-            risk_links[0].click()
-            time.sleep(1)
-            self.assertTrue(
-                self.is_text_present_on_page(text="Risk Acceptance")
-                or self.is_text_present_on_page(text="Test Risk Acceptance"),
-            )
-        else:
-            # Risk acceptance may be listed differently
-            self.assertFalse(self.is_error_message_present())
+        self.assertTrue(len(risk_links) > 0, "Could not find Test Risk Acceptance link")
+        risk_links[0].click()
+        time.sleep(1)
+        self.assertTrue(
+            self.is_text_present_on_page(text="Risk Acceptance")
+            or self.is_text_present_on_page(text="Test Risk Acceptance"),
+        )
+
+    @on_exception_html_source_logger
+    def test_download_risk_acceptance_proof(self):
+        """Download the proof file from a risk acceptance (regression test for #14467)."""
+        driver = self.driver
+        # Navigate to the Ad Hoc Engagement where the RA was created
+        self.goto_all_engagements_overview(driver)
+        time.sleep(1)
+        driver.find_element(By.LINK_TEXT, "Ad Hoc Engagement").click()
+        time.sleep(1)
+        # Click the proof download link ("Yes") in the Risk Acceptance table
+        download_links = driver.find_elements(By.CSS_SELECTOR, "a[href*='risk_acceptance'][href*='download']")
+        self.assertTrue(len(download_links) > 0, "Could not find proof download link on engagement page")
+        download_links[0].click()
+        time.sleep(2)
+        # Verify no 500 error occurred
+        self.assertFalse(self.is_error_message_present())
+        body_text = driver.find_element(By.TAG_NAME, "body").text
+        self.assertNotIn("Internal Server Error", body_text)
 
     @on_exception_html_source_logger
     def test_delete_risk_acceptance(self):
-        """Delete a risk acceptance."""
+        """Delete a risk acceptance from the Ad Hoc Engagement."""
         driver = self.driver
-        eng_url = self._get_engagement_url(driver)
-        self.assertIsNotNone(eng_url, "Could not find Beta Test engagement")
+        self.goto_all_engagements_overview(driver)
         time.sleep(1)
-        # Find delete links for risk acceptances
-        delete_links = driver.find_elements(By.CSS_SELECTOR, "a[href*='risk_acceptance'][href*='delete']")
-        if len(delete_links) > 0:
-            delete_links[0].click()
-            time.sleep(1)
-            # Confirm delete - the URL pattern is a GET request that performs delete
-            # Check if we're on a confirmation page
-            confirm_btns = driver.find_elements(By.CSS_SELECTOR, "input.btn.btn-danger")
-            if len(confirm_btns) > 0:
-                confirm_btns[0].click()
-            else:
-                button_btns = driver.find_elements(By.CSS_SELECTOR, "button.btn.btn-danger")
-                if len(button_btns) > 0:
-                    button_btns[0].click()
+        driver.find_element(By.LINK_TEXT, "Ad Hoc Engagement").click()
+        time.sleep(1)
+        # Find delete form for risk acceptance (uses POST via hidden form)
+        delete_forms = driver.find_elements(By.CSS_SELECTOR, "form[id^='delete-risk_acceptance-form']")
+        if len(delete_forms) > 0:
+            # The delete link triggers the form via JS; submit the form directly
+            driver.execute_script("arguments[0].submit()", delete_forms[0])
             time.sleep(1)
             self.assertTrue(
                 self.is_success_message_present(text="Risk acceptance deleted successfully")
@@ -154,6 +140,7 @@ def suite():
     suite.addTest(RiskAcceptanceTest("test_enable_full_risk_acceptance"))
     suite.addTest(RiskAcceptanceTest("test_add_risk_acceptance"))
     suite.addTest(RiskAcceptanceTest("test_view_risk_acceptance"))
+    suite.addTest(RiskAcceptanceTest("test_download_risk_acceptance_proof"))
     suite.addTest(RiskAcceptanceTest("test_delete_risk_acceptance"))
     suite.addTest(ProductTest("test_delete_product"))
     return suite
