diff --git a/api/CHANGELOG.md b/api/CHANGELOG.md index 90480975be..3edcd8e215 100644 --- a/api/CHANGELOG.md +++ b/api/CHANGELOG.md @@ -23,6 +23,7 @@ All notable changes to the **Prowler API** are documented in this file. - Finding groups muted filter, counters, metadata extraction and mute reaggregation [(#10477)](https://github.com/prowler-cloud/prowler/pull/10477) - Finding groups `check_title__icontains` resolution, `name__icontains` resource filter and `resource_group` field in `/resources` response [(#10486)](https://github.com/prowler-cloud/prowler/pull/10486) - Membership `post_delete` signal using raw FK ids to avoid `DoesNotExist` during cascade deletions [(#10497)](https://github.com/prowler-cloud/prowler/pull/10497) +- Finding group resources endpoints returning false 404 when filters match no results, and `sort` parameter being ignored [(#10510)](https://github.com/prowler-cloud/prowler/pull/10510) ### 🔐 Security diff --git a/api/src/backend/api/tests/test_views.py b/api/src/backend/api/tests/test_views.py index 49026a1cd6..9aad2bcff6 100644 --- a/api/src/backend/api/tests/test_views.py +++ b/api/src/backend/api/tests/test_views.py @@ -16011,6 +16011,191 @@ class TestFindingGroupViewSet: # Should still return the 2 resources within the date range assert len(response.json()["data"]) == 2 + def test_resources_status_filter_returns_empty_not_404( + self, authenticated_client, finding_groups_fixture + ): + """Test that filtering by status on a valid check returns empty list, not 404.""" + # s3_bucket_public_access has only FAIL findings, filtering by PASS should return [] + response = authenticated_client.get( + reverse( + "finding-group-resources", kwargs={"pk": "s3_bucket_public_access"} + ), + {"filter[inserted_at]": TODAY, "filter[status]": "PASS"}, + ) + assert response.status_code == status.HTTP_200_OK + assert response.json()["data"] == [] + + def test_resources_nonexistent_check_still_404( + self, authenticated_client, finding_groups_fixture + ): + """Test that a truly nonexistent check_id still returns 404.""" + response = authenticated_client.get( + reverse("finding-group-resources", kwargs={"pk": "totally_fake_check"}), + {"filter[inserted_at]": TODAY}, + ) + assert response.status_code == status.HTTP_404_NOT_FOUND + + def test_resources_sort_by_status_ascending( + self, authenticated_client, finding_groups_fixture + ): + """Test sort=status returns PASS before FAIL.""" + # ec2_instance_public_ip has 1 PASS (resource1) and 1 FAIL (resource2) + response = authenticated_client.get( + reverse( + "finding-group-resources", + kwargs={"pk": "ec2_instance_public_ip"}, + ), + {"filter[inserted_at]": TODAY, "sort": "status"}, + ) + assert response.status_code == status.HTTP_200_OK + data = response.json()["data"] + assert len(data) == 2 + assert data[0]["attributes"]["status"] == "PASS" + assert data[1]["attributes"]["status"] == "FAIL" + + def test_resources_sort_by_status_descending( + self, authenticated_client, finding_groups_fixture + ): + """Test sort=-status returns FAIL before PASS.""" + response = authenticated_client.get( + reverse( + "finding-group-resources", + kwargs={"pk": "ec2_instance_public_ip"}, + ), + {"filter[inserted_at]": TODAY, "sort": "-status"}, + ) + assert response.status_code == status.HTTP_200_OK + data = response.json()["data"] + assert len(data) == 2 + assert data[0]["attributes"]["status"] == "FAIL" + assert data[1]["attributes"]["status"] == "PASS" + + def test_resources_sort_invalid_field_returns_400( + self, authenticated_client, finding_groups_fixture + ): + """Test that an invalid sort field returns 400.""" + response = authenticated_client.get( + reverse( + "finding-group-resources", kwargs={"pk": "s3_bucket_public_access"} + ), + {"filter[inserted_at]": TODAY, "sort": "invalid_field"}, + ) + assert response.status_code == status.HTTP_400_BAD_REQUEST + + def test_latest_resources_status_filter_returns_empty_not_404( + self, authenticated_client, finding_groups_fixture + ): + """Test latest resources with status filter on valid check returns empty, not 404.""" + response = authenticated_client.get( + reverse( + "finding-group-latest_resources", + kwargs={"check_id": "s3_bucket_public_access"}, + ), + {"filter[status]": "PASS"}, + ) + assert response.status_code == status.HTTP_200_OK + assert response.json()["data"] == [] + + def test_latest_resources_sort_by_status( + self, authenticated_client, finding_groups_fixture + ): + """Test latest resources sort=status returns PASS before FAIL.""" + response = authenticated_client.get( + reverse( + "finding-group-latest_resources", + kwargs={"check_id": "ec2_instance_public_ip"}, + ), + {"sort": "status"}, + ) + assert response.status_code == status.HTTP_200_OK + data = response.json()["data"] + assert len(data) == 2 + assert data[0]["attributes"]["status"] == "PASS" + assert data[1]["attributes"]["status"] == "FAIL" + + def test_resources_nonexistent_check_missing_date_returns_400( + self, authenticated_client, finding_groups_fixture + ): + """Nonexistent check_id with missing required date filter returns 400, not 404.""" + response = authenticated_client.get( + reverse("finding-group-resources", kwargs={"pk": "totally_fake_check"}), + ) + # FindingGroupFilter requires inserted_at — validation fires before existence check + assert response.status_code == status.HTTP_400_BAD_REQUEST + + def test_resources_nonexistent_check_invalid_sort_returns_400( + self, authenticated_client, finding_groups_fixture + ): + """Nonexistent check_id with invalid sort returns 400, not 404.""" + response = authenticated_client.get( + reverse("finding-group-resources", kwargs={"pk": "totally_fake_check"}), + {"filter[inserted_at]": TODAY, "sort": "invalid_field"}, + ) + assert response.status_code == status.HTTP_400_BAD_REQUEST + + def test_resources_empty_sort_falls_back_to_default_order( + self, authenticated_client, finding_groups_fixture + ): + """Degenerate sort values should behave like no sort, not raise 500.""" + all_ids = set() + for page_num in (1, 2): + response = authenticated_client.get( + reverse( + "finding-group-resources", + kwargs={"pk": "s3_bucket_public_access"}, + ), + { + "filter[inserted_at]": TODAY, + "sort": ",", + "page[size]": 1, + "page[number]": page_num, + }, + ) + assert response.status_code == status.HTTP_200_OK + data = response.json()["data"] + assert len(data) == 1 + all_ids.add(data[0]["id"]) + assert len(all_ids) == 2 + + def test_resources_sort_pagination_stability( + self, authenticated_client, finding_groups_fixture + ): + """Sort with small page size returns all resources without duplicates or gaps.""" + # s3_bucket_public_access has 2 resources, both FAIL — they tie on status + all_ids = set() + for page_num in (1, 2): + response = authenticated_client.get( + reverse( + "finding-group-resources", + kwargs={"pk": "s3_bucket_public_access"}, + ), + { + "filter[inserted_at]": TODAY, + "sort": "status", + "page[size]": 1, + "page[number]": page_num, + }, + ) + assert response.status_code == status.HTTP_200_OK + data = response.json()["data"] + assert len(data) == 1 + all_ids.add(data[0]["id"]) + # Both pages should return different resources (no duplicates) + assert len(all_ids) == 2 + + def test_latest_resources_nonexistent_check_invalid_sort_returns_400( + self, authenticated_client, finding_groups_fixture + ): + """Nonexistent check_id with invalid sort on latest returns 400, not 404.""" + response = authenticated_client.get( + reverse( + "finding-group-latest_resources", + kwargs={"check_id": "totally_fake_check"}, + ), + {"sort": "invalid_field"}, + ) + assert response.status_code == status.HTTP_400_BAD_REQUEST + # Test provider_id filter actually filters data def test_finding_groups_provider_id_filter_actually_filters( self, authenticated_client, finding_groups_fixture, providers_fixture diff --git a/api/src/backend/api/v1/views.py b/api/src/backend/api/v1/views.py index 7d5beeb66a..fc5b1a9245 100644 --- a/api/src/backend/api/v1/views.py +++ b/api/src/backend/api/v1/views.py @@ -7103,23 +7103,40 @@ class FindingGroupViewSet(BaseRLSViewSet): return results - def _validate_sort_fields(self, sort_param): - """Validate and map JSON:API sort fields for aggregated finding groups.""" - sort_field_map = { - "check_id": "check_id", - "check_title": "check_title", - "severity": "severity_order", - "fail_count": "fail_count", - "pass_count": "pass_count", - "muted_count": "muted_count", - "new_count": "new_count", - "changed_count": "changed_count", - "resources_total": "resources_total", - "resources_fail": "resources_fail", - "first_seen_at": "agg_first_seen_at", - "last_seen_at": "agg_last_seen_at", - "failing_since": "agg_failing_since", - } + _FINDING_GROUP_SORT_MAP = { + "check_id": "check_id", + "check_title": "check_title", + "severity": "severity_order", + "fail_count": "fail_count", + "pass_count": "pass_count", + "muted_count": "muted_count", + "new_count": "new_count", + "changed_count": "changed_count", + "resources_total": "resources_total", + "resources_fail": "resources_fail", + "first_seen_at": "agg_first_seen_at", + "last_seen_at": "agg_last_seen_at", + "failing_since": "agg_failing_since", + } + + _RESOURCE_SORT_MAP = { + "status": "status_order", + "severity": "severity_order", + "first_seen_at": "first_seen_at", + "last_seen_at": "last_seen_at", + "resource.uid": "resource_uid", + "resource.name": "resource_name", + "resource.region": "resource_region", + "resource.service": "resource_service", + "resource.type": "resource_type", + "provider.uid": "provider_uid", + "provider.alias": "provider_alias", + } + + def _validate_sort_fields(self, sort_param, sort_field_map=None): + """Validate and map JSON:API sort fields using the given field map.""" + if sort_field_map is None: + sort_field_map = self._FINDING_GROUP_SORT_MAP ordering = [] for field in sort_param.split(","): @@ -7129,7 +7146,6 @@ class FindingGroupViewSet(BaseRLSViewSet): is_desc = field.startswith("-") raw_field = field[1:] if is_desc else field if raw_field not in sort_field_map: - # Validate sort fields explicitly to return JSON:API 400 instead of FieldError. raise ValidationError( [ { @@ -7253,7 +7269,58 @@ class FindingGroupViewSet(BaseRLSViewSet): ), ) .filter(resource_id__isnull=False) - .order_by("resource_id") + ) + + # Annotations needed for each sort field (lightweight versions for ordering) + _RESOURCE_SORT_ANNOTATIONS = { + "status_order": lambda: Max( + Case( + When(finding__status="FAIL", finding__muted=False, then=Value(3)), + When(finding__status="PASS", finding__muted=False, then=Value(2)), + default=Value(1), + output_field=IntegerField(), + ) + ), + "severity_order": lambda: Max( + Case( + *[ + When(finding__severity=severity, then=Value(order)) + for severity, order in SEVERITY_ORDER.items() + ], + output_field=IntegerField(), + ) + ), + "first_seen_at": lambda: Min("finding__first_seen_at"), + "last_seen_at": lambda: Max("finding__inserted_at"), + "resource_uid": lambda: Max("resource__uid"), + "resource_name": lambda: Max("resource__name"), + "resource_region": lambda: Max("resource__region"), + "resource_service": lambda: Max("resource__service"), + "resource_type": lambda: Max("resource__type"), + "provider_uid": lambda: Max("resource__provider__uid"), + "provider_alias": lambda: Max("resource__provider__alias"), + } + + def _build_resource_ordering_queryset( + self, filtered_queryset, resource_ids, tenant_id, ordering + ): + """Build a lightweight aggregation with only the columns needed for sorting.""" + mapping_qs = self._build_resource_mapping_queryset( + filtered_queryset, resource_ids=resource_ids, tenant_id=tenant_id + ) + + # Collect only the annotations required by the requested ordering + annotations = {} + for field in ordering: + col = field.lstrip("-") + if col != "resource_id" and col in self._RESOURCE_SORT_ANNOTATIONS: + annotations[col] = self._RESOURCE_SORT_ANNOTATIONS[col]() + + return ( + mapping_qs.values("resource_id") + .annotate(**annotations) + .filter(resource_id__isnull=False) + .order_by(*ordering) ) def _post_process_resources(self, resource_data): @@ -7336,11 +7403,17 @@ class FindingGroupViewSet(BaseRLSViewSet): ) return self._aggregate_daily_summaries(clean_queryset) - def _sorted_paginated_response(self, request, aggregated_queryset): + def _sorted_paginated_response( + self, + request, + aggregated_queryset, + ): """Apply ordering, pagination, post-processing, and return the Response.""" sort_param = request.query_params.get("sort") if sort_param: - ordering = self._validate_sort_fields(sort_param) + ordering = self._validate_sort_fields( + sort_param, self._FINDING_GROUP_SORT_MAP + ) if ordering: aggregated_queryset = aggregated_queryset.order_by(*ordering) else: @@ -7358,6 +7431,88 @@ class FindingGroupViewSet(BaseRLSViewSet): serializer = self.get_serializer(processed_data, many=True) return Response(serializer.data) + def _validate_resource_sort(self, request): + """Validate the sort parameter for resource endpoints (raises 400 if invalid).""" + sort_param = request.query_params.get("sort") + if sort_param: + self._validate_sort_fields(sort_param, self._RESOURCE_SORT_MAP) + + def _paginated_resource_response( + self, request, filtered_queryset, resource_ids, tenant_id + ): + """Paginate and return resources. + + Without sort: paginate lightweight resource IDs first, aggregate only the page. + With sort: build a lightweight ordering subquery (resource_id + sort keys), + paginate that, then aggregate full details only for the page. + """ + sort_param = request.query_params.get("sort") + + if sort_param: + ordering = self._validate_sort_fields(sort_param, self._RESOURCE_SORT_MAP) + if ordering: + if "resource_id" not in {field.lstrip("-") for field in ordering}: + ordering.append("resource_id") + + # Phase 1: lightweight aggregation with only sort keys, paginate + ordering_qs = self._build_resource_ordering_queryset( + filtered_queryset, + resource_ids=resource_ids, + tenant_id=tenant_id, + ordering=ordering, + ) + page = self.paginate_queryset(ordering_qs) + if page is not None: + page_ids = [row["resource_id"] for row in page] + resource_data = self._build_resource_aggregation( + filtered_queryset, resource_ids=page_ids, tenant_id=tenant_id + ) + # Re-sort to match the page ordering + id_order = {rid: idx for idx, rid in enumerate(page_ids)} + results = self._post_process_resources(resource_data) + results.sort(key=lambda r: id_order.get(r["resource_id"], 0)) + serializer = FindingGroupResourceSerializer(results, many=True) + return self.get_paginated_response(serializer.data) + + page_ids = [row["resource_id"] for row in ordering_qs] + resource_data = self._build_resource_aggregation( + filtered_queryset, resource_ids=page_ids, tenant_id=tenant_id + ) + id_order = {rid: idx for idx, rid in enumerate(page_ids)} + results = self._post_process_resources(resource_data) + results.sort(key=lambda r: id_order.get(r["resource_id"], 0)) + serializer = FindingGroupResourceSerializer(results, many=True) + return Response(serializer.data) + + # No sort (or only empty sort fragments): paginate lightweight resource IDs + # first, aggregate only the page. + mapping_qs = self._build_resource_mapping_queryset( + filtered_queryset, resource_ids=resource_ids, tenant_id=tenant_id + ) + resource_id_qs = ( + mapping_qs.values_list("resource_id", flat=True) + .distinct() + .order_by("resource_id") + ) + + page_ids = self.paginate_queryset(resource_id_qs) + if page_ids is not None: + resource_data = self._build_resource_aggregation( + filtered_queryset, resource_ids=page_ids, tenant_id=tenant_id + ) + id_order = {rid: idx for idx, rid in enumerate(page_ids)} + results = self._post_process_resources(resource_data) + results.sort(key=lambda r: id_order.get(r["resource_id"], 0)) + serializer = FindingGroupResourceSerializer(results, many=True) + return self.get_paginated_response(serializer.data) + + resource_data = self._build_resource_aggregation( + filtered_queryset, resource_ids=resource_ids, tenant_id=tenant_id + ).order_by("resource_id") + results = self._post_process_resources(resource_data) + serializer = FindingGroupResourceSerializer(results, many=True) + return Response(serializer.data) + def list(self, request, *args, **kwargs): """ List finding groups with aggregation and filtering. @@ -7431,57 +7586,33 @@ class FindingGroupViewSet(BaseRLSViewSet): check_id = pk queryset = self._get_finding_queryset() - # Apply date filters from request to Finding queryset + # 1. Normalize and split params normalized_params = self._normalize_jsonapi_params(request.query_params) finding_params, resource_params = self._split_resource_filters( normalized_params ) + # 2. Validate all inputs before any DB existence check filterset = FindingGroupFilter(finding_params, queryset=queryset) if not filterset.is_valid(): raise ValidationError(filterset.errors) + # Access .qs to trigger filter_queryset validation (e.g. required date filters) filtered_queryset = filterset.qs - - # Filter by check_id - filtered_queryset = filtered_queryset.filter(check_id=check_id) - - # Check if any findings exist for this check_id - if not filtered_queryset.exists(): - raise NotFound(f"Finding group '{check_id}' not found.") - resource_ids = self._resource_ids_from_params( resource_params, request.tenant_id ) - mapping_queryset = self._build_resource_mapping_queryset( - filtered_queryset, - resource_ids=resource_ids, - tenant_id=request.tenant_id, - ) - resource_id_queryset = ( - mapping_queryset.values_list("resource_id", flat=True) - .distinct() - .order_by("resource_id") - ) + self._validate_resource_sort(request) - page_ids = self.paginate_queryset(resource_id_queryset) - if page_ids is not None: - resource_data = self._build_resource_aggregation( - filtered_queryset, - resource_ids=page_ids, - tenant_id=request.tenant_id, - ) - results = self._post_process_resources(resource_data) - serializer = FindingGroupResourceSerializer(results, many=True) - return self.get_paginated_response(serializer.data) + # 3. Check if the finding group exists (scoped to tenant/RBAC, ignoring user filters) + if not queryset.filter(check_id=check_id).exists(): + raise NotFound(f"Finding group '{check_id}' not found.") - resource_data = self._build_resource_aggregation( - filtered_queryset, - resource_ids=resource_ids, - tenant_id=request.tenant_id, + # 4. Narrow to check_id + filtered_queryset = filtered_queryset.filter(check_id=check_id) + + return self._paginated_resource_response( + request, filtered_queryset, resource_ids, request.tenant_id ) - results = self._post_process_resources(resource_data) - serializer = FindingGroupResourceSerializer(results, many=True) - return Response(serializer.data) @extend_schema( summary="List resources for a finding group from latest scans", @@ -7525,55 +7656,31 @@ class FindingGroupViewSet(BaseRLSViewSet): if key.startswith("inserted_at"): del normalized_params[key] + # 1. Normalize and split params finding_params, resource_params = self._split_resource_filters( normalized_params ) + # 2. Validate all inputs before any DB existence check filterset = LatestFindingGroupFilter(finding_params, queryset=queryset) if not filterset.is_valid(): raise ValidationError(filterset.errors) filtered_queryset = filterset.qs + resource_ids = self._resource_ids_from_params( + resource_params, request.tenant_id + ) + self._validate_resource_sort(request) - # Filter to latest scans and check_id + # 3. Check if the finding group exists (scoped to tenant/RBAC + latest scans) + if not queryset.filter(scan_id__in=latest_scan_ids, check_id=check_id).exists(): + raise NotFound(f"Finding group '{check_id}' not found.") + + # 4. Narrow to latest scans + check_id filtered_queryset = filtered_queryset.filter( scan_id__in=latest_scan_ids, check_id=check_id, ) - # Check if any findings exist for this check_id - if not filtered_queryset.exists(): - raise NotFound(f"Finding group '{check_id}' not found.") - - resource_ids = self._resource_ids_from_params( - resource_params, request.tenant_id + return self._paginated_resource_response( + request, filtered_queryset, resource_ids, request.tenant_id ) - mapping_queryset = self._build_resource_mapping_queryset( - filtered_queryset, - resource_ids=resource_ids, - tenant_id=request.tenant_id, - ) - resource_id_queryset = ( - mapping_queryset.values_list("resource_id", flat=True) - .distinct() - .order_by("resource_id") - ) - - page_ids = self.paginate_queryset(resource_id_queryset) - if page_ids is not None: - resource_data = self._build_resource_aggregation( - filtered_queryset, - resource_ids=page_ids, - tenant_id=request.tenant_id, - ) - results = self._post_process_resources(resource_data) - serializer = FindingGroupResourceSerializer(results, many=True) - return self.get_paginated_response(serializer.data) - - resource_data = self._build_resource_aggregation( - filtered_queryset, - resource_ids=resource_ids, - tenant_id=request.tenant_id, - ) - results = self._post_process_resources(resource_data) - serializer = FindingGroupResourceSerializer(results, many=True) - return Response(serializer.data)