fix(api): false 404 and sorting on finding group resources endpoints (#10510)

This commit is contained in:
Adrián Peña
2026-03-30 12:47:16 +02:00
committed by GitHub
parent 6df74529d6
commit 2a8b6261e1
3 changed files with 387 additions and 94 deletions
+1
View File
@@ -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
+185
View File
@@ -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
+201 -94
View File
@@ -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)