diff --git a/api/src/backend/api/filters.py b/api/src/backend/api/filters.py index faa6fc1707..74f36a18cd 100644 --- a/api/src/backend/api/filters.py +++ b/api/src/backend/api/filters.py @@ -40,7 +40,6 @@ from api.models import ( ProviderComplianceScore, ProviderGroup, ProviderSecret, - ProviderStatusChoices, Resource, ResourceTag, Role, @@ -320,17 +319,10 @@ class ProviderFilter(FilterSet): choices=Provider.ProviderChoices.choices, lookup_expr="in", ) - status = ChoiceFilter( - choices=ProviderStatusChoices.choices, - help_text="""Filter by provider connection status. - Valid values: pending, checking, connected, error.""", - ) - status__in = ChoiceInFilter( - field_name="status", - choices=ProviderStatusChoices.choices, - lookup_expr="in", - help_text="""Filter by multiple provider connection statuses. - Accepts comma-separated values: pending,connected,error""", + available = BooleanFilter( + help_text="""Filter by provider availability. Set to True to return only + available providers, or False to return only unavailable providers + (ephemeral accounts that no longer exist).""" ) class Meta: @@ -342,7 +334,7 @@ class ProviderFilter(FilterSet): "alias": ["exact", "icontains", "in"], "inserted_at": ["gte", "lte"], "updated_at": ["gte", "lte"], - "status": ["exact", "in"], + "available": ["exact"], } filter_overrides = { ProviderEnumField: { diff --git a/api/src/backend/api/migrations/0068_provider_available.py b/api/src/backend/api/migrations/0068_provider_available.py new file mode 100644 index 0000000000..3eb17331ba --- /dev/null +++ b/api/src/backend/api/migrations/0068_provider_available.py @@ -0,0 +1,19 @@ +# Migration to add available field to Provider model +from django.db import migrations, models + + +class Migration(migrations.Migration): + dependencies = [ + ("api", "0067_tenant_compliance_summary"), + ] + + operations = [ + migrations.AddField( + model_name="provider", + name="available", + field=models.BooleanField( + default=True, + help_text="Whether the provider account still exists. If False, connection checks are skipped.", + ), + ), + ] diff --git a/api/src/backend/api/migrations/0068_provider_status.py b/api/src/backend/api/migrations/0068_provider_status.py deleted file mode 100644 index 173b83ea61..0000000000 --- a/api/src/backend/api/migrations/0068_provider_status.py +++ /dev/null @@ -1,62 +0,0 @@ -# Migration to add status field to Provider model -from django.db import migrations, models - -from api.db_router import MainRouter - - -def populate_provider_status(apps, schema_editor): - """ - Populate status field based on existing connected field values. - - Migration logic for production: - - connected=True → status="connected" (successful connection) - - connected=False → status="error" (connection was attempted but failed) - - connected=NULL → status="pending" (never attempted to connect) - """ - Provider = apps.get_model("api", "Provider") - db_alias = MainRouter.admin_db - - Provider.objects.using(db_alias).filter(connected=True).update(status="connected") - Provider.objects.using(db_alias).filter(connected=False).update(status="error") - Provider.objects.using(db_alias).filter(connected__isnull=True).update( - status="pending" - ) - - -def reverse_populate(apps, schema_editor): - """ - Reverse the status population. - """ - Provider = apps.get_model("api", "Provider") - db_alias = MainRouter.admin_db - - Provider.objects.using(db_alias).all().update(status=None) - - -class Migration(migrations.Migration): - dependencies = [ - ("api", "0067_tenant_compliance_summary"), - ] - - operations = [ - migrations.AddField( - model_name="provider", - name="status", - field=models.CharField( - blank=True, - choices=[ - ("pending", "Pending"), - ("checking", "Checking"), - ("connected", "Connected"), - ("error", "Error"), - ], - default="pending", - max_length=20, - null=True, - ), - ), - migrations.RunPython( - populate_provider_status, - reverse_code=reverse_populate, - ), - ] diff --git a/api/src/backend/api/models.py b/api/src/backend/api/models.py index ee5e9091a8..68604cd92a 100644 --- a/api/src/backend/api/models.py +++ b/api/src/backend/api/models.py @@ -110,17 +110,6 @@ class PermissionChoices(models.TextChoices): NONE = "none", _("No permissions") -class ProviderStatusChoices(models.TextChoices): - """ - Represents the connection status states for a Provider. - """ - - PENDING = "pending", _("Pending") - CHECKING = "checking", _("Checking") - CONNECTED = "connected", _("Connected") - ERROR = "error", _("Error") - - class ActiveProviderManager(models.Manager): def get_queryset(self): return super().get_queryset().filter(self.active_provider_filter()) @@ -430,12 +419,9 @@ class Provider(RowLevelSecurityProtectedModel): ) connected = models.BooleanField(null=True, blank=True) connection_last_checked_at = models.DateTimeField(null=True, blank=True) - status = models.CharField( - max_length=20, - choices=ProviderStatusChoices.choices, - default=ProviderStatusChoices.PENDING, - null=True, - blank=True, + available = models.BooleanField( + default=True, + help_text="Whether the provider account still exists. If False, connection checks are skipped.", ) metadata = models.JSONField(default=dict, blank=True) scanner_args = models.JSONField(default=dict, blank=True) diff --git a/api/src/backend/api/specs/v1.yaml b/api/src/backend/api/specs/v1.yaml index 5524e253e4..128067619c 100644 --- a/api/src/backend/api/specs/v1.yaml +++ b/api/src/backend/api/specs/v1.yaml @@ -6646,33 +6646,13 @@ paths: included. Providers with no connection attempt (status is null) are excluded from this filter. - in: query - name: filter[status] + name: filter[available] schema: - type: string - enum: - - pending - - checking - - connected - - error + type: boolean description: |- - Filter by provider connection status. - Valid values: pending, checking, connected, error. - - in: query - name: filter[status__in] - schema: - type: array - items: - type: string - enum: - - pending - - checking - - connected - - error - description: |- - Filter by multiple provider connection statuses. - Accepts comma-separated values: pending,connected,error - explode: false - style: form + Filter by provider availability. Set to True to return only + available providers, or False to return only unavailable providers + (ephemeral accounts that no longer exist). - in: query name: filter[id] schema: @@ -15927,6 +15907,10 @@ components: 'Production AWS Account', 'Dev Environment' maxLength: 100 minLength: 3 + available: + type: boolean + description: Whether the provider account still exists. Set to False + if the provider is ephemeral and no longer exists. required: - data PatchedRoleProviderGroupRelationshipRequest: @@ -16887,6 +16871,10 @@ components: nullable: true maxLength: 100 minLength: 3 + available: + type: boolean + description: Whether the provider account still exists. If False, connection + checks are skipped. connection: type: object properties: @@ -16895,13 +16883,6 @@ components: last_checked_at: type: string format: date-time - status: - type: string - enum: - - pending - - checking - - connected - - error readOnly: true required: - provider diff --git a/api/src/backend/api/v1/serializers.py b/api/src/backend/api/v1/serializers.py index 42b60f48b5..fcd413c217 100644 --- a/api/src/backend/api/v1/serializers.py +++ b/api/src/backend/api/v1/serializers.py @@ -877,6 +877,7 @@ class ProviderSerializer(RLSSerializer): "provider", "uid", "alias", + "available", "connection", # "scanner_args", "secret", @@ -890,7 +891,6 @@ class ProviderSerializer(RLSSerializer): "properties": { "connected": {"type": "boolean"}, "last_checked_at": {"type": "string", "format": "date-time"}, - "status": {"type": "string"}, }, } ) @@ -898,7 +898,6 @@ class ProviderSerializer(RLSSerializer): return { "connected": obj.connected, "last_checked_at": obj.connection_last_checked_at, - "status": obj.status, } @@ -919,6 +918,7 @@ class ProviderIncludeSerializer(RLSSerializer): "provider", "uid", "alias", + "available", "connection", # "scanner_args", ] @@ -929,7 +929,6 @@ class ProviderIncludeSerializer(RLSSerializer): "properties": { "connected": {"type": "boolean"}, "last_checked_at": {"type": "string", "format": "date-time"}, - "status": {"type": "string"}, }, } ) @@ -937,7 +936,6 @@ class ProviderIncludeSerializer(RLSSerializer): return { "connected": obj.connected, "last_checked_at": obj.connection_last_checked_at, - "status": obj.status, } @@ -966,19 +964,23 @@ class ProviderCreateSerializer(RLSSerializer, BaseWriteSerializer): class ProviderUpdateSerializer(BaseWriteSerializer): """ Serializer for updating the Provider model. - Only allows "alias" and "scanner_args" fields to be updated. + Only allows "alias", "available" and "scanner_args" fields to be updated. """ class Meta: model = Provider fields = [ "alias", + "available", # "scanner_args" ] extra_kwargs = { "alias": { "help_text": "Human readable name to identify the provider, e.g. 'Production AWS Account', 'Dev Environment'", - } + }, + "available": { + "help_text": "Whether the provider account still exists. Set to False if the provider is ephemeral and no longer exists.", + }, } diff --git a/api/src/backend/tasks/jobs/connection.py b/api/src/backend/tasks/jobs/connection.py index 6350e5d338..25a16a4cae 100644 --- a/api/src/backend/tasks/jobs/connection.py +++ b/api/src/backend/tasks/jobs/connection.py @@ -3,12 +3,7 @@ from datetime import datetime, timezone import openai from celery.utils.log import get_task_logger -from api.models import ( - Integration, - LighthouseConfiguration, - Provider, - ProviderStatusChoices, -) +from api.models import Integration, LighthouseConfiguration, Provider from api.utils import ( prowler_integration_connection_test, prowler_provider_connection_test, @@ -17,6 +12,32 @@ from api.utils import ( logger = get_task_logger(__name__) +def _is_provider_not_found_error(error: Exception) -> bool: + """ + Check if the error indicates the provider account no longer exists. + + Args: + error: The exception raised during connection test. + + Returns: + bool: True if the error indicates the provider account doesn't exist. + """ + error_str = str(error).lower() + not_found_patterns = [ + "account not found", + "subscription not found", + "project not found", + "invalidclienttokenid", + "the security token included in the request is invalid", + "subscriptionnotfound", + "resource not found", + "does not exist", + "was not found", + "no such account", + ] + return any(pattern in error_str for pattern in not_found_patterns) + + def check_provider_connection(provider_id: str): """ Business logic to check the connection status of a provider. @@ -35,9 +56,16 @@ def check_provider_connection(provider_id: str): """ provider_instance = Provider.objects.get(pk=provider_id) - # Set status to CHECKING before the connection test - provider_instance.status = ProviderStatusChoices.CHECKING - provider_instance.save(update_fields=["status"]) + # Skip connection check if provider is marked as unavailable + if not provider_instance.available: + logger.info( + f"Skipping connection check for provider {provider_id}: marked as unavailable" + ) + return { + "connected": False, + "error": "Provider is marked as unavailable", + "skipped": True, + } try: connection_result = prowler_provider_connection_test(provider_instance) @@ -45,20 +73,30 @@ def check_provider_connection(provider_id: str): logger.warning( f"Unexpected exception checking {provider_instance.provider} provider connection: {str(e)}" ) - # Set status to ERROR on exception - provider_instance.status = ProviderStatusChoices.ERROR provider_instance.connected = False provider_instance.connection_last_checked_at = datetime.now(tz=timezone.utc) + + # Mark provider as unavailable if the error indicates the account doesn't exist + if _is_provider_not_found_error(e): + provider_instance.available = False + logger.warning( + f"Provider {provider_id} marked as unavailable: account not found" + ) + provider_instance.save() raise e provider_instance.connected = connection_result.is_connected provider_instance.connection_last_checked_at = datetime.now(tz=timezone.utc) - provider_instance.status = ( - ProviderStatusChoices.CONNECTED - if connection_result.is_connected - else ProviderStatusChoices.ERROR - ) + + # Check if connection failed due to provider not found + if not connection_result.is_connected and connection_result.error: + if _is_provider_not_found_error(connection_result.error): + provider_instance.available = False + logger.warning( + f"Provider {provider_id} marked as unavailable: account not found" + ) + provider_instance.save() connection_error = f"{connection_result.error}" if connection_result.error else None diff --git a/api/src/backend/tasks/jobs/scan.py b/api/src/backend/tasks/jobs/scan.py index 904a8eabde..880522f398 100644 --- a/api/src/backend/tasks/jobs/scan.py +++ b/api/src/backend/tasks/jobs/scan.py @@ -14,6 +14,7 @@ from config.env import env from config.settings.celery import CELERY_DEADLOCK_ATTEMPTS from django.db import IntegrityError, OperationalError from django.db.models import Case, Count, IntegerField, Prefetch, Q, Sum, When +from tasks.jobs.connection import _is_provider_not_found_error from tasks.jobs.queries import ( COMPLIANCE_UPSERT_PROVIDER_SCORE_SQL, COMPLIANCE_UPSERT_TENANT_SUMMARY_SQL, @@ -39,7 +40,6 @@ from api.models import ( MuteRule, Processor, Provider, - ProviderStatusChoices, Resource, ResourceFindingMapping, ResourceScanSummary, @@ -764,6 +764,20 @@ def perform_prowler_scan( with rls_transaction(tenant_id): provider_instance = Provider.objects.get(pk=provider_id) scan_instance = Scan.objects.get(pk=scan_id) + + # Skip scan if provider is marked as unavailable + if not provider_instance.available: + logger.info( + f"Skipping scan for provider {provider_id}: marked as unavailable" + ) + scan_instance.state = StateChoices.FAILED + scan_instance.started_at = datetime.now(tz=timezone.utc) + scan_instance.completed_at = datetime.now(tz=timezone.utc) + scan_instance.save() + raise ProviderConnectionError( + f"Provider {provider_instance.provider} is marked as unavailable" + ) + scan_instance.state = StateChoices.EXECUTING scan_instance.started_at = datetime.now(tz=timezone.utc) scan_instance.save() @@ -802,10 +816,14 @@ def perform_prowler_scan( provider_instance, mutelist_processor ) provider_instance.connected = True - provider_instance.status = ProviderStatusChoices.CONNECTED except Exception as e: provider_instance.connected = False - provider_instance.status = ProviderStatusChoices.ERROR + # Mark provider as unavailable if the error indicates the account doesn't exist + if _is_provider_not_found_error(e): + provider_instance.available = False + logger.warning( + f"Provider {provider_id} marked as unavailable: account not found" + ) exc = ProviderConnectionError( f"Provider {provider_instance.provider} is not connected: {e}" ) diff --git a/api/src/backend/tasks/tests/test_connection.py b/api/src/backend/tasks/tests/test_connection.py index 16b6fc9f37..3ca54858f0 100644 --- a/api/src/backend/tasks/tests/test_connection.py +++ b/api/src/backend/tasks/tests/test_connection.py @@ -9,17 +9,12 @@ from tasks.jobs.connection import ( check_provider_connection, ) -from api.models import ( - Integration, - LighthouseConfiguration, - Provider, - ProviderStatusChoices, -) +from api.models import Integration, LighthouseConfiguration, Provider @pytest.mark.django_db -def test_provider_created_with_pending_status(tenants_fixture): - """Test that a newly created provider has PENDING status.""" +def test_provider_created_with_default_available(tenants_fixture): + """Test that a newly created provider has available=True by default.""" provider = Provider.objects.create( provider="aws", uid="123456789012", @@ -27,7 +22,7 @@ def test_provider_created_with_pending_status(tenants_fixture): tenant_id=tenants_fixture[0].id, ) - assert provider.status == ProviderStatusChoices.PENDING + assert provider.available is True assert provider.connected is None assert provider.connection_last_checked_at is None @@ -45,9 +40,6 @@ def test_check_provider_connection( ): provider = Provider.objects.create(**provider_data, tenant_id=tenants_fixture[0].id) - # Verify initial state is PENDING - assert provider.status == ProviderStatusChoices.PENDING - mock_test_connection_result = MagicMock() mock_test_connection_result.is_connected = True @@ -60,40 +52,10 @@ def test_check_provider_connection( mock_provider_connection_test.assert_called_once() assert provider.connected is True - assert provider.status == ProviderStatusChoices.CONNECTED assert provider.connection_last_checked_at is not None assert provider.connection_last_checked_at <= datetime.now(tz=timezone.utc) -@patch("tasks.jobs.connection.Provider.objects.get") -@patch("tasks.jobs.connection.prowler_provider_connection_test") -@pytest.mark.django_db -def test_check_provider_connection_sets_checking_status( - mock_provider_connection_test, mock_provider_get -): - """Test that status is set to CHECKING before connection test runs.""" - mock_provider_instance = MagicMock() - mock_provider_instance.provider = Provider.ProviderChoices.AWS.value - mock_provider_get.return_value = mock_provider_instance - - captured_status = None - - def capture_status_during_test(provider): - nonlocal captured_status - captured_status = provider.status - result = MagicMock() - result.is_connected = True - result.error = None - return result - - mock_provider_connection_test.side_effect = capture_status_during_test - - check_provider_connection(provider_id="provider_id") - - # Verify status was CHECKING when prowler_provider_connection_test was called - assert captured_status == ProviderStatusChoices.CHECKING - - @patch("tasks.jobs.connection.Provider.objects.get") @pytest.mark.django_db def test_check_provider_connection_unsupported_provider(mock_provider_get): @@ -115,6 +77,7 @@ def test_check_provider_connection_exception( ): mock_provider_instance = MagicMock() mock_provider_instance.provider = Provider.ProviderChoices.AWS.value + mock_provider_instance.available = True mock_provider_get.return_value = mock_provider_instance mock_provider_connection_test.return_value = MagicMock() @@ -125,12 +88,77 @@ def test_check_provider_connection_exception( assert result["connected"] is False assert result["error"] is not None - - assert ( - mock_provider_instance.save.call_count == 2 - ) # Once for CHECKING, once for ERROR + assert mock_provider_instance.save.call_count == 1 + assert mock_provider_instance.connected is False + + +@pytest.mark.django_db +def test_check_provider_connection_skips_unavailable_provider(tenants_fixture): + """Test that connection check is skipped when provider is marked as unavailable.""" + provider = Provider.objects.create( + provider="aws", + uid="123456789012", + alias="aws-test", + available=False, + tenant_id=tenants_fixture[0].id, + ) + + result = check_provider_connection(provider_id=str(provider.id)) + + assert result["connected"] is False + assert result["error"] == "Provider is marked as unavailable" + assert result["skipped"] is True + + provider.refresh_from_db() + # Provider should not have been modified + assert provider.connected is None + assert provider.connection_last_checked_at is None + + +@patch("tasks.jobs.connection.Provider.objects.get") +@patch("tasks.jobs.connection.prowler_provider_connection_test") +@pytest.mark.django_db +def test_check_provider_connection_marks_unavailable_on_not_found_error( + mock_provider_connection_test, mock_provider_get +): + """Test that provider is marked as unavailable when account not found error occurs.""" + mock_provider_instance = MagicMock() + mock_provider_instance.provider = Provider.ProviderChoices.AWS.value + mock_provider_instance.available = True + mock_provider_get.return_value = mock_provider_instance + + mock_provider_connection_test.return_value = MagicMock() + mock_provider_connection_test.return_value.is_connected = False + mock_provider_connection_test.return_value.error = Exception( + "The security token included in the request is invalid" + ) + + result = check_provider_connection(provider_id="provider_id") + + assert result["connected"] is False + assert mock_provider_instance.available is False + assert mock_provider_instance.connected is False + + +@patch("tasks.jobs.connection.Provider.objects.get") +@patch("tasks.jobs.connection.prowler_provider_connection_test") +@pytest.mark.django_db +def test_check_provider_connection_exception_marks_unavailable_on_not_found( + mock_provider_connection_test, mock_provider_get +): + """Test that provider is marked as unavailable when connection test raises account not found exception.""" + mock_provider_instance = MagicMock() + mock_provider_instance.provider = Provider.ProviderChoices.AWS.value + mock_provider_instance.available = True + mock_provider_get.return_value = mock_provider_instance + + mock_provider_connection_test.side_effect = Exception("Account not found") + + with pytest.raises(Exception, match="Account not found"): + check_provider_connection(provider_id="provider_id") + + assert mock_provider_instance.available is False assert mock_provider_instance.connected is False - assert mock_provider_instance.status == ProviderStatusChoices.ERROR @pytest.mark.parametrize( diff --git a/api/src/backend/tasks/tests/test_scan.py b/api/src/backend/tasks/tests/test_scan.py index cbb21a32de..8902b17b54 100644 --- a/api/src/backend/tasks/tests/test_scan.py +++ b/api/src/backend/tasks/tests/test_scan.py @@ -34,7 +34,6 @@ from api.models import ( Finding, MuteRule, Provider, - ProviderStatusChoices, Resource, Scan, StateChoices, @@ -259,7 +258,6 @@ class TestPerformScan: provider.refresh_from_db() assert provider.connected is False - assert provider.status == ProviderStatusChoices.ERROR assert isinstance(provider.connection_last_checked_at, datetime) @pytest.mark.parametrize(