From 848b3fcf74358c7ee87a0eb0fdec47542e52fb20 Mon Sep 17 00:00:00 2001 From: Cody Maffucci <46459665+Maffooch@users.noreply.github.com> Date: Fri, 30 Oct 2026 12:02:38 -0600 Subject: [PATCH 1/2] fix(findings-api): honor ?engagement=, ?product= and ?prod_type= on /api/v2/findings/ A Finding has no engagement, product or prod_type field and ApiFindingFilter declared no filter by those names, so django-filter dropped the parameters and the list answered with every finding the caller could see. Declare them as short names for the test__engagement, test__engagement__product and test__engagement__product__prod_type filters, and document them on the Findings pages. Co-Authored-By: Claude Opus 5.5 --- .../engagements_tests/OS__findings.md | 2 + .../engagements_tests/PRO__findings.md | 2 + dojo/finding/api/filters.py | 5 + .../test_api_finding_relation_filters.py | 124 ++++++++++++++++++ 4 files changed, 133 insertions(+) create mode 100644 unittests/test_api_finding_relation_filters.py diff --git a/docs/content/asset_modelling/engagements_tests/OS__findings.md b/docs/content/asset_modelling/engagements_tests/OS__findings.md index 61f1c4d0721..d4a02abc96e 100644 --- a/docs/content/asset_modelling/engagements_tests/OS__findings.md +++ b/docs/content/asset_modelling/engagements_tests/OS__findings.md @@ -28,6 +28,8 @@ Overall, Findings are designed to work with the Asset Hierarchy to standardize y Findings are accessible via the sidebar. The submenu provides access to Open and Closed Findings, All Findings (regardless of Open or Closed status), [Risk Accepted Findings](/triage_findings/findings_workflows/os__risk_acceptance/), as well as Findings Templates. Individual Findings are also accessible from within the Test that contains them. +Through the API, `/api/v2/findings/?engagement=` lists the Findings of one Engagement, `?product=` those of one Asset and `?prod_type=` those of one Organization. Each takes a comma-separated list of ids and combines with every other filter. The longer `test__engagement`, `test__engagement__product` and `test__engagement__product__prod_type` names do the same. + ![image](images/osfindings_ss1.png) ### Permissions diff --git a/docs/content/asset_modelling/engagements_tests/PRO__findings.md b/docs/content/asset_modelling/engagements_tests/PRO__findings.md index b01fdb6b183..d368bcb29f8 100644 --- a/docs/content/asset_modelling/engagements_tests/PRO__findings.md +++ b/docs/content/asset_modelling/engagements_tests/PRO__findings.md @@ -26,6 +26,8 @@ Overall, Findings are designed to work with the Asset Hierarchy to standardize y ## Accessing Findings Findings are accessible from the sidebar under **Act > Explore > Findings**. The submenu provides access to **Active** and **Mitigated** Findings, **All** Findings (regardless of Open or Closed status), Finding **Groups**, **Risk Accepted** Findings and Finding **Templates**; new Findings are created with the **New Finding** button at the top of a Findings list. Individual Findings are also accessible from within the Test that contains them. +Through the API, `/api/v2/findings/?engagement=` lists the Findings of one Engagement, `?product=` those of one Asset and `?prod_type=` those of one Organization. Each takes a comma-separated list of ids and combines with every other filter. The longer `test__engagement`, `test__engagement__product` and `test__engagement__product__prod_type` names do the same. + [Risk Accepted Findings](/triage_findings/findings_workflows/os__risk_acceptance/) are listed under **Act > Explore > Findings > Risk Accepted**; the Risk Acceptance records themselves are managed from **Act > Risk Acceptances** in the sidebar. ![image](images/profindings_ss1.png) diff --git a/dojo/finding/api/filters.py b/dojo/finding/api/filters.py index f09608fa137..446d51fe10f 100644 --- a/dojo/finding/api/filters.py +++ b/dojo/finding/api/filters.py @@ -117,6 +117,11 @@ class ApiFindingFilter(DojoFilter): test__engagement = NumberInFilter(field_name="test__engagement", lookup_expr="in") test__engagement__product = NumberInFilter(field_name="test__engagement__product", lookup_expr="in") test__engagement__product__prod_type = NumberInFilter(field_name="test__engagement__product__prod_type", lookup_expr="in") + # Short names for the three filters above. A Finding has no engagement / product / prod_type field, so + # without these django-filter dropped ?engagement= silently and listed every finding the caller can see. + engagement = NumberInFilter(field_name="test__engagement", lookup_expr="in", label="Engagement") + product = NumberInFilter(field_name="test__engagement__product", lookup_expr="in", label=labels.ASSET_LABEL) + prod_type = NumberInFilter(field_name="test__engagement__product__prod_type", lookup_expr="in", label=labels.ORG_LABEL) finding_group = NumberInFilter(method="filter_finding_group") # ExistsRiskAcceptanceFilter diff --git a/unittests/test_api_finding_relation_filters.py b/unittests/test_api_finding_relation_filters.py new file mode 100644 index 00000000000..28328b3d3f4 --- /dev/null +++ b/unittests/test_api_finding_relation_filters.py @@ -0,0 +1,124 @@ +from django.urls import reverse +from django.utils.timezone import now +from rest_framework.test import APIClient + +from dojo.filters import ApiFindingFilter +from dojo.models import ( + Dojo_User, + Engagement, + Finding, + Product, + Product_Type, + Test, + Test_Type, +) + +from .dojo_test_case import DojoTestCase + + +class TestApiFindingRelationFilters(DojoTestCase): + + """ + Regression: ?engagement= on /api/v2/findings/ was not a declared filter. A Finding has no + engagement field, so django-filter dropped the parameter without an error and the list answered + with every finding the caller could see. engagement, product and prod_type are now short names + for the test__engagement, test__engagement__product and test__engagement__product__prod_type + filters. + """ + + @classmethod + def setUpTestData(cls): + cls.user = Dojo_User.objects.create(username="rel_filter_user", is_staff=True, is_superuser=True) + cls.test_type = Test_Type.objects.create(name="Relation Filter Test Type") + cls.type_a = Product_Type.objects.create(name="Relation Filter PT A") + cls.type_b = Product_Type.objects.create(name="Relation Filter PT B") + cls.product_a = Product.objects.create(name="Relation Filter A", prod_type=cls.type_a, description="a") + cls.product_b = Product.objects.create(name="Relation Filter B", prod_type=cls.type_b, description="b") + cls.findings = {} + # Two engagements on product A, so engagement and product answer different sets. + for key, product, severities in ( + ("a1", cls.product_a, ["High", "High", "Low"]), + ("a2", cls.product_a, ["High", "Medium"]), + ("b", cls.product_b, ["High", "Critical", "Low", "Info"]), + ): + engagement = Engagement.objects.create( + name=f"Relation Filter {key}", product=product, target_start=now(), target_end=now(), + ) + setattr(cls, f"eng_{key}", engagement) + test = Test.objects.create( + engagement=engagement, test_type=cls.test_type, target_start=now(), target_end=now(), + ) + cls.findings[key] = { + Finding.objects.create( + title=f"Relation Filter {key} {index}", test=test, reporter=cls.user, severity=severity, + ).id + for index, severity in enumerate(severities) + } + cls.scope = Finding.objects.filter(test__engagement__product__in=[cls.product_a, cls.product_b]) + + def _filtered(self, data): + filterset = ApiFindingFilter(data=data, queryset=self.scope) + self.assertTrue(filterset.is_valid(), msg=filterset.errors) + return set(filterset.qs.values_list("id", flat=True)) + + def test_short_names_are_declared_on_the_long_form_fields(self): + for alias, field_name in ( + ("engagement", "test__engagement"), + ("product", "test__engagement__product"), + ("prod_type", "test__engagement__product__prod_type"), + ): + with self.subTest(alias=alias): + declared = ApiFindingFilter.base_filters.get(alias) + self.assertIsNotNone(declared, msg=f"{alias} is not declared, so django-filter drops it") + self.assertEqual(declared.field_name, field_name) + self.assertEqual(declared.lookup_expr, "in") + + def test_engagement(self): + self.assertEqual(self._filtered({"engagement": str(self.eng_a1.id)}), self.findings["a1"]) + self.assertEqual( + self._filtered({"engagement": f"{self.eng_a1.id},{self.eng_b.id}"}), + self.findings["a1"] | self.findings["b"], + ) + + def test_product(self): + self.assertEqual( + self._filtered({"product": str(self.product_a.id)}), self.findings["a1"] | self.findings["a2"], + ) + + def test_prod_type(self): + self.assertEqual(self._filtered({"prod_type": str(self.type_b.id)}), self.findings["b"]) + + def test_short_names_match_the_long_forms(self): + for alias, long_form, value in ( + ("engagement", "test__engagement", self.eng_a2.id), + ("product", "test__engagement__product", self.product_b.id), + ("prod_type", "test__engagement__product__prod_type", self.type_a.id), + ): + with self.subTest(alias=alias): + self.assertEqual(self._filtered({alias: str(value)}), self._filtered({long_form: str(value)})) + + def test_combines_with_other_filters(self): + high_a1 = set(self.scope.filter(id__in=self.findings["a1"], severity="High").values_list("id", flat=True)) + self.assertEqual(self._filtered({"engagement": str(self.eng_a1.id), "severity": "High"}), high_a1) + self.assertEqual(self._filtered({"engagement": str(self.eng_b.id), "product": str(self.product_a.id)}), set()) + + def test_non_numeric_value_is_invalid(self): + filterset = ApiFindingFilter(data={"engagement": "abc"}, queryset=self.scope) + self.assertFalse(filterset.is_valid()) + self.assertIn("engagement", filterset.errors) + + def test_api_list_is_scoped_and_counted(self): + client = APIClient() + client.force_authenticate(user=self.user) + url = reverse("finding-list") + page1 = client.get(url, {"engagement": self.eng_b.id, "limit": 3}, secure=True) + page2 = client.get(url, {"engagement": self.eng_b.id, "limit": 3, "offset": 3}, secure=True) + self.assertEqual(page1.status_code, 200, page1.content[:500]) + self.assertEqual(page1.data["count"], 4) + self.assertEqual(page2.data["count"], 4) + self.assertIsNone(page2.data["next"]) + ids = {f["id"] for f in page1.data["results"]} | {f["id"] for f in page2.data["results"]} + self.assertEqual(ids, self.findings["b"]) + # A parameter that is not a filter is still ignored rather than rejected. + unknown = client.get(url, {"engagement": self.eng_b.id, "not_a_filter": "1", "limit": 100}, secure=True) + self.assertEqual({f["id"] for f in unknown.data["results"]}, self.findings["b"]) From e49d5b5d62bc90a8d442b94f553d7957431883a8 Mon Sep 17 00:00:00 2001 From: Cody Maffucci <46459665+Maffooch@users.noreply.github.com> Date: Fri, 30 Oct 2026 14:58:21 -0600 Subject: [PATCH 2/2] test(findings-api): pin that the short names do not widen for a product member; note the change in the 3.5 upgrade page Co-Authored-By: Claude Opus 5.5 --- docs/content/releases/os_upgrading/3.5.md | 8 ++++-- .../test_api_finding_relation_filters.py | 26 +++++++++++++++++++ 2 files changed, 32 insertions(+), 2 deletions(-) diff --git a/docs/content/releases/os_upgrading/3.5.md b/docs/content/releases/os_upgrading/3.5.md index a65adf278f0..5b298097dda 100644 --- a/docs/content/releases/os_upgrading/3.5.md +++ b/docs/content/releases/os_upgrading/3.5.md @@ -2,6 +2,10 @@ title: 'Upgrading to DefectDojo Version 3.5.x' toc_hide: true weight: -20261005 -description: No special instructions. +description: Findings API engagement, product and prod_type filters now apply. --- -There are no special instructions for upgrading to 3.5.x. Check the [Release Notes](https://github.com/DefectDojo/django-DefectDojo/releases/tag/3.5.0) for the contents of the release. +There are no special instructions for upgrading to 3.5.x beyond the API change below. Check the [Release Notes](https://github.com/DefectDojo/django-DefectDojo/releases/tag/3.5.0) for the contents of the release. + +## Findings API: `engagement`, `product` and `prod_type` filters now apply + +Before 3.5.x, `/api/v2/findings/` ignored the `engagement`, `product` and `prod_type` query parameters and returned every Finding the caller could see. They are now applied, as short names for `test__engagement`, `test__engagement__product` and `test__engagement__product__prod_type`. A client that already sends them gets only the Findings of that Engagement, Asset or Organization, so expect a smaller `count` and different pages. A non-numeric value is now rejected with a 400. Clients that use the `test__engagement*` parameters see no change. diff --git a/unittests/test_api_finding_relation_filters.py b/unittests/test_api_finding_relation_filters.py index 28328b3d3f4..6155b15f7bf 100644 --- a/unittests/test_api_finding_relation_filters.py +++ b/unittests/test_api_finding_relation_filters.py @@ -2,13 +2,16 @@ from django.utils.timezone import now from rest_framework.test import APIClient +from dojo.authorization.roles_permissions import Roles from dojo.filters import ApiFindingFilter from dojo.models import ( Dojo_User, Engagement, Finding, Product, + Product_Member, Product_Type, + Role, Test, Test_Type, ) @@ -122,3 +125,26 @@ def test_api_list_is_scoped_and_counted(self): # A parameter that is not a filter is still ignored rather than rejected. unknown = client.get(url, {"engagement": self.eng_b.id, "not_a_filter": "1", "limit": 100}, secure=True) self.assertEqual({f["id"] for f in unknown.data["results"]}, self.findings["b"]) + + def test_api_does_not_widen_for_a_member_of_one_product(self): + # A Reader of product A only. The short names narrow what the caller may already see, so asking for + # product B's engagement, product or product type is an empty page, while A's own ids still answer. + reader = Dojo_User.objects.create(username="rel_filter_reader") + Product_Member.objects.create(user=reader, product=self.product_a, role=Role.objects.get(id=Roles.Reader)) + self.product_a.authorized_users.add(reader) + client = APIClient() + client.force_authenticate(user=reader) + url = reverse("finding-list") + for params in ( + {"engagement": self.eng_b.id}, + {"product": self.product_b.id}, + {"prod_type": self.type_b.id}, + ): + with self.subTest(params=params): + response = client.get(url, {**params, "limit": 100}, secure=True) + self.assertEqual(response.status_code, 200, response.content[:500]) + self.assertEqual(response.data["count"], 0) + self.assertEqual(response.data["results"], []) + own = client.get(url, {"engagement": self.eng_a2.id, "limit": 100}, secure=True) + self.assertEqual(own.data["count"], len(self.findings["a2"])) + self.assertEqual({f["id"] for f in own.data["results"]}, self.findings["a2"])