fix(integrations): Disallow modifying a repo's integration (#111739)

Author: cmanallen

src/sentry/integrations/api/endpoints/organization_repository_details.py

@@ -12,13 +12,10 @@
 from sentry.api.base import cell_silo_endpoint
 from sentry.api.bases.organization import OrganizationEndpoint, OrganizationIntegrationsPermission
 from sentry.api.exceptions import ResourceDoesNotExist
-from sentry.api.fields.empty_integer import EmptyIntegerField
 from sentry.api.serializers import serialize
 from sentry.api.serializers.models.repository import RepositorySerializer as RepositoryApiSerializer
 from sentry.constants import ObjectStatus
 from sentry.deletions.models.scheduleddeletion import CellScheduledDeletion
-from sentry.hybridcloud.rpc import coerce_id_from
-from sentry.integrations.services.integration import integration_service
 from sentry.models.commit import Commit
 from sentry.models.organization import Organization
 from sentry.models.repository import Repository
@@ -37,7 +34,6 @@ class RepositorySerializer(serializers.Serializer):
     )
     name = serializers.CharField(required=False)
     url = serializers.URLField(required=False, allow_blank=True)
-    integrationId = EmptyIntegerField(required=False, allow_null=True)
 
 
 @cell_silo_endpoint
@@ -71,6 +67,11 @@ def put(self, request: Request, organization: Organization, repo_id) -> Response
         if repo.status == ObjectStatus.DELETION_IN_PROGRESS:
             return Response(status=400)
 
+        if "integrationId" in request.data:
+            return Response(
+                {"detail": "Changing the repository provider is not allowed"}, status=400
+            )
+
         serializer = RepositorySerializer(data=request.data, partial=True)
 
         if not serializer.is_valid():
@@ -85,18 +86,6 @@ def put(self, request: Request, organization: Organization, repo_id) -> Response
                 update_kwargs["status"] = ObjectStatus.HIDDEN
             else:
                 raise NotImplementedError
-        if result.get("integrationId"):
-            integration = integration_service.get_integration(
-                integration_id=result["integrationId"],
-                organization_id=coerce_id_from(organization),
-                status=ObjectStatus.ACTIVE,
-            )
-            if integration is None:
-                return Response({"detail": "Invalid integration id"}, status=400)
-
-            update_kwargs["integration_id"] = integration.id
-            update_kwargs["provider"] = f"integrations:{integration.provider}"
-
         if update_kwargs:
             old_status = repo.status
             with transaction.atomic(router.db_for_write(Repository)):

tests/sentry/integrations/api/endpoints/test_organization_repository_details.py

@@ -9,9 +9,7 @@
 from sentry.models.commit import Commit
 from sentry.models.options.organization_option import OrganizationOption
 from sentry.models.repository import Repository
-from sentry.silo.base import SiloMode
 from sentry.testutils.cases import APITestCase
-from sentry.testutils.silo import assume_test_silo_mode
 
 
 class OrganizationRepositoryGetTest(APITestCase):
@@ -211,35 +209,29 @@ def test_put(self) -> None:
         self.login_as(user=self.user)
 
         org = self.create_organization(owner=self.user, name="baz")
-        integration = self.create_integration(
-            organization=org, provider="example", name="Example", external_id="example:1"
-        )
 
         repo = Repository.objects.create(
             name="example", organization_id=org.id, status=ObjectStatus.DISABLED
         )
 
         url = reverse("sentry-api-0-organization-repository-details", args=[org.slug, repo.id])
-        response = self.client.put(url, data={"status": "visible", "integrationId": integration.id})
+        response = self.client.put(url, data={"status": "visible"})
 
         assert response.status_code == 200
 
         repo = Repository.objects.get(id=repo.id)
         assert repo.status == ObjectStatus.ACTIVE
-        assert repo.integration_id == integration.id
 
     def test_put_cancel_deletion(self) -> None:
         self.login_as(user=self.user)
 
         org = self.create_organization(owner=self.user, name="baz")
-        integration = self.create_integration(
-            organization=org, provider="example", name="Example", external_id="example:1"
-        )
 
         repo = Repository.objects.create(
             name="uuid-name",
             external_id="uuid-external-id",
             organization_id=org.id,
+            provider="integrations:example",
             status=ObjectStatus.PENDING_DELETION,
             config={"pending_deletion_name": "example-name"},
         )
@@ -256,13 +248,12 @@ def test_put_cancel_deletion(self) -> None:
         )
 
         url = reverse("sentry-api-0-organization-repository-details", args=[org.slug, repo.id])
-        response = self.client.put(url, data={"status": "visible", "integrationId": integration.id})
+        response = self.client.put(url, data={"status": "visible"})
 
         assert response.status_code == 200
 
         repo = Repository.objects.get(id=repo.id)
         assert repo.status == ObjectStatus.ACTIVE
-        assert repo.integration_id == integration.id
         assert repo.provider == "integrations:example"
         assert repo.name == "example-name"
         assert repo.external_id == "example-external-id"
@@ -332,37 +323,30 @@ def test_put_hide_repo_with_commits(self, mock_cleanup_task: MagicMock) -> None:
             }
         )
 
-    def test_put_bad_integration_org(self) -> None:
+    def test_put_rejects_integration_id(self) -> None:
         self.login_as(user=self.user)
 
         org = self.create_organization(owner=self.user, name="baz")
-        with assume_test_silo_mode(SiloMode.CONTROL):
-            integration = self.create_provider_integration(provider="example", name="example")
+        integration = self.create_integration(
+            organization=org, provider="example", name="Example", external_id="example:1"
+        )
 
-        repo = Repository.objects.create(name="example", organization_id=org.id)
+        repo = Repository.objects.create(
+            name="example",
+            organization_id=org.id,
+            provider="integrations:github",
+            integration_id=None,
+        )
 
         url = reverse("sentry-api-0-organization-repository-details", args=[org.slug, repo.id])
-        # integration isn't linked to org
         response = self.client.put(url, data={"status": "visible", "integrationId": integration.id})
 
         assert response.status_code == 400
-        assert response.data["detail"] == "Invalid integration id"
-        assert Repository.objects.get(id=repo.id).name == "example"
-
-    def test_put_bad_integration_id(self) -> None:
-        self.login_as(user=self.user)
-
-        org = self.create_organization(owner=self.user, name="baz")
+        assert response.data["detail"] == "Changing the repository provider is not allowed"
 
-        repo = Repository.objects.create(name="example", organization_id=org.id)
-
-        url = reverse("sentry-api-0-organization-repository-details", args=[org.slug, repo.id])
-        # integration isn't linked to org
-        response = self.client.put(url, data={"status": "visible", "integrationId": "notanumber"})
-
-        assert response.status_code == 400
-        assert response.data == {"integrationId": ["A valid integer is required."]}
-        assert Repository.objects.get(id=repo.id).name == "example"
+        repo = Repository.objects.get(id=repo.id)
+        assert repo.provider == "integrations:github"
+        assert repo.integration_id is None
 
     @patch("sentry.tasks.seer.cleanup.cleanup_seer_repository_preferences.apply_async")
     def test_put_hide_repo_triggers_cleanup(self, mock_cleanup_task: MagicMock) -> None: