From 60314e781f0a408ff46349a119ee35c6a6f42132 Mon Sep 17 00:00:00 2001 From: Alejandro Bailo <59607668+alejandrobailo@users.noreply.github.com> Date: Wed, 28 May 2025 16:30:28 +0200 Subject: [PATCH 01/12] feat: enhance CustomDropdownFilter (#7868) --- ui/CHANGELOG.md | 1 + .../ui/custom/custom-dropdown-filter.tsx | 258 +++++++++--------- 2 files changed, 136 insertions(+), 123 deletions(-) diff --git a/ui/CHANGELOG.md b/ui/CHANGELOG.md index 443c6bbe03..d554b44c2d 100644 --- a/ui/CHANGELOG.md +++ b/ui/CHANGELOG.md @@ -12,6 +12,7 @@ All notable changes to the **Prowler UI** are documented in this file. - Add `Provider UID` filter to scans page. [(#7820)](https://github.com/prowler-cloud/prowler/pull/7820) - Download report behaviour updated to show feedback based on API response. [(#7758)](https://github.com/prowler-cloud/prowler/pull/7758) - Missing KISA and ProwlerThreat icons added to the compliance page. [(#7860)(https://github.com/prowler-cloud/prowler/pull/7860)] +- Improve CustomDropdownFilter component. [(#7868)(https://github.com/prowler-cloud/prowler/pull/7868)] --- diff --git a/ui/components/ui/custom/custom-dropdown-filter.tsx b/ui/components/ui/custom/custom-dropdown-filter.tsx index 5664122e89..254efa5024 100644 --- a/ui/components/ui/custom/custom-dropdown-filter.tsx +++ b/ui/components/ui/custom/custom-dropdown-filter.tsx @@ -10,167 +10,183 @@ import { PopoverTrigger, ScrollShadow, } from "@nextui-org/react"; -import { XCircle } from "lucide-react"; +import { ChevronDown, X } from "lucide-react"; import { useSearchParams } from "next/navigation"; import React, { useCallback, useEffect, useMemo, useState } from "react"; -import { PlusCircleIcon } from "@/components/icons"; -import { useUrlFilters } from "@/hooks/use-url-filters"; import { CustomDropdownFilterProps } from "@/types"; import { EntityInfoShort } from "../entities"; -const filterSelectedClass = - "inline-flex items-center border py-1 text-xs transition-colors border-transparent bg-default-500 text-secondary-foreground hover:bg-default-500/80 rounded-md px-2 font-normal"; - -export const CustomDropdownFilter: React.FC = ({ +export const CustomDropdownFilter = ({ filter, onFilterChange, -}) => { +}: CustomDropdownFilterProps) => { const searchParams = useSearchParams(); - const { clearFilter } = useUrlFilters(); const [groupSelected, setGroupSelected] = useState(new Set()); - const [pendingClearFilter, setPendingClearFilter] = useState( - null, + const [isOpen, setIsOpen] = useState(false); + + const filterValues = useMemo(() => filter?.values || [], [filter?.values]); + const selectedValues = Array.from(groupSelected).filter( + (value) => value !== "all", ); + const isAllSelected = + selectedValues.length === filterValues.length && filterValues.length > 0; - const allFilterKeys = useMemo(() => filter?.values || [], [filter?.values]); - - const getActiveFilter = useMemo(() => { - const currentFilters: Record = {}; - Array.from(searchParams.entries()).forEach(([key, value]) => { - if (key.startsWith("filter[") && key.endsWith("]")) { - const filterKey = key.slice(7, -1); - if (filter && filter.key === filterKey) { - // eslint-disable-next-line security/detect-object-injection - currentFilters[filterKey] = value; - } - } - }); - return currentFilters; - }, [searchParams, filter]); - - const memoizedFilterValues = useMemo( - () => filter?.values || [], - [filter?.values], - ); + const activeFilterValue = useMemo(() => { + const filterParam = searchParams.get(`filter[${filter?.key}]`); + return filterParam ? filterParam.split(",") : []; + }, [searchParams, filter?.key]); + // Sync URL state with component state useEffect(() => { - if (filter && getActiveFilter[filter.key]) { - const activeValues = getActiveFilter[filter.key].split(","); - const newSelection = new Set(activeValues); - if (newSelection.size === memoizedFilterValues.length) { + if (activeFilterValue.length > 0) { + const newSelection = new Set(activeFilterValue); + if (newSelection.size === filterValues.length) { newSelection.add("all"); } setGroupSelected(newSelection); } else { setGroupSelected(new Set()); } - }, [getActiveFilter, filter?.key, memoizedFilterValues, filter]); + }, [activeFilterValue, filterValues.length]); + + const updateSelection = useCallback( + (newValues: string[]) => { + const actualValues = newValues.filter((key) => key !== "all"); + const newSelection = new Set(actualValues); + + // Auto-add "all" if all items are selected + if ( + actualValues.length === filterValues.length && + filterValues.length > 0 + ) { + newSelection.add("all"); + } + + setGroupSelected(newSelection); + + // Notify parent with actual values (excluding "all") + onFilterChange?.(filter.key, actualValues); + }, + [filterValues.length, onFilterChange, filter.key], + ); const onSelectionChange = useCallback( (keys: string[]) => { - setGroupSelected((prevGroupSelected) => { - const newSelection = new Set(keys); + const currentSelection = Array.from(groupSelected); + const newKeys = new Set(keys); + const oldKeys = new Set(currentSelection); - if ( - newSelection.size === allFilterKeys.length && - !newSelection.has("all") - ) { - return new Set(["all", ...allFilterKeys]); - } else if (prevGroupSelected.has("all")) { - newSelection.delete("all"); - return new Set(allFilterKeys.filter((key) => newSelection.has(key))); - } - return newSelection; - }); + // Check if "all" was just toggled + const allWasSelected = oldKeys.has("all"); + const allIsSelected = newKeys.has("all"); - if (onFilterChange && filter) { - const selectedValues = keys.filter((key) => key !== "all"); - onFilterChange(filter.key, selectedValues); + if (allIsSelected && !allWasSelected) { + // "all" was just selected - select all items + updateSelection(filterValues); + } else if (!allIsSelected && allWasSelected) { + // "all" was just deselected - deselect all items + updateSelection([]); + } else if (allIsSelected && allWasSelected) { + // "all" was already selected, but individual items changed + // Remove "all" and keep only the individual selections + const individualSelections = keys.filter((key) => key !== "all"); + updateSelection(individualSelections); + } else { + // Normal individual selection without "all" + updateSelection(keys); } }, - [allFilterKeys, onFilterChange, filter], + [groupSelected, updateSelection, filterValues], ); - const handleSelectAllClick = useCallback(() => { - setGroupSelected((prevGroupSelected: Set) => { - const newSelection: Set = prevGroupSelected.has("all") - ? new Set() - : new Set(["all", ...allFilterKeys]); + const handleClearAll = useCallback( + (e: React.MouseEvent) => { + e.stopPropagation(); + updateSelection([]); + }, + [updateSelection], + ); - if (onFilterChange && filter) { - const selectedValues = Array.from(newSelection).filter( - (key) => key !== "all", - ); - onFilterChange(filter.key, selectedValues); - } - - return newSelection; - }); - }, [allFilterKeys, onFilterChange, filter]); - - // Update the pending clear filter - const onClearFilter = useCallback((filterKey: string) => { - setPendingClearFilter(filterKey); - }, []); - - // Execute the update in the router after the render - useEffect(() => { - if (pendingClearFilter && filter) { - clearFilter(pendingClearFilter); - setPendingClearFilter(null); // Reset the state - } - }, [pendingClearFilter, clearFilter, filter]); + const getDisplayLabel = useCallback( + (value: string) => { + const entity = filter.valueLabelMapping?.find((entry) => entry[value])?.[ + value + ]; + return entity?.alias || entity?.uid || value; + }, + [filter.valueLabelMapping], + ); return ( -
- - -
+
= ({ wrapper: "checkbox-update", }} value="all" - isSelected={groupSelected.has("all")} - onClick={handleSelectAllClick} > Select All - {memoizedFilterValues.map((value) => { - // Find the corresponding entity from valueLabelMapping - const matchingEntry = filter.valueLabelMapping?.find( + {filterValues.map((value) => { + const entity = filter.valueLabelMapping?.find( (entry) => entry[value], - ); - const entity = matchingEntry?.[value]; + )?.[value]; return ( Date: Wed, 28 May 2025 16:43:58 +0200 Subject: [PATCH 02/12] fix(tests): typo in m365 domain test (#7866) --- api/src/backend/api/tests/test_views.py | 2 +- prowler/CHANGELOG.md | 1 + 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/api/src/backend/api/tests/test_views.py b/api/src/backend/api/tests/test_views.py index f6c2c56c38..ffc9d22802 100644 --- a/api/src/backend/api/tests/test_views.py +++ b/api/src/backend/api/tests/test_views.py @@ -921,7 +921,7 @@ class TestProviderViewSet: }, { "provider": "m365", - "uid": "TestingPro.onMirosoft.com", + "uid": "TestingPro.onmicrosoft.com", "alias": "test", }, { diff --git a/prowler/CHANGELOG.md b/prowler/CHANGELOG.md index ccf9fb4a7b..2f6de1ce04 100644 --- a/prowler/CHANGELOG.md +++ b/prowler/CHANGELOG.md @@ -21,6 +21,7 @@ All notable changes to the **Prowler SDK** are documented in this file. - Add `repository_default_branch_requires_codeowners_review` check for GitHub provider. [(#7753)](https://github.com/prowler-cloud/prowler/pull/7753) - Add NIS 2 compliance framework for AWS. [(7839)](https://github.com/prowler-cloud/prowler/pull/7839) - Add NIS 2 compliance framework for Azure. [(7857)](https://github.com/prowler-cloud/prowler/pull/7857) +- Add search bar in Dashboard Overview page. [(#7804)](https://github.com/prowler-cloud/prowler/pull/7804) ### Fixed - Fix `m365_powershell test_credentials` to use sanitized credentials. [(#7761)](https://github.com/prowler-cloud/prowler/pull/7761) From 12987ec9f91ad735235e8cd163890a08e337835e Mon Sep 17 00:00:00 2001 From: Hugo Pereira Brito <101209179+HugoPBrito@users.noreply.github.com> Date: Wed, 28 May 2025 16:48:49 +0200 Subject: [PATCH 03/12] fix(admincenter): service and group visibility (#7870) --- ...dmincenter_groups_not_public_visibility.py | 2 +- .../admincenter/admincenter_service.py | 4 +- ...enter_groups_not_public_visibility_test.py | 88 +++++++++++++++++++ 3 files changed, 90 insertions(+), 4 deletions(-) diff --git a/prowler/providers/m365/services/admincenter/admincenter_groups_not_public_visibility/admincenter_groups_not_public_visibility.py b/prowler/providers/m365/services/admincenter/admincenter_groups_not_public_visibility/admincenter_groups_not_public_visibility.py index d61fdec68d..f38f94f64f 100644 --- a/prowler/providers/m365/services/admincenter/admincenter_groups_not_public_visibility/admincenter_groups_not_public_visibility.py +++ b/prowler/providers/m365/services/admincenter/admincenter_groups_not_public_visibility/admincenter_groups_not_public_visibility.py @@ -36,7 +36,7 @@ class admincenter_groups_not_public_visibility(Check): report.status = "FAIL" report.status_extended = f"Group {group.name} has {group.visibility} visibility and should be Private." - if group.visibility != "Public": + if group.visibility and group.visibility != "Public": report.status = "PASS" report.status_extended = ( f"Group {group.name} has {group.visibility} visibility." diff --git a/prowler/providers/m365/services/admincenter/admincenter_service.py b/prowler/providers/m365/services/admincenter/admincenter_service.py index feac9c8375..32b2b12805 100644 --- a/prowler/providers/m365/services/admincenter/admincenter_service.py +++ b/prowler/providers/m365/services/admincenter/admincenter_service.py @@ -11,8 +11,6 @@ from prowler.providers.m365.m365_provider import M365Provider class AdminCenter(M365Service): def __init__(self, provider: M365Provider): super().__init__(provider) - if self.powershell: - self.powershell.close() self.organization_config = None self.sharing_policy = None @@ -203,7 +201,7 @@ class DirectoryRole(BaseModel): class Group(BaseModel): id: str name: str - visibility: str + visibility: Optional[str] class Domain(BaseModel): diff --git a/tests/providers/m365/services/admincenter/admincenter_groups_not_public_visibility/admincenter_groups_not_public_visibility_test.py b/tests/providers/m365/services/admincenter/admincenter_groups_not_public_visibility/admincenter_groups_not_public_visibility_test.py index 81209d67a4..7391bd102e 100644 --- a/tests/providers/m365/services/admincenter/admincenter_groups_not_public_visibility/admincenter_groups_not_public_visibility_test.py +++ b/tests/providers/m365/services/admincenter/admincenter_groups_not_public_visibility/admincenter_groups_not_public_visibility_test.py @@ -114,3 +114,91 @@ class Test_admincenter_groups_not_public_visibility: assert result[0].resource_name == "Group1" assert result[0].resource_id == id_group1 assert result[0].location == "global" + + def test_admincenter_group_public_visibility(self): + admincenter_client = mock.MagicMock + admincenter_client.audited_tenant = "audited_tenant" + admincenter_client.audited_domain = DOMAIN + + with ( + mock.patch( + "prowler.providers.common.provider.Provider.get_global_provider", + return_value=set_mocked_m365_provider(), + ), + mock.patch( + "prowler.providers.m365.lib.powershell.m365_powershell.M365PowerShell.connect_exchange_online" + ), + mock.patch( + "prowler.providers.m365.services.admincenter.admincenter_groups_not_public_visibility.admincenter_groups_not_public_visibility.admincenter_client", + new=admincenter_client, + ), + ): + from prowler.providers.m365.services.admincenter.admincenter_groups_not_public_visibility.admincenter_groups_not_public_visibility import ( + admincenter_groups_not_public_visibility, + ) + from prowler.providers.m365.services.admincenter.admincenter_service import ( + Group, + ) + + id_group1 = str(uuid4()) + + admincenter_client.groups = { + id_group1: Group(id=id_group1, name="Group1", visibility="Public"), + } + + check = admincenter_groups_not_public_visibility() + result = check.execute() + assert len(result) == 1 + assert result[0].status == "FAIL" + assert ( + result[0].status_extended + == "Group Group1 has Public visibility and should be Private." + ) + assert result[0].resource == admincenter_client.groups[id_group1].dict() + assert result[0].resource_name == "Group1" + assert result[0].resource_id == id_group1 + assert result[0].location == "global" + + def test_admincenter_group_none_visibility(self): + admincenter_client = mock.MagicMock + admincenter_client.audited_tenant = "audited_tenant" + admincenter_client.audited_domain = DOMAIN + + with ( + mock.patch( + "prowler.providers.common.provider.Provider.get_global_provider", + return_value=set_mocked_m365_provider(), + ), + mock.patch( + "prowler.providers.m365.lib.powershell.m365_powershell.M365PowerShell.connect_exchange_online" + ), + mock.patch( + "prowler.providers.m365.services.admincenter.admincenter_groups_not_public_visibility.admincenter_groups_not_public_visibility.admincenter_client", + new=admincenter_client, + ), + ): + from prowler.providers.m365.services.admincenter.admincenter_groups_not_public_visibility.admincenter_groups_not_public_visibility import ( + admincenter_groups_not_public_visibility, + ) + from prowler.providers.m365.services.admincenter.admincenter_service import ( + Group, + ) + + id_group1 = str(uuid4()) + + admincenter_client.groups = { + id_group1: Group(id=id_group1, name="Group1", visibility=None), + } + + check = admincenter_groups_not_public_visibility() + result = check.execute() + assert len(result) == 1 + assert result[0].status == "FAIL" + assert ( + result[0].status_extended + == "Group Group1 has None visibility and should be Private." + ) + assert result[0].resource == admincenter_client.groups[id_group1].dict() + assert result[0].resource_name == "Group1" + assert result[0].resource_id == id_group1 + assert result[0].location == "global" From 48c9ed8a79347049810278b8da4d4dbbb3fbd87e Mon Sep 17 00:00:00 2001 From: sumit-tft <70506234+sumit-tft@users.noreply.github.com> Date: Thu, 29 May 2025 11:22:36 +0530 Subject: [PATCH 04/12] fix(ui): increase limit to retrieve more than 10 scan list (#7865) --- ui/CHANGELOG.md | 3 +++ ui/app/(prowler)/compliance/page.tsx | 1 + 2 files changed, 4 insertions(+) diff --git a/ui/CHANGELOG.md b/ui/CHANGELOG.md index d554b44c2d..01480c765e 100644 --- a/ui/CHANGELOG.md +++ b/ui/CHANGELOG.md @@ -14,6 +14,9 @@ All notable changes to the **Prowler UI** are documented in this file. - Missing KISA and ProwlerThreat icons added to the compliance page. [(#7860)(https://github.com/prowler-cloud/prowler/pull/7860)] - Improve CustomDropdownFilter component. [(#7868)(https://github.com/prowler-cloud/prowler/pull/7868)] +### 🐞 Fixes +- Retrieve more than 10 scans in /compliance page. [(#7865)](https://github.com/prowler-cloud/prowler/pull/7865) + --- ## [v1.7.1] (Prowler v5.7.1) diff --git a/ui/app/(prowler)/compliance/page.tsx b/ui/app/(prowler)/compliance/page.tsx index e3dab9e762..56b6dc9d62 100644 --- a/ui/app/(prowler)/compliance/page.tsx +++ b/ui/app/(prowler)/compliance/page.tsx @@ -33,6 +33,7 @@ export default async function Compliance({ filters: { "filter[state]": "completed", }, + pageSize: 50, }); if (!scansData?.data) { From 921f94ebbf9d3cd5eec5984e497fe626b6e42907 Mon Sep 17 00:00:00 2001 From: Pepe Fagoaga Date: Thu, 29 May 2025 12:32:57 +0545 Subject: [PATCH 05/12] fix(k8s): UID validation for valid context names (#7871) --- api/CHANGELOG.md | 1 + api/src/backend/api/models.py | 2 +- api/src/backend/api/tests/test_views.py | 10 ++++++++++ 3 files changed, 12 insertions(+), 1 deletion(-) diff --git a/api/CHANGELOG.md b/api/CHANGELOG.md index f49253fb45..6459e17086 100644 --- a/api/CHANGELOG.md +++ b/api/CHANGELOG.md @@ -16,6 +16,7 @@ All notable changes to the **Prowler API** are documented in this file. ### Fixed - Fixed task lookup to use task_kwargs instead of task_args for scan report resolution. [(#7830)](https://github.com/prowler-cloud/prowler/pull/7830) +- Fixed Kubernetes UID validation to allow valid context names [(#7871)](https://github.com/prowler-cloud/prowler/pull/7871) --- diff --git a/api/src/backend/api/models.py b/api/src/backend/api/models.py index 28b0845a3a..2b4fec00cf 100644 --- a/api/src/backend/api/models.py +++ b/api/src/backend/api/models.py @@ -242,7 +242,7 @@ class Provider(RowLevelSecurityProtectedModel): @staticmethod def validate_kubernetes_uid(value): if not re.match( - r"^[a-z0-9][A-Za-z0-9_.:\/-]{1,250}$", + r"^[a-zA-Z0-9][a-zA-Z0-9._@:\/-]{1,250}$", value, ): raise ModelValidationError( diff --git a/api/src/backend/api/tests/test_views.py b/api/src/backend/api/tests/test_views.py index ffc9d22802..ac6c6b51a6 100644 --- a/api/src/backend/api/tests/test_views.py +++ b/api/src/backend/api/tests/test_views.py @@ -914,6 +914,16 @@ class TestProviderViewSet: "uid": "gke_aaaa-dev_europe-test1_dev-aaaa-test-cluster-long-name-123456789", "alias": "GKE", }, + { + "provider": "kubernetes", + "uid": "gke_project/cluster-name", + "alias": "GKE", + }, + { + "provider": "kubernetes", + "uid": "admin@k8s-demo", + "alias": "test", + }, { "provider": "azure", "uid": "8851db6b-42e5-4533-aa9e-30a32d67e875", From 5d043cc929886a8d28dce66b3d7d3cd75b1e2524 Mon Sep 17 00:00:00 2001 From: Alison Vilela Date: Thu, 29 May 2025 03:05:23 -0400 Subject: [PATCH 06/12] fix(awslambda): aws service awslambda not working (#7869) --- prowler/lib/check/models.py | 5 ++--- tests/lib/check/models_test.py | 2 +- 2 files changed, 3 insertions(+), 4 deletions(-) diff --git a/prowler/lib/check/models.py b/prowler/lib/check/models.py index 875350c757..9c8ee40eb6 100644 --- a/prowler/lib/check/models.py +++ b/prowler/lib/check/models.py @@ -329,9 +329,8 @@ class CheckMetadata(BaseModel): checks = set() if service: - # This is a special case for the AWS provider since `lambda` is a reserved keyword in Python - if service == "awslambda": - service = "lambda" + if service == "lambda": + service = "awslambda" checks = { check_name for check_name, check_metadata in bulk_checks_metadata.items() diff --git a/tests/lib/check/models_test.py b/tests/lib/check/models_test.py index 414de90288..fdbb8b6d70 100644 --- a/tests/lib/check/models_test.py +++ b/tests/lib/check/models_test.py @@ -37,7 +37,7 @@ mock_metadata_lambda = CheckMetadata( CheckID="awslambda_function_url_public", CheckTitle="Check 1", CheckType=["type1"], - ServiceName="lambda", + ServiceName="awslambda", SubServiceName="subservice1", ResourceIdTemplate="template1", Severity="high", From a89e3598f22133d1a03e31020215172d18108dac Mon Sep 17 00:00:00 2001 From: Sergio Garcia Date: Thu, 29 May 2025 13:20:53 +0200 Subject: [PATCH 07/12] fix(gcp): test connection by verifying token (#7882) --- prowler/providers/gcp/gcp_provider.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/prowler/providers/gcp/gcp_provider.py b/prowler/providers/gcp/gcp_provider.py index 789c434bb4..63c1c6d51d 100644 --- a/prowler/providers/gcp/gcp_provider.py +++ b/prowler/providers/gcp/gcp_provider.py @@ -515,9 +515,9 @@ class GcpProvider(Provider): credentials=session, ) - # Test the connection using the Service Usage API since it is enabled by default - client = discovery.build("serviceusage", "v1", credentials=session) - request = client.services().list(parent=f"projects/{project_id}") + # Test the connection using OAuth2 API to verify token validity + client = discovery.build("oauth2", "v2", credentials=session) + request = client.tokeninfo() request.execute() return Connection(is_connected=True) From 71ac703e6fdc728cd8854dab7ee1ea8a9c6c450b Mon Sep 17 00:00:00 2001 From: Daniel Barranquero <74871504+danibarranqueroo@users.noreply.github.com> Date: Thu, 29 May 2025 16:38:15 +0200 Subject: [PATCH 08/12] fix(api): connection correctly reflected (#7831) Co-authored-by: Pepe Fagoaga --- api/CHANGELOG.md | 3 +++ api/src/backend/tasks/jobs/scan.py | 8 +++++++- api/src/backend/tasks/tests/test_scan.py | 5 +++++ 3 files changed, 15 insertions(+), 1 deletion(-) diff --git a/api/CHANGELOG.md b/api/CHANGELOG.md index 6459e17086..e7ad901fc4 100644 --- a/api/CHANGELOG.md +++ b/api/CHANGELOG.md @@ -10,6 +10,9 @@ All notable changes to the **Prowler API** are documented in this file. ### Changed - Renamed field encrypted_password to password for M365 provider [(#7784)](https://github.com/prowler-cloud/prowler/pull/7784) +### Fixed +- Fixed the connection status verification before launching a scan [(#7831)](https://github.com/prowler-cloud/prowler/pull/7831) + --- ## [v1.8.2] (Prowler v5.7.2) diff --git a/api/src/backend/tasks/jobs/scan.py b/api/src/backend/tasks/jobs/scan.py index 81fbfbb0e9..1f4f9a8b94 100644 --- a/api/src/backend/tasks/jobs/scan.py +++ b/api/src/backend/tasks/jobs/scan.py @@ -124,6 +124,7 @@ def perform_prowler_scan( unique_resources = set() scan_resource_cache: set[tuple[str, str, str, str]] = set() start_time = time.time() + exc = None with rls_transaction(tenant_id): provider_instance = Provider.objects.get(pk=provider_id) @@ -139,7 +140,7 @@ def perform_prowler_scan( provider_instance.connected = True except Exception as e: provider_instance.connected = False - raise ValueError( + exc = ValueError( f"Provider {provider_instance.provider} is not connected: {e}" ) finally: @@ -148,6 +149,11 @@ def perform_prowler_scan( ) provider_instance.save() + # If the provider is not connected, raise an exception outside the transaction. + # If raised within the transaction, the transaction will be rolled back and the provider will not be marked as not connected. + if exc: + raise exc + prowler_scan = ProwlerScan(provider=prowler_provider, checks=checks_to_execute) resource_cache = {} diff --git a/api/src/backend/tasks/tests/test_scan.py b/api/src/backend/tasks/tests/test_scan.py index 8880b8bd03..a5fde62963 100644 --- a/api/src/backend/tasks/tests/test_scan.py +++ b/api/src/backend/tasks/tests/test_scan.py @@ -1,5 +1,6 @@ import json import uuid +from datetime import datetime from unittest.mock import MagicMock, patch import pytest @@ -206,6 +207,10 @@ class TestPerformScan: scan.refresh_from_db() assert scan.state == StateChoices.FAILED + provider.refresh_from_db() + assert provider.connected is False + assert isinstance(provider.connection_last_checked_at, datetime) + @pytest.mark.parametrize( "last_status, new_status, expected_delta", [ From 6c3653c483375182452822755dd2cb657c123ebd Mon Sep 17 00:00:00 2001 From: Hugo Pereira Brito <101209179+HugoPBrito@users.noreply.github.com> Date: Fri, 30 May 2025 10:01:32 +0200 Subject: [PATCH 09/12] fix(docs): remove warning of encrypted password for cloud (#7886) --- .../tutorials/microsoft365/getting-started-m365.md | 14 -------------- 1 file changed, 14 deletions(-) diff --git a/docs/tutorials/microsoft365/getting-started-m365.md b/docs/tutorials/microsoft365/getting-started-m365.md index f94707d86b..f648b073af 100644 --- a/docs/tutorials/microsoft365/getting-started-m365.md +++ b/docs/tutorials/microsoft365/getting-started-m365.md @@ -176,20 +176,6 @@ Follow these steps to assign the role: ![App Overview](./img/app-overview.png) -???+ warning - For Prowler Cloud encrypted password is still needed (when we update Prowler Cloud and regular password is accepted this warning will be deleted), so the password that you paste in the next step should be generated following this steps: - - - UNIX: Open a PowerShell cmd with a [supported version](../../getting-started/requirements.md#supported-powershell-versions) and then run the following command: - - ```console - $securePassword = ConvertTo-SecureString "examplepassword" -AsPlainText -Force - $encryptedPassword = $securePassword | ConvertFrom-SecureString - Write-Output $encryptedPassword - 6500780061006d0070006c006500700061007300730077006f0072006400 - ``` - - - Windows: Install WSL using `wsl --install -d Ubuntu-22.04`, then open the Ubuntu terminal, install powershell and run the same command above. - 2. Go to Prowler Cloud/App and paste: From 878e4e0bbcf903d7651a06cf98394bfb52759d41 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Adri=C3=A1n=20Jes=C3=BAs=20Pe=C3=B1a=20Rodr=C3=ADguez?= Date: Fri, 30 May 2025 10:07:32 +0200 Subject: [PATCH 10/12] fix: add new get method to avoid race conditions when creating async tasks (#7876) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Víctor Fernández Poyatos --- .env | 4 +++ api/CHANGELOG.md | 1 + api/src/backend/api/models.py | 40 ++++++++++++++++++++++++ api/src/backend/api/tests/test_models.py | 37 +++++++++++++++++++++- api/src/backend/api/v1/views.py | 8 ++--- 5 files changed, 85 insertions(+), 5 deletions(-) diff --git a/.env b/.env index 1fa497f3e6..1f53b81153 100644 --- a/.env +++ b/.env @@ -24,6 +24,10 @@ POSTGRES_USER=prowler POSTGRES_PASSWORD=postgres POSTGRES_DB=prowler_db +# Celery-Prowler task settings +TASK_RETRY_DELAY_SECONDS=0.1 +TASK_RETRY_ATTEMPTS=5 + # Valkey settings # If running Valkey and celery on host, use localhost, else use 'valkey' VALKEY_HOST=valkey diff --git a/api/CHANGELOG.md b/api/CHANGELOG.md index e7ad901fc4..937c090f85 100644 --- a/api/CHANGELOG.md +++ b/api/CHANGELOG.md @@ -20,6 +20,7 @@ All notable changes to the **Prowler API** are documented in this file. ### Fixed - Fixed task lookup to use task_kwargs instead of task_args for scan report resolution. [(#7830)](https://github.com/prowler-cloud/prowler/pull/7830) - Fixed Kubernetes UID validation to allow valid context names [(#7871)](https://github.com/prowler-cloud/prowler/pull/7871) +- Fixed a race condition when creating background tasks [(#7876)](https://github.com/prowler-cloud/prowler/pull/7876). --- diff --git a/api/src/backend/api/models.py b/api/src/backend/api/models.py index 2b4fec00cf..9564399868 100644 --- a/api/src/backend/api/models.py +++ b/api/src/backend/api/models.py @@ -1,7 +1,9 @@ import json import re +import time from uuid import UUID, uuid4 +from config.env import env from cryptography.fernet import Fernet from django.conf import settings from django.contrib.auth.models import AbstractBaseUser @@ -352,6 +354,42 @@ class ProviderGroupMembership(RowLevelSecurityProtectedModel): resource_name = "provider_groups-provider" +class TaskManager(models.Manager): + def get_with_retry( + self, + id: str, + max_retries: int = None, + delay_seconds: float = None, + ): + """ + Retry fetching a Task by ID in case it hasn't been created yet. + + Args: + id (str): The Celery task ID (expected to match Task model PK). + max_retries (int, optional): Number of retry attempts. Defaults to env TASK_RETRY_ATTEMPTS or 5. + delay_seconds (float, optional): Delay between retries in seconds. Defaults to env TASK_RETRY_DELAY_SECONDS or 0.1. + + Returns: + Task: The retrieved Task instance. + + Raises: + Task.DoesNotExist: If the task is not found after all retries. + """ + max_retries = max_retries or env.int("TASK_RETRY_ATTEMPTS", default=5) + delay_seconds = delay_seconds or env.float( + "TASK_RETRY_DELAY_SECONDS", default=0.1 + ) + + for _attempt in range(max_retries): + try: + return self.get(id=id) + except self.model.DoesNotExist: + time.sleep(delay_seconds) + raise self.model.DoesNotExist( + f"Task with ID {id} not found after {max_retries} retries." + ) + + class Task(RowLevelSecurityProtectedModel): id = models.UUIDField(primary_key=True, default=uuid4, editable=False) inserted_at = models.DateTimeField(auto_now_add=True, editable=False) @@ -364,6 +402,8 @@ class Task(RowLevelSecurityProtectedModel): blank=True, ) + objects = TaskManager() + class Meta(RowLevelSecurityProtectedModel.Meta): db_table = "tasks" diff --git a/api/src/backend/api/tests/test_models.py b/api/src/backend/api/tests/test_models.py index c2beeb3583..de2ec7c59e 100644 --- a/api/src/backend/api/tests/test_models.py +++ b/api/src/backend/api/tests/test_models.py @@ -1,6 +1,9 @@ +import uuid +from unittest import mock + import pytest -from api.models import Resource, ResourceTag +from api.models import Resource, ResourceTag, Task @pytest.mark.django_db @@ -120,3 +123,35 @@ class TestResourceModel: # compliance={}, # ) # assert Finding.objects.filter(uid=long_uid).exists() + + +@pytest.mark.django_db +class TestTaskManager: + def test_get_with_retry_success(self): + task_id = uuid.uuid4() + call_counter = {"count": 0} + + def side_effect(*args, **kwargs): + if call_counter["count"] < 2: + call_counter["count"] += 1 + raise Task.DoesNotExist() + return Task(id=task_id) + + with mock.patch.object(Task.objects, "get", side_effect=side_effect): + task = Task.objects.get_with_retry( + task_id, max_retries=5, delay_seconds=0.01 + ) + + assert task.id == task_id + assert call_counter["count"] == 2 + + def test_get_with_retry_fail(self): + non_existent_id = uuid.uuid4() + + with mock.patch.object(Task.objects, "get", side_effect=Task.DoesNotExist): + with pytest.raises(Task.DoesNotExist) as excinfo: + Task.objects.get_with_retry( + non_existent_id, max_retries=3, delay_seconds=0.01 + ) + + assert str(non_existent_id) in str(excinfo.value) diff --git a/api/src/backend/api/v1/views.py b/api/src/backend/api/v1/views.py index b8679217f9..8fc87d2808 100644 --- a/api/src/backend/api/v1/views.py +++ b/api/src/backend/api/v1/views.py @@ -1086,7 +1086,7 @@ class ProviderViewSet(BaseRLSViewSet): task = check_provider_connection_task.delay( provider_id=pk, tenant_id=self.request.tenant_id ) - prowler_task = Task.objects.get(id=task.id) + prowler_task = Task.objects.get_with_retry(id=task.id) serializer = TaskSerializer(prowler_task) return Response( data=serializer.data, @@ -1109,7 +1109,7 @@ class ProviderViewSet(BaseRLSViewSet): task = delete_provider_task.delay( provider_id=pk, tenant_id=self.request.tenant_id ) - prowler_task = Task.objects.get(id=task.id) + prowler_task = Task.objects.get_with_retry(id=task.id) serializer = TaskSerializer(prowler_task) return Response( data=serializer.data, @@ -1489,10 +1489,10 @@ class ScanViewSet(BaseRLSViewSet): }, ) + prowler_task = Task.objects.get_with_retry(id=task.id) scan.task_id = task.id scan.save(update_fields=["task_id"]) - prowler_task = Task.objects.get(id=task.id) self.response_serializer_class = TaskSerializer output_serializer = self.get_serializer(prowler_task) @@ -2823,7 +2823,7 @@ class ScheduleViewSet(BaseRLSViewSet): with transaction.atomic(): task = schedule_provider_scan(provider_instance) - prowler_task = Task.objects.get(id=task.id) + prowler_task = Task.objects.get_with_retry(id=task.id) self.response_serializer_class = TaskSerializer output_serializer = self.get_serializer(prowler_task) From b256c1062275771fed52904e6f133b159fa620ae Mon Sep 17 00:00:00 2001 From: Hugo Pereira Brito <101209179+HugoPBrito@users.noreply.github.com> Date: Fri, 30 May 2025 10:24:49 +0200 Subject: [PATCH 11/12] chore: replace `Directory.Read.All` permission to `Domain.Read.All` for Azure (#7888) --- docs/getting-started/requirements.md | 4 ++-- docs/tutorials/azure/create-prowler-service-principal.md | 4 ++-- docs/tutorials/azure/getting-started-azure.md | 4 ++-- docs/tutorials/microsoft365/getting-started-m365.md | 4 ++-- prowler/CHANGELOG.md | 1 + 5 files changed, 9 insertions(+), 8 deletions(-) diff --git a/docs/getting-started/requirements.md b/docs/getting-started/requirements.md index 6e79f86bba..e5d6087c1f 100644 --- a/docs/getting-started/requirements.md +++ b/docs/getting-started/requirements.md @@ -70,7 +70,7 @@ The other three cases does not need additional configuration, `--az-cli-auth` an Prowler for Azure needs two types of permission scopes to be set: - **Microsoft Entra ID permissions**: used to retrieve metadata from the identity assumed by Prowler and specific Entra checks (not mandatory to have access to execute the tool). The permissions required by the tool are the following: - - `Directory.Read.All` + - `Domain.Read.All` - `Policy.Read.All` - `UserAuthenticationMethod.Read.All` (used only for the Entra checks related with multifactor authentication) - **Subscription scope permissions**: required to launch the checks against your resources, mandatory to launch the tool. It is required to add the following RBAC builtin roles per subscription to the entity that is going to be assumed by the tool: @@ -199,7 +199,7 @@ Since this is a delegated permission authentication method, necessary permission Prowler for M365 requires two types of permission scopes to be set (if you want to run the full provider including PowerShell checks). Both must be configured using Microsoft Entra ID: - **Service Principal Application Permissions**: These are set at the **application** level and are used to retrieve data from the identity being assessed: - - `Directory.Read.All`: Required for all services. + - `Domain.Read.All`: Required for all services. - `Policy.Read.All`: Required for all services. - `User.Read` (IMPORTANT: this must be set as **delegated**): Required for the sign-in. - `SharePointTenantSettings.Read.All`: Required for SharePoint service. diff --git a/docs/tutorials/azure/create-prowler-service-principal.md b/docs/tutorials/azure/create-prowler-service-principal.md index 02ab7ea629..25c170df97 100644 --- a/docs/tutorials/azure/create-prowler-service-principal.md +++ b/docs/tutorials/azure/create-prowler-service-principal.md @@ -40,7 +40,7 @@ az ad sp create-for-rbac --name "ProwlerApp" To allow Prowler to retrieve metadata from the identity assumed and run specific Entra checks, it is needed to assign the following permissions: -- `Directory.Read.All` +- `Domain.Read.All` - `Policy.Read.All` - `UserAuthenticationMethod.Read.All` (used only for the Entra checks related with multifactor authentication) @@ -58,7 +58,7 @@ To assign the permissions you can make it from the Azure Portal or using the Azu 5. Then click on "+ Add a permission" and select "Microsoft Graph" 6. Once in the "Microsoft Graph" view, select "Application permissions" 7. Finally, search for "Directory", "Policy" and "UserAuthenticationMethod" select the following permissions: - - `Directory.Read.All` + - `Domain.Read.All` - `Policy.Read.All` - `UserAuthenticationMethod.Read.All` 8. Click on "Add permissions" to apply the new permissions. diff --git a/docs/tutorials/azure/getting-started-azure.md b/docs/tutorials/azure/getting-started-azure.md index b9331703e8..5dead442d7 100644 --- a/docs/tutorials/azure/getting-started-azure.md +++ b/docs/tutorials/azure/getting-started-azure.md @@ -90,7 +90,7 @@ A Service Principal is required to grant Prowler the necessary privileges. Assign the following Microsoft Graph permissions: - - Directory.Read.All + - Domain.Read.All - Policy.Read.All @@ -107,7 +107,7 @@ Assign the following Microsoft Graph permissions: 3. Search and select: - - `Directory.Read.All` + - `Domain.Read.All` - `Policy.Read.All` - `UserAuthenticationMethod.Read.All` diff --git a/docs/tutorials/microsoft365/getting-started-m365.md b/docs/tutorials/microsoft365/getting-started-m365.md index f648b073af..5e0364faaf 100644 --- a/docs/tutorials/microsoft365/getting-started-m365.md +++ b/docs/tutorials/microsoft365/getting-started-m365.md @@ -96,7 +96,7 @@ With this done you will have all the needed keys, summarized in the following ta Assign the following Microsoft Graph permissions: -- `Directory.Read.All`: Required for all services. +- `Domain.Read.All`: Required for all services. - `Policy.Read.All`: Required for all services. - `SharePointTenantSettings.Read.All`: Required for SharePoint service. - `AuditLog.Read.All`: Required for Entra service. @@ -114,7 +114,7 @@ Follow these steps to assign the permissions: 3. Search and select every permission below and once all are selected click on `Add permissions`: - - `Directory.Read.All` + - `Domain.Read.All` - `Policy.Read.All` - `SharePointTenantSettings.Read.All` - `AuditLog.Read.All`: Required for Entra service. diff --git a/prowler/CHANGELOG.md b/prowler/CHANGELOG.md index 2f6de1ce04..34e9fc3037 100644 --- a/prowler/CHANGELOG.md +++ b/prowler/CHANGELOG.md @@ -28,6 +28,7 @@ All notable changes to the **Prowler SDK** are documented in this file. - Fix `admincenter_users_admins_reduced_license_footprint` check logic to pass when admin user has no license. [(#7779)](https://github.com/prowler-cloud/prowler/pull/7779) - Fix `m365_powershell` to close the PowerShell sessions in msgraph services. [(#7816)](https://github.com/prowler-cloud/prowler/pull/7816) - Fix `defender_ensure_notify_alerts_severity_is_high`check to accept high or lower severity. [(#7862)](https://github.com/prowler-cloud/prowler/pull/7862) +- Replace `Directory.Read.All` permission with `Domain.Read.All` which is more restrictive. [(#7888)](https://github.com/prowler-cloud/prowler/pull/7888) --- From 4888c277133f6191ea40bc37dd2cee7d674704f7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?C=C3=A9sar=20Arroba?= <19954079+cesararroba@users.noreply.github.com> Date: Fri, 30 May 2025 13:55:57 +0200 Subject: [PATCH 12/12] chore: fix commit sha when a pr is merged (#7889) --- .github/workflows/pull-request-merged.yml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.github/workflows/pull-request-merged.yml b/.github/workflows/pull-request-merged.yml index 090950f79b..7bc05dc3c4 100644 --- a/.github/workflows/pull-request-merged.yml +++ b/.github/workflows/pull-request-merged.yml @@ -18,7 +18,7 @@ jobs: - name: Set short git commit SHA id: vars run: | - shortSha=$(git rev-parse --short ${{ github.sha }}) + shortSha=$(git rev-parse --short ${{ github.event.pull_request.merge_commit_sha }}) echo "SHORT_SHA=${shortSha}" >> $GITHUB_ENV - name: Trigger pull request @@ -28,7 +28,7 @@ jobs: repository: ${{ secrets.CLOUD_DISPATCH }} event-type: prowler-pull-request-merged client-payload: '{ - "PROWLER_COMMIT_SHA": "${{ github.sha }}", + "PROWLER_COMMIT_SHA": "${{ github.event.pull_request.merge_commit_sha }}", "PROWLER_COMMIT_SHORT_SHA": "${{ env.SHORT_SHA }}", "PROWLER_PR_TITLE": "${{ github.event.pull_request.title }}", "PROWLER_PR_LABELS": ${{ toJson(github.event.pull_request.labels.*.name) }},