diff --git a/api/CHANGELOG.md b/api/CHANGELOG.md index 321534c8f5..120b04dbb3 100644 --- a/api/CHANGELOG.md +++ b/api/CHANGELOG.md @@ -2,6 +2,22 @@ All notable changes to the **Prowler API** are documented in this file. +## [1.32.2] (Prowler UNRELEASED) + +### 🐞 Fixed + +- `scan-perform` no longer reports an error when a provider is deleted during a running scan [(#11696)](https://github.com/prowler-cloud/prowler/pull/11696) + +--- + +## [1.32.1] (Prowler v5.31.1) + +### 🐞 Fixed + +- API key auth no longer mutates `TenantAPIKey.objects` during admin DB lookups [(#11686)](https://github.com/prowler-cloud/prowler/pull/11686) + +--- + ## [1.32.0] (Prowler v5.31.0) ### πŸš€ Added diff --git a/api/src/backend/api/authentication.py b/api/src/backend/api/authentication.py index 1e9eee46ff..755bd64e39 100644 --- a/api/src/backend/api/authentication.py +++ b/api/src/backend/api/authentication.py @@ -1,11 +1,14 @@ +from math import isfinite from uuid import UUID from api.db_router import MainRouter from api.models import TenantAPIKey, TenantAPIKeyManager from cryptography.fernet import InvalidToken +from django.core.exceptions import ObjectDoesNotExist from django.utils import timezone from drf_simple_apikey.backends import APIKeyAuthentication as BaseAPIKeyAuth from drf_simple_apikey.crypto import get_crypto +from drf_simple_apikey.settings import package_settings from rest_framework.authentication import BaseAuthentication from rest_framework.exceptions import AuthenticationFailed from rest_framework.request import Request @@ -21,18 +24,49 @@ class TenantAPIKeyAuthentication(BaseAPIKeyAuth): def _authenticate_credentials(self, request, key): """ Override to use admin connection, bypassing RLS during authentication. - Delegates to parent after temporarily routing model queries to admin DB. """ - # Temporarily point the model's manager to admin database - original_objects = self.model.objects - self.model.objects = self.model.objects.using(MainRouter.admin_db) + try: + payload = self.key_crypto.decrypt(key) + except ValueError: + raise AuthenticationFailed("Invalid API Key.") + + if not isinstance(payload, dict): + raise AuthenticationFailed("Invalid API Key.") + + payload_pk = payload.get("_pk") + payload_exp = payload.get("_exp") + if ( + not isinstance(payload_pk, str) + or isinstance(payload_exp, bool) + or not isinstance(payload_exp, (int, float)) + or not isfinite(payload_exp) + ): + raise AuthenticationFailed("Invalid API Key.") try: - # Call parent method which will now use admin database - return super()._authenticate_credentials(request, key) - finally: - # Restore original manager - self.model.objects = original_objects + api_key_pk = UUID(payload_pk) + except ValueError: + raise AuthenticationFailed("Invalid API Key.") + + if payload_exp < timezone.now().timestamp(): + raise AuthenticationFailed("API Key has already expired.") + + try: + api_key = self.model.objects.using(MainRouter.admin_db).get(id=api_key_pk) + except ObjectDoesNotExist: + raise AuthenticationFailed("No entity matching this api key.") + + if api_key.revoked: + raise AuthenticationFailed("This API Key has been revoked.") + + client_ip = request.META.get(package_settings.IP_ADDRESS_HEADER) + if api_key.blacklisted_ips and client_ip in api_key.blacklisted_ips: + raise AuthenticationFailed("Access denied from blacklisted IP.") + + if api_key.whitelisted_ips and client_ip not in api_key.whitelisted_ips: + raise AuthenticationFailed("Access restricted to specific IP addresses.") + + return api_key.entity, key def authenticate(self, request: Request): prefixed_key = self.get_key(request) diff --git a/api/src/backend/api/tests/test_authentication.py b/api/src/backend/api/tests/test_authentication.py index fa72b395b3..d05a55ce5a 100644 --- a/api/src/backend/api/tests/test_authentication.py +++ b/api/src/backend/api/tests/test_authentication.py @@ -7,6 +7,7 @@ import pytest from api.authentication import SSEAuthentication, TenantAPIKeyAuthentication from api.db_router import MainRouter from api.models import TenantAPIKey +from django.db.models.query import QuerySet from django.test import RequestFactory from rest_framework.exceptions import AuthenticationFailed @@ -64,6 +65,54 @@ class TestTenantAPIKeyAuthentication: # Verify the manager was restored assert TenantAPIKey.objects == original_manager + def test_authenticate_credentials_keeps_manager_during_lookup( + self, auth_backend, api_keys_fixture, request_factory + ): + """Authentication must not expose a QuerySet as the model manager.""" + api_key = api_keys_fixture[0] + raw_key = api_key._raw_key + _, encrypted_key = raw_key.split(TenantAPIKey.objects.separator, 1) + + original_get = QuerySet.get + manager_has_create_api_key = [] + + def observe_manager(queryset, *args, **kwargs): + manager_has_create_api_key.append( + hasattr(TenantAPIKey.objects, "create_api_key") + ) + return original_get(queryset, *args, **kwargs) + + request = request_factory.get("/") + + with patch.object(QuerySet, "get", observe_manager): + auth_backend._authenticate_credentials(request, encrypted_key) + + assert manager_has_create_api_key + assert all(manager_has_create_api_key) + + @pytest.mark.parametrize( + "payload", + [ + {"_pk": str(uuid4()), "_exp": "not-a-timestamp"}, + { + "_pk": "not-a-uuid", + "_exp": (datetime.now(UTC) + timedelta(days=1)).timestamp(), + }, + {"_pk": str(uuid4()), "_exp": True}, + ], + ) + def test_authenticate_credentials_rejects_malformed_payloads( + self, auth_backend, request_factory, payload + ): + """Malformed decrypted payloads fail as authentication errors.""" + request = request_factory.get("/") + encrypted_key = auth_backend.key_crypto.generate(payload) + + with pytest.raises(AuthenticationFailed) as exc_info: + auth_backend._authenticate_credentials(request, encrypted_key) + + assert str(exc_info.value.detail) == "Invalid API Key." + def test_authenticate_credentials_restores_manager_on_exception( self, auth_backend, request_factory ): diff --git a/api/src/backend/config/django/testing.py b/api/src/backend/config/django/testing.py index a1e5c29fb3..9951478bfd 100644 --- a/api/src/backend/config/django/testing.py +++ b/api/src/backend/config/django/testing.py @@ -39,3 +39,7 @@ SIMPLE_JWT["ALGORITHM"] = "HS256" # noqa: F405 SIMPLE_JWT["SIGNING_KEY"] = env.str( # noqa: F405 "DJANGO_TOKEN_SIGNING_KEY", "insecure-testing-jwt-signing-key-do-not-use-in-prod" ) + +# Tests don't need secure password hashing; PBKDF2 (~hundreds of ms per call) +# dominates fixture setup time across every create_user()/check_password(). +PASSWORD_HASHERS = ["django.contrib.auth.hashers.MD5PasswordHasher"] diff --git a/api/src/backend/conftest.py b/api/src/backend/conftest.py index 54aa99ab98..af58730e7d 100644 --- a/api/src/backend/conftest.py +++ b/api/src/backend/conftest.py @@ -69,6 +69,107 @@ TEST_USER = "dev@prowler.com" TEST_PASSWORD = "testing_psswd" +def _install_compliance_catalog_test_cache() -> None: + """Memoize the heavy SDK catalog loaders for the whole test session. + + ``get_bulk_compliance_frameworks_universal`` re-reads and Pydantic-validates + ~100 compliance JSONs (β‰ˆ20 MB) and ``CheckMetadata.get_bulk`` re-reads ~1k + check metadata files on *every* call. Production amortizes this through the + per-process lazy caches (``PROWLER_CHECKS`` / ``PROWLER_COMPLIANCE_OVERVIEW_TEMPLATE``) + and ``warm_compliance_caches``, but the test suite parametrizes over every + provider and deliberately resets the API-level caches, so the same catalogs + were re-parsed dozens of times across the suite (β‰ˆ3s/call locally, β‰ˆ19s under + coverage in CI). + + The catalog files are immutable during a run and callers treat the parsed + objects as read-only, so caching the result per provider is safe. This is the + test-only equivalent of an ``lru_cache`` on the SDK functions, without + changing SDK behavior in production. + + A second, lower-level cache memoizes ``load_compliance_framework_universal`` + **per file path**. ``get_bulk_compliance_frameworks_universal`` parses *every* + compliance JSON and only then filters by provider, so a per-provider cache + still re-parses all ~100 files on the first load of each provider. The + per-path cache makes the first provider parse the files once and every other + provider/test reuse the already-parsed ``ComplianceFramework`` objects (only + the cheap ``listdir`` + filtering re-runs). ``_load_jsons_from_dir`` calls + ``load_compliance_framework_universal`` as a module global, so patching the + attribute is picked up without touching the SDK. + + Installed at conftest import time (before test modules are collected) so that + even ``from ... import get_bulk_compliance_frameworks_universal`` bindings in + the test modules resolve to the cached wrapper. + """ + import prowler.lib.check.compliance_models as compliance_models + from prowler.lib.check.models import CheckMetadata + + original_bulk_frameworks = ( + compliance_models.get_bulk_compliance_frameworks_universal + ) + original_get_bulk = CheckMetadata.get_bulk + original_load = compliance_models.load_compliance_framework_universal + + def cached_bulk_frameworks(provider): + if provider not in _COMPLIANCE_FRAMEWORK_CACHE: + _COMPLIANCE_FRAMEWORK_CACHE[provider] = original_bulk_frameworks(provider) + return _COMPLIANCE_FRAMEWORK_CACHE[provider] + + def cached_get_bulk(provider): + if provider not in _COMPLIANCE_CHECKS_CACHE: + _COMPLIANCE_CHECKS_CACHE[provider] = original_get_bulk(provider) + return _COMPLIANCE_CHECKS_CACHE[provider] + + def cached_load(path): + if path not in _COMPLIANCE_PATH_CACHE: + _COMPLIANCE_PATH_CACHE[path] = original_load(path) + return _COMPLIANCE_PATH_CACHE[path] + + compliance_models.get_bulk_compliance_frameworks_universal = cached_bulk_frameworks + compliance_models.load_compliance_framework_universal = cached_load + CheckMetadata.get_bulk = staticmethod(cached_get_bulk) + + # ``api.compliance`` does ``from ... import get_bulk_compliance_frameworks_universal`` + # so it holds its own binding; patch it too in case it was imported first. + import api.compliance as api_compliance + + api_compliance.get_bulk_compliance_frameworks_universal = cached_bulk_frameworks + + +# Module-scoped so the ``_compliance_cache_guard`` fixture below can reset them. +# Keeping them out of ``_install_compliance_catalog_test_cache``'s local scope is +# what makes the caches resettable between tests; the wrappers above close over +# these names, and the original loaders stay referenced so patched behaviour is +# still honoured. +_COMPLIANCE_FRAMEWORK_CACHE: dict[str, dict] = {} +_COMPLIANCE_CHECKS_CACHE: dict[str, dict] = {} +_COMPLIANCE_PATH_CACHE: dict[str, object] = {} + + +_install_compliance_catalog_test_cache() + + +@pytest.fixture(autouse=True) +def _compliance_cache_guard(request): + """Reset the compliance catalog caches after any test that used ``monkeypatch``. + + The session-wide caches in ``_install_compliance_catalog_test_cache`` let the + read-only, parametrized compliance tests parse the ~100 catalog JSONs once + instead of dozens of times. A test that swaps a loader (or mutates a returned + object) could otherwise leak that state into later tests through the shared + dicts. Using ``monkeypatch`` as the opt-in signal keeps the full speed-up for + catalog-reading tests while giving patching tests a clean slate afterwards; + the next test simply repopulates the caches from disk. + """ + yield + if "monkeypatch" in request.fixturenames: + _COMPLIANCE_FRAMEWORK_CACHE.clear() + _COMPLIANCE_CHECKS_CACHE.clear() + _COMPLIANCE_PATH_CACHE.clear() + import api.compliance as api_compliance + + api_compliance.AVAILABLE_COMPLIANCE_FRAMEWORKS.clear() + + def today_after_n_days(n_days: int) -> str: return datetime.strftime( datetime.today().date() + timedelta(days=n_days), "%Y-%m-%d" diff --git a/api/src/backend/tasks/jobs/scan.py b/api/src/backend/tasks/jobs/scan.py index dcaacf6642..d69e0c8941 100644 --- a/api/src/backend/tasks/jobs/scan.py +++ b/api/src/backend/tasks/jobs/scan.py @@ -19,7 +19,7 @@ from api.db_utils import ( psycopg_connection, rls_transaction, ) -from api.exceptions import ProviderConnectionError +from api.exceptions import ProviderConnectionError, ProviderDeletedException from api.models import ( AttackSurfaceOverview, ComplianceOverviewSummary, @@ -48,7 +48,7 @@ from celery.utils.log import get_task_logger from config.django.base import DJANGO_FINDINGS_BATCH_SIZE from config.env import env from config.settings.celery import CELERY_DEADLOCK_ATTEMPTS -from django.db import IntegrityError, OperationalError +from django.db import DatabaseError, IntegrityError, OperationalError, transaction from django.db.models import ( Case, Count, @@ -117,6 +117,20 @@ ATTACK_SURFACE_PROVIDER_COMPATIBILITY = { _ATTACK_SURFACE_MAPPING_CACHE: dict[str, dict] = {} +def _save_scan_instance( + scan_instance: Scan, provider_id: str, update_fields: list[str] +) -> None: + try: + with transaction.atomic(): # Savepoint for not killing the `rls_transaction` + scan_instance.save(update_fields=update_fields) + except DatabaseError: + if Scan.objects.filter(pk=scan_instance.id).exists(): + raise + raise ProviderDeletedException( + f"Provider '{provider_id}' for scan '{scan_instance.id}' was deleted during the scan" + ) from None + + def aggregate_category_counts( categories: list[str], severity: str, @@ -1029,13 +1043,18 @@ def perform_prowler_scan( group_resources_cache: dict[str, set] = {} start_time = time.time() exc = None + skip_final_scan_update = False with rls_transaction(tenant_id): provider_instance = Provider.objects.get(pk=provider_id) scan_instance = Scan.objects.get(pk=scan_id) scan_instance.state = StateChoices.EXECUTING scan_instance.started_at = datetime.now(tz=UTC) - scan_instance.save(update_fields=["state", "started_at", "updated_at"]) + _save_scan_instance( + scan_instance, + provider_id, + ["state", "started_at", "updated_at"], + ) # Find the mutelist processor if it exists with rls_transaction(tenant_id, using=READ_REPLICA_ALIAS): @@ -1101,7 +1120,7 @@ def perform_prowler_scan( # Throttle scan_instance progress writes to avoid hammering the writer: # only persist when progress moves by at least `PROGRESS_THROTTLE_DELTA` - # OR `PROGRESS_THROTTLE_SECONDS` have elapsed. The final progress (1.0) + # OR `PROGRESS_THROTTLE_SECONDS` have elapsed. The final progress (100) # always persists in the `finally` block below. last_persisted_progress = -1.0 last_persisted_progress_at = 0.0 @@ -1143,7 +1162,11 @@ def perform_prowler_scan( ): with rls_transaction(tenant_id): scan_instance.progress = progress - scan_instance.save(update_fields=["progress", "updated_at"]) + _save_scan_instance( + scan_instance, + provider_id, + ["progress", "updated_at"], + ) last_persisted_progress = progress last_persisted_progress_at = now @@ -1170,26 +1193,39 @@ def perform_prowler_scan( batch_size=SCAN_DB_BATCH_SIZE, ) + except ProviderDeletedException as e: + logger.warning(str(e)) + exception = e + skip_final_scan_update = True except Exception as e: logger.error(f"Error performing scan {scan_id}: {e}") exception = e scan_instance.state = StateChoices.FAILED finally: - with rls_transaction(tenant_id): - scan_instance.duration = time.time() - start_time - scan_instance.completed_at = datetime.now(tz=UTC) - scan_instance.unique_resource_count = len(unique_resources) - scan_instance.save( - update_fields=[ - "state", - "duration", - "completed_at", - "unique_resource_count", - "progress", - "updated_at", - ] - ) + if not skip_final_scan_update: + try: + with rls_transaction(tenant_id): + scan_instance.duration = time.time() - start_time + scan_instance.completed_at = datetime.now(tz=UTC) + scan_instance.unique_resource_count = len(unique_resources) + if exception is None: + scan_instance.progress = 100 + _save_scan_instance( + scan_instance, + provider_id, + [ + "state", + "duration", + "completed_at", + "unique_resource_count", + "progress", + "updated_at", + ], + ) + except ProviderDeletedException as e: + logger.warning(str(e)) + exception = e if exception is not None: raise exception diff --git a/api/src/backend/tasks/tests/test_scan.py b/api/src/backend/tasks/tests/test_scan.py index 17b3b65fc4..2d251b247f 100644 --- a/api/src/backend/tasks/tests/test_scan.py +++ b/api/src/backend/tasks/tests/test_scan.py @@ -9,7 +9,7 @@ from unittest.mock import MagicMock, patch import pytest from api.db_router import MainRouter -from api.exceptions import ProviderConnectionError +from api.exceptions import ProviderConnectionError, ProviderDeletedException from api.models import ( Finding, MuteRule, @@ -262,6 +262,75 @@ class TestPerformScan: assert provider.connected is False assert isinstance(provider.connection_last_checked_at, datetime) + def test_perform_prowler_scan_provider_deleted_during_progress_update( + self, + tenants_fixture, + scans_fixture, + providers_fixture, + ): + tenant = tenants_fixture[0] + scan = scans_fixture[0] + provider = providers_fixture[0] + + tenant_id = str(tenant.id) + scan_id = str(scan.id) + provider_id = str(provider.id) + + def scan_results(): + Provider.objects.filter(pk=provider_id).delete() + yield 50, [] + + with ( + patch( + "tasks.jobs.scan.initialize_prowler_provider", + return_value=MagicMock(), + ), + patch("tasks.jobs.scan.ProwlerScan") as mock_prowler_scan_class, + patch("tasks.jobs.scan.logger.error") as mock_logger_error, + ): + mock_prowler_scan_instance = MagicMock() + mock_prowler_scan_instance.scan.return_value = scan_results() + mock_prowler_scan_class.return_value = mock_prowler_scan_instance + + with pytest.raises(ProviderDeletedException): + perform_prowler_scan(tenant_id, scan_id, provider_id, []) + + mock_logger_error.assert_not_called() + assert not Scan.objects.filter(pk=scan_id).exists() + + def test_perform_prowler_scan_sets_final_progress_when_progress_updates_are_throttled( + self, + tenants_fixture, + scans_fixture, + providers_fixture, + ): + tenant = tenants_fixture[0] + scan = scans_fixture[0] + provider = providers_fixture[0] + + tenant_id = str(tenant.id) + scan_id = str(scan.id) + provider_id = str(provider.id) + + with ( + patch( + "tasks.jobs.scan.initialize_prowler_provider", + return_value=MagicMock(), + ), + patch("tasks.jobs.scan.ProwlerScan") as mock_prowler_scan_class, + patch("tasks.jobs.scan.PROGRESS_THROTTLE_DELTA", 200), + patch("tasks.jobs.scan.PROGRESS_THROTTLE_SECONDS", 3600), + ): + mock_prowler_scan_instance = MagicMock() + mock_prowler_scan_instance.scan.return_value = [(99, []), (100, [])] + mock_prowler_scan_class.return_value = mock_prowler_scan_instance + + perform_prowler_scan(tenant_id, scan_id, provider_id, []) + + scan.refresh_from_db() + assert scan.state == StateChoices.COMPLETED + assert scan.progress == 100 + @pytest.mark.parametrize( "last_status, new_status, expected_delta", [ diff --git a/prowler/CHANGELOG.md b/prowler/CHANGELOG.md index b040b32a78..1d6dd42262 100644 --- a/prowler/CHANGELOG.md +++ b/prowler/CHANGELOG.md @@ -2,6 +2,23 @@ All notable changes to the **Prowler SDK** are documented in this file. +## [5.32.0] (Prowler UNRELEASED) + +### πŸš€ Added + +- `entra_conditional_access_policy_explicitly_targets_azure_devops` check for M365 provider, verifying at least one enabled Conditional Access policy explicitly includes the Azure DevOps cloud application instead of relying on a broad "All cloud apps" policy [(#11182)](https://github.com/prowler-cloud/prowler/pull/11182) +- `entra_conditional_access_policy_no_exclusion_gaps` check for M365 provider, verifying every user, group, role, or application excluded from an enabled Conditional Access policy stays in scope of another enabled policy [(#11577)](https://github.com/prowler-cloud/prowler/pull/11577) + +--- + +## [5.31.1] (Prowler v5.31.1) + +### 🐞 Fixed + +- Alibaba Cloud `ram_password_policy_number` and `cs_kubernetes_cluster_check_weekly` checks not being loaded due to missing implementation and package files [(#11683)](https://github.com/prowler-cloud/prowler/pull/11683) + +--- + ## [5.31.0] (Prowler v5.31.0) ### πŸš€ Added diff --git a/prowler/providers/alibabacloud/services/cs/cs_kubernetes_cluster_check_weekly/__init__.py b/prowler/providers/alibabacloud/services/cs/cs_kubernetes_cluster_check_weekly/__init__.py new file mode 100644 index 0000000000..e69de29bb2 diff --git a/prowler/providers/alibabacloud/services/ram/ram_password_policy_number/ram_password_policy_number.py b/prowler/providers/alibabacloud/services/ram/ram_password_policy_number/ram_password_policy_number.py new file mode 100644 index 0000000000..305570289c --- /dev/null +++ b/prowler/providers/alibabacloud/services/ram/ram_password_policy_number/ram_password_policy_number.py @@ -0,0 +1,34 @@ +from prowler.lib.check.models import Check, CheckReportAlibabaCloud +from prowler.providers.alibabacloud.services.ram.ram_client import ram_client + + +class ram_password_policy_number(Check): + """Check if RAM password policy requires at least one number.""" + + def execute(self) -> list[CheckReportAlibabaCloud]: + findings = [] + + if ram_client.password_policy: + report = CheckReportAlibabaCloud( + metadata=self.metadata(), resource=ram_client.password_policy + ) + report.region = ram_client.region + report.resource_id = f"{ram_client.audited_account}-password-policy" + report.resource_arn = ( + f"acs:ram::{ram_client.audited_account}:password-policy" + ) + + if ram_client.password_policy.require_numbers: + report.status = "PASS" + report.status_extended = ( + "RAM password policy requires at least one number." + ) + else: + report.status = "FAIL" + report.status_extended = ( + "RAM password policy does not require at least one number." + ) + + findings.append(report) + + return findings diff --git a/prowler/providers/m365/services/entra/entra_conditional_access_policy_explicitly_targets_azure_devops/__init__.py b/prowler/providers/m365/services/entra/entra_conditional_access_policy_explicitly_targets_azure_devops/__init__.py new file mode 100644 index 0000000000..e69de29bb2 diff --git a/prowler/providers/m365/services/entra/entra_conditional_access_policy_explicitly_targets_azure_devops/entra_conditional_access_policy_explicitly_targets_azure_devops.metadata.json b/prowler/providers/m365/services/entra/entra_conditional_access_policy_explicitly_targets_azure_devops/entra_conditional_access_policy_explicitly_targets_azure_devops.metadata.json new file mode 100644 index 0000000000..c352854003 --- /dev/null +++ b/prowler/providers/m365/services/entra/entra_conditional_access_policy_explicitly_targets_azure_devops/entra_conditional_access_policy_explicitly_targets_azure_devops.metadata.json @@ -0,0 +1,43 @@ +{ + "Provider": "m365", + "CheckID": "entra_conditional_access_policy_explicitly_targets_azure_devops", + "CheckTitle": "Conditional Access Policy explicitly targets Azure DevOps", + "CheckType": [], + "ServiceName": "entra", + "SubServiceName": "", + "ResourceIdTemplate": "", + "Severity": "medium", + "ResourceType": "NotDefined", + "ResourceGroup": "IAM", + "Description": "Microsoft Entra **Conditional Access** is verified to have at least one **enabled** policy that explicitly includes the **Azure DevOps** cloud application. Policies targeting **All** cloud apps do not satisfy this check because the goal is to verify that Azure DevOps has been deliberately considered.", + "Risk": "Without an explicit Conditional Access policy for Azure DevOps, organizations may rely on broad policies that do not account for Azure DevOps-specific access patterns such as CLI, IDE plug-ins, PAT-based workflows, source code access, build pipelines, secrets, and service connections.", + "RelatedUrl": "", + "AdditionalURLs": [ + "https://learn.microsoft.com/en-us/graph/api/resources/conditionalaccesspolicy?view=graph-rest-1.0", + "https://learn.microsoft.com/en-us/graph/api/resources/conditionalaccessapplications?view=graph-rest-1.0", + "https://learn.microsoft.com/en-us/azure/devops/organizations/accounts/manage-conditional-access", + "https://learn.microsoft.com/en-us/troubleshoot/entra/entra-id/governance/verify-first-party-apps-sign-in" + ], + "Remediation": { + "Code": { + "CLI": "", + "NativeIaC": "", + "Other": "1. Navigate to the Microsoft Entra admin center (https://entra.microsoft.com).\n2. Expand **Protection** > **Conditional Access** and select **Policies**.\n3. Create or edit a policy for Azure DevOps.\n4. Under **Target resources**, select **Include** > **Select apps** and choose **Azure DevOps**.\n5. Configure the required grant or session controls for your organization.\n6. Set the policy to **Report-only** until validated, then enable it.", + "Terraform": "" + }, + "Recommendation": { + "Text": "Create and enable a Conditional Access policy that explicitly includes the Azure DevOps cloud application, then configure the appropriate access controls for your organization.", + "Url": "https://hub.prowler.com/check/entra_conditional_access_policy_explicitly_targets_azure_devops" + } + }, + "Categories": [ + "identity-access", + "trust-boundaries", + "e3" + ], + "DependsOn": [], + "RelatedTo": [ + "entra_conditional_access_policy_all_apps_all_users" + ], + "Notes": "Azure DevOps Services uses appId 499b84ac-1321-427f-aa17-267ca6975798." +} diff --git a/prowler/providers/m365/services/entra/entra_conditional_access_policy_explicitly_targets_azure_devops/entra_conditional_access_policy_explicitly_targets_azure_devops.py b/prowler/providers/m365/services/entra/entra_conditional_access_policy_explicitly_targets_azure_devops/entra_conditional_access_policy_explicitly_targets_azure_devops.py new file mode 100644 index 0000000000..f1e874fc91 --- /dev/null +++ b/prowler/providers/m365/services/entra/entra_conditional_access_policy_explicitly_targets_azure_devops/entra_conditional_access_policy_explicitly_targets_azure_devops.py @@ -0,0 +1,57 @@ +"""Check if a Conditional Access policy explicitly targets Azure DevOps.""" + +from prowler.lib.check.models import Check, CheckReportM365 +from prowler.providers.m365.services.entra.entra_client import entra_client +from prowler.providers.m365.services.entra.entra_service import ( + ConditionalAccessPolicyState, +) + +AZURE_DEVOPS_APP_ID = "499b84ac-1321-427f-aa17-267ca6975798" + + +class entra_conditional_access_policy_explicitly_targets_azure_devops(Check): + """Check that an enabled Conditional Access policy explicitly targets Azure DevOps.""" + + def execute(self) -> list[CheckReportM365]: + """Execute the check for explicit Azure DevOps targeting. + + Returns: + A list of reports containing the result of the check. + """ + findings = [] + report = CheckReportM365( + metadata=self.metadata(), + resource={}, + resource_name="Conditional Access Policies", + resource_id="conditionalAccessPolicies", + ) + report.status = "FAIL" + report.status_extended = ( + "No enabled Conditional Access Policy explicitly targets Azure DevOps." + ) + + for policy in entra_client.conditional_access_policies.values(): + if policy.state != ConditionalAccessPolicyState.ENABLED: + continue + + if not policy.conditions.application_conditions: + continue + + if ( + AZURE_DEVOPS_APP_ID + not in policy.conditions.application_conditions.included_applications + ): + continue + + report = CheckReportM365( + metadata=self.metadata(), + resource=policy, + resource_name=policy.display_name, + resource_id=policy.id, + ) + report.status = "PASS" + report.status_extended = f"Conditional Access Policy {policy.display_name} explicitly targets Azure DevOps." + break + + findings.append(report) + return findings diff --git a/prowler/providers/m365/services/entra/entra_conditional_access_policy_no_exclusion_gaps/__init__.py b/prowler/providers/m365/services/entra/entra_conditional_access_policy_no_exclusion_gaps/__init__.py new file mode 100644 index 0000000000..e69de29bb2 diff --git a/prowler/providers/m365/services/entra/entra_conditional_access_policy_no_exclusion_gaps/entra_conditional_access_policy_no_exclusion_gaps.metadata.json b/prowler/providers/m365/services/entra/entra_conditional_access_policy_no_exclusion_gaps/entra_conditional_access_policy_no_exclusion_gaps.metadata.json new file mode 100644 index 0000000000..35c80b8054 --- /dev/null +++ b/prowler/providers/m365/services/entra/entra_conditional_access_policy_no_exclusion_gaps/entra_conditional_access_policy_no_exclusion_gaps.metadata.json @@ -0,0 +1,42 @@ +{ + "Provider": "m365", + "CheckID": "entra_conditional_access_policy_no_exclusion_gaps", + "CheckTitle": "Conditional Access exclusions are covered by another policy (no exclusion gaps)", + "CheckType": [], + "ServiceName": "entra", + "SubServiceName": "", + "ResourceIdTemplate": "", + "Severity": "medium", + "ResourceType": "NotDefined", + "ResourceGroup": "IAM", + "Description": "Verifies that every object excluded from an enabled Microsoft Entra **Conditional Access** policy (users, groups, roles, or applications) is still included by at least one enabled policy, so the exclusion keeps a compensating control. The Directory Synchronization Accounts role and confirmed emergency access (break glass) accounts are treated as intentional and not reported.", + "Risk": "An object excluded from a Conditional Access policy but never included by any other enabled policy sits completely outside Conditional Access enforcement. This creates a silent **MFA bypass** and **lateral movement** path: a principal exempted as a one-off remains permanently uncontrolled if no compensating policy covers it.", + "RelatedUrl": "", + "AdditionalURLs": [ + "https://learn.microsoft.com/en-us/entra/identity/conditional-access/plan-conditional-access", + "https://learn.microsoft.com/en-us/graph/api/resources/conditionalaccesspolicy?view=graph-rest-1.0", + "https://learn.microsoft.com/en-us/graph/api/resources/conditionalaccessusers?view=graph-rest-1.0", + "https://learn.microsoft.com/en-us/entra/identity/role-based-access-control/security-emergency-access" + ], + "Remediation": { + "Code": { + "CLI": "", + "NativeIaC": "", + "Other": "1. Navigate to Protection > Conditional Access > Policies in the Microsoft Entra admin center.\n2. For each object reported as an exclusion gap, decide whether the exclusion is still required.\n3. If the exclusion must stay, add the object to the Include scope of another enabled Conditional Access policy that enforces compensating controls (for example MFA).\n4. If the exclusion is no longer required, remove it so the object falls back under the original policy.\n5. Re-run the check to confirm no exclusion gaps remain.", + "Terraform": "" + }, + "Recommendation": { + "Text": "Ensure every object excluded from a Conditional Access policy is included by at least one other enabled policy that applies compensating controls. Reserve exclusions for break-glass accounts and the Directory Synchronization Accounts role, and review exclusion lists regularly so that exempted principals never drift outside Conditional Access enforcement.", + "Url": "https://hub.prowler.com/check/entra_conditional_access_policy_no_exclusion_gaps" + } + }, + "Categories": [ + "identity-access" + ], + "DependsOn": [], + "RelatedTo": [ + "entra_conditional_access_policy_directory_sync_account_excluded", + "entra_emergency_access_exclusion" + ], + "Notes": "Covers user, group, role, and application exclusions. Platform and location exclusions are out of scope because they are scoping conditions rather than principals removed from enforcement. Service-principal exclusions require additional fields on the ConditionalAccessPolicy service model." +} diff --git a/prowler/providers/m365/services/entra/entra_conditional_access_policy_no_exclusion_gaps/entra_conditional_access_policy_no_exclusion_gaps.py b/prowler/providers/m365/services/entra/entra_conditional_access_policy_no_exclusion_gaps/entra_conditional_access_policy_no_exclusion_gaps.py new file mode 100644 index 0000000000..0974581759 --- /dev/null +++ b/prowler/providers/m365/services/entra/entra_conditional_access_policy_no_exclusion_gaps/entra_conditional_access_policy_no_exclusion_gaps.py @@ -0,0 +1,262 @@ +"""Check that Conditional Access exclusions do not create coverage gaps.""" + +from collections import Counter, defaultdict + +from prowler.lib.check.models import Check, CheckReportM365 +from prowler.providers.m365.services.entra.entra_client import entra_client +from prowler.providers.m365.services.entra.entra_service import ( + ConditionalAccessGrantControl, + ConditionalAccessPolicyState, +) + +# Directory Synchronization Accounts built-in role template ID. Prowler enforces +# excluding this role (see entra_conditional_access_policy_directory_sync_account_excluded); +# it is intended to have no fallback, so it never counts as a gap here. +DIRECTORY_SYNC_ROLE_TEMPLATE_ID = "d29b2b05-8046-44ba-8758-1e26182fcf32" + + +class entra_conditional_access_policy_no_exclusion_gaps(Check): + """Check that objects excluded from Conditional Access policies remain covered. + + Excluding a principal from a Conditional Access (CA) policy is only safe when + that principal is still covered by *some* enabled CA policy that enforces + compensating controls. An object excluded everywhere and included nowhere + sits completely outside CA enforcement, which is how MFA bypass and lateral + movement against admin accounts happen in real incidents. + + For every enabled CA policy this check walks each exclusion collection and + verifies the excluded object is still in scope of another enabled policy: one + that includes it (explicitly, or via the "All" wildcard) and does not itself + exclude it. A wildcard belonging to the policy that excludes the object does + not count, so a one-off exclusion with no compensating policy is reported as + a gap. + + Only principals and target apps are evaluated (users, groups, roles, + applications). Platform and location exclusions are scoping conditions rather + than principals removed from enforcement, so they are out of scope. + + - PASS: Every excluded object stays in scope of another enabled policy, or no + enabled policy uses any exclusion. + - FAIL: At least one excluded object is in scope of no other enabled policy. + """ + + # (label, conditions attribute, included attr, excluded attr, wildcard token). + # The wildcard token, when present in an include collection, scopes a policy + # to every object of that type. Groups and roles have no wildcard: they are + # always explicit identifiers and transitive group/role expansion is out of + # scope for v1, so an excluded group/role is only "covered" when the same + # identifier is explicitly included by another enabled policy. + _COLLECTIONS = [ + ("users", "user_conditions", "included_users", "excluded_users", "All"), + ("groups", "user_conditions", "included_groups", "excluded_groups", None), + ("roles", "user_conditions", "included_roles", "excluded_roles", None), + ( + "applications", + "application_conditions", + "included_applications", + "excluded_applications", + "All", + ), + ] + + def execute(self) -> list[CheckReportM365]: + """Execute the Conditional Access exclusion-gap check. + + Returns: + list[CheckReportM365]: A single-element list with the aggregate result. + """ + report = CheckReportM365( + metadata=self.metadata(), + resource={}, + resource_name="Conditional Access Policies", + resource_id="conditionalAccessPolicies", + ) + + enabled_policies = [ + policy + for policy in entra_client.conditional_access_policies.values() + if policy.state == ConditionalAccessPolicyState.ENABLED + ] + + if not enabled_policies: + report.status = "PASS" + report.status_extended = ( + "No enabled Conditional Access policies found; " + "no exclusion coverage gaps are possible." + ) + return [report] + + emergency_users, emergency_groups = self._emergency_access_objects() + + # gaps: type label -> set of excluded object IDs with no compensating policy + gaps = defaultdict(set) + any_exclusion = False + + for policy in enabled_policies: + for ( + label, + conditions_attr, + included_attr, + excluded_attr, + wildcard, + ) in self._COLLECTIONS: + conditions = getattr(policy.conditions, conditions_attr) + if not conditions: + continue + for object_id in getattr(conditions, excluded_attr): + any_exclusion = True + if self._is_expected_exclusion( + label, object_id, emergency_users, emergency_groups + ): + continue + if not self._is_covered( + object_id, + conditions_attr, + included_attr, + excluded_attr, + wildcard, + enabled_policies, + ): + gaps[label].add(object_id) + + if not any_exclusion: + report.status = "PASS" + report.status_extended = ( + "No enabled Conditional Access policy uses exclusions; " + "no coverage gaps are possible." + ) + return [report] + + if not gaps: + report.status = "PASS" + report.status_extended = ( + "Every object excluded from an enabled Conditional Access policy is " + "still in scope of another enabled policy, so a compensating control " + "remains in effect." + ) + return [report] + + report.status = "FAIL" + report.status_extended = ( + "Conditional Access exclusion gaps found " + f"({self._format_gaps(gaps, self._build_name_index())}). These objects " + "are excluded but in scope of no other enabled policy, leaving them " + "outside CA enforcement." + ) + return [report] + + def _build_name_index(self) -> dict: + """Map excluded object IDs to display names per type, for readable findings. + + Users, groups, and applications resolve to their display name; roles have + no loaded name catalog, so role template IDs are shown as-is. Unresolved + IDs (for example deleted principals still referenced by a policy) fall + back to the raw identifier. + """ + users = { + uid: user.name + for uid, user in (getattr(entra_client, "users", {}) or {}).items() + if getattr(user, "name", None) + } + groups = { + group.id: group.name + for group in (getattr(entra_client, "groups", []) or []) + if getattr(group, "name", None) + } + applications = { + sp.app_id: sp.name + for sp in (getattr(entra_client, "service_principals", {}) or {}).values() + if getattr(sp, "app_id", None) and getattr(sp, "name", None) + } + return {"users": users, "groups": groups, "applications": applications} + + def _is_covered( + self, + object_id, + conditions_attr, + included_attr, + excluded_attr, + wildcard, + enabled_policies, + ) -> bool: + """Return True if any enabled policy keeps ``object_id`` in scope. + + A policy keeps the object in scope when it includes it β€”explicitly or via + the type's wildcard tokenβ€” and does not also exclude it. The wildcard of a + policy that itself excludes the object does not count, which is what makes + a one-off exclusion with no compensating policy a real gap. + """ + for policy in enabled_policies: + conditions = getattr(policy.conditions, conditions_attr) + if not conditions: + continue + if object_id in getattr(conditions, excluded_attr): + continue + included = getattr(conditions, included_attr) + if object_id in included or (wildcard is not None and wildcard in included): + return True + return False + + def _emergency_access_objects(self) -> tuple[set, set]: + """Return user and group IDs that act as emergency access (break-glass). + + Objects excluded from *every* enabled (enforced) Conditional Access policy + with a Block grant control are intended, compensating gaps and must not be + reported here. Only ENABLED policies count: report-only policies are not + enforced, so including them would dilute the "excluded everywhere" check + and could hide a genuine break-glass account (consistent with execute()). + """ + blocking_policies = [ + policy + for policy in entra_client.conditional_access_policies.values() + if policy.state == ConditionalAccessPolicyState.ENABLED + and ConditionalAccessGrantControl.BLOCK + in policy.grant_controls.built_in_controls + ] + if not blocking_policies: + return set(), set() + + total = len(blocking_policies) + excluded_users = Counter() + excluded_groups = Counter() + for policy in blocking_policies: + user_conditions = policy.conditions.user_conditions + if not user_conditions: + continue + for user_id in user_conditions.excluded_users: + excluded_users[user_id] += 1 + for group_id in user_conditions.excluded_groups: + excluded_groups[group_id] += 1 + + emergency_users = {uid for uid, n in excluded_users.items() if n == total} + emergency_groups = {gid for gid, n in excluded_groups.items() if n == total} + return emergency_users, emergency_groups + + def _is_expected_exclusion( + self, label, object_id, emergency_users, emergency_groups + ) -> bool: + """Exclusions that are intentional by design and must not count as gaps.""" + if label == "roles" and object_id == DIRECTORY_SYNC_ROLE_TEMPLATE_ID: + return True + if label == "users" and object_id in emergency_users: + return True + if label == "groups" and object_id in emergency_groups: + return True + return False + + def _format_gaps(self, gaps, name_index) -> str: + """Render the orphaned objects grouped by type, by display name when known. + + Each ID is shown as its display name when resolvable; unresolved IDs (and + all roles, which have no name catalog) fall back to the raw identifier. + """ + parts = [] + for label in ("users", "groups", "roles", "applications"): + if label not in gaps: + continue + names = name_index.get(label, {}) + rendered = sorted( + names.get(object_id, object_id) for object_id in gaps[label] + ) + parts.append(f"{label}: {', '.join(rendered)}") + return " | ".join(parts) diff --git a/tests/providers/alibabacloud/services/ram/ram_password_policy_number/ram_password_policy_number_test.py b/tests/providers/alibabacloud/services/ram/ram_password_policy_number/ram_password_policy_number_test.py new file mode 100644 index 0000000000..cc9b9ab35e --- /dev/null +++ b/tests/providers/alibabacloud/services/ram/ram_password_policy_number/ram_password_policy_number_test.py @@ -0,0 +1,67 @@ +from unittest import mock + +from tests.providers.alibabacloud.alibabacloud_fixtures import ( + set_mocked_alibabacloud_provider, +) + + +class TestRamPasswordPolicyNumber: + def test_numbers_not_required_fails(self): + ram_client = mock.MagicMock() + ram_client.audited_account = "1234567890" + ram_client.region = "cn-hangzhou" + + with ( + mock.patch( + "prowler.providers.common.provider.Provider.get_global_provider", + return_value=set_mocked_alibabacloud_provider(), + ), + mock.patch( + "prowler.providers.alibabacloud.services.ram.ram_password_policy_number.ram_password_policy_number.ram_client", + new=ram_client, + ), + ): + from prowler.providers.alibabacloud.services.ram.ram_password_policy_number.ram_password_policy_number import ( + ram_password_policy_number, + ) + from prowler.providers.alibabacloud.services.ram.ram_service import ( + PasswordPolicy, + ) + + ram_client.password_policy = PasswordPolicy(require_numbers=False) + + check = ram_password_policy_number() + result = check.execute() + + assert len(result) == 1 + assert result[0].status == "FAIL" + + def test_numbers_required_passes(self): + ram_client = mock.MagicMock() + ram_client.audited_account = "1234567890" + ram_client.region = "cn-hangzhou" + + with ( + mock.patch( + "prowler.providers.common.provider.Provider.get_global_provider", + return_value=set_mocked_alibabacloud_provider(), + ), + mock.patch( + "prowler.providers.alibabacloud.services.ram.ram_password_policy_number.ram_password_policy_number.ram_client", + new=ram_client, + ), + ): + from prowler.providers.alibabacloud.services.ram.ram_password_policy_number.ram_password_policy_number import ( + ram_password_policy_number, + ) + from prowler.providers.alibabacloud.services.ram.ram_service import ( + PasswordPolicy, + ) + + ram_client.password_policy = PasswordPolicy(require_numbers=True) + + check = ram_password_policy_number() + result = check.execute() + + assert len(result) == 1 + assert result[0].status == "PASS" diff --git a/tests/providers/m365/services/entra/entra_conditional_access_policy_explicitly_targets_azure_devops/entra_conditional_access_policy_explicitly_targets_azure_devops_test.py b/tests/providers/m365/services/entra/entra_conditional_access_policy_explicitly_targets_azure_devops/entra_conditional_access_policy_explicitly_targets_azure_devops_test.py new file mode 100644 index 0000000000..c120034e1e --- /dev/null +++ b/tests/providers/m365/services/entra/entra_conditional_access_policy_explicitly_targets_azure_devops/entra_conditional_access_policy_explicitly_targets_azure_devops_test.py @@ -0,0 +1,191 @@ +from unittest import mock +from uuid import uuid4 + +from prowler.providers.m365.services.entra.entra_service import ( + ApplicationsConditions, + ConditionalAccessGrantControl, + ConditionalAccessPolicyState, + Conditions, + GrantControlOperator, + GrantControls, + PersistentBrowser, + SessionControls, + SignInFrequency, + SignInFrequencyInterval, + UsersConditions, +) +from tests.providers.m365.m365_fixtures import DOMAIN, set_mocked_m365_provider + +CHECK_MODULE_PATH = "prowler.providers.m365.services.entra.entra_conditional_access_policy_explicitly_targets_azure_devops.entra_conditional_access_policy_explicitly_targets_azure_devops" + +AZURE_DEVOPS_APP_ID = "499b84ac-1321-427f-aa17-267ca6975798" + + +def _make_session_controls(): + """Return default session controls for test policies.""" + return SessionControls( + persistent_browser=PersistentBrowser(is_enabled=False, mode="always"), + sign_in_frequency=SignInFrequency( + is_enabled=False, + frequency=None, + type=None, + interval=SignInFrequencyInterval.EVERY_TIME, + ), + ) + + +def _make_conditions(included_applications=None): + """Return Conditions with the given included applications.""" + return Conditions( + application_conditions=ApplicationsConditions( + included_applications=included_applications or ["All"], + excluded_applications=[], + included_user_actions=[], + ), + user_conditions=UsersConditions( + included_groups=[], + excluded_groups=[], + included_users=["All"], + excluded_users=[], + included_roles=[], + excluded_roles=[], + ), + client_app_types=[], + user_risk_levels=[], + ) + + +def _make_grant_controls(): + """Return default grant controls for test policies.""" + return GrantControls( + built_in_controls=[ConditionalAccessGrantControl.MFA], + operator=GrantControlOperator.AND, + authentication_strength=None, + ) + + +def _make_policy(state, included_applications=None, display_name="Azure DevOps Policy"): + """Return a ConditionalAccessPolicy for tests.""" + from prowler.providers.m365.services.entra.entra_service import ( + ConditionalAccessPolicy, + ) + + return ConditionalAccessPolicy( + id=str(uuid4()), + display_name=display_name, + conditions=_make_conditions(included_applications=included_applications), + grant_controls=_make_grant_controls(), + session_controls=_make_session_controls(), + state=state, + ) + + +class Test_entra_conditional_access_policy_explicitly_targets_azure_devops: + def _run_check(self, policies): + entra_client = mock.MagicMock + entra_client.audited_tenant = "audited_tenant" + entra_client.audited_domain = DOMAIN + with ( + mock.patch( + "prowler.providers.common.provider.Provider.get_global_provider", + return_value=set_mocked_m365_provider(), + ), + mock.patch( + f"{CHECK_MODULE_PATH}.entra_client", + new=entra_client, + ), + ): + from prowler.providers.m365.services.entra.entra_conditional_access_policy_explicitly_targets_azure_devops.entra_conditional_access_policy_explicitly_targets_azure_devops import ( + entra_conditional_access_policy_explicitly_targets_azure_devops, + ) + + entra_client.conditional_access_policies = policies + + check = entra_conditional_access_policy_explicitly_targets_azure_devops() + return check.execute() + + def test_no_conditional_access_policies(self): + result = self._run_check({}) + assert len(result) == 1 + assert result[0].status == "FAIL" + assert ( + result[0].status_extended + == "No enabled Conditional Access Policy explicitly targets Azure DevOps." + ) + assert result[0].resource == {} + assert result[0].resource_name == "Conditional Access Policies" + assert result[0].resource_id == "conditionalAccessPolicies" + assert result[0].location == "global" + + def test_enabled_policy_targets_azure_devops(self): + policy = _make_policy( + ConditionalAccessPolicyState.ENABLED, + included_applications=[AZURE_DEVOPS_APP_ID], + ) + result = self._run_check({policy.id: policy}) + assert len(result) == 1 + assert result[0].status == "PASS" + assert ( + result[0].status_extended + == f"Conditional Access Policy {policy.display_name} explicitly targets Azure DevOps." + ) + assert result[0].resource_name == policy.display_name + assert result[0].resource_id == policy.id + + def test_enabled_policy_targets_all_apps_only(self): + policy = _make_policy( + ConditionalAccessPolicyState.ENABLED, included_applications=["All"] + ) + result = self._run_check({policy.id: policy}) + assert len(result) == 1 + assert result[0].status == "FAIL" + assert ( + result[0].status_extended + == "No enabled Conditional Access Policy explicitly targets Azure DevOps." + ) + + def test_disabled_policy_targets_azure_devops(self): + policy = _make_policy( + ConditionalAccessPolicyState.DISABLED, + included_applications=[AZURE_DEVOPS_APP_ID], + ) + result = self._run_check({policy.id: policy}) + assert len(result) == 1 + assert result[0].status == "FAIL" + assert ( + result[0].status_extended + == "No enabled Conditional Access Policy explicitly targets Azure DevOps." + ) + + def test_report_only_policy_targets_azure_devops(self): + policy = _make_policy( + ConditionalAccessPolicyState.ENABLED_FOR_REPORTING, + included_applications=[AZURE_DEVOPS_APP_ID], + ) + result = self._run_check({policy.id: policy}) + assert len(result) == 1 + assert result[0].status == "FAIL" + assert ( + result[0].status_extended + == "No enabled Conditional Access Policy explicitly targets Azure DevOps." + ) + + def test_multiple_policies_one_targets_azure_devops(self): + non_matching = _make_policy( + ConditionalAccessPolicyState.ENABLED, + included_applications=["All"], + display_name="All Apps Policy", + ) + matching = _make_policy( + ConditionalAccessPolicyState.ENABLED, + included_applications=["some-other-app", AZURE_DEVOPS_APP_ID], + display_name="Dedicated Azure DevOps Policy", + ) + result = self._run_check({non_matching.id: non_matching, matching.id: matching}) + assert len(result) == 1 + assert result[0].status == "PASS" + assert result[0].resource_id == matching.id + assert ( + result[0].status_extended + == f"Conditional Access Policy {matching.display_name} explicitly targets Azure DevOps." + ) diff --git a/tests/providers/m365/services/entra/entra_conditional_access_policy_no_exclusion_gaps/entra_conditional_access_policy_no_exclusion_gaps_test.py b/tests/providers/m365/services/entra/entra_conditional_access_policy_no_exclusion_gaps/entra_conditional_access_policy_no_exclusion_gaps_test.py new file mode 100644 index 0000000000..30629c2a29 --- /dev/null +++ b/tests/providers/m365/services/entra/entra_conditional_access_policy_no_exclusion_gaps/entra_conditional_access_policy_no_exclusion_gaps_test.py @@ -0,0 +1,424 @@ +import re +from unittest import mock +from uuid import uuid4 + +from prowler.providers.m365.services.entra.entra_service import ( + ApplicationsConditions, + ConditionalAccessGrantControl, + ConditionalAccessPolicy, + ConditionalAccessPolicyState, + Conditions, + GrantControlOperator, + GrantControls, + PersistentBrowser, + PlatformConditions, + SessionControls, + SignInFrequency, + SignInFrequencyInterval, + UsersConditions, +) +from tests.providers.m365.m365_fixtures import DOMAIN, set_mocked_m365_provider + +CHECK_PATH = "prowler.providers.m365.services.entra.entra_conditional_access_policy_no_exclusion_gaps.entra_conditional_access_policy_no_exclusion_gaps" +DIRECTORY_SYNC_ROLE_TEMPLATE_ID = "d29b2b05-8046-44ba-8758-1e26182fcf32" + + +def _policy( + display_name="Policy", + state=ConditionalAccessPolicyState.ENABLED, + included_users=None, + excluded_users=None, + included_groups=None, + excluded_groups=None, + included_roles=None, + excluded_roles=None, + included_applications=None, + excluded_applications=None, + include_platforms=None, + exclude_platforms=None, + block=False, +) -> ConditionalAccessPolicy: + """Build a fully-populated ConditionalAccessPolicy for tests. + + Args: + display_name: Policy display name. + state: Policy state (default ENABLED). + included_users: Included user IDs, or None. + excluded_users: Excluded user IDs, or None. + included_groups: Included group IDs, or None. + excluded_groups: Excluded group IDs, or None. + included_roles: Included role template IDs, or None. + excluded_roles: Excluded role template IDs, or None. + included_applications: Included application IDs, or None. + excluded_applications: Excluded application IDs, or None. + include_platforms: Included platform names, or None. + exclude_platforms: Excluded platform names, or None. + block: Whether the policy uses a Block grant control (default False). + + Returns: + A ConditionalAccessPolicy instance with the specified conditions. + """ + return ConditionalAccessPolicy( + id=str(uuid4()), + display_name=display_name, + conditions=Conditions( + application_conditions=ApplicationsConditions( + included_applications=included_applications or [], + excluded_applications=excluded_applications or [], + included_user_actions=[], + ), + user_conditions=UsersConditions( + included_groups=included_groups or [], + excluded_groups=excluded_groups or [], + included_users=included_users or [], + excluded_users=excluded_users or [], + included_roles=included_roles or [], + excluded_roles=excluded_roles or [], + ), + platform_conditions=PlatformConditions( + include_platforms=include_platforms or [], + exclude_platforms=exclude_platforms or [], + ), + ), + grant_controls=GrantControls( + built_in_controls=( + [ConditionalAccessGrantControl.BLOCK] + if block + else [ConditionalAccessGrantControl.MFA] + ), + operator=GrantControlOperator.AND, + authentication_strength=None, + ), + session_controls=SessionControls( + persistent_browser=PersistentBrowser(is_enabled=False, mode="always"), + sign_in_frequency=SignInFrequency( + is_enabled=False, + frequency=None, + type=None, + interval=SignInFrequencyInterval.EVERY_TIME, + ), + ), + state=state, + ) + + +def _run( + policies: list[ConditionalAccessPolicy], + users=None, + groups=None, + service_principals=None, +) -> list: + """Run the check with a mocked entra_client holding the given policies. + + Args: + policies: ConditionalAccessPolicy objects to inject into the mocked client. + users: Optional id -> User mapping used to resolve display names. + groups: Optional list of Group objects used to resolve display names. + service_principals: Optional id -> ServicePrincipal mapping for app names. + + Returns: + The list of check report objects returned by ``execute()``. + """ + entra_client = mock.MagicMock() + entra_client.audited_tenant = "audited_tenant" + entra_client.audited_domain = DOMAIN + with ( + mock.patch( + "prowler.providers.common.provider.Provider.get_global_provider", + return_value=set_mocked_m365_provider(), + ), + mock.patch(f"{CHECK_PATH}.entra_client", new=entra_client), + ): + from prowler.providers.m365.services.entra.entra_conditional_access_policy_no_exclusion_gaps.entra_conditional_access_policy_no_exclusion_gaps import ( + entra_conditional_access_policy_no_exclusion_gaps, + ) + + entra_client.conditional_access_policies = {p.id: p for p in policies} + entra_client.users = users or {} + entra_client.groups = groups or [] + entra_client.service_principals = service_principals or {} + check = entra_conditional_access_policy_no_exclusion_gaps() + return check.execute() + + +class Test_entra_conditional_access_policy_no_exclusion_gaps: + """Tests for the Conditional Access exclusion-gap check. + + Verifies that objects excluded from enabled Conditional Access policies stay + in scope of another enabled policy (explicitly or via the type's wildcard), + with the directory-sync role and break-glass accounts treated as intended + exclusions. + """ + + def test_no_policies(self): + result = _run([]) + assert len(result) == 1 + assert result[0].status == "PASS" + assert "No enabled Conditional Access policies" in result[0].status_extended + assert result[0].resource == {} + assert result[0].resource_name == "Conditional Access Policies" + assert result[0].resource_id == "conditionalAccessPolicies" + assert result[0].location == "global" + + def test_only_disabled_policies(self): + result = _run( + [ + _policy( + state=ConditionalAccessPolicyState.DISABLED, + included_users=["All"], + excluded_users=["user-1"], + ) + ] + ) + assert result[0].status == "PASS" + assert "No enabled Conditional Access policies" in result[0].status_extended + + def test_report_only_policies_out_of_scope(self): + # An exclusion in a report-only policy must not be evaluated. + result = _run( + [ + _policy( + state=ConditionalAccessPolicyState.ENABLED_FOR_REPORTING, + included_users=["All"], + excluded_users=["orphan-user"], + ) + ] + ) + assert result[0].status == "PASS" + assert "No enabled Conditional Access policies" in result[0].status_extended + + def test_no_exclusions_used(self): + result = _run([_policy(included_users=["All"], included_applications=["All"])]) + assert result[0].status == "PASS" + assert "no coverage gaps are possible" in result[0].status_extended + + def test_exclusion_covered_by_another_policy(self): + # Policy A excludes user-1; Policy B includes user-1 explicitly -> covered. + result = _run( + [ + _policy( + display_name="A", included_users=["All"], excluded_users=["user-1"] + ), + _policy(display_name="B", included_users=["user-1"]), + ] + ) + assert result[0].status == "PASS" + assert "compensating control" in result[0].status_extended + + def test_user_exclusion_gap(self): + # user-1 is excluded but never included anywhere -> FAIL. + result = _run( + [ + _policy( + display_name="A", included_users=["All"], excluded_users=["user-1"] + ) + ] + ) + assert result[0].status == "FAIL" + assert "users: user-1" in result[0].status_extended + + def test_gap_reports_display_name_when_resolvable(self): + # A resolvable user shows its display name; an unresolved user (e.g. + # deleted but still referenced) falls back to its raw ID. + from prowler.providers.m365.services.entra.entra_service import User + + result = _run( + [ + _policy( + display_name="A", + included_users=["All"], + excluded_users=["user-1", "ghost-2"], + ) + ], + users={ + "user-1": User( + id="user-1", + name="Alice Admin", + on_premises_sync_enabled=False, + ) + }, + ) + assert result[0].status == "FAIL" + assert "Alice Admin" in result[0].status_extended + assert "user-1" not in result[0].status_extended + # Unresolved ID still surfaces as the raw identifier. + assert "ghost-2" in result[0].status_extended + + def test_group_and_role_gaps_reported_by_type(self): + result = _run( + [ + _policy( + display_name="P", + included_users=["All"], + excluded_groups=["group-x"], + excluded_roles=["role-y"], + ) + ] + ) + assert result[0].status == "FAIL" + assert "groups: group-x" in result[0].status_extended + assert "roles: role-y" in result[0].status_extended + + def test_application_exclusion_gap(self): + result = _run( + [ + _policy( + display_name="AppPolicy", + included_applications=["All"], + excluded_applications=["app-123"], + ) + ] + ) + assert result[0].status == "FAIL" + assert "applications: app-123" in result[0].status_extended + + def test_application_exclusion_covered(self): + result = _run( + [ + _policy( + display_name="A", + included_applications=["All"], + excluded_applications=["app-123"], + ), + _policy(display_name="B", included_applications=["app-123"]), + ] + ) + assert result[0].status == "PASS" + + def test_exclusion_covered_by_all_wildcard_in_another_policy(self): + # Policy A excludes user-1; Policy B targets "All" users and does NOT + # exclude user-1, so user-1 stays in scope of B -> covered -> PASS. + # The "All" wildcard of the policy that excludes the user (A) must not + # count, but the wildcard of an unrelated policy (B) does. + result = _run( + [ + _policy( + display_name="A", + included_users=["All"], + excluded_users=["user-1"], + ), + _policy(display_name="B", included_users=["All"]), + ] + ) + assert result[0].status == "PASS" + assert "compensating control" in result[0].status_extended + + def test_exclusion_only_wildcard_is_self_excluding_is_gap(self): + # The only "All" users policy is the one that excludes user-1, and no + # other policy covers user-1 -> real gap -> FAIL. This is the case + # #11375's global-union "All" handling would have wrongly passed. + result = _run( + [ + _policy( + display_name="A", + included_users=["All"], + excluded_users=["user-1"], + ), + _policy( + display_name="B", + included_users=["All"], + excluded_users=["user-1"], + ), + ] + ) + assert result[0].status == "FAIL" + assert "users: user-1" in result[0].status_extended + + def test_platform_exclusions_are_ignored(self): + # Platform exclusions are scoping conditions, not principals removed from + # enforcement, so they are out of scope even with no covering policy. + result = _run( + [ + _policy( + display_name="MDM", + included_users=["All"], + exclude_platforms=["android", "ios", "macos", "linux"], + ) + ] + ) + assert result[0].status == "PASS" + + def test_directory_sync_role_exclusion_skipped(self): + # Dir-sync role excluded with no fallback must NOT be a gap. + result = _run( + [ + _policy( + display_name="P", + included_users=["All"], + excluded_roles=[DIRECTORY_SYNC_ROLE_TEMPLATE_ID], + ) + ] + ) + assert result[0].status == "PASS" + assert "compensating control" in result[0].status_extended + + def test_emergency_access_user_exclusion_skipped(self): + # A break-glass user excluded from EVERY enabled blocking policy is an + # intended gap and must not be reported. + emergency = "breakglass-user" + result = _run( + [ + _policy( + display_name="Block1", + block=True, + included_users=["All"], + excluded_users=[emergency], + ), + _policy( + display_name="Block2", + block=True, + included_users=["All"], + excluded_users=[emergency], + ), + ] + ) + assert result[0].status == "PASS" + assert "compensating control" in result[0].status_extended + + def test_emergency_access_ignores_report_only_blocking_policy(self): + # A break-glass user excluded from every ENABLED blocking policy is an + # intended gap, even if a report-only (non-enforced) blocking policy that + # does NOT exclude them also exists. Report-only policies must not dilute + # the emergency determination. + emergency = "breakglass-user" + result = _run( + [ + _policy( + display_name="Block1", + block=True, + included_users=["All"], + excluded_users=[emergency], + ), + _policy( + display_name="Block2", + block=True, + included_users=["All"], + excluded_users=[emergency], + ), + _policy( + display_name="ReportOnlyBlock", + block=True, + state=ConditionalAccessPolicyState.ENABLED_FOR_REPORTING, + included_users=["All"], + ), + ] + ) + assert result[0].status == "PASS" + assert "compensating control" in result[0].status_extended + + def test_mixed_gap_and_covered(self): + # user-1 covered, user-2 orphaned -> FAIL listing only user-2. + result = _run( + [ + _policy( + display_name="A", + included_users=["All"], + excluded_users=["user-1", "user-2"], + ), + _policy(display_name="B", included_users=["user-1"]), + ] + ) + assert result[0].status == "FAIL" + assert "user-2" in result[0].status_extended + # user-1 is covered, so it must not appear as a gap (whitespace-robust). + assert not re.search(r"\busers:\s*user-1\b", result[0].status_extended) diff --git a/ui/CHANGELOG.md b/ui/CHANGELOG.md index 6e3d91a6d0..334db67d75 100644 --- a/ui/CHANGELOG.md +++ b/ui/CHANGELOG.md @@ -4,12 +4,24 @@ All notable changes to the **Prowler UI** are documented in this file. ## [1.32.0] (Prowler UNRELEASED) +### πŸš€ Added + +- Filter the Overview, Findings, Resources, Scans, and Providers views by provider group [(#11659)](https://github.com/prowler-cloud/prowler/pull/11659) + ### πŸ”„ Changed - Sentry, Google Tag Manager, and PostHog now load their `UI_*` config only when the matching enable flag (`UI_SENTRY_ENABLE` / `UI_GOOGLE_TAG_MANAGER_ENABLE` / `UI_POSTHOG_ENABLE`) is `"true"` (default off); the deprecated legacy names (`NEXT_PUBLIC_*`, `POSTHOG_KEY`/`POSTHOG_HOST`) still activate without the flag [(#11682)](https://github.com/prowler-cloud/prowler/pull/11682) --- +## [1.31.1] (Prowler v5.31.1) + +### πŸ”„ Changed + +- Schedule Scans provider table and launch flows now use provider schedule fields, restore OSS daily scheduling, default to the next local scan hour, and clarify provider selection in launch scan [(#11684)](https://github.com/prowler-cloud/prowler/pull/11684) + +--- + ## [1.31.0] (Prowler v5.31.0) ### πŸš€ Added diff --git a/ui/actions/finding-groups/finding-groups.ts b/ui/actions/finding-groups/finding-groups.ts index bf5df80aae..faa697cf32 100644 --- a/ui/actions/finding-groups/finding-groups.ts +++ b/ui/actions/finding-groups/finding-groups.ts @@ -2,6 +2,7 @@ import { redirect } from "next/navigation"; +import type { FindingsFilterParam } from "@/actions/findings/findings-filters"; import { apiBaseUrl, composeSort, @@ -15,7 +16,6 @@ import { } from "@/lib"; import { appendSanitizedProviderFilters } from "@/lib/provider-filters"; import { handleApiResponse } from "@/lib/server-actions-helper"; -import { FilterParam } from "@/types/filters"; /** * Maps filter[search] to filter[check_title__icontains] for finding-groups. @@ -39,7 +39,7 @@ function mapSearchFilter( * finding-group resources sub-endpoint. These must be stripped before * calling the resources API to avoid empty results. */ -const FINDING_GROUP_RESOURCE_UNSUPPORTED_FILTERS: FilterParam[] = [ +const FINDING_GROUP_RESOURCE_UNSUPPORTED_FILTERS: FindingsFilterParam[] = [ "filter[service__in]", "filter[scan__in]", "filter[scan_id]", @@ -53,7 +53,7 @@ function normalizeFindingGroupResourceFilters( Object.entries(filters).filter( ([key]) => !FINDING_GROUP_RESOURCE_UNSUPPORTED_FILTERS.includes( - key as FilterParam, + key as FindingsFilterParam, ), ), ); diff --git a/ui/actions/findings/findings-filters.ts b/ui/actions/findings/findings-filters.ts new file mode 100644 index 0000000000..268d71e7cd --- /dev/null +++ b/ui/actions/findings/findings-filters.ts @@ -0,0 +1,40 @@ +import { FILTER_FIELD, FilterParam } from "@/types/filters"; + +/** Findings-only filter fields not shared with other views. */ +// eslint-disable-next-line @typescript-eslint/no-unused-vars +const FINDINGS_EXTRA_FIELD = { + DELTA_IN: "delta__in", + SCAN_EXACT: "scan", + SCAN_ID: "scan_id", + SCAN_ID_IN: "scan_id__in", + INSERTED_AT: "inserted_at", + INSERTED_AT_GTE: "inserted_at__gte", + INSERTED_AT_LTE: "inserted_at__lte", + MUTED: "muted", +} as const; + +type FindingsExtraField = + (typeof FINDINGS_EXTRA_FIELD)[keyof typeof FINDINGS_EXTRA_FIELD]; + +/** + * URL filter param keys the findings view supports, e.g. `filter[severity__in]`. + * Composed from the shared fields it uses plus a few findings-only extras + * (alternate scan/date/delta forms not used by other views). + */ +export type FindingsFilterParam = FilterParam< + // findings uses provider_id, not provider_uid + | (typeof FILTER_FIELD)[ + | "PROVIDER_TYPE" + | "PROVIDER_ID" + | "PROVIDER_GROUPS" + | "REGION" + | "SERVICE" + | "SEVERITY" + | "STATUS" + | "DELTA" + | "RESOURCE_TYPE" + | "CATEGORY" + | "RESOURCE_GROUPS" + | "SCAN"] + | FindingsExtraField +>; diff --git a/ui/actions/manage-groups/manage-groups.test.ts b/ui/actions/manage-groups/manage-groups.test.ts new file mode 100644 index 0000000000..c016832a1f --- /dev/null +++ b/ui/actions/manage-groups/manage-groups.test.ts @@ -0,0 +1,138 @@ +import { beforeEach, describe, expect, it, vi } from "vitest"; + +const { + fetchMock, + getAuthHeadersMock, + handleApiErrorMock, + handleApiResponseMock, +} = vi.hoisted(() => ({ + fetchMock: vi.fn(), + getAuthHeadersMock: vi.fn(), + handleApiErrorMock: vi.fn(), + handleApiResponseMock: vi.fn(), +})); + +vi.mock("next/cache", () => ({ + revalidatePath: vi.fn(), +})); + +vi.mock("next/navigation", () => ({ + redirect: vi.fn(), +})); + +vi.mock("@/lib", () => ({ + apiBaseUrl: "https://api.example.com/api/v1", + getAuthHeaders: getAuthHeadersMock, + getErrorMessage: vi.fn(), +})); + +vi.mock("@/lib/server-actions-helper", () => ({ + handleApiError: handleApiErrorMock, + handleApiResponse: handleApiResponseMock, +})); + +import { getAllProviderGroups } from "./manage-groups"; + +const makeGroup = (id: string, name: string) => ({ + type: "provider-groups" as const, + id, + attributes: { name, inserted_at: "", updated_at: "" }, + relationships: { + providers: { meta: { count: 0 }, data: [] }, + roles: { meta: { count: 0 }, data: [] }, + }, + links: { self: "" }, +}); + +const makePage = ( + data: ReturnType[], + page: number, + pages: number, +) => ({ + links: { first: "", last: "", next: null, prev: null }, + data, + meta: { pagination: { page, pages, count: data.length } }, +}); + +describe("getAllProviderGroups", () => { + beforeEach(() => { + vi.clearAllMocks(); + vi.stubGlobal("fetch", fetchMock); + getAuthHeadersMock.mockResolvedValue({ Authorization: "Bearer token" }); + fetchMock.mockResolvedValue(new Response(null, { status: 200 })); + }); + + it("merges every page into a single response with collapsed pagination", async () => { + handleApiResponseMock + .mockResolvedValueOnce( + makePage( + [makeGroup("g1", "Group 1"), makeGroup("g2", "Group 2")], + 1, + 2, + ), + ) + .mockResolvedValueOnce(makePage([makeGroup("g3", "Group 3")], 2, 2)); + + const result = await getAllProviderGroups(); + + expect(fetchMock).toHaveBeenCalledTimes(2); + expect(result?.data.map((group) => group.id)).toEqual(["g1", "g2", "g3"]); + expect(result?.meta.pagination).toMatchObject({ + page: 1, + pages: 1, + count: 3, + }); + }); + + it("stops after the first page when there is only one page", async () => { + handleApiResponseMock.mockResolvedValueOnce( + makePage([makeGroup("g1", "Group 1")], 1, 1), + ); + + const result = await getAllProviderGroups(); + + expect(fetchMock).toHaveBeenCalledTimes(1); + expect(result?.data).toHaveLength(1); + }); + + it("returns undefined when the first page has no data", async () => { + handleApiResponseMock.mockResolvedValueOnce(makePage([], 1, 1)); + + const result = await getAllProviderGroups(); + + expect(result).toBeUndefined(); + }); + + it("returns undefined when the request throws", async () => { + fetchMock.mockRejectedValueOnce(new Error("network down")); + + const result = await getAllProviderGroups(); + + expect(result).toBeUndefined(); + }); + + it("returns undefined when a later page resolves to an error payload", async () => { + handleApiResponseMock + .mockResolvedValueOnce(makePage([makeGroup("g1", "Group 1")], 1, 2)) + .mockResolvedValueOnce({ error: "Forbidden", status: 403 }); + + const result = await 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 933dabbdbc..c916a89d39 100644 --- a/ui/actions/manage-groups/manage-groups.ts +++ b/ui/actions/manage-groups/manage-groups.ts @@ -51,6 +51,87 @@ export const getProviderGroups = async ({ } }; +/** + * Fetches all provider groups by iterating through every page. + * Used to populate filter dropdowns (e.g. the Provider Group selector) without + * the pagination cap that `getProviderGroups` applies for the management table. + */ +export const getAllProviderGroups = async (): Promise< + ProviderGroupsResponse | undefined +> => { + const pageSize = 100; // Larger page size to minimize API calls + const maxPages = 50; // Safety limit: 50 pages Γ— 100 = 5000 groups max + let currentPage = 1; + const allGroups: ProviderGroupsResponse["data"] = []; + let lastResponse: ProviderGroupsResponse | undefined; + 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()); + url.searchParams.append("page[size]", pageSize.toString()); + + const response = await fetch(url.toString(), { headers }); + const data = (await handleApiResponse(response)) as + | ProviderGroupsResponse + | { error: string; status?: number } + | undefined; + + // A later page resolving to an API error payload must abort rather than + // be treated as "no more pages", which would silently truncate groups. + if (data && "error" in data) { + console.error("Error fetching all provider groups:", data.error); + return undefined; + } + + if (!data?.data || data.data.length === 0) { + hasMorePages = false; + continue; + } + + allGroups.push(...data.data); + lastResponse = data; + + const totalPages = data.meta?.pagination?.pages || 1; + if (currentPage >= totalPages) { + hasMorePages = false; + } else { + currentPage++; + } + } + + if (hasMorePages && currentPage > maxPages) { + console.error( + `Error fetching all provider groups: exceeded max page limit (${maxPages})`, + ); + return undefined; + } + + if (lastResponse) { + return { + ...lastResponse, + data: allGroups, + meta: { + ...lastResponse.meta, + pagination: { + ...lastResponse.meta?.pagination, + page: 1, + pages: 1, + count: allGroups.length, + }, + }, + }; + } + + return undefined; + } catch (error) { + console.error("Error fetching all provider groups:", error); + return undefined; + } +}; + export const getProviderGroupInfoById = async (providerGroupId: string) => { const headers = await getAuthHeaders({ contentType: false }); const url = new URL(`${apiBaseUrl}/provider-groups/${providerGroupId}`); diff --git a/ui/actions/overview/overview-filters.ts b/ui/actions/overview/overview-filters.ts new file mode 100644 index 0000000000..186977f9ce --- /dev/null +++ b/ui/actions/overview/overview-filters.ts @@ -0,0 +1,16 @@ +import { FILTER_FIELD, FilterParam } from "@/types/filters"; + +/** + * URL filter param keys the overview dashboard scopes its widgets by. Overview has + * no single action; its widgets read these keys from the URL filters. + */ +export type OverviewFilterParam = FilterParam< + (typeof FILTER_FIELD)["PROVIDER_TYPE" | "PROVIDER_ID" | "PROVIDER_GROUPS"] +>; + +/** The `filter[...]` keys overview widgets read from the URL. */ +export const OVERVIEW_FILTER_PARAM = { + PROVIDER_TYPE: `filter[${FILTER_FIELD.PROVIDER_TYPE}]`, + PROVIDER_ID: `filter[${FILTER_FIELD.PROVIDER_ID}]`, + PROVIDER_GROUPS: `filter[${FILTER_FIELD.PROVIDER_GROUPS}]`, +} as const satisfies Record; diff --git a/ui/actions/providers/providers-filters.ts b/ui/actions/providers/providers-filters.ts new file mode 100644 index 0000000000..71184c0b81 --- /dev/null +++ b/ui/actions/providers/providers-filters.ts @@ -0,0 +1,18 @@ +import { FILTER_FIELD, FilterParam } from "@/types/filters"; +import { PROVIDERS_PAGE_FILTER } from "@/types/providers-table"; + +/** + * URL filter param keys the providers list supports, e.g. `filter[provider__in]`. + * Provider scope plus its providers-only extras (`provider__in` API param, + * `connected` status). + */ +export type ProvidersFilterParam = FilterParam< + | (typeof FILTER_FIELD)["PROVIDER_TYPE" | "PROVIDER_GROUPS" | "PROVIDER_UID"] + | (typeof PROVIDERS_PAGE_FILTER)["PROVIDER" | "STATUS"] +>; + +/** `filter[...]` keys used when mapping the provider-type filter to the API param. */ +export const PROVIDERS_FILTER_PARAM = { + PROVIDER: `filter[${PROVIDERS_PAGE_FILTER.PROVIDER}]`, + PROVIDER_TYPE: `filter[${PROVIDERS_PAGE_FILTER.PROVIDER_TYPE}]`, +} as const satisfies Record; diff --git a/ui/actions/resources/resources-filters.ts b/ui/actions/resources/resources-filters.ts new file mode 100644 index 0000000000..01132943a6 --- /dev/null +++ b/ui/actions/resources/resources-filters.ts @@ -0,0 +1,25 @@ +import { FILTER_FIELD, FilterParam } from "@/types/filters"; + +/** Resources-only filter fields not shared with other views. */ +// eslint-disable-next-line @typescript-eslint/no-unused-vars +const RESOURCES_EXTRA_FIELD = { + TYPE: "type__in", + GROUPS: "groups__in", +} as const; + +type ResourcesExtraField = + (typeof RESOURCES_EXTRA_FIELD)[keyof typeof RESOURCES_EXTRA_FIELD]; + +/** + * URL filter param keys the resources view supports, e.g. `filter[type__in]`. + * The shared core plus its resources-only dimensions (`type__in`, `groups__in`). + */ +export type ResourcesFilterParam = FilterParam< + | (typeof FILTER_FIELD)[ + | "PROVIDER_TYPE" + | "PROVIDER_ID" + | "PROVIDER_GROUPS" + | "REGION" + | "SERVICE"] + | ResourcesExtraField +>; diff --git a/ui/actions/scans/scans-filters.ts b/ui/actions/scans/scans-filters.ts new file mode 100644 index 0000000000..19958e9fa5 --- /dev/null +++ b/ui/actions/scans/scans-filters.ts @@ -0,0 +1,34 @@ +import { FILTER_FIELD, FilterParam } from "@/types/filters"; + +/** + * Provider filter fields used to match/clear synthetic pending scan rows β€” the + * `__in` forms (shared with real scan rows) plus the exact forms, and the + * provider-group `__in` form so pending rows honor the group filter too. + */ +export const SCANS_PROVIDER_FILTER_FIELD = { + PROVIDER_IN: FILTER_FIELD.PROVIDER, + PROVIDER: "provider", + PROVIDER_TYPE_IN: FILTER_FIELD.PROVIDER_TYPE, + PROVIDER_TYPE: "provider_type", + PROVIDER_GROUPS_IN: FILTER_FIELD.PROVIDER_GROUPS, +} as const; + +/** Scans-only filter fields not shared with other views. */ +// eslint-disable-next-line @typescript-eslint/no-unused-vars +const SCANS_EXTRA_FIELD = { + STATE: "state__in", + TRIGGER: "trigger", +} as const; + +type ScansExtraField = + (typeof SCANS_EXTRA_FIELD)[keyof typeof SCANS_EXTRA_FIELD]; + +/** + * URL filter param keys the scans view supports, e.g. `filter[state__in]`. + * Provider scope (scans filters accounts by provider id) including provider + * groups and the exact pending-row provider forms, plus the scans-only dimensions. + */ +export type ScansFilterParam = FilterParam< + | (typeof SCANS_PROVIDER_FILTER_FIELD)[keyof typeof SCANS_PROVIDER_FILTER_FIELD] + | ScansExtraField +>; diff --git a/ui/app/(prowler)/_overview/_components/accounts-selector.test.tsx b/ui/app/(prowler)/_overview/_components/accounts-selector.test.tsx index 8a738ef5e1..e69d0427ee 100644 --- a/ui/app/(prowler)/_overview/_components/accounts-selector.test.tsx +++ b/ui/app/(prowler)/_overview/_components/accounts-selector.test.tsx @@ -2,7 +2,7 @@ import { render, screen, within } from "@testing-library/react"; import userEvent from "@testing-library/user-event"; import { describe, expect, it, vi } from "vitest"; -import { FilterType } from "@/types/filters"; +import { FILTER_FIELD } from "@/types/filters"; import { AccountsSelector } from "./accounts-selector"; @@ -147,9 +147,24 @@ describe("AccountsSelector", () => { placeholder: "Search Providers...", emptyMessage: "No Providers found.", }); + expect(screen.getByText("All Providers")).toBeInTheDocument(); expect(screen.getByText("Production AWS")).toBeInTheDocument(); }); + it("supports contextual placeholder and empty-selection copy", () => { + render( + , + ); + + expect(screen.getByText("Select a Provider")).toBeInTheDocument(); + expect(screen.getByText("No provider selected")).toBeInTheDocument(); + }); + it("allows disabling search explicitly", () => { render(); @@ -174,7 +189,7 @@ describe("AccountsSelector", () => { render( , ); diff --git a/ui/app/(prowler)/_overview/_components/accounts-selector.tsx b/ui/app/(prowler)/_overview/_components/accounts-selector.tsx index 128784517f..ab5c27fd6c 100644 --- a/ui/app/(prowler)/_overview/_components/accounts-selector.tsx +++ b/ui/app/(prowler)/_overview/_components/accounts-selector.tsx @@ -17,7 +17,7 @@ import { MultiSelectValue, } from "@/components/shadcn/select/multiselect"; import { useUrlFilters } from "@/hooks/use-url-filters"; -import { type AccountFilterKey, FilterType } from "@/types/filters"; +import { type AccountFilterKey, FILTER_FIELD } from "@/types/filters"; import { getProviderDisplayName, type ProviderProps, @@ -32,6 +32,9 @@ interface AccountsSelectorBaseProps { id?: string; disabledValues?: string[]; closeOnSelect?: boolean; + placeholder?: string; + emptySelectionLabel?: string; + clearSelectionLabel?: string; } /** Batch mode: caller controls both pending state and notification callback (all-or-nothing). */ @@ -65,7 +68,7 @@ export function AccountsSelector({ providers, onBatchChange, selectedValues, - filterKey = FilterType.PROVIDER_ID, + filterKey = FILTER_FIELD.PROVIDER_ID, id = "accounts-selector", disabledValues = [], search = { @@ -73,6 +76,9 @@ export function AccountsSelector({ emptyMessage: "No Providers found.", }, closeOnSelect = false, + placeholder = "All Providers", + emptySelectionLabel = "All selected", + clearSelectionLabel = "Select All", }: AccountsSelectorProps) { const searchParams = useSearchParams(); const { navigateWithParams } = useUrlFilters(); @@ -85,7 +91,7 @@ export function AccountsSelector({ const visibleProviders = providers; const getProviderValue = (provider: ProviderProps) => - filterKey === FilterType.PROVIDER_UID + filterKey === FILTER_FIELD.PROVIDER_UID ? provider.attributes.uid : provider.id; const disabledValuesSet = new Set(disabledValues); @@ -163,7 +169,7 @@ export function AccountsSelector({ onOpenChange={closeOnSelect ? setSelectorOpen : undefined} > - {selectedLabel() || } + {selectedLabel() || } {visibleProviders.length > 0 ? ( @@ -187,7 +193,9 @@ export function AccountsSelector({ } }} > - {selectedIds.length === 0 ? "All selected" : "Select All"} + {selectedIds.length === 0 + ? emptySelectionLabel + : clearSelectionLabel} {visibleProviders.map((p) => { const value = getProviderValue(p); 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 b8479432e6..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 @@ -3,11 +3,16 @@ import { getFindingsBySeverity, SeverityByProviderType, } from "@/actions/overview"; +import { OVERVIEW_FILTER_PARAM } from "@/actions/overview/overview-filters"; import { getAllProviders } from "@/actions/providers"; 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, @@ -16,27 +21,31 @@ export async function RiskPipelineViewSSR({ }) { const filters = pickFilterParams(searchParams); - const providerTypeFilter = filters["filter[provider_type__in]"]; - const providerIdFilter = filters["filter[provider_id__in]"]; + const providerTypeFilter = filters[OVERVIEW_FILTER_PARAM.PROVIDER_TYPE]; + const providerIdFilter = filters[OVERVIEW_FILTER_PARAM.PROVIDER_ID]; + const providerGroupsFilter = filters[OVERVIEW_FILTER_PARAM.PROVIDER_GROUPS]; // Fetch providers list to know account types const providersListResponse = await getAllProviders(); const allProviders = providersListResponse?.data || []; + // 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 = parseFilterIds(providerGroupsFilter); + const scopedProviders = scopeProvidersByGroup(allProviders, selectedGroupIds); + // Build severityByProviderType based on filters const severityByProviderType: SeverityByProviderType = {}; let selectedProviderTypes: string[] | undefined; 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 = {}; for (const accountId of selectedAccountIds) { - const provider = allProviders.find((p) => p.id === accountId); + const provider = scopedProviders.find((p) => p.id === accountId); if (provider) { const type = provider.attributes.provider.toLowerCase(); if (!accountsByType[type]) { @@ -70,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({ @@ -93,9 +102,10 @@ export async function RiskPipelineViewSSR({ } } } else { - // Case: No filters - get all provider types and make parallel calls + // Case: No account/type filter - enumerate provider types (scoped to the + // selected groups when a group filter is active) and make parallel calls. const allProviderTypes = Array.from( - new Set(allProviders.map((p) => p.attributes.provider.toLowerCase())), + new Set(scopedProviders.map((p) => p.attributes.provider.toLowerCase())), ); const severityPromises = allProviderTypes.map(async (providerType) => { 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 887eb7a5d5..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,31 +22,19 @@ export async function RiskPlotSSR({ }) { const filters = pickFilterParams(searchParams); - const providerTypeFilter = filters["filter[provider_type__in]"]; - const providerIdFilter = filters["filter[provider_id__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 (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 9e0802d4ff..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,10 +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]"); - if (providerType) filters["filter[provider_type__in]"] = providerType; - if (providerId) filters["filter[provider_id__in]"] = providerId; + 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/app/(prowler)/findings/page.tsx b/ui/app/(prowler)/findings/page.tsx index 65e8eca9cd..96c1ca05dc 100644 --- a/ui/app/(prowler)/findings/page.tsx +++ b/ui/app/(prowler)/findings/page.tsx @@ -6,6 +6,7 @@ import { getLatestFindingGroups, } from "@/actions/finding-groups"; import { getLatestMetadataInfo, getMetadataInfo } from "@/actions/findings"; +import { getAllProviderGroups } from "@/actions/manage-groups/manage-groups"; import { getAllProviders } from "@/actions/providers"; import { getScan, getScans } from "@/actions/scans"; import { SeedFromFindingsButton } from "@/app/(prowler)/alerts/_components"; @@ -36,8 +37,9 @@ export default async function Findings({ const { encodedSort } = extractSortAndKey(resolvedSearchParams); const { filters, query } = extractFiltersAndQuery(resolvedSearchParams); - const [providersData, scansData] = await Promise.all([ + const [providersData, providerGroupsData, scansData] = await Promise.all([ getAllProviders(), + getAllProviderGroups(), getScans({ pageSize: 50 }), ]); @@ -99,6 +101,7 @@ export default async function Findings({
; }) { const resolvedSearchParams = await searchParams; - const providersData = await getAllProviders(); + const [providersData, providerGroupsData] = await Promise.all([ + getAllProviders(), + getAllProviderGroups(), + ]); return (
+
diff --git a/ui/app/(prowler)/providers/page.tsx b/ui/app/(prowler)/providers/page.tsx index e6186df10a..72b0af9d7f 100644 --- a/ui/app/(prowler)/providers/page.tsx +++ b/ui/app/(prowler)/providers/page.tsx @@ -118,6 +118,7 @@ const ProvidersTabContent = async ({ isCloud={process.env.NEXT_PUBLIC_IS_CLOUD_ENV === "true"} filters={providersView.filters} providers={providersView.providers} + providerGroups={providersView.providerGroups} metadata={providersView.metadata} rows={providersView.rows} /> diff --git a/ui/app/(prowler)/providers/providers-page.utils.test.ts b/ui/app/(prowler)/providers/providers-page.utils.test.ts index 5727807d85..4c712ff081 100644 --- a/ui/app/(prowler)/providers/providers-page.utils.test.ts +++ b/ui/app/(prowler)/providers/providers-page.utils.test.ts @@ -18,6 +18,10 @@ const schedulesActionsMock = vi.hoisted(() => ({ getSchedules: vi.fn(), })); +const manageGroupsActionsMock = vi.hoisted(() => ({ + getAllProviderGroups: vi.fn(), +})); + vi.mock("@/actions/providers", () => providersActionsMock); vi.mock( "@/actions/organizations/organizations", @@ -25,6 +29,7 @@ vi.mock( ); vi.mock("@/actions/scans", () => scansActionsMock); vi.mock("@/actions/schedules", () => schedulesActionsMock); +vi.mock("@/actions/manage-groups/manage-groups", () => manageGroupsActionsMock); import { SearchParamsProps } from "@/types"; import { ProvidersApiResponse } from "@/types/providers"; @@ -886,6 +891,103 @@ describe("loadProvidersAccountsViewData", () => { ).toBeUndefined(); }); + it("uses provider schedule attributes as authoritative when scan_hour is null", async () => { + // Given β€” provider-1 still has a materialized scheduled scan row, but the + // provider payload says the schedule was removed. + providersActionsMock.getProviders.mockResolvedValue({ + ...providersResponse, + data: [ + { + ...providersResponse.data[0], + attributes: { + ...providersResponse.data[0].attributes, + scan_enabled: true, + scan_frequency: SCHEDULE_FREQUENCY.DAILY, + scan_hour: null, + scan_timezone: "UTC", + scan_interval_hours: null, + scan_day_of_week: null, + scan_day_of_month: null, + next_scan_at: null, + last_scan_at: null, + }, + }, + providersResponse.data[1], + ], + }); + providersActionsMock.getAllProviders.mockResolvedValue(providersResponse); + scansActionsMock.getScans.mockResolvedValue({ + data: [ + { + type: "scans", + id: "scan-1", + attributes: { trigger: "scheduled", state: "scheduled" }, + relationships: { + provider: { data: { type: "providers", id: "provider-1" } }, + }, + }, + ], + }); + schedulesActionsMock.getSchedules.mockResolvedValue({ + data: [buildSchedule("provider-1", { scan_hour: 9 })], + }); + + // When + const viewData = await loadProvidersAccountsViewData({ + searchParams: {} satisfies SearchParamsProps, + isCloud: false, + }); + + // Then + const providerRow = findProviderRow(viewData.rows, "provider-1"); + expect(providerRow?.hasSchedule).toBe(false); + expect(providerRow?.scheduleSummary).toBeUndefined(); + expect(providerRow?.lastScanAt).toBeNull(); + }); + + it("builds provider schedule and last scan values from the provider payload", async () => { + // Given + providersActionsMock.getProviders.mockResolvedValue({ + ...providersResponse, + data: [ + { + ...providersResponse.data[0], + attributes: { + ...providersResponse.data[0].attributes, + scan_enabled: true, + scan_frequency: SCHEDULE_FREQUENCY.MONTHLY, + scan_hour: 8, + scan_timezone: "Europe/Madrid", + scan_interval_hours: null, + scan_day_of_week: null, + scan_day_of_month: 24, + next_scan_at: "2026-06-24T06:00:00Z", + last_scan_at: "2026-06-23T06:00:00Z", + }, + }, + providersResponse.data[1], + ], + }); + providersActionsMock.getAllProviders.mockResolvedValue(providersResponse); + scansActionsMock.getScans.mockResolvedValue({ data: [] }); + schedulesActionsMock.getSchedules.mockResolvedValue({ error: "Not found" }); + + // When + const viewData = await loadProvidersAccountsViewData({ + searchParams: {} satisfies SearchParamsProps, + isCloud: false, + }); + + // Then + const providerRow = findProviderRow(viewData.rows, "provider-1"); + expect(providerRow?.hasSchedule).toBe(true); + expect(providerRow?.scheduleSummary?.cadence).toBe("Monthly on the 24th"); + expect(providerRow?.scheduleSummary?.nextScanAt).toBe( + "2026-06-24T06:00:00Z", + ); + expect(providerRow?.lastScanAt).toBe("2026-06-23T06:00:00Z"); + }); + it("ignores paused or unconfigured schedules", async () => { // Given β€” provider-1 paused (disabled), provider-2 never configured. providersActionsMock.getProviders.mockResolvedValue(providersResponse); @@ -913,8 +1015,10 @@ describe("loadProvidersAccountsViewData", () => { ); }); - it("falls back to scan-based detection when /schedules is unavailable (OSS)", async () => { - // Given β€” /schedules errors, but provider-1 has a materialized scheduled scan. + it("does not infer provider schedules from materialized scans when /schedules is unavailable", async () => { + // Given β€” /schedules errors, and provider-1 still has a materialized + // scheduled scan. That scan is historical execution state, not schedule + // configuration. providersActionsMock.getProviders.mockResolvedValue(providersResponse); providersActionsMock.getAllProviders.mockResolvedValue(providersResponse); scansActionsMock.getScans.mockResolvedValue({ @@ -937,9 +1041,9 @@ describe("loadProvidersAccountsViewData", () => { isCloud: false, }); - // Then β€” scan-based path still flags the provider; no throw from the error. + // Then β€” only provider scan_* fields or /schedules can mark a schedule. expect(findProviderRow(viewData.rows, "provider-1")?.hasSchedule).toBe( - true, + false, ); expect(findProviderRow(viewData.rows, "provider-2")?.hasSchedule).toBe( false, diff --git a/ui/app/(prowler)/providers/providers-page.utils.ts b/ui/app/(prowler)/providers/providers-page.utils.ts index 32ce2a5321..52a56c4fe0 100644 --- a/ui/app/(prowler)/providers/providers-page.utils.ts +++ b/ui/app/(prowler)/providers/providers-page.utils.ts @@ -1,9 +1,10 @@ +import { getAllProviderGroups } from "@/actions/manage-groups/manage-groups"; import { listOrganizationsSafe, listOrganizationUnitsSafe, } from "@/actions/organizations/organizations"; import { getAllProviders, getProviders } from "@/actions/providers"; -import { getScans } from "@/actions/scans"; +import { PROVIDERS_FILTER_PARAM } from "@/actions/providers/providers-filters"; import { getSchedules } from "@/actions/schedules"; import { extractFiltersAndQuery, @@ -11,6 +12,7 @@ import { } from "@/lib/helper-filters"; import { buildProviderScheduleSummary, + buildScheduleAttributesFromProvider, buildSchedulesByProviderId, isScheduleConfigured, } from "@/lib/schedules"; @@ -33,7 +35,7 @@ import { ProvidersTableRow, ProvidersTableRowsInput, } from "@/types/providers-table"; -import { SCAN_TRIGGER, ScanProps, ScanScheduleSummary } from "@/types/scans"; +import { ScanScheduleSummary } from "@/types/scans"; import { ScheduleAttributes } from "@/types/schedules"; const PROVIDERS_STATUS_MAPPING = [ @@ -114,26 +116,6 @@ const createProviderGroupLookup = ( return lookup; }; -const ACTIVE_SCAN_STATES = new Set(["scheduled", "available", "executing"]); - -const buildScheduledProviderIds = (scans: ScanProps[]): Set => { - const scheduled = new Set(); - - for (const scan of scans) { - if ( - scan.attributes.trigger === SCAN_TRIGGER.SCHEDULED && - ACTIVE_SCAN_STATES.has(scan.attributes.state) - ) { - const providerId = scan.relationships.provider?.data?.id; - if (providerId) { - scheduled.add(providerId); - } - } - } - - return scheduled; -}; - // A schedule is backed by the Provider row itself, so its `/schedules` entry // exists before the first scheduled Scan is materialized β€” only enabled, // configured ones carry a displayable cadence summary. @@ -145,17 +127,33 @@ const buildProviderScheduleSummaryFor = ( ? buildProviderScheduleSummary(attributes, now) : undefined; +const getProviderLastScanAt = ( + provider: ProvidersApiResponse["data"][number], +): string | null => { + if ( + Object.prototype.hasOwnProperty.call(provider.attributes, "last_scan_at") + ) { + return provider.attributes.last_scan_at ?? null; + } + + return provider.attributes.connection.last_checked_at ?? null; +}; + const enrichProviders = ( providersResponse: ProvidersApiResponse | undefined, - scanScheduledProviderIds: Set, schedulesByProviderId: Record, ): ProvidersProviderRow[] => { const providerGroupLookup = createProviderGroupLookup(providersResponse); const now = new Date(); return (providersResponse?.data ?? []).map((provider) => { + const providerScheduleAttributes = buildScheduleAttributesFromProvider( + provider.attributes, + ); + const scheduleAttributes = + providerScheduleAttributes ?? schedulesByProviderId[provider.id]; const scheduleSummary = buildProviderScheduleSummaryFor( - schedulesByProviderId[provider.id], + scheduleAttributes, now, ); @@ -167,11 +165,11 @@ const enrichProviders = ( (providerGroup: { id: string }) => providerGroupLookup.get(providerGroup.id) ?? "Unknown Group", ) ?? [], - // A fired scheduled scan OR a configured schedule that hasn't fired yet. - hasSchedule: - scanScheduledProviderIds.has(provider.id) || - scheduleSummary !== undefined, + // Provider scan_* fields are authoritative when present; otherwise we + // only fall back to the /schedules resource, never materialized scans. + hasSchedule: scheduleSummary !== undefined, scheduleSummary, + lastScanAt: getProviderLastScanAt(provider), }; }); }; @@ -488,13 +486,12 @@ export async function loadProvidersAccountsViewData({ // Map provider_type__in (used by ProviderTypeSelector) to provider__in (API param) const providerTypeFilter = - providerFilters[`filter[${PROVIDERS_PAGE_FILTER.PROVIDER_TYPE}]`]; + providerFilters[PROVIDERS_FILTER_PARAM.PROVIDER_TYPE]; if (providerTypeFilter) { - providerFilters[`filter[${PROVIDERS_PAGE_FILTER.PROVIDER}]`] = - providerTypeFilter; + providerFilters[PROVIDERS_FILTER_PARAM.PROVIDER] = providerTypeFilter; } - delete providerFilters[`filter[${PROVIDERS_PAGE_FILTER.PROVIDER_TYPE}]`]; + delete providerFilters[PROVIDERS_FILTER_PARAM.PROVIDER_TYPE]; const emptyOrganizationsResponse: OrganizationListResponse = { data: [], @@ -506,7 +503,7 @@ export async function loadProvidersAccountsViewData({ const [ providersResponse, allProvidersResponse, - scansResponse, + allProviderGroupsResponse, schedulesResponse, organizationsResponse, organizationUnitsResponse, @@ -523,18 +520,10 @@ export async function loadProvidersAccountsViewData({ // Unfiltered fetch for ProviderTypeSelector β€” only needs distinct types; // TODO: Replace with a dedicated lightweight endpoint when available. resolveActionResult(getAllProviders()), - // Fetch active scheduled scans to flag providers whose schedule has fired. - resolveActionResult( - getScans({ - pageSize: 500, - filters: { - "filter[trigger]": SCAN_TRIGGER.SCHEDULED, - "filter[state__in]": "scheduled,available", - }, - }), - ), - // Fetch configured schedules to also flag providers whose schedule has not - // fired yet (best-effort: absent in OSS, where the helper yields no ids). + // Unfiltered fetch for the Provider Group selector dropdown. + resolveActionResult(getAllProviderGroups()), + // Fetch configured schedules as a fallback when provider scan_* fields are + // absent (best-effort: typically empty in OSS). resolveActionResult(getSchedules()), isCloud ? listOrganizationsSafe() @@ -544,18 +533,11 @@ export async function loadProvidersAccountsViewData({ : Promise.resolve(emptyOrganizationUnitsResponse), ]); - const scanScheduledProviderIds = buildScheduledProviderIds( - scansResponse?.data ?? [], - ); const schedulesByProviderId = buildSchedulesByProviderId(schedulesResponse); const orgs = organizationsResponse?.data ?? []; const ous = organizationUnitsResponse?.data ?? []; - const providers = enrichProviders( - providersResponse, - scanScheduledProviderIds, - schedulesByProviderId, - ); + const providers = enrichProviders(providersResponse, schedulesByProviderId); const rows = buildProvidersTableRows({ isCloud, @@ -568,6 +550,7 @@ export async function loadProvidersAccountsViewData({ filters: createProvidersFilters(), metadata: providersResponse?.meta, providers: allProvidersResponse?.data ?? [], + providerGroups: allProviderGroupsResponse?.data ?? [], rows, }; } diff --git a/ui/app/(prowler)/resources/page.tsx b/ui/app/(prowler)/resources/page.tsx index fb7e5ab63d..9c9227160c 100644 --- a/ui/app/(prowler)/resources/page.tsx +++ b/ui/app/(prowler)/resources/page.tsx @@ -1,5 +1,6 @@ import { Suspense } from "react"; +import { getAllProviderGroups } from "@/actions/manage-groups/manage-groups"; import { getAllProviders } from "@/actions/providers"; import { getLatestMetadataInfo, @@ -37,19 +38,23 @@ export default async function Resources({ const initialResourceId = resolvedSearchParams.resourceId?.toString(); - const [metadataInfoData, providersData, resourceByIdData] = await Promise.all( - [ - (hasDateOrScan ? getMetadataInfo : getLatestMetadataInfo)({ - query, - filters: outputFilters, - sort: encodedSort, - }), - getAllProviders(), - initialResourceId - ? getResourceById(initialResourceId, { include: ["provider"] }) - : Promise.resolve(undefined), - ], - ); + const [ + metadataInfoData, + providersData, + providerGroupsData, + resourceByIdData, + ] = await Promise.all([ + (hasDateOrScan ? getMetadataInfo : getLatestMetadataInfo)({ + query, + filters: outputFilters, + sort: encodedSort, + }), + getAllProviders(), + getAllProviderGroups(), + initialResourceId + ? getResourceById(initialResourceId, { include: ["provider"] }) + : Promise.resolve(undefined), + ]); const processedResource = resourceByIdData?.data ? (() => { @@ -80,6 +85,7 @@ export default async function Resources({
; + `filter[${SCANS_PROVIDER_FILTER_FIELD.PROVIDER_IN}]`, + `filter[${SCANS_PROVIDER_FILTER_FIELD.PROVIDER}]`, +] as const satisfies ReadonlyArray; const PROVIDER_TYPE_FILTER_KEYS = [ - `filter[${PENDING_ROW_PROVIDER_FILTER.PROVIDER_TYPE_IN}]`, - `filter[${PENDING_ROW_PROVIDER_FILTER.PROVIDER_TYPE}]`, -] as const satisfies ReadonlyArray; + `filter[${SCANS_PROVIDER_FILTER_FIELD.PROVIDER_TYPE_IN}]`, + `filter[${SCANS_PROVIDER_FILTER_FIELD.PROVIDER_TYPE}]`, +] as const satisfies ReadonlyArray; + +const PROVIDER_GROUP_FILTER_KEYS = [ + `filter[${SCANS_PROVIDER_FILTER_FIELD.PROVIDER_GROUPS_IN}]`, +] as const satisfies ReadonlyArray; const getFilterSearchQuery = ( filters: Record, @@ -86,7 +83,7 @@ const parseCsvParam = (value?: string | string[]): string[] => { const getFirstSearchParam = ( searchParams: SearchParamsProps, - keys: ReadonlyArray, + keys: ReadonlyArray, ): string | string[] | undefined => { for (const key of keys) { const value = searchParams[key]; @@ -107,11 +104,18 @@ const filterProvidersForPendingRows = ( const types = parseCsvParam( getFirstSearchParam(searchParams, PROVIDER_TYPE_FILTER_KEYS), ); + const groups = parseCsvParam( + getFirstSearchParam(searchParams, PROVIDER_GROUP_FILTER_KEYS), + ); return providers.filter( (provider) => (ids.length === 0 || ids.includes(provider.id)) && - (types.length === 0 || types.includes(provider.attributes.provider)), + (types.length === 0 || types.includes(provider.attributes.provider)) && + (groups.length === 0 || + (provider.relationships?.provider_groups?.data ?? []).some((group) => + groups.includes(group.id), + )), ); }; @@ -177,8 +181,12 @@ export default async function Scans({ const session = await auth(); const resolvedSearchParams = await searchParams; - const providersData = await getAllProviders(); + const [providersData, providerGroupsData] = await Promise.all([ + getAllProviders(), + getAllProviderGroups(), + ]); const providers = providersData?.data ?? []; + const providerGroups = providerGroupsData?.data ?? []; const connectedProviders = providers.filter( (provider: ProviderProps) => @@ -229,6 +237,7 @@ export default async function Scans({ ) : ( diff --git a/ui/components/filters/data-filters.ts b/ui/components/filters/data-filters.ts index 2457b1a8b2..83969d0362 100644 --- a/ui/components/filters/data-filters.ts +++ b/ui/components/filters/data-filters.ts @@ -1,5 +1,5 @@ import { CONNECTION_STATUS_MAPPING } from "@/lib/helper-filters"; -import { FilterOption, FilterType } from "@/types/filters"; +import { FILTER_FIELD, FilterOption } from "@/types/filters"; import { PROVIDER_DISPLAY_NAMES, PROVIDER_TYPES, @@ -64,19 +64,19 @@ export const filterScans = [ //Static filters for findings export const filterFindings = [ { - key: FilterType.SEVERITY, + key: FILTER_FIELD.SEVERITY, labelCheckboxGroup: "Severity", values: ["critical", "high", "medium", "low", "informational"], index: 0, }, { - key: FilterType.STATUS, + key: FILTER_FIELD.STATUS, labelCheckboxGroup: "Status", values: ["PASS", "FAIL", "MANUAL"], index: 1, }, { - key: FilterType.DELTA, + key: FILTER_FIELD.DELTA, labelCheckboxGroup: "Delta", values: ["new", "changed"], index: 2, diff --git a/ui/components/filters/provider-account-selectors.test.tsx b/ui/components/filters/provider-account-selectors.test.tsx index a8b8bd1689..01868d0e8c 100644 --- a/ui/components/filters/provider-account-selectors.test.tsx +++ b/ui/components/filters/provider-account-selectors.test.tsx @@ -1,7 +1,7 @@ import { render } from "@testing-library/react"; import { beforeEach, describe, expect, it, vi } from "vitest"; -import { FilterType } from "@/types/filters"; +import { FILTER_FIELD } from "@/types/filters"; import type { ProviderProps } from "@/types/providers"; import { ProviderAccountSelectors } from "./provider-account-selectors"; @@ -171,7 +171,7 @@ describe("ProviderAccountSelectors", () => { render( , @@ -230,7 +230,7 @@ describe("ProviderAccountSelectors", () => { ({ + navigateWithParamsMock: vi.fn(), +})); + +let currentSearchParams = new URLSearchParams(); + +vi.mock("next/navigation", () => ({ + useSearchParams: () => currentSearchParams, +})); + +vi.mock("@/hooks/use-url-filters", () => ({ + useUrlFilters: () => ({ + navigateWithParams: navigateWithParamsMock, + }), +})); + +vi.mock("@/components/shadcn/select/multiselect", () => ({ + MultiSelect: ({ + children, + onValuesChange, + }: { + children: React.ReactNode; + onValuesChange: (values: string[]) => void; + }) => ( +
+
+ ), + MultiSelectTrigger: ({ children }: { children: React.ReactNode }) => ( +
{children}
+ ), + MultiSelectValue: ({ placeholder }: { placeholder: string }) => ( + {placeholder} + ), + MultiSelectContent: ({ + children, + search, + }: { + children: React.ReactNode; + search?: unknown; + }) => { + multiSelectContentSpy(search); + return
{children}
; + }, + MultiSelectItem: ({ + children, + value, + keywords, + }: { + children: React.ReactNode; + value: string; + keywords?: string[]; + }) => ( +
+ {children} +
+ ), +})); + +const makeGroup = (id: string, name: string): ProviderGroup => ({ + type: "provider-groups", + id, + attributes: { name, inserted_at: "", updated_at: "" }, + relationships: { + providers: { meta: { count: 0 }, data: [] }, + roles: { meta: { count: 0 }, data: [] }, + }, + links: { self: "" }, +}); + +const groups = [ + makeGroup("group-1", "Production"), + makeGroup("group-2", "Dev"), +]; + +describe("ProviderGroupSelector", () => { + beforeEach(() => { + vi.clearAllMocks(); + currentSearchParams = new URLSearchParams(); + }); + + it("stays visible with the placeholder and empty message when there are no provider groups", () => { + render(); + + // Control is still rendered (visible even with zero groups)... + expect(screen.getByText("All Provider Groups")).toBeInTheDocument(); + // ...and the single empty state is the MultiSelect's own emptyMessage, + // not a duplicate custom message. + expect(multiSelectContentSpy).toHaveBeenCalledWith({ + placeholder: "Search Provider Groups...", + emptyMessage: "No Provider Groups found.", + }); + expect( + screen.queryByText("No Provider Groups available"), + ).not.toBeInTheDocument(); + }); + + it("passes searchable dropdown defaults to MultiSelectContent and lists groups", () => { + render(); + + expect(multiSelectContentSpy).toHaveBeenCalledWith({ + placeholder: "Search Provider Groups...", + emptyMessage: "No Provider Groups found.", + }); + expect(screen.getByText("Production")).toBeInTheDocument(); + expect(screen.getByText("Dev")).toBeInTheDocument(); + }); + + it("allows disabling search explicitly", () => { + render(); + + expect(multiSelectContentSpy).toHaveBeenLastCalledWith(false); + }); + + it("passes the group name as a search keyword", () => { + render(); + + expect( + screen.getByText("Production").closest("[data-value]"), + ).toHaveAttribute("data-keywords", expect.stringContaining("Production")); + }); + + it("disables select all when nothing is selected", () => { + render(); + + expect( + screen.getByRole("option", { name: /select all Provider Groups/i }), + ).toHaveAttribute("aria-disabled", "true"); + expect(screen.getByText("All selected")).toBeInTheDocument(); + }); + + it("shows the selected count in the trigger when multiple groups are selected", () => { + render( + , + ); + + const trigger = screen.getByTestId("trigger"); + expect( + within(trigger).getByText("2 Provider Groups selected"), + ).toBeInTheDocument(); + }); + + it("shows the single group name in the trigger when one group is selected", () => { + render( + , + ); + + const trigger = screen.getByTestId("trigger"); + expect(within(trigger).getByText("Production")).toBeInTheDocument(); + }); + + it("instant mode: writes the selection to filter[provider_groups__in] in the URL", () => { + render(); + + fireEvent.click(screen.getByTestId("mock-select-group-2")); + + expect(navigateWithParamsMock).toHaveBeenCalledTimes(1); + const params = new URLSearchParams(); + navigateWithParamsMock.mock.calls[0][0](params); + expect(params.get("filter[provider_groups__in]")).toBe("group-2"); + }); + + it("instant mode: clearing deletes the filter key and the extra paramsToDeleteOnChange keys", () => { + currentSearchParams = new URLSearchParams( + "filter[provider_groups__in]=group-1&page=3&scanId=abc", + ); + render( + , + ); + + fireEvent.click( + screen.getByRole("option", { name: /select all Provider Groups/i }), + ); + + expect(navigateWithParamsMock).toHaveBeenCalledTimes(1); + const params = new URLSearchParams( + "filter[provider_groups__in]=group-1&page=3&scanId=abc", + ); + navigateWithParamsMock.mock.calls[0][0](params); + expect(params.has("filter[provider_groups__in]")).toBe(false); + expect(params.has("page")).toBe(false); + expect(params.has("scanId")).toBe(false); + }); + + it("defaults the control id and links the sr-only label to it", () => { + render(); + + const label = screen.getByText(/Filter by Provider Group/i); + expect(label).toHaveAttribute("for", "provider-group-selector"); + expect(label).toHaveAttribute("id", "provider-group-selector-label"); + }); + + it("applies a custom id so multiple instances don't collide", () => { + render( + , + ); + + const label = screen.getByText(/Filter by Provider Group/i); + expect(label).toHaveAttribute("for", "resources-provider-group"); + expect(label).toHaveAttribute("id", "resources-provider-group-label"); + }); + + it("does not navigate on clear when nothing is selected", () => { + render(); + + fireEvent.click( + screen.getByRole("option", { name: /select all Provider Groups/i }), + ); + + expect(navigateWithParamsMock).not.toHaveBeenCalled(); + }); +}); diff --git a/ui/components/filters/provider-group-selector.tsx b/ui/components/filters/provider-group-selector.tsx new file mode 100644 index 0000000000..e7194d694b --- /dev/null +++ b/ui/components/filters/provider-group-selector.tsx @@ -0,0 +1,171 @@ +"use client"; + +import { useSearchParams } from "next/navigation"; + +import { + MultiSelect, + MultiSelectContent, + MultiSelectItem, + type MultiSelectSearchProp, + MultiSelectTrigger, + MultiSelectValue, +} from "@/components/shadcn/select/multiselect"; +import { useUrlFilters } from "@/hooks/use-url-filters"; +import type { ProviderGroup } from "@/types/components"; +import { FILTER_FIELD } from "@/types/filters"; + +const PROVIDER_GROUP_FILTER_KEY = FILTER_FIELD.PROVIDER_GROUPS; +const URL_FILTER_KEY = `filter[${PROVIDER_GROUP_FILTER_KEY}]`; + +/** Common props shared by both batch and instant modes. */ +interface ProviderGroupSelectorBaseProps { + groups: ProviderGroup[]; + search?: MultiSelectSearchProp; + /** DOM id for the control; pass a unique one when rendering more than one. */ + id?: string; + /** + * Instant mode only: extra URL params to delete when the selection changes + * (e.g. ["page", "scanId"]), mirroring ProviderAccountSelectors. Ignored in + * batch mode, where the parent owns URL updates. + */ + paramsToDeleteOnChange?: string[]; +} + +/** Batch mode: caller controls both pending state and notification callback (all-or-nothing). */ +interface ProviderGroupSelectorBatchProps + extends ProviderGroupSelectorBaseProps { + /** + * Called instead of navigating immediately. + * Use this on pages that batch filter changes (e.g. Findings). + * + * @param filterKey - The raw filter key without "filter[]" wrapper, e.g. "provider_groups__in" + * @param values - The selected values array + */ + onBatchChange: (filterKey: string, values: string[]) => void; + /** + * Pending selected values controlled by the parent. + * Reflects pending state before Apply is clicked. + */ + selectedValues: string[]; +} + +/** Instant mode: URL-driven β€” neither callback nor controlled value. */ +interface ProviderGroupSelectorInstantProps + extends ProviderGroupSelectorBaseProps { + onBatchChange?: never; + selectedValues?: never; +} + +type ProviderGroupSelectorProps = + | ProviderGroupSelectorBatchProps + | ProviderGroupSelectorInstantProps; + +export function ProviderGroupSelector({ + groups, + onBatchChange, + selectedValues, + id = "provider-group-selector", + search = { + placeholder: "Search Provider Groups...", + emptyMessage: "No Provider Groups found.", + }, + paramsToDeleteOnChange = [], +}: ProviderGroupSelectorProps) { + const searchParams = useSearchParams(); + const { navigateWithParams } = useUrlFilters(); + const labelId = `${id}-label`; + + const current = searchParams.get(URL_FILTER_KEY) || ""; + const urlSelectedIds = current ? current.split(",").filter(Boolean) : []; + + // In batch mode, use the parent-controlled pending values; otherwise, use URL state. + const selectedIds = onBatchChange ? selectedValues : urlSelectedIds; + + const handleMultiValueChange = (ids: string[]) => { + if (onBatchChange) { + onBatchChange(PROVIDER_GROUP_FILTER_KEY, ids); + return; + } + navigateWithParams((params) => { + if (ids.length > 0) { + params.set(URL_FILTER_KEY, ids.join(",")); + } else { + params.delete(URL_FILTER_KEY); + } + paramsToDeleteOnChange.forEach((key) => params.delete(key)); + }); + }; + + const selectedLabel = () => { + if (selectedIds.length === 0) return null; + if (selectedIds.length === 1) { + const group = groups.find((g) => g.id === selectedIds[0]); + return ( + + {group ? group.attributes.name : selectedIds[0]} + + ); + } + return ( + + {selectedIds.length} Provider Groups selected + + ); + }; + + return ( +
+ + + + {selectedLabel() || ( + + )} + + + {/* No items when empty: the MultiSelect's own emptyMessage is the + single empty state (avoids a duplicate "none" message). */} + {groups.length > 0 && ( + <> +
{ + if (selectedIds.length === 0) return; + handleMultiValueChange([]); + }} + onKeyDown={(e) => { + if (e.key === "Enter" || e.key === " ") { + e.preventDefault(); + if (selectedIds.length === 0) return; + handleMultiValueChange([]); + } + }} + > + {selectedIds.length === 0 ? "All selected" : "Select All"} +
+ {groups.map((group) => ( + + {group.attributes.name} + + ))} + + )} +
+
+
+ ); +} diff --git a/ui/components/findings/findings-filters.tsx b/ui/components/findings/findings-filters.tsx index 349769f3ad..cf25eb9529 100644 --- a/ui/components/findings/findings-filters.tsx +++ b/ui/components/findings/findings-filters.tsx @@ -14,12 +14,14 @@ import { FilterSummaryStrip, } from "@/components/filters/filter-summary-strip"; import { ProviderAccountSelectors } from "@/components/filters/provider-account-selectors"; +import { ProviderGroupSelector } from "@/components/filters/provider-group-selector"; import { Button } from "@/components/shadcn"; import { ExpandableSection } from "@/components/ui/expandable-section"; import { DataTableFilterCustom } from "@/components/ui/table/data-table-filter-custom"; import { useFilterBatch } from "@/hooks/use-filter-batch"; import { getCategoryLabel, getGroupLabel } from "@/lib/categories"; -import { FilterType, ScanEntity } from "@/types"; +import { FILTER_FIELD, ScanEntity } from "@/types"; +import { ProviderGroup } from "@/types/components"; import { DATA_TABLE_FILTER_MODE } from "@/types/filters"; import { ProviderProps } from "@/types/providers"; @@ -31,6 +33,8 @@ import { interface FindingsFiltersProps { /** Provider data for provider/account filter controls. */ providers: ProviderProps[]; + /** Provider groups for the provider group filter control. */ + providerGroups?: ProviderGroup[]; completedScanIds: string[]; scanDetails: { [key: string]: ScanEntity }[]; uniqueRegions: string[]; @@ -70,6 +74,10 @@ const FILTER_GRID_ITEM_CLASS = "min-w-0"; export const FindingsFilterBatchControls = ({ providers, + // Undefined = caller opted out (the alert editor shares this component but + // loads no groups); an empty array still renders the control, so it stays + // visible even when a tenant has no groups yet. + providerGroups, completedScanIds, scanDetails, uniqueRegions, @@ -97,7 +105,7 @@ export const FindingsFilterBatchControls = ({ const customFilters = [ ...filterFindings - .filter((filter) => !isAlertsEdit || filter.key !== FilterType.STATUS) + .filter((filter) => !isAlertsEdit || filter.key !== FILTER_FIELD.STATUS) .map((filter) => ({ ...filter, labelFormatter: (value: string) => @@ -107,32 +115,32 @@ export const FindingsFilterBatchControls = ({ }), })), { - key: FilterType.REGION, + key: FILTER_FIELD.REGION, labelCheckboxGroup: "Regions", values: uniqueRegions, index: 3, }, { - key: FilterType.SERVICE, + key: FILTER_FIELD.SERVICE, labelCheckboxGroup: "Services", values: uniqueServices, index: 4, }, { - key: FilterType.RESOURCE_TYPE, + key: FILTER_FIELD.RESOURCE_TYPE, labelCheckboxGroup: "Resource Type", values: uniqueResourceTypes, index: 8, }, { - key: FilterType.CATEGORY, + key: FILTER_FIELD.CATEGORY, labelCheckboxGroup: "Category", values: uniqueCategories, labelFormatter: getCategoryLabel, index: 5, }, { - key: FilterType.RESOURCE_GROUPS, + key: FILTER_FIELD.RESOURCE_GROUPS, labelCheckboxGroup: "Resource Group", values: uniqueGroups, labelFormatter: getGroupLabel, @@ -142,14 +150,14 @@ export const FindingsFilterBatchControls = ({ ? [] : [ { - key: FilterType.SCAN, + key: FILTER_FIELD.SCAN, labelCheckboxGroup: "Scan ID", values: completedScanIds, width: "wide" as const, valueLabelMapping: scanDetails, labelFormatter: (value: string) => getFindingsFilterDisplayValue( - `filter[${FilterType.SCAN}]`, + `filter[${FILTER_FIELD.SCAN}]`, value, { providers, @@ -167,6 +175,7 @@ export const FindingsFilterBatchControls = ({ appliedFilters, { providers, + providerGroups, scans: scanDetails, }, ); @@ -174,6 +183,7 @@ export const FindingsFilterBatchControls = ({ changedFilters, { providers, + providerGroups, scans: scanDetails, }, ); @@ -199,15 +209,26 @@ export const FindingsFilterBatchControls = ({ : undefined; const providerAccountControls = (className: string) => ( - + <> + + {providerGroups !== undefined && ( +
+ +
+ )} + ); const alertEditFilterGrid = hasCustomFilters ? ( diff --git a/ui/components/findings/findings-filters.utils.test.ts b/ui/components/findings/findings-filters.utils.test.ts index aa376f360d..69ce0605d2 100644 --- a/ui/components/findings/findings-filters.utils.test.ts +++ b/ui/components/findings/findings-filters.utils.test.ts @@ -1,5 +1,6 @@ import { describe, expect, it } from "vitest"; +import { ProviderGroup } from "@/types/components"; import { ProviderProps } from "@/types/providers"; import { ScanEntity } from "@/types/scans"; @@ -8,6 +9,19 @@ import { getFindingsFilterDisplayValue, } from "./findings-filters.utils"; +const providerGroups: ProviderGroup[] = [ + { + type: "provider-groups", + id: "group-1", + attributes: { name: "Production", inserted_at: "", updated_at: "" }, + relationships: { + providers: { meta: { count: 0 }, data: [] }, + roles: { meta: { count: 0 }, data: [] }, + }, + links: { self: "" }, + }, +]; + function makeProvider( overrides: Partial & { id: string }, ): ProviderProps { @@ -98,6 +112,24 @@ describe("getFindingsFilterDisplayValue", () => { ).toBe("missing-provider"); }); + it("shows the provider group name for provider_groups filters instead of the raw group id", () => { + expect( + getFindingsFilterDisplayValue("filter[provider_groups__in]", "group-1", { + providerGroups, + }), + ).toBe("Production"); + }); + + it("keeps the raw value when the provider group cannot be resolved", () => { + expect( + getFindingsFilterDisplayValue( + "filter[provider_groups__in]", + "missing-group", + { providerGroups }, + ), + ).toBe("missing-group"); + }); + it("shows the resolved scan badge label for scan filters instead of formatting the raw scan id", () => { expect( getFindingsFilterDisplayValue("filter[scan__in]", "scan-1", { scans }), @@ -230,6 +262,22 @@ describe("buildFindingsFilterChips", () => { ]); }); + it("labels provider group chips and resolves their names", () => { + const chips = buildFindingsFilterChips( + { "filter[provider_groups__in]": ["group-1"] }, + { providerGroups }, + ); + + expect(chips).toEqual([ + { + key: "filter[provider_groups__in]", + label: "Provider Group", + value: "group-1", + displayValue: "Production", + }, + ]); + }); + it("treats filter[delta] and filter[delta__in] identically", () => { // Given const chipsSingular = buildFindingsFilterChips({ diff --git a/ui/components/findings/findings-filters.utils.ts b/ui/components/findings/findings-filters.utils.ts index b2a935aec0..4cb9eef394 100644 --- a/ui/components/findings/findings-filters.utils.ts +++ b/ui/components/findings/findings-filters.utils.ts @@ -1,8 +1,12 @@ +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 { FilterParam } from "@/types/filters"; +import { ProviderGroup } from "@/types/components"; import { getProviderDisplayName, ProviderProps } from "@/types/providers"; import { ScanEntity } from "@/types/scans"; import { SEVERITY_DISPLAY_NAMES } from "@/types/severities"; @@ -10,6 +14,7 @@ import { SEVERITY_DISPLAY_NAMES } from "@/types/severities"; interface GetFindingsFilterDisplayValueOptions { providers?: ProviderProps[]; scans?: Array<{ [scanId: string]: ScanEntity }>; + providerGroups?: ProviderGroup[]; } const FINDING_DELTA_DISPLAY_NAMES: Record = { @@ -42,7 +47,7 @@ function getScanDisplayValue( } export function getFindingsFilterDisplayValue( - filterKey: string, + filterKey: FindingsFilterParam, value: string, options: GetFindingsFilterDisplayValueOptions = {}, ): string { @@ -53,6 +58,9 @@ export function getFindingsFilterDisplayValue( if (filterKey === "filter[provider_id__in]") { return getProviderAccountDisplayValue(value, options.providers || []); } + if (filterKey === "filter[provider_groups__in]") { + return getProviderGroupDisplayValue(value, options.providerGroups || []); + } if (filterKey === "filter[scan__in]" || filterKey === "filter[scan]") { return getScanDisplayValue(value, options.scans || []); } @@ -95,12 +103,14 @@ export function getFindingsFilterDisplayValue( /** * Maps raw filter param keys (e.g. "filter[severity__in]") to human-readable labels. * Used to render chips in the FilterSummaryStrip. - * Typed as Record so TypeScript enforces exhaustiveness β€” any - * addition to FilterParam will cause a compile error here if the label is missing. + * Typed as Record so TypeScript enforces exhaustiveness + * β€” any addition to the findings filter set will cause a compile error here if the + * label is missing. */ -export const FILTER_KEY_LABELS: Record = { +export const FILTER_KEY_LABELS: Record = { "filter[provider_type__in]": "Provider", "filter[provider_id__in]": "Account", + "filter[provider_groups__in]": "Provider Group", "filter[severity__in]": "Severity", "filter[status__in]": "Status", "filter[delta__in]": "Delta", @@ -115,12 +125,15 @@ export const FILTER_KEY_LABELS: Record = { "filter[scan_id]": "Scan", "filter[scan_id__in]": "Scan", "filter[inserted_at]": "Date", + "filter[inserted_at__gte]": "Date", + "filter[inserted_at__lte]": "Date", "filter[muted]": "Muted", }; interface BuildFindingsFilterChipsOptions { providers?: ProviderProps[]; scans?: Array<{ [scanId: string]: ScanEntity }>; + providerGroups?: ProviderGroup[]; includeMuted?: boolean; } @@ -142,13 +155,13 @@ export function buildFindingsFilterChips( Object.entries(pendingFilters).forEach(([key, values]) => { if (!values || values.length === 0) return; if (key === "filter[muted]" && !options.includeMuted) return; - const label = FILTER_KEY_LABELS[key as FilterParam] ?? key; + const label = FILTER_KEY_LABELS[key as FindingsFilterParam] ?? key; const visibleValues = values; if (visibleValues.length === 0) return; const displayValues = visibleValues.map((value) => - getFindingsFilterDisplayValue(key, value, options), + getFindingsFilterDisplayValue(key as FindingsFilterParam, value, options), ); const chip: FilterChip = { diff --git a/ui/components/providers/link-to-scans.tsx b/ui/components/providers/link-to-scans.tsx index fee1ec407d..cbdfa573a5 100644 --- a/ui/components/providers/link-to-scans.tsx +++ b/ui/components/providers/link-to-scans.tsx @@ -5,13 +5,12 @@ import { formatLocalTimeWithZone } from "@/lib/date-utils"; import type { ScanScheduleSummary } from "@/types/scans"; interface LinkToScansProps { - hasSchedule: boolean; schedule?: ScanScheduleSummary; } // Matches the scans table Schedule column: cadence on top, next-run local time -// underneath. Falls back to a plain label when the cadence is unknown. -export const LinkToScans = ({ hasSchedule, schedule }: LinkToScansProps) => { +// underneath. Falls back to None when no configured schedule is present. +export const LinkToScans = ({ schedule }: LinkToScansProps) => { if (schedule) { return ( { ); } - return ( - - {hasSchedule ? "Daily" : "None"} - - ); + return None; }; diff --git a/ui/components/providers/organizations/org-launch-scan.test.tsx b/ui/components/providers/organizations/org-launch-scan.test.tsx index f7f7fb11a3..cd1175c5f6 100644 --- a/ui/components/providers/organizations/org-launch-scan.test.tsx +++ b/ui/components/providers/organizations/org-launch-scan.test.tsx @@ -429,7 +429,7 @@ describe("OrgLaunchScan", () => { }); // Then - expect(screen.getByText(/reached your scan limit/i)).toBeInTheDocument(); + expect(screen.getByText(/exceeded the usage limit/i)).toBeInTheDocument(); expect(updateSchedulesBulkMock).not.toHaveBeenCalled(); expect(launchOrganizationScansMock).not.toHaveBeenCalled(); }); diff --git a/ui/components/providers/organizations/org-launch-scan.tsx b/ui/components/providers/organizations/org-launch-scan.tsx index b8fde57c65..8747d28126 100644 --- a/ui/components/providers/organizations/org-launch-scan.tsx +++ b/ui/components/providers/organizations/org-launch-scan.tsx @@ -363,8 +363,16 @@ export function OrgLaunchScan({ {isBlocked ? (

- You have reached your scan limit, so additional scans are not - available right now. + You have exceeded the usage limit of one provider. You can add + more providers and run unlimited scans by adding a subscription.{" "} + + Manage Billing +

) : isAdvanced ? ( diff --git a/ui/components/providers/providers-filters.test.tsx b/ui/components/providers/providers-filters.test.tsx index b6e6655247..37080b3f77 100644 --- a/ui/components/providers/providers-filters.test.tsx +++ b/ui/components/providers/providers-filters.test.tsx @@ -16,6 +16,10 @@ vi.mock("@/app/(prowler)/_overview/_components/provider-type-selector", () => ({ ProviderTypeSelector: () =>
Provider type selector
, })); +vi.mock("@/components/filters/provider-group-selector", () => ({ + ProviderGroupSelector: () =>
Provider group selector
, +})); + vi.mock("@/components/filters/clear-filters-button", () => ({ ClearFiltersButton: () => , })); diff --git a/ui/components/providers/providers-filters.tsx b/ui/components/providers/providers-filters.tsx index 285bae4c1e..d488a5849c 100644 --- a/ui/components/providers/providers-filters.tsx +++ b/ui/components/providers/providers-filters.tsx @@ -5,6 +5,7 @@ import type { ReactNode } from "react"; import { ProviderTypeSelector } from "@/app/(prowler)/_overview/_components/provider-type-selector"; import { ClearFiltersButton } from "@/components/filters/clear-filters-button"; +import { ProviderGroupSelector } from "@/components/filters/provider-group-selector"; import { MultiSelect, MultiSelectContent, @@ -18,6 +19,7 @@ import { EntityInfo } from "@/components/ui/entities/entity-info"; import { useUrlFilters } from "@/hooks/use-url-filters"; import { isConnectionStatus, isGroupFilterEntity } from "@/lib/helper-filters"; import { FilterEntity, FilterOption, ProviderEntity } from "@/types"; +import { ProviderGroup } from "@/types/components"; import { GroupFilterEntity, ProviderConnectionStatus, @@ -31,12 +33,14 @@ function isNonEmptyString(value: string | null | undefined): value is string { interface ProvidersFiltersProps { filters: FilterOption[]; providers: ProviderProps[]; + providerGroups?: ProviderGroup[]; actions?: ReactNode; } export const ProvidersFilters = ({ filters, providers, + providerGroups = [], actions, }: ProvidersFiltersProps) => { const { updateFilter } = useUrlFilters(); @@ -153,6 +157,9 @@ export const ProvidersFilters = ({
+
+ +
{sortedFilters.map((filter) => { const selectedValues = getSelectedValues(filter); return ( diff --git a/ui/components/providers/table/column-providers.test.tsx b/ui/components/providers/table/column-providers.test.tsx new file mode 100644 index 0000000000..c2e9abc18c --- /dev/null +++ b/ui/components/providers/table/column-providers.test.tsx @@ -0,0 +1,131 @@ +import { render, screen } from "@testing-library/react"; +import type { ReactNode } from "react"; +import { describe, expect, it, vi } from "vitest"; + +import { + PROVIDERS_ROW_TYPE, + type ProvidersProviderRow, + type ProvidersTableRow, +} from "@/types/providers-table"; + +import { getColumnProviders } from "./column-providers"; + +vi.mock("@/components/shadcn", () => ({ + Badge: ({ children }: { children: ReactNode }) => {children}, +})); + +vi.mock("@/components/shadcn/checkbox/checkbox", () => ({ + Checkbox: () => null, +})); + +vi.mock("@/components/ui/code-snippet/code-snippet", () => ({ + CodeSnippet: ({ value }: { value: string }) => {value}, +})); + +vi.mock("@/components/ui/entities", () => ({ + DateWithTime: ({ dateTime }: { dateTime: string | null }) => ( + + ), + EntityInfo: ({ + entityAlias, + entityId, + }: { + entityAlias?: string; + entityId?: string; + }) => {entityAlias ?? entityId}, +})); + +vi.mock("@/components/ui/table", () => ({ + DataTableColumnHeader: ({ title }: { title: string }) => {title}, +})); + +vi.mock("@/components/ui/table/data-table-expand-all-toggle", () => ({ + DataTableExpandAllToggle: () => null, +})); + +vi.mock("@/components/ui/table/data-table-expandable-cell", () => ({ + DataTableExpandableCell: ({ children }: { children: ReactNode }) => ( +
{children}
+ ), +})); + +vi.mock("../link-to-scans", () => ({ + LinkToScans: () => null, +})); + +vi.mock("./data-table-row-actions", () => ({ + DataTableRowActions: () => null, +})); + +const providerRow: ProvidersProviderRow = { + id: "provider-1", + rowType: PROVIDERS_ROW_TYPE.PROVIDER, + type: "providers", + attributes: { + provider: "aws", + uid: "123456789012", + alias: "Production", + status: "completed", + resources: 0, + connection: { + connected: true, + last_checked_at: "2026-01-01T00:00:00Z", + }, + scanner_args: { + only_logs: false, + excluded_checks: [], + aws_retries_max_attempts: 3, + }, + inserted_at: "2026-01-01T00:00:00Z", + updated_at: "2026-01-01T00:00:00Z", + created_by: { + object: "user", + id: "user-1", + }, + }, + relationships: { + secret: { data: { id: "secret-1", type: "secrets" } }, + provider_groups: { meta: { count: 0 }, data: [] }, + }, + groupNames: [], + hasSchedule: false, +}; + +function renderLastScanCell(row: ProvidersTableRow) { + const lastScanColumn = getColumnProviders( + {}, + [], + [], + [], + vi.fn(), + vi.fn(), + vi.fn(), + ).find((column) => column.id === "lastScan"); + + const cell = lastScanColumn?.cell; + if (typeof cell !== "function") { + throw new Error("Last Scan column cell renderer not found"); + } + + const element = cell({ + row: { original: row }, + } as unknown as Parameters[0]); + + render(<>{element as ReactNode}); +} + +describe("getColumnProviders", () => { + it("falls back to connection last_checked_at when lastScanAt is undefined", () => { + renderLastScanCell({ ...providerRow, lastScanAt: undefined }); + + expect(screen.getByText("2026-01-01T00:00:00Z")).toBeVisible(); + expect(screen.queryByText("Never")).not.toBeInTheDocument(); + }); + + it("treats a null lastScanAt as authoritative", () => { + renderLastScanCell({ ...providerRow, lastScanAt: null }); + + expect(screen.getByText("Never")).toBeVisible(); + expect(screen.queryByText("2026-01-01T00:00:00Z")).not.toBeInTheDocument(); + }); +}); diff --git a/ui/components/providers/table/column-providers.tsx b/ui/components/providers/table/column-providers.tsx index c88c61b78e..3f93cd2b45 100644 --- a/ui/components/providers/table/column-providers.tsx +++ b/ui/components/providers/table/column-providers.tsx @@ -227,22 +227,25 @@ export function getColumnProviders( return -; } - const lastCheckedAt = (row.original as ProvidersProviderRow).attributes - .connection.last_checked_at; + const provider = row.original as ProvidersProviderRow; + const lastScanAt = + provider.lastScanAt !== undefined + ? provider.lastScanAt + : provider.attributes.connection.last_checked_at; - if (!lastCheckedAt) { + if (!lastScanAt) { return ( Never ); } - return ; + return ; }, enableSorting: false, }, { id: "scanSchedule", - size: 140, + size: 180, header: ({ column }) => ( ), @@ -255,12 +258,7 @@ export function getColumnProviders( ); } - return ( - - ); + return ; }, enableSorting: false, }, diff --git a/ui/components/providers/wizard/steps/launch-step.test.tsx b/ui/components/providers/wizard/steps/launch-step.test.tsx index 6cf7d3cfa5..90bc0b4b5d 100644 --- a/ui/components/providers/wizard/steps/launch-step.test.tsx +++ b/ui/components/providers/wizard/steps/launch-step.test.tsx @@ -79,7 +79,7 @@ describe("LaunchStep", () => { scanOnDemandMock.mockResolvedValue({ data: { id: "scan-1" } }); }); - it("defaults to run now and locks schedule mode outside Cloud", async () => { + it("defaults to daily schedule mode and locks advanced cadence outside Cloud", async () => { // Given const onFooterChange = vi.fn(); seedConnectedProvider(); @@ -94,19 +94,20 @@ describe("LaunchStep", () => { // Then expect(screen.getByText("Account Connected!")).toBeInTheDocument(); - expect(screen.getByRole("radio", { name: "Run now" })).toBeChecked(); expect( screen.getByRole("radio", { name: "On a schedule" }), - ).toBeDisabled(); + ).toBeChecked(); + expect(screen.getByRole("radio", { name: "Run now" })).not.toBeChecked(); expect( - screen.queryByRole("combobox", { name: /repeats/i }), - ).not.toBeInTheDocument(); + screen.getByRole("radio", { name: "On a schedule" }), + ).toBeEnabled(); + expect(screen.getByRole("combobox", { name: /repeats/i })).toBeDisabled(); await waitFor(() => expect(onFooterChange).toHaveBeenCalled()); - expect(lastFooterConfig(onFooterChange)?.actionLabel).toBe("Launch scan"); + expect(lastFooterConfig(onFooterChange)?.actionLabel).toBe("Save"); }); - it("launches only an on-demand scan and never creates a legacy daily schedule", async () => { + it("saves a legacy daily schedule by default", async () => { // Given const onClose = vi.fn(); const onFooterChange = vi.fn(); @@ -127,9 +128,43 @@ describe("LaunchStep", () => { }); // Then - await waitFor(() => expect(scanOnDemandMock).toHaveBeenCalledTimes(1)); - const sentFormData = scanOnDemandMock.mock.calls[0]?.[0] as FormData; + await waitFor(() => expect(scheduleDailyMock).toHaveBeenCalledTimes(1)); + const sentFormData = scheduleDailyMock.mock.calls[0]?.[0] as FormData; expect(sentFormData.get("providerId")).toBe("provider-1"); + expect(scanOnDemandMock).not.toHaveBeenCalled(); + expect(updateScheduleMock).not.toHaveBeenCalled(); + expect(onClose).toHaveBeenCalledTimes(1); + }); + + it("launches only an on-demand scan when run now is selected", async () => { + // Given + const user = userEvent.setup(); + const onClose = vi.fn(); + const onFooterChange = vi.fn(); + seedConnectedProvider(); + + render( + , + ); + await waitFor(() => expect(onFooterChange).toHaveBeenCalled()); + + // When + await user.click(screen.getByRole("radio", { name: "Run now" })); + await waitFor(() => + expect(lastFooterConfig(onFooterChange)?.actionLabel).toBe( + "Launch scan", + ), + ); + await act(async () => { + lastFooterConfig(onFooterChange)?.onAction?.(); + }); + + // Then + await waitFor(() => expect(scanOnDemandMock).toHaveBeenCalledTimes(1)); expect(scheduleDailyMock).not.toHaveBeenCalled(); expect(updateScheduleMock).not.toHaveBeenCalled(); expect(onClose).toHaveBeenCalledTimes(1); @@ -438,7 +473,7 @@ describe("LaunchStep", () => { ); // Then - expect(screen.getByText(/reached your scan limit/i)).toBeInTheDocument(); + expect(screen.getByText(/exceeded the usage limit/i)).toBeInTheDocument(); await waitFor(() => expect(onFooterChange).toHaveBeenCalled()); expect(lastFooterConfig(onFooterChange)?.actionDisabled).toBe(true); }); diff --git a/ui/components/providers/wizard/steps/launch-step.tsx b/ui/components/providers/wizard/steps/launch-step.tsx index 51a5727f63..27af2b09f2 100644 --- a/ui/components/providers/wizard/steps/launch-step.tsx +++ b/ui/components/providers/wizard/steps/launch-step.tsx @@ -93,17 +93,19 @@ export function LaunchStep({ const capability = capabilityProp ?? getScanScheduleCapability(isCloud()); const isManualOnly = capability === SCAN_SCHEDULE_CAPABILITY.MANUAL_ONLY; const isAdvanced = capability === SCAN_SCHEDULE_CAPABILITY.ADVANCED; + const isDailyLegacy = capability === SCAN_SCHEDULE_CAPABILITY.DAILY_LEGACY; const isBlocked = capability === SCAN_SCHEDULE_CAPABILITY.BLOCKED; + const canUseScheduleMode = isAdvanced || isDailyLegacy; const [isLaunching, setIsLaunching] = useState(false); const [mode, setMode] = useState( - isAdvanced ? LAUNCH_MODE.SCHEDULE : LAUNCH_MODE.NOW, + canUseScheduleMode ? LAUNCH_MODE.SCHEDULE : LAUNCH_MODE.NOW, ); const form = useForm({ resolver: zodResolver(scheduleFormSchema), defaultValues: getScheduleFormDefaults(), }); - const isScheduleMode = isAdvanced && mode === LAUNCH_MODE.SCHEDULE; + const isScheduleMode = canUseScheduleMode && mode === LAUNCH_MODE.SCHEDULE; const isLimitBlocked = mode === LAUNCH_MODE.NOW && isScanLimitReached; const isActionBlocked = isLaunching || @@ -129,10 +131,10 @@ export function LaunchStep({ })(); useEffect(() => { - if (!isAdvanced && mode !== LAUNCH_MODE.NOW) { + if (!canUseScheduleMode && mode !== LAUNCH_MODE.NOW) { setMode(LAUNCH_MODE.NOW); } - }, [isAdvanced, mode]); + }, [canUseScheduleMode, mode]); const launchOnDemandScan = async (): Promise => { if (!providerId || isBlocked) return null; @@ -327,10 +329,10 @@ export function LaunchStep({ On a schedule - {!isAdvanced && + {!canUseScheduleMode && !isBlocked && (isManualOnly ? ( @@ -341,7 +343,7 @@ export function LaunchStep({ - {!isAdvanced && !isBlocked && ( + {isManualOnly && !isBlocked && (

Scheduled scans are not available for this account. Run now to get immediate findings. @@ -350,8 +352,16 @@ export function LaunchStep({ {(isLimitBlocked || isBlocked) && (

- You have reached your scan limit, so additional scans are not - available right now. + You have exceeded the usage limit of one provider. You can add more + providers and run unlimited scans by adding a subscription.{" "} + + Manage Billing +

)} @@ -361,6 +371,8 @@ export function LaunchStep({ disabled={isLaunching || !providerId} showLaunchInitialScan showNextScheduledCopy + canUseAdvancedSchedule={isAdvanced} + showCloudUpgradeBadge={isDailyLegacy} /> )}
diff --git a/ui/components/resources/resources-filters.tsx b/ui/components/resources/resources-filters.tsx index d6b90a6c87..f493c2a385 100644 --- a/ui/components/resources/resources-filters.tsx +++ b/ui/components/resources/resources-filters.tsx @@ -11,11 +11,13 @@ import { FilterSummaryStrip, } from "@/components/filters/filter-summary-strip"; import { ProviderAccountSelectors } from "@/components/filters/provider-account-selectors"; +import { ProviderGroupSelector } from "@/components/filters/provider-group-selector"; import { Button } from "@/components/shadcn"; import { ExpandableSection } from "@/components/ui/expandable-section"; import { DataTableFilterCustom } from "@/components/ui/table"; import { useFilterBatch } from "@/hooks/use-filter-batch"; import { getGroupLabel } from "@/lib/categories"; +import { ProviderGroup } from "@/types/components"; import { DATA_TABLE_FILTER_MODE } from "@/types/filters"; import { ProviderProps } from "@/types/providers"; @@ -26,6 +28,7 @@ import { interface ResourcesFiltersProps { providers: ProviderProps[]; + providerGroups?: ProviderGroup[]; uniqueRegions: string[]; uniqueServices: string[]; uniqueResourceTypes: string[]; @@ -40,6 +43,7 @@ const FILTER_CONTROL_COLUMN_CLASS = export const ResourcesFilters = ({ providers, + providerGroups = [], uniqueRegions, uniqueServices, uniqueResourceTypes, @@ -93,10 +97,12 @@ export const ResourcesFilters = ({ const appliedFilterChips: FilterChip[] = buildResourcesFilterChips( appliedFilters, providers, + providerGroups, ); const pendingFilterChips: FilterChip[] = buildResourcesFilterChips( changedFilters, providers, + providerGroups, ); const appliedCount = countVisibleFilterKeys(appliedFilters); const showAppliedRow = appliedFilterChips.length > 0; @@ -178,6 +184,13 @@ export const ResourcesFilters = ({ providerSelectorClassName={FILTER_CONTROL_COLUMN_CLASS} accountSelectorClassName={FILTER_CONTROL_COLUMN_CLASS} /> +
+ +
{hasCustomFilters && (
@@ -175,7 +172,6 @@ export function ScanScheduleFields({ render={({ field }) => ( ( -
- Repeats - {cloudUpgradeBadge} -
+ Repeats