fix(api): prevent 500 on Jira integrations with sparse fieldsets (#12261)

api/changelog.d/integrations-sparse-fieldset-jira.fixed.md

@@ -0,0 +1 @@
+Requesting integrations with a sparse fieldset that leaves out `configuration` no longer returns HTTP 500 errors when the tenant has a Jira integration

api/src/backend/api/tests/test_rbac.py

@@ -2,6 +2,7 @@ import json
 from unittest.mock import ANY, Mock, patch
 
 import pytest
+from api.db_utils import rls_transaction
 from api.models import (
     Integration,
     IntegrationProviderRelationship,
@@ -1291,22 +1292,6 @@ class TestLimitedVisibility:
             response.json()["data"]["relationships"]["providers"]["meta"]["count"] == 1
         )
 
-    @pytest.fixture
-    def jira_integration(self, tenants_fixture):
-        # Jira is a tenant-wide integration: it is not attached to any provider
-        return Integration.objects.create(
-            tenant_id=tenants_fixture[0].id,
-            enabled=True,
-            connected=True,
-            integration_type=Integration.IntegrationChoices.JIRA,
-            configuration={"projects": {"TEST": "Test project"}},
-            credentials={
-                "domain": "test",
-                "user_mail": "a@b.com",
-                "api_token": "token",
-            },
-        )
-
     @pytest.fixture
     def out_of_scope_integration(self, tenants_fixture, provider_factory):
         tenant_id = tenants_fixture[0].id
@@ -1332,7 +1317,7 @@ class TestLimitedVisibility:
         self,
         authenticated_client_rbac_limited,
         integrations_fixture,
-        jira_integration,
+        jira_integration_fixture,
         aws_provider_pair,
     ):
         # Integration 2 is attached to both providers, so make both visible to the role
@@ -1348,13 +1333,16 @@ class TestLimitedVisibility:
         assert response.status_code == status.HTTP_200_OK
         integration_ids = [item["id"] for item in response.json()["data"]]
         # The tenant-wide Jira integration is visible without unlimited visibility
-        assert str(jira_integration.id) in integration_ids
+        assert str(jira_integration_fixture.id) in integration_ids
         # Integrations attached to more than one visible provider are not duplicated
         assert integration_ids.count(str(integrations_fixture[1].id)) == 1
         assert response.json()["meta"]["pagination"]["count"] == len(integration_ids)
 
     def test_integrations_list_without_provider_groups_keeps_tenant_wide_integration(
-        self, authenticated_client_rbac_limited, integrations_fixture, jira_integration
+        self,
+        authenticated_client_rbac_limited,
+        integrations_fixture,
+        jira_integration_fixture,
     ):
         # A role with no provider group at all sees no provider, but still needs Jira
         RoleProviderGroupRelationship.objects.all().delete()
@@ -1363,7 +1351,7 @@ class TestLimitedVisibility:
 
         assert response.status_code == status.HTTP_200_OK
         integration_ids = [item["id"] for item in response.json()["data"]]
-        assert integration_ids == [str(jira_integration.id)]
+        assert integration_ids == [str(jira_integration_fixture.id)]
 
     def test_integrations_include_providers_hides_out_of_scope_providers(
         self, authenticated_client_rbac_limited, integrations_fixture, aws_provider_pair
@@ -1382,13 +1370,19 @@ class TestLimitedVisibility:
         assert str(hidden_provider.id) not in included_ids
 
     def test_integrations_list_with_sparse_fields(
-        self, authenticated_client_rbac_limited, integrations_fixture
+        self,
+        authenticated_client_rbac_limited,
+        integrations_fixture,
+        jira_integration_fixture,
     ):
         response = authenticated_client_rbac_limited.get(
             reverse("integration-list"), {"fields[integrations]": "enabled"}
         )
 
         assert response.status_code == status.HTTP_200_OK
+        assert str(jira_integration_fixture.id) in [
+            item["id"] for item in response.json()["data"]
+        ]
         assert all(
             list(item["attributes"].keys()) == ["enabled"]
             for item in response.json()["data"]
@@ -1424,7 +1418,10 @@ class TestLimitedVisibility:
         assert response.status_code == status.HTTP_404_NOT_FOUND
 
     def test_integration_update_allowed_when_fully_visible(
-        self, authenticated_client_rbac_limited, integrations_fixture, jira_integration
+        self,
+        authenticated_client_rbac_limited,
+        integrations_fixture,
+        jira_integration_fixture,
     ):
         # Integration 1 is only related to provider1, which the role can access
         integration = integrations_fixture[0]
@@ -1458,20 +1455,21 @@ class TestLimitedVisibility:
         payload = {
             "data": {
                 "type": "integrations",
-                "id": str(jira_integration.id),
+                "id": str(jira_integration_fixture.id),
                 "attributes": {"enabled": False},
             }
         }
 
         response = authenticated_client_rbac_limited.patch(
-            reverse("integration-detail", kwargs={"pk": jira_integration.id}),
+            reverse("integration-detail", kwargs={"pk": jira_integration_fixture.id}),
             data=json.dumps(payload),
             content_type="application/vnd.api+json",
         )
 
         assert response.status_code == status.HTTP_200_OK
-        jira_integration.refresh_from_db()
-        assert jira_integration.enabled is False
+        with rls_transaction(str(jira_integration_fixture.tenant_id)):
+            jira_integration_fixture.refresh_from_db()
+        assert jira_integration_fixture.enabled is False
 
     def test_integration_create_rejects_out_of_scope_provider(
         self, authenticated_client_rbac_limited, aws_provider_pair
@@ -1571,7 +1569,10 @@ class TestLimitedVisibility:
         assert Integration.objects.filter(id=integration.id).exists()
 
     def test_integration_delete_allowed_when_fully_visible(
-        self, authenticated_client_rbac_limited, integrations_fixture, jira_integration
+        self,
+        authenticated_client_rbac_limited,
+        integrations_fixture,
+        jira_integration_fixture,
     ):
         # Integration 1 is only related to provider1, which the role can access
         integration = integrations_fixture[0]
@@ -1585,20 +1586,20 @@ class TestLimitedVisibility:
 
         # Tenant-wide integrations have no provider restricting the role
         response = authenticated_client_rbac_limited.delete(
-            reverse("integration-detail", kwargs={"pk": jira_integration.id})
+            reverse("integration-detail", kwargs={"pk": jira_integration_fixture.id})
         )
 
         assert response.status_code == status.HTTP_204_NO_CONTENT
 
     def test_jira_issue_types_allowed_without_unlimited_visibility(
-        self, authenticated_client_rbac_limited, jira_integration
+        self, authenticated_client_rbac_limited, jira_integration_fixture
     ):
         with patch("api.v1.views.initialize_prowler_integration") as mock_jira:
             mock_jira.return_value.get_available_issue_types.return_value = ["Task"]
             response = authenticated_client_rbac_limited.get(
                 reverse(
                     "integration-jira-issue-types",
-                    kwargs={"integration_pk": jira_integration.id},
+                    kwargs={"integration_pk": jira_integration_fixture.id},
                 ),
                 {"project_key": "TEST"},
             )
@@ -1634,12 +1635,12 @@ class TestLimitedVisibility:
         assert response.status_code == status.HTTP_404_NOT_FOUND
 
     def test_jira_dispatches_allowed_without_unlimited_visibility(
-        self, authenticated_client_rbac_limited, jira_integration
+        self, authenticated_client_rbac_limited, jira_integration_fixture
     ):
         response = authenticated_client_rbac_limited.post(
             reverse(
                 "integration-jira-dispatches",
-                kwargs={"integration_pk": jira_integration.id},
+                kwargs={"integration_pk": jira_integration_fixture.id},
             ),
             data=json.dumps({}),
             content_type="application/vnd.api+json",

api/src/backend/api/tests/test_serializers.py

@@ -6,10 +6,13 @@ from api.v1.serializer_utils.integrations import (
 from api.v1.serializer_utils.providers import ProviderSecretField
 from api.v1.serializers import (
     ImageProviderSecret,
+    IntegrationSerializer,
+    IntegrationUpdateSerializer,
     KubernetesProviderSecret,
     OracleCloudProviderSecret,
 )
 from rest_framework.exceptions import ValidationError
+from rest_framework.test import APIRequestFactory
 
 
 class TestS3ConfigSerializer:
@@ -352,3 +355,25 @@ current-context: test-context
 
         assert not serializer.is_valid()
         assert "kubeconfig_content" in serializer.errors
+
+
+@pytest.mark.django_db
+class TestIntegrationSerializerJiraDomain:
+    """The serialized Jira `domain` must not reach the model instance."""
+
+    @pytest.mark.parametrize(
+        "serializer_class", [IntegrationSerializer, IntegrationUpdateSerializer]
+    )
+    def test_to_representation_does_not_mutate_configuration(
+        self, serializer_class, jira_integration_fixture
+    ):
+        # `IntegrationUpdateSerializer` exposes a `HyperlinkedIdentityField`
+        context = {"request": APIRequestFactory().get("/")}
+        representation = serializer_class(
+            jira_integration_fixture, context=context
+        ).data
+
+        assert representation["configuration"]["domain"] == "test"
+        assert jira_integration_fixture.configuration == {
+            "projects": {"TEST": "Test project"}
+        }

api/src/backend/api/tests/test_views.py

@@ -23,6 +23,7 @@ from api.attack_paths import (
 )
 from api.compliance import get_compliance_frameworks
 from api.db_router import MainRouter
+from api.db_utils import rls_transaction
 from api.models import (
     AttackSurfaceOverview,
     ComplianceOverviewSummary,
@@ -13498,6 +13499,73 @@ class TestIntegrationViewSet:
                 f"Expected type '{expected_type}' not found in included data"
             )
 
+    # Serializing a Jira integration reads `configuration` to add the domain from the
+    # credentials, and a sparse fieldset can leave that field out of the representation
+
+    def test_integrations_list_sparse_fields_without_configuration(
+        self, authenticated_client, jira_integration_fixture
+    ):
+        response = authenticated_client.get(
+            reverse("integration-list"),
+            {"fields[integrations]": "enabled,integration_type"},
+        )
+
+        assert response.status_code == status.HTTP_200_OK
+        attributes = response.json()["data"][0]["attributes"]
+        assert sorted(attributes.keys()) == ["enabled", "integration_type"]
+
+    def test_integrations_retrieve_sparse_fields_without_configuration(
+        self, authenticated_client, jira_integration_fixture
+    ):
+        response = authenticated_client.get(
+            reverse("integration-detail", kwargs={"pk": jira_integration_fixture.id}),
+            {"fields[integrations]": "enabled,integration_type"},
+        )
+
+        assert response.status_code == status.HTTP_200_OK
+        assert "configuration" not in response.json()["data"]["attributes"]
+
+    def test_integrations_partial_update_sparse_fields_without_configuration(
+        self, authenticated_client, jira_integration_fixture
+    ):
+        data = {
+            "data": {
+                "type": "integrations",
+                "id": str(jira_integration_fixture.id),
+                "attributes": {"enabled": False},
+            }
+        }
+
+        url = reverse("integration-detail", kwargs={"pk": jira_integration_fixture.id})
+        response = authenticated_client.patch(
+            f"{url}?fields[integrations]=enabled,integration_type",
+            data=json.dumps(data),
+            content_type="application/vnd.api+json",
+        )
+
+        assert response.status_code == status.HTTP_200_OK
+        assert "configuration" not in response.json()["data"]["attributes"]
+        with rls_transaction(str(jira_integration_fixture.tenant_id)):
+            jira_integration_fixture.refresh_from_db()
+        assert jira_integration_fixture.enabled is False
+        # Omitting `configuration` from the fieldset must not rewrite it, and the
+        # serialized `domain` must not leak into the stored value
+        assert jira_integration_fixture.configuration == {
+            "projects": {"TEST": "Test project"}
+        }
+
+    def test_integrations_retrieve_jira_keeps_domain_in_configuration(
+        self, authenticated_client, jira_integration_fixture
+    ):
+        response = authenticated_client.get(
+            reverse("integration-detail", kwargs={"pk": jira_integration_fixture.id})
+        )
+
+        assert response.status_code == status.HTTP_200_OK
+        configuration = response.json()["data"]["attributes"]["configuration"]
+        assert configuration["domain"] == "test"
+        assert configuration["projects"] == {"TEST": "Test project"}
+
     @pytest.mark.parametrize(
         "integration_type, configuration, credentials",
         [

api/src/backend/api/v1/serializers.py

@@ -3006,10 +3006,15 @@ class IntegrationSerializer(IntegrationProviderVisibilityMixin, RLSSerializer):
         representation = self.hide_restricted_providers(
             super().to_representation(instance)
         )
-        if instance.integration_type == Integration.IntegrationChoices.JIRA:
-            representation["configuration"].update(
-                {"domain": instance.credentials.get("domain")}
-            )
+        # `configuration` is missing when the request asks for a subset of the fields
+        if (
+            instance.integration_type == Integration.IntegrationChoices.JIRA
+            and "configuration" in representation
+        ):
+            representation["configuration"] = {
+                **representation["configuration"],
+                "domain": instance.credentials.get("domain"),
+            }
         return representation
 
 
@@ -3143,11 +3148,16 @@ class IntegrationUpdateSerializer(
         representation = self.hide_restricted_providers(
             super().to_representation(instance)
         )
-        # Ensure JIRA integrations show updated domain in configuration from credentials
-        if instance.integration_type == Integration.IntegrationChoices.JIRA:
-            representation["configuration"].update(
-                {"domain": instance.credentials.get("domain")}
-            )
+        # Ensure JIRA integrations show updated domain in configuration from credentials.
+        # `configuration` is missing when the request asks for a subset of the fields
+        if (
+            instance.integration_type == Integration.IntegrationChoices.JIRA
+            and "configuration" in representation
+        ):
+            representation["configuration"] = {
+                **representation["configuration"],
+                "domain": instance.credentials.get("domain"),
+            }
         return representation
 
 

api/src/backend/conftest.py

@@ -1450,6 +1450,26 @@ def integrations_fixture(aws_provider_pair):
     return integration1, integration2
 
 
+@pytest.fixture
+def jira_integration_fixture(tenants_fixture):
+    # Jira is a tenant-wide integration: it is not attached to any provider, and its
+    # `domain` is read from the credentials when the integration is serialized
+    tenant_id = tenants_fixture[0].id
+    with rls_transaction(str(tenant_id)):
+        return Integration.objects.create(
+            tenant_id=tenant_id,
+            enabled=True,
+            connected=True,
+            integration_type=Integration.IntegrationChoices.JIRA,
+            configuration={"projects": {"TEST": "Test project"}},
+            credentials={
+                "domain": "test",
+                "user_mail": "a@b.com",
+                "api_token": "token",
+            },
+        )
+
+
 @pytest.fixture
 def backfill_scan_metadata_fixture(scans_fixture, findings_fixture):
     for scan_instance in scans_fixture: