From d90e4449f9b7d0fbbcf15800cdbcac1021b59258 Mon Sep 17 00:00:00 2001 From: "Pablo F.G" Date: Tue, 23 Jun 2026 09:24:31 +0200 Subject: [PATCH] feat(ui): address alan feedback --- .../manage-groups/manage-groups.test.ts | 15 ++ ui/actions/manage-groups/manage-groups.ts | 9 +- .../_overview/_lib/provider-scope.test.ts | 162 ++++++++++++++++++ .../_overview/_lib/provider-scope.ts | 71 ++++++++ .../risk-pipeline-view.ssr.tsx | 28 ++- .../graphs-tabs/risk-plot/risk-plot.ssr.tsx | 46 ++--- .../finding-severity-over-time.tsx | 17 +- .../findings/findings-filters.utils.ts | 13 +- .../resources/resources-filters.utils.ts | 9 +- ui/lib/helper-filters.test.ts | 30 ++++ ui/lib/helper-filters.ts | 14 ++ 11 files changed, 340 insertions(+), 74 deletions(-) create mode 100644 ui/app/(prowler)/_overview/_lib/provider-scope.test.ts create mode 100644 ui/app/(prowler)/_overview/_lib/provider-scope.ts diff --git a/ui/actions/manage-groups/manage-groups.test.ts b/ui/actions/manage-groups/manage-groups.test.ts index d86fdce94c..c016832a1f 100644 --- a/ui/actions/manage-groups/manage-groups.test.ts +++ b/ui/actions/manage-groups/manage-groups.test.ts @@ -120,4 +120,19 @@ describe("getAllProviderGroups", () => { expect(result).toBeUndefined(); }); + + it("returns undefined instead of a truncated list when the max-page cap is hit", async () => { + // Given an API that always reports more pages than the 50-page safety cap + handleApiResponseMock.mockImplementation((response: Response) => { + void response; + return Promise.resolve(makePage([makeGroup("g", "Group")], 1, 9999)); + }); + + // When fetching every page + const result = await getAllProviderGroups(); + + // Then it must not return a partial/truncated list; bail out instead + expect(result).toBeUndefined(); + expect(fetchMock).toHaveBeenCalledTimes(50); + }); }); diff --git a/ui/actions/manage-groups/manage-groups.ts b/ui/actions/manage-groups/manage-groups.ts index 201c76de3a..c916a89d39 100644 --- a/ui/actions/manage-groups/manage-groups.ts +++ b/ui/actions/manage-groups/manage-groups.ts @@ -59,7 +59,6 @@ export const getProviderGroups = async ({ export const getAllProviderGroups = async (): Promise< ProviderGroupsResponse | undefined > => { - const headers = await getAuthHeaders({ contentType: false }); const pageSize = 100; // Larger page size to minimize API calls const maxPages = 50; // Safety limit: 50 pages × 100 = 5000 groups max let currentPage = 1; @@ -68,6 +67,7 @@ export const getAllProviderGroups = async (): Promise< let hasMorePages = true; try { + const headers = await getAuthHeaders({ contentType: false }); while (hasMorePages && currentPage <= maxPages) { const url = new URL(`${apiBaseUrl}/provider-groups`); url.searchParams.append("page[number]", currentPage.toString()); @@ -102,6 +102,13 @@ export const getAllProviderGroups = async (): Promise< } } + if (hasMorePages && currentPage > maxPages) { + console.error( + `Error fetching all provider groups: exceeded max page limit (${maxPages})`, + ); + return undefined; + } + if (lastResponse) { return { ...lastResponse, diff --git a/ui/app/(prowler)/_overview/_lib/provider-scope.test.ts b/ui/app/(prowler)/_overview/_lib/provider-scope.test.ts new file mode 100644 index 0000000000..3071f84837 --- /dev/null +++ b/ui/app/(prowler)/_overview/_lib/provider-scope.test.ts @@ -0,0 +1,162 @@ +import { describe, expect, it } from "vitest"; + +import { ProviderProps } from "@/types/providers"; + +import { + filterProvidersByScope, + parseFilterIds, + scopeProvidersByGroup, +} from "./provider-scope"; + +const makeProvider = ( + id: string, + provider: string, + groupIds: string[] = [], +): ProviderProps => + ({ + id, + attributes: { provider }, + relationships: { + provider_groups: { + data: groupIds.map((gid) => ({ type: "provider-groups", id: gid })), + }, + }, + }) as unknown as ProviderProps; + +describe("parseFilterIds", () => { + it("returns an empty array for undefined", () => { + // Given / When / Then + expect(parseFilterIds(undefined)).toEqual([]); + }); + + it("returns an empty array for an empty string", () => { + // Given an empty param value (e.g. "filter[provider_groups__in]=") + // When / Then it must not produce a [""] match + expect(parseFilterIds("")).toEqual([]); + }); + + it("drops whitespace-only and empty segments", () => { + // Given a blank/whitespace value + // When / Then + expect(parseFilterIds(" ")).toEqual([]); + expect(parseFilterIds(",")).toEqual([]); + expect(parseFilterIds("a,,b")).toEqual(["a", "b"]); + }); + + it("splits and trims comma-separated ids", () => { + expect(parseFilterIds(" a , b ")).toEqual(["a", "b"]); + }); + + it("normalizes array param values", () => { + expect(parseFilterIds(["a", "", "b"])).toEqual(["a", "b"]); + }); +}); + +describe("scopeProvidersByGroup", () => { + const providers = [ + makeProvider("p1", "aws", ["g1"]), + makeProvider("p2", "gcp", ["g2"]), + makeProvider("p3", "azure", []), + ]; + + it("returns every provider when no group is selected", () => { + expect(scopeProvidersByGroup(providers, [])).toEqual(providers); + }); + + it("keeps only providers that belong to a selected group", () => { + // When scoping to g1 + const result = scopeProvidersByGroup(providers, ["g1"]); + + // Then only the g1 member remains + expect(result.map((p) => p.id)).toEqual(["p1"]); + }); + + it("excludes providers with no group memberships", () => { + expect(scopeProvidersByGroup(providers, ["g2"]).map((p) => p.id)).toEqual([ + "p2", + ]); + }); +}); + +describe("filterProvidersByScope", () => { + const providers = [ + makeProvider("p1", "aws", ["g1"]), + makeProvider("p2", "gcp", ["g1"]), + makeProvider("p3", "aws", ["g2"]), + makeProvider("p4", "azure", []), + ]; + + it("returns every provider when no dimension is set", () => { + const result = filterProvidersByScope(providers, { + providerIds: [], + providerTypes: [], + providerGroupIds: [], + }); + + expect(result).toEqual(providers); + }); + + it("filters by provider id", () => { + const result = filterProvidersByScope(providers, { + providerIds: ["p2"], + providerTypes: [], + providerGroupIds: [], + }); + + expect(result.map((p) => p.id)).toEqual(["p2"]); + }); + + it("filters by provider type case-insensitively", () => { + const result = filterProvidersByScope(providers, { + providerIds: [], + providerTypes: ["AWS"], + providerGroupIds: [], + }); + + expect(result.map((p) => p.id)).toEqual(["p1", "p3"]); + }); + + it("filters by provider group", () => { + const result = filterProvidersByScope(providers, { + providerIds: [], + providerTypes: [], + providerGroupIds: ["g1"], + }); + + expect(result.map((p) => p.id)).toEqual(["p1", "p2"]); + }); + + it("composes group AND type (the risk-plot regression)", () => { + // Given both a group and a type filter are active + // When combining group g1 with type aws + const result = filterProvidersByScope(providers, { + providerIds: [], + providerTypes: ["aws"], + providerGroupIds: ["g1"], + }); + + // Then only providers matching BOTH survive (p1), not all aws or all g1 + expect(result.map((p) => p.id)).toEqual(["p1"]); + }); + + it("composes id AND group", () => { + // p3 is aws/g2; selecting it together with group g1 yields nothing + const result = filterProvidersByScope(providers, { + providerIds: ["p3"], + providerTypes: [], + providerGroupIds: ["g1"], + }); + + expect(result).toEqual([]); + }); + + it("composes all three dimensions", () => { + const result = filterProvidersByScope(providers, { + providerIds: ["p1", "p2"], + providerTypes: ["aws"], + providerGroupIds: ["g1"], + }); + + expect(result.map((p) => p.id)).toEqual(["p1"]); + }); +}); diff --git a/ui/app/(prowler)/_overview/_lib/provider-scope.ts b/ui/app/(prowler)/_overview/_lib/provider-scope.ts new file mode 100644 index 0000000000..49973b201b --- /dev/null +++ b/ui/app/(prowler)/_overview/_lib/provider-scope.ts @@ -0,0 +1,71 @@ +import { ProviderProps } from "@/types/providers"; + +export interface ProviderScopeFilters { + providerIds: string[]; + providerTypes: string[]; + providerGroupIds: string[]; +} + +/** + * Normalize a comma-separated filter param into trimmed, non-empty ids. + * Guards against blank values (e.g. an empty "filter[...]=" param) so they are + * treated as "no filter" instead of matching against an empty-string id. + */ +export const parseFilterIds = ( + value: string | string[] | undefined, +): string[] => { + if (value === undefined) return []; + const raw = Array.isArray(value) ? value.join(",") : value; + return raw + .split(",") + .map((id) => id.trim()) + .filter((id) => id.length > 0); +}; + +const belongsToGroup = (provider: ProviderProps, groupIds: string[]): boolean => + provider.relationships.provider_groups?.data?.some((group) => + groupIds.includes(group.id), + ) ?? false; + +/** + * Keep only providers belonging to one of the selected groups. An empty group + * list means "no group filter" and returns every provider unchanged. + */ +export const scopeProvidersByGroup = ( + providers: ProviderProps[], + groupIds: string[], +): ProviderProps[] => + groupIds.length === 0 + ? providers + : providers.filter((p) => belongsToGroup(p, groupIds)); + +/** + * Filter providers by every active scope dimension (id, type, group) combined + * with AND. Each empty dimension is skipped, so a provider is kept only when it + * satisfies all the filters that are actually set. + */ +export const filterProvidersByScope = ( + providers: ProviderProps[], + { providerIds, providerTypes, providerGroupIds }: ProviderScopeFilters, +): ProviderProps[] => { + const normalizedTypes = providerTypes.map((type) => type.toLowerCase()); + + return providers.filter((provider) => { + if (providerIds.length > 0 && !providerIds.includes(provider.id)) { + return false; + } + if ( + normalizedTypes.length > 0 && + !normalizedTypes.includes(provider.attributes.provider.toLowerCase()) + ) { + return false; + } + if ( + providerGroupIds.length > 0 && + !belongsToGroup(provider, providerGroupIds) + ) { + return false; + } + return true; + }); +}; diff --git a/ui/app/(prowler)/_overview/graphs-tabs/risk-pipeline-view/risk-pipeline-view.ssr.tsx b/ui/app/(prowler)/_overview/graphs-tabs/risk-pipeline-view/risk-pipeline-view.ssr.tsx index 19c63692fd..2a4b100379 100644 --- a/ui/app/(prowler)/_overview/graphs-tabs/risk-pipeline-view/risk-pipeline-view.ssr.tsx +++ b/ui/app/(prowler)/_overview/graphs-tabs/risk-pipeline-view/risk-pipeline-view.ssr.tsx @@ -9,6 +9,10 @@ import { SankeyChart } from "@/components/graphs/sankey-chart"; import { SearchParamsProps } from "@/types"; import { pickFilterParams } from "../../_lib/filter-params"; +import { + parseFilterIds, + scopeProvidersByGroup, +} from "../../_lib/provider-scope"; export async function RiskPipelineViewSSR({ searchParams, @@ -27,18 +31,8 @@ export async function RiskPipelineViewSSR({ // Scope the provider set to the selected groups so we enumerate only their // provider types below (the per-type API calls also carry the group filter). - const selectedGroupIds = providerGroupsFilter - ? String(providerGroupsFilter) - .split(",") - .map((id) => id.trim()) - : []; - const scopedProviders = selectedGroupIds.length - ? allProviders.filter((p) => - p.relationships.provider_groups?.data?.some((group) => - selectedGroupIds.includes(group.id), - ), - ) - : allProviders; + const selectedGroupIds = parseFilterIds(providerGroupsFilter); + const scopedProviders = scopeProvidersByGroup(allProviders, selectedGroupIds); // Build severityByProviderType based on filters const severityByProviderType: SeverityByProviderType = {}; @@ -46,9 +40,7 @@ export async function RiskPipelineViewSSR({ if (providerIdFilter) { // Case: Accounts are selected - group by provider type and make parallel calls - const selectedAccountIds = String(providerIdFilter) - .split(",") - .map((id) => id.trim()); + const selectedAccountIds = parseFilterIds(providerIdFilter); // Group selected accounts by provider type const accountsByType: Record = {}; @@ -87,9 +79,9 @@ export async function RiskPipelineViewSSR({ } } else if (providerTypeFilter) { // Case: Provider types are selected - make parallel calls for each type - selectedProviderTypes = String(providerTypeFilter) - .split(",") - .map((t) => t.trim().toLowerCase()); + selectedProviderTypes = parseFilterIds(providerTypeFilter).map((type) => + type.toLowerCase(), + ); const severityPromises = selectedProviderTypes.map(async (providerType) => { const response = await getFindingsBySeverity({ diff --git a/ui/app/(prowler)/_overview/graphs-tabs/risk-plot/risk-plot.ssr.tsx b/ui/app/(prowler)/_overview/graphs-tabs/risk-plot/risk-plot.ssr.tsx index 6f42261a14..1f4d3625d4 100644 --- a/ui/app/(prowler)/_overview/graphs-tabs/risk-plot/risk-plot.ssr.tsx +++ b/ui/app/(prowler)/_overview/graphs-tabs/risk-plot/risk-plot.ssr.tsx @@ -1,5 +1,6 @@ import { Info } from "lucide-react"; +import { OVERVIEW_FILTER_PARAM } from "@/actions/overview/overview-filters"; import { adaptToRiskPlotData, getProvidersRiskData, @@ -8,6 +9,10 @@ import { getAllProviders } from "@/actions/providers"; import { SearchParamsProps } from "@/types"; import { pickFilterParams } from "../../_lib/filter-params"; +import { + filterProvidersByScope, + parseFilterIds, +} from "../../_lib/provider-scope"; import { RiskPlotClient } from "./risk-plot-client"; export async function RiskPlotSSR({ @@ -17,42 +22,19 @@ export async function RiskPlotSSR({ }) { const filters = pickFilterParams(searchParams); - const providerTypeFilter = filters["filter[provider_type__in]"]; - const providerIdFilter = filters["filter[provider_id__in]"]; - const providerGroupsFilter = filters["filter[provider_groups__in]"]; - // Fetch all providers const providersListResponse = await getAllProviders(); const allProviders = providersListResponse?.data || []; - // Filter providers based on search params - let filteredProviders = allProviders; - - if (providerIdFilter) { - // Filter by specific provider IDs - const selectedIds = String(providerIdFilter) - .split(",") - .map((id) => id.trim()); - filteredProviders = allProviders.filter((p) => selectedIds.includes(p.id)); - } else if (providerGroupsFilter) { - // Filter by provider group membership - const selectedGroupIds = String(providerGroupsFilter) - .split(",") - .map((id) => id.trim()); - filteredProviders = allProviders.filter((p) => - p.relationships.provider_groups?.data?.some((group) => - selectedGroupIds.includes(group.id), - ), - ); - } else if (providerTypeFilter) { - // Filter by provider types - const selectedTypes = String(providerTypeFilter) - .split(",") - .map((t) => t.trim().toLowerCase()); - filteredProviders = allProviders.filter((p) => - selectedTypes.includes(p.attributes.provider.toLowerCase()), - ); - } + // Compose every active provider-scope filter with AND so combining e.g. a + // group and a type narrows to providers matching both. + const filteredProviders = filterProvidersByScope(allProviders, { + providerIds: parseFilterIds(filters[OVERVIEW_FILTER_PARAM.PROVIDER_ID]), + providerTypes: parseFilterIds(filters[OVERVIEW_FILTER_PARAM.PROVIDER_TYPE]), + providerGroupIds: parseFilterIds( + filters[OVERVIEW_FILTER_PARAM.PROVIDER_GROUPS], + ), + }); // No providers to show if (filteredProviders.length === 0) { diff --git a/ui/app/(prowler)/_overview/severity-over-time/_components/finding-severity-over-time.tsx b/ui/app/(prowler)/_overview/severity-over-time/_components/finding-severity-over-time.tsx index c39c462bfb..b65b6a21f0 100644 --- a/ui/app/(prowler)/_overview/severity-over-time/_components/finding-severity-over-time.tsx +++ b/ui/app/(prowler)/_overview/severity-over-time/_components/finding-severity-over-time.tsx @@ -3,6 +3,7 @@ import { useRouter, useSearchParams } from "next/navigation"; import { useState } from "react"; +import { OVERVIEW_FILTER_PARAM } from "@/actions/overview/overview-filters"; import { getSeverityTrendsByTimeRange } from "@/actions/overview/severity-trends"; import { LineChart } from "@/components/graphs/line-chart"; import { LineConfig, LineDataPoint } from "@/components/graphs/types"; @@ -42,12 +43,16 @@ export const FindingSeverityOverTime = ({ const getActiveProviderFilters = (): Record => { const filters: Record = {}; - const providerType = searchParams.get("filter[provider_type__in]"); - const providerId = searchParams.get("filter[provider_id__in]"); - const providerGroups = searchParams.get("filter[provider_groups__in]"); - if (providerType) filters["filter[provider_type__in]"] = providerType; - if (providerId) filters["filter[provider_id__in]"] = providerId; - if (providerGroups) filters["filter[provider_groups__in]"] = providerGroups; + const providerType = searchParams.get(OVERVIEW_FILTER_PARAM.PROVIDER_TYPE); + const providerId = searchParams.get(OVERVIEW_FILTER_PARAM.PROVIDER_ID); + const providerGroups = searchParams.get( + OVERVIEW_FILTER_PARAM.PROVIDER_GROUPS, + ); + if (providerType) + filters[OVERVIEW_FILTER_PARAM.PROVIDER_TYPE] = providerType; + if (providerId) filters[OVERVIEW_FILTER_PARAM.PROVIDER_ID] = providerId; + if (providerGroups) + filters[OVERVIEW_FILTER_PARAM.PROVIDER_GROUPS] = providerGroups; return filters; }; diff --git a/ui/components/findings/findings-filters.utils.ts b/ui/components/findings/findings-filters.utils.ts index 1b7d13b42b..4cb9eef394 100644 --- a/ui/components/findings/findings-filters.utils.ts +++ b/ui/components/findings/findings-filters.utils.ts @@ -1,7 +1,10 @@ import type { FindingsFilterParam } from "@/actions/findings/findings-filters"; import type { FilterChip } from "@/components/filters/filter-summary-strip"; import { formatLabel, getCategoryLabel, getGroupLabel } from "@/lib/categories"; -import { getScanEntityLabel } from "@/lib/helper-filters"; +import { + getProviderGroupDisplayValue, + getScanEntityLabel, +} from "@/lib/helper-filters"; import { FINDING_STATUS_DISPLAY_NAMES } from "@/types"; import { ProviderGroup } from "@/types/components"; import { getProviderDisplayName, ProviderProps } from "@/types/providers"; @@ -31,14 +34,6 @@ function getProviderAccountDisplayValue( return provider.attributes.alias || provider.attributes.uid || providerId; } -function getProviderGroupDisplayValue( - groupId: string, - groups: ProviderGroup[], -): string { - const group = groups.find((item) => item.id === groupId); - return group?.attributes.name || groupId; -} - function getScanDisplayValue( scanId: string, scans: Array<{ [scanId: string]: ScanEntity }>, diff --git a/ui/components/resources/resources-filters.utils.ts b/ui/components/resources/resources-filters.utils.ts index b327b3d62f..805cf2ace1 100644 --- a/ui/components/resources/resources-filters.utils.ts +++ b/ui/components/resources/resources-filters.utils.ts @@ -1,6 +1,7 @@ import type { ResourcesFilterParam } from "@/actions/resources/resources-filters"; import type { FilterChip } from "@/components/filters/filter-summary-strip"; import { formatLabel, getGroupLabel } from "@/lib/categories"; +import { getProviderGroupDisplayValue } from "@/lib/helper-filters"; import type { ProviderGroup } from "@/types/components"; import type { ProviderProps } from "@/types/providers"; import { getProviderDisplayName } from "@/types/providers"; @@ -27,14 +28,6 @@ function getProviderAccountDisplayValue( return provider.attributes.alias || provider.attributes.uid || providerId; } -function getProviderGroupDisplayValue( - groupId: string, - groups: ProviderGroup[], -): string { - const group = groups.find((item) => item.id === groupId); - return group?.attributes.name || groupId; -} - export function getResourcesFilterDisplayValue( filterKey: string, value: string, diff --git a/ui/lib/helper-filters.test.ts b/ui/lib/helper-filters.test.ts index 858a95e68f..c538e00e5c 100644 --- a/ui/lib/helper-filters.test.ts +++ b/ui/lib/helper-filters.test.ts @@ -1,14 +1,23 @@ import { describe, expect, it } from "vitest"; +import type { ProviderGroup } from "@/types/components"; import type { ScanEntity } from "@/types/scans"; import { + getProviderGroupDisplayValue, getScanEntityLabel, hasDateFilter, hasDateOrScanFilter, hasHistoricalFindingFilter, } from "./helper-filters"; +const makeProviderGroup = (id: string, name: string): ProviderGroup => + ({ + type: "provider-groups", + id, + attributes: { name, inserted_at: "", updated_at: "" }, + }) as ProviderGroup; + function makeScan(overrides: Partial = {}): ScanEntity { return { id: "scan-1", @@ -25,6 +34,27 @@ function makeScan(overrides: Partial = {}): ScanEntity { }; } +describe("getProviderGroupDisplayValue", () => { + const groups = [ + makeProviderGroup("g1", "Production"), + makeProviderGroup("g2", "Staging"), + ]; + + it("resolves the group name when the id matches", () => { + expect(getProviderGroupDisplayValue("g1", groups)).toBe("Production"); + }); + + it("falls back to the raw id when the group is not found", () => { + expect(getProviderGroupDisplayValue("unknown", groups)).toBe("unknown"); + }); + + it("falls back to the raw id when the group name is empty", () => { + expect( + getProviderGroupDisplayValue("g3", [makeProviderGroup("g3", "")]), + ).toBe("g3"); + }); +}); + describe("hasDateOrScanFilter", () => { it("returns true for scan filters", () => { expect(hasDateOrScanFilter({ "filter[scan__in]": "scan-1" })).toBe(true); diff --git a/ui/lib/helper-filters.ts b/ui/lib/helper-filters.ts index ae510bdc83..cfc1717f98 100644 --- a/ui/lib/helper-filters.ts +++ b/ui/lib/helper-filters.ts @@ -1,4 +1,5 @@ import { ProviderProps, ProvidersApiResponse, ScanProps } from "@/types"; +import { ProviderGroup } from "@/types/components"; import { FilterEntity } from "@/types/filters"; import { getProviderDisplayName, @@ -119,6 +120,19 @@ export function getScanEntityLabel(scan: ScanEntity): string { return providerLabel || scanName; } +/** + * Resolves the display name for a provider group filter value, falling back to + * the raw id when the group can't be resolved. Shared by the findings and + * resources filter utils so their chips stay in sync. + */ +export function getProviderGroupDisplayValue( + groupId: string, + groups: ProviderGroup[], +): string { + const group = groups.find((item) => item.id === groupId); + return group?.attributes.name || groupId; +} + /** * Creates a scan details mapping for filters from completed scans. * Used to provide detailed information for scan filters in the UI.