diff --git a/api/src/backend/api/tests/test_views.py b/api/src/backend/api/tests/test_views.py index f2a0c299f2..2f155543b8 100644 --- a/api/src/backend/api/tests/test_views.py +++ b/api/src/backend/api/tests/test_views.py @@ -1712,6 +1712,20 @@ class TestProviderViewSet: ) assert response.status_code == status.HTTP_404_NOT_FOUND + def test_providers_connection_unavailable_provider( + self, authenticated_client, providers_fixture + ): + """Test that connection check returns 400 for unavailable provider.""" + provider1, *_ = providers_fixture + provider1.available = False + provider1.save() + + response = authenticated_client.post( + reverse("provider-connection", kwargs={"pk": provider1.id}) + ) + assert response.status_code == status.HTTP_400_BAD_REQUEST + assert response.json()["errors"][0]["code"] == "provider_unavailable" + @pytest.mark.parametrize( "filter_name, filter_value, expected_count", ( @@ -2887,6 +2901,35 @@ class TestScanViewSet: response.json()["errors"][0]["source"]["pointer"] == "/data/attributes/name" ) + def test_scans_create_unavailable_provider( + self, authenticated_client, providers_fixture + ): + """Test that creating a scan for an unavailable provider returns 400.""" + provider1, *_ = providers_fixture + provider1.available = False + provider1.save() + + scan_json_payload = { + "data": { + "type": "scans", + "attributes": { + "name": "Test Scan", + "trigger": Scan.TriggerChoices.MANUAL, + }, + "relationships": { + "provider": {"data": {"type": "providers", "id": str(provider1.id)}} + }, + } + } + + response = authenticated_client.post( + reverse("scan-list"), + data=scan_json_payload, + content_type=API_JSON_CONTENT_TYPE, + ) + assert response.status_code == status.HTTP_400_BAD_REQUEST + assert "unavailable provider" in response.json()["errors"][0]["detail"].lower() + def test_scans_partial_update(self, authenticated_client, scans_fixture): scan1, *_ = scans_fixture new_name = "Updated Scan Name" diff --git a/api/src/backend/api/v1/serializers.py b/api/src/backend/api/v1/serializers.py index fcd413c217..88515b2171 100644 --- a/api/src/backend/api/v1/serializers.py +++ b/api/src/backend/api/v1/serializers.py @@ -1066,6 +1066,14 @@ class ScanCreateSerializer(RLSSerializer, BaseWriteSerializer): "name", ] + def validate_provider(self, provider): + if not provider.available: + raise serializers.ValidationError( + "Cannot create scan for unavailable provider. " + "The provider no longer exists." + ) + return provider + def create(self, validated_data): # provider = validated_data.get("provider") diff --git a/api/src/backend/api/v1/views.py b/api/src/backend/api/v1/views.py index ffdfc1005e..7a081cffdc 100644 --- a/api/src/backend/api/v1/views.py +++ b/api/src/backend/api/v1/views.py @@ -1551,7 +1551,23 @@ class ProviderViewSet(DisablePaginationMixin, BaseRLSViewSet): ) @action(detail=True, methods=["post"], url_name="connection") def connection(self, request, pk=None): - get_object_or_404(Provider, pk=pk) + provider = get_object_or_404(Provider, pk=pk) + + if not provider.available: + return Response( + data={ + "errors": [ + { + "detail": "Cannot check connection for unavailable provider. " + "The provider no longer exists.", + "status": "400", + "code": "provider_unavailable", + } + ] + }, + status=status.HTTP_400_BAD_REQUEST, + ) + with transaction.atomic(): task = check_provider_connection_task.delay( provider_id=pk, tenant_id=self.request.tenant_id diff --git a/api/src/backend/tasks/jobs/connection.py b/api/src/backend/tasks/jobs/connection.py index 25a16a4cae..bd673459be 100644 --- a/api/src/backend/tasks/jobs/connection.py +++ b/api/src/backend/tasks/jobs/connection.py @@ -12,32 +12,6 @@ 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. @@ -76,27 +50,12 @@ def check_provider_connection(provider_id: str): 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) - # 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 880522f398..880027d95e 100644 --- a/api/src/backend/tasks/jobs/scan.py +++ b/api/src/backend/tasks/jobs/scan.py @@ -14,7 +14,6 @@ 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, @@ -818,12 +817,6 @@ def perform_prowler_scan( provider_instance.connected = True except Exception as e: provider_instance.connected = False - # 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 3ca54858f0..acac3d840f 100644 --- a/api/src/backend/tasks/tests/test_connection.py +++ b/api/src/backend/tasks/tests/test_connection.py @@ -115,52 +115,6 @@ def test_check_provider_connection_skips_unavailable_provider(tenants_fixture): 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 - - @pytest.mark.parametrize( "lighthouse_data", [ diff --git a/api/src/backend/tasks/tests/test_scan.py b/api/src/backend/tasks/tests/test_scan.py index 8902b17b54..6ec28612ef 100644 --- a/api/src/backend/tasks/tests/test_scan.py +++ b/api/src/backend/tasks/tests/test_scan.py @@ -260,6 +260,34 @@ class TestPerformScan: assert provider.connected is False assert isinstance(provider.connection_last_checked_at, datetime) + @patch("api.db_utils.rls_transaction") + def test_perform_prowler_scan_unavailable_provider( + self, + mock_rls_transaction, + tenants_fixture, + scans_fixture, + providers_fixture, + ): + """Test that scan fails immediately for unavailable provider.""" + tenant = tenants_fixture[0] + scan = scans_fixture[0] + provider = providers_fixture[0] + + # Mark provider as unavailable + provider.available = False + provider.save() + + tenant_id = str(tenant.id) + scan_id = str(scan.id) + provider_id = str(provider.id) + checks_to_execute = ["check1", "check2"] + + with pytest.raises(ProviderConnectionError, match="marked as unavailable"): + perform_prowler_scan(tenant_id, scan_id, provider_id, checks_to_execute) + + scan.refresh_from_db() + assert scan.state == StateChoices.FAILED + @pytest.mark.parametrize( "last_status, new_status, expected_delta", [