Browse Source

fix(integrations) Fix validation error when deleting unpublished sentryapps (#59803)

Update mediator.Param() type to use RpcUser

Fixes SENTRY-17R0
Fixes HC-985
Mark Story 1 year ago
parent
commit
df34124d39

+ 1 - 7
src/sentry/api/endpoints/integrations/sentry_apps/installation/details.py

@@ -13,10 +13,7 @@ from sentry.api.serializers.rest_framework import SentryAppInstallationSerialize
 from sentry.mediators import InstallationNotifier
 from sentry.mediators.sentry_app_installations.updater import Updater
 from sentry.models.integrations.sentry_app_installation import SentryAppInstallation
-from sentry.models.user import User
-from sentry.services.hybrid_cloud.user import RpcUser
 from sentry.utils.audit import create_audit_entry
-from sentry.utils.functional import extract_lazy_object
 
 
 @control_silo_endpoint
@@ -32,12 +29,9 @@ class SentryAppInstallationDetailsEndpoint(SentryAppInstallationBaseEndpoint):
 
     def delete(self, request: Request, installation) -> Response:
         installation = SentryAppInstallation.objects.get(id=installation.id)
-        user = extract_lazy_object(request.user)
-        if isinstance(user, RpcUser):
-            user = User.objects.get(id=user.id)
         with transaction.atomic(using=router.db_for_write(SentryAppInstallation)):
             try:
-                InstallationNotifier.run(install=installation, user=user, action="deleted")
+                InstallationNotifier.run(install=installation, user=request.user, action="deleted")
             # if the error is from a request exception, log the error and continue
             except RequestException as exc:
                 sentry_sdk.capture_exception(exc)

+ 2 - 2
src/sentry/mediators/sentry_app_installations/installation_notifier.py

@@ -5,14 +5,14 @@ from sentry.coreapi import APIUnauthorized
 from sentry.mediators.mediator import Mediator
 from sentry.mediators.param import Param
 from sentry.models.integrations.sentry_app_installation import SentryAppInstallation
-from sentry.models.user import User
+from sentry.services.hybrid_cloud.user.model import RpcUser
 from sentry.utils.cache import memoize
 from sentry.utils.sentry_apps import send_and_save_webhook_request
 
 
 class InstallationNotifier(Mediator):
     install = Param(SentryAppInstallation)
-    user = Param(User)
+    user = Param(RpcUser)
     action = Param(str)
     using = router.db_for_write(SentryAppInstallation)
 

+ 3 - 3
tests/sentry/api/endpoints/test_organization_sentry_app_installation_details.py

@@ -8,6 +8,7 @@ from sentry.constants import SentryAppInstallationStatus
 from sentry.mediators.token_exchange.grant_exchanger import GrantExchanger
 from sentry.models.auditlogentry import AuditLogEntry
 from sentry.services.hybrid_cloud.app import app_service
+from sentry.services.hybrid_cloud.user.service import user_service
 from sentry.testutils.cases import APITestCase
 from sentry.testutils.silo import control_silo_test
 
@@ -87,13 +88,12 @@ class DeleteSentryAppInstallationDetailsTest(SentryAppInstallationDetailsTest):
     def test_delete_install(self, record, run):
         responses.add(url="https://example.com/webhook", method=responses.POST, body=b"")
         self.login_as(user=self.user)
+        rpc_user = user_service.get_user(user_id=self.user.id)
         response = self.client.delete(self.url, format="json")
         assert AuditLogEntry.objects.filter(
             event=audit_log.get_event_id("SENTRY_APP_UNINSTALL")
         ).exists()
-        run.assert_called_once_with(
-            install=self.orm_installation2, user=self.user, action="deleted"
-        )
+        run.assert_called_once_with(install=self.orm_installation2, user=rpc_user, action="deleted")
         record.assert_called_with(
             "sentry_app.uninstalled",
             user_id=self.user.id,

+ 17 - 0
tests/sentry/api/endpoints/test_sentry_app_details.py

@@ -6,6 +6,7 @@ from sentry import audit_log, deletions
 from sentry.constants import SentryAppStatus
 from sentry.models.auditlogentry import AuditLogEntry
 from sentry.models.integrations.sentry_app import SentryApp
+from sentry.models.integrations.sentry_app_installation import SentryAppInstallation
 from sentry.models.organizationmember import OrganizationMember
 from sentry.silo import SiloMode
 from sentry.testutils.cases import APITestCase
@@ -470,6 +471,22 @@ class DeleteSentryAppDetailsTest(SentryAppDetailsTest):
             sentry_app=self.unpublished_app.slug,
         )
 
+    def test_delete_unpublished_app_with_installs(self):
+        installation = self.create_sentry_app_installation(
+            organization=self.organization,
+            slug=self.unpublished_app.slug,
+            user=self.user,
+        )
+        self.login_as(user=self.superuser, superuser=True)
+        url = reverse("sentry-api-0-sentry-app-details", args=[self.unpublished_app.slug])
+        response = self.client.delete(url)
+        assert response.status_code == 204
+
+        assert AuditLogEntry.objects.filter(
+            event=audit_log.get_event_id("SENTRY_APP_REMOVE")
+        ).exists()
+        assert not SentryAppInstallation.objects.filter(id=installation.id).exists()
+
     def test_cannot_delete_published_app(self):
         self.login_as(user=self.superuser, superuser=True)
         url = reverse("sentry-api-0-sentry-app-details", args=[self.published_app.slug])

+ 7 - 5
tests/sentry/mediators/sentry_app_installations/test_installation_notifier.py

@@ -5,6 +5,7 @@ import pytest
 
 from sentry.coreapi import APIUnauthorized
 from sentry.mediators.sentry_app_installations.installation_notifier import InstallationNotifier
+from sentry.services.hybrid_cloud.user.service import user_service
 from sentry.testutils.cases import TestCase
 from sentry.testutils.silo import control_silo_test
 from sentry.utils import json
@@ -39,10 +40,11 @@ class TestInstallationNotifier(TestCase):
             user=self.user,
             prevent_token_exchange=True,
         )
+        self.rpc_user = user_service.get_user(user_id=self.user.id)
 
     @patch("sentry.utils.sentry_apps.webhooks.safe_urlopen", return_value=MockResponseInstance)
     def test_task_enqueued(self, safe_urlopen):
-        InstallationNotifier.run(install=self.install, user=self.user, action="created")
+        InstallationNotifier.run(install=self.install, user=self.rpc_user, action="created")
 
         ((args, kwargs),) = safe_urlopen.call_args_list
 
@@ -71,7 +73,7 @@ class TestInstallationNotifier(TestCase):
 
     @patch("sentry.utils.sentry_apps.webhooks.safe_urlopen", return_value=MockResponseInstance)
     def test_uninstallation_enqueued(self, safe_urlopen):
-        InstallationNotifier.run(install=self.install, user=self.user, action="deleted")
+        InstallationNotifier.run(install=self.install, user=self.rpc_user, action="deleted")
 
         ((args, kwargs),) = safe_urlopen.call_args_list
 
@@ -101,14 +103,14 @@ class TestInstallationNotifier(TestCase):
     @patch("sentry.utils.sentry_apps.webhooks.safe_urlopen")
     def test_invalid_installation_action(self, safe_urlopen):
         with pytest.raises(APIUnauthorized):
-            InstallationNotifier.run(install=self.install, user=self.user, action="updated")
+            InstallationNotifier.run(install=self.install, user=self.rpc_user, action="updated")
 
         assert not safe_urlopen.called
 
     @patch("sentry.utils.sentry_apps.webhooks.safe_urlopen", return_value=MockResponseInstance)
     def test_webhook_request_saved(self, safe_urlopen):
-        InstallationNotifier.run(install=self.install, user=self.user, action="created")
-        InstallationNotifier.run(install=self.install, user=self.user, action="deleted")
+        InstallationNotifier.run(install=self.install, user=self.rpc_user, action="created")
+        InstallationNotifier.run(install=self.install, user=self.rpc_user, action="deleted")
 
         buffer = SentryAppWebhookRequestsBuffer(self.sentry_app)
         requests = buffer.get_requests()