ref(hc): OrganizationIntegration and Organization related api fixes for hybrid cloud (#51965)

Split out some subclasses and enable more stable=True tests.

---------

Co-authored-by: Mark Story <mark@mark-story.com>
Co-authored-by: getsantry[bot] <66042841+getsantry[bot]@users.noreply.github.com>

pyproject.toml

@@ -149,9 +149,7 @@ module = [
     "sentry.api.bases.group",
     "sentry.api.bases.incident",
     "sentry.api.bases.integration",
-    "sentry.api.bases.organization",
     "sentry.api.bases.organization_events",
-    "sentry.api.bases.organization_integrations",
     "sentry.api.bases.organization_request_change",
     "sentry.api.bases.organizationmember",
     "sentry.api.bases.project",

src/sentry/api/base.py

@@ -586,7 +586,7 @@ class ReleaseAnalyticsMixin:
         )
 
 
-def resolve_region(request: Request):
+def resolve_region(request: Request) -> Optional[str]:
     subdomain = getattr(request, "subdomain", None)
     if subdomain is None:
         return None

src/sentry/api/bases/integration.py

@@ -2,15 +2,14 @@ from __future__ import annotations
 
 import sys
 import traceback
-from typing import Any, Mapping
+from typing import Any, Optional
 
 from rest_framework.request import Request
 from rest_framework.response import Response
-from sentry_sdk import Scope
 
 from sentry.utils.sdk import capture_exception
 
-from .organization import OrganizationEndpoint, OrganizationPermission
+from .organization import ControlSiloOrganizationEndpoint, OrganizationEndpoint
 
 # This GET scope map is ideally a public endpoint but for now
 # we are allowing for anyone who has member permissions or above.
@@ -26,21 +25,44 @@ PARANOID_GET = (
 )
 
 
-class IntegrationEndpoint(OrganizationEndpoint):
-    permission_classes = (OrganizationPermission,)
+def _handle_exception(
+    exc: Exception,
+) -> Optional[Response]:
+    if hasattr(exc, "code") and exc.code == 503:
+        sys.stderr.write(traceback.format_exc())
+        event_id = capture_exception(exc)
+        context = {"detail": str(exc), "errorId": event_id}
+        response = Response(context, status=503)
+        response.exception = True
+        return response
+    return None
+
+
+class IntegrationEndpoint(ControlSiloOrganizationEndpoint):
+    """
+    Baseclass for integration endpoints in control silo that need integration exception handling
+    """
+
+    def handle_exception(
+        self,
+        request: Request,
+        exc: Exception,
+        *args: Any,
+        **kwds: Any,
+    ) -> Response:
+        return _handle_exception(exc) or super().handle_exception(request, exc, *args, **kwds)
+
+
+class RegionIntegrationEndpoint(OrganizationEndpoint):
+    """
+    Baseclass for integration endpoints in region silo that need integration exception handling
+    """
 
     def handle_exception(
         self,
         request: Request,
         exc: Exception,
-        handler_context: Mapping[str, Any] | None = None,
-        scope: Scope | None = None,
+        *args: Any,
+        **kwds: Any,
     ) -> Response:
-        if hasattr(exc, "code") and exc.code == 503:
-            sys.stderr.write(traceback.format_exc())
-            event_id = capture_exception(exc)
-            context = {"detail": str(exc), "errorId": event_id}
-            response = Response(context, status=503)
-            response.exception = True
-            return response
-        return super().handle_exception(request, exc, handler_context, scope)
+        return _handle_exception(exc) or super().handle_exception(request, exc, *args, **kwds)

src/sentry/api/bases/organization.py

@@ -1,6 +1,6 @@
 from __future__ import annotations
 
-from typing import Any, Optional, Set
+from typing import Any, Optional, Set, cast
 
 import sentry_sdk
 from django.core.cache import cache
@@ -24,7 +24,11 @@ from sentry.models.apikey import is_api_key_auth
 from sentry.models.environment import Environment
 from sentry.models.orgauthtoken import is_org_auth_token_auth
 from sentry.models.release import Release
-from sentry.services.hybrid_cloud.organization import RpcOrganization, RpcUserOrganizationContext
+from sentry.services.hybrid_cloud.organization import (
+    RpcOrganization,
+    RpcUserOrganizationContext,
+    organization_service,
+)
 from sentry.utils import auth
 from sentry.utils.hashlib import hash_values
 from sentry.utils.numbers import format_grouped_length
@@ -46,28 +50,36 @@ class OrganizationPermission(SentryPermission):
     def is_not_2fa_compliant(
         self, request: Request, organization: RpcOrganization | Organization
     ) -> bool:
-        return (
-            organization.flags.require_2fa
-            and not request.user.has_2fa()
-            and not is_active_superuser(request)
-        )
+        if not organization.flags.require_2fa:
+            return False
+
+        if request.user.has_2fa():  # type: ignore
+            return False
+
+        if is_active_superuser(request):
+            return False
+
+        return True
 
     def needs_sso(self, request: Request, organization: Organization | RpcOrganization) -> bool:
         # XXX(dcramer): this is very similar to the server-rendered views
         # logic for checking valid SSO
         if not request.access.requires_sso:
             return False
-        if not auth.has_completed_sso(request, organization.id):
+        if not auth.has_completed_sso(request, cast(int, organization.id)):
             return True
         if not request.access.sso_is_valid:
             return True
         return False
 
     def has_object_permission(
-        self, request: Request, view: object, organization: Organization
+        self,
+        request: Request,
+        view: object,
+        organization: Organization | RpcOrganization | RpcUserOrganizationContext,
     ) -> bool:
         self.determine_access(request, organization)
-        allowed_scopes = set(self.scope_map.get(request.method, []))
+        allowed_scopes = set(self.scope_map.get(request.method or "", []))
         return any(request.access.has_scope(s) for s in allowed_scopes)
 
     def is_member_disabled_from_limit(
@@ -184,6 +196,47 @@ class OrgAuthTokenPermission(OrganizationPermission):
     }
 
 
+class ControlSiloOrganizationEndpoint(Endpoint):
+    """
+    A base class for endpoints that use an organization scoping but lives in the control silo
+    """
+
+    permission_classes = (OrganizationPermission,)
+
+    def convert_args(
+        self, request: Request, organization_slug: str | None = None, *args: Any, **kwargs: Any
+    ) -> tuple[tuple[Any, ...], dict[str, Any]]:
+        if resolve_region(request) is None:
+            subdomain = getattr(request, "subdomain", None)
+            if subdomain is not None and subdomain != organization_slug:
+                raise ResourceDoesNotExist
+
+        if not organization_slug:
+            raise ResourceDoesNotExist
+
+        organization_context = organization_service.get_organization_by_slug(
+            slug=organization_slug, only_visible=False, user_id=request.user.id  # type: ignore
+        )
+        if organization_context is None:
+            raise ResourceDoesNotExist
+
+        with sentry_sdk.start_span(op="check_object_permissions_on_organization"):
+            self.check_object_permissions(request, organization_context)
+
+        bind_organization_context(organization_context.organization)
+
+        # Track the 'active' organization when the request came from
+        # a cookie based agent (react app)
+        # Never track any org (regardless of whether the user does or doesn't have
+        # membership in that org) when the user is in active superuser mode
+        if request.auth is None and request.user and not is_active_superuser(request):
+            auth.set_active_org(request, organization_context.organization.slug)
+
+        kwargs["organization_context"] = organization_context
+        kwargs["organization"] = organization_context.organization
+        return (args, kwargs)
+
+
 class OrganizationEndpoint(Endpoint):
     permission_classes = (OrganizationPermission,)
 
@@ -233,7 +286,7 @@ class OrganizationEndpoint(Endpoint):
                 if not project_ids:
                     return []
             else:
-                project_ids = self.get_requested_project_ids_unchecked(request)
+                project_ids = self.get_requested_project_ids_unchecked(request)  # type: ignore
 
         return self._get_projects_by_id(
             project_ids,
@@ -278,10 +331,10 @@ class OrganizationEndpoint(Endpoint):
                     or include_all_accessible
                 ):
                     span.set_tag("mode", "has_project_access")
-                    func = request.access.has_project_access
+                    func = request.access.has_project_access  # type: ignore
                 else:
                     span.set_tag("mode", "has_project_membership")
-                    func = request.access.has_project_membership
+                    func = request.access.has_project_membership  # type: ignore
                 projects = [p for p in qs if func(p)]
 
         project_ids = {p.id for p in projects}
@@ -408,7 +461,7 @@ class OrganizationEndpoint(Endpoint):
 
         bind_organization_context(organization)
 
-        request._request.organization = organization
+        request._request.organization = organization  # type: ignore
 
         # Track the 'active' organization when the request came from
         # a cookie based agent (react app)
@@ -437,16 +490,18 @@ class OrganizationReleasesBaseEndpoint(OrganizationEndpoint):
         """
         has_valid_api_key = False
         if is_api_key_auth(request.auth):
-            if request.auth.organization_id != organization.id:
+            if request.auth.organization_id != organization.id:  # type: ignore
                 return []
-            has_valid_api_key = request.auth.has_scope(
+            has_valid_api_key = request.auth.has_scope(  # type: ignore
                 "project:releases"
-            ) or request.auth.has_scope("project:write")
+            ) or request.auth.has_scope(  # type: ignore
+                "project:write"
+            )
 
         if is_org_auth_token_auth(request.auth):
-            if request.auth.organization_id != organization.id:
+            if request.auth.organization_id != organization.id:  # type: ignore
                 return []
-            has_valid_api_key = request.auth.has_scope("org:ci")
+            has_valid_api_key = request.auth.has_scope("org:ci")  # type: ignore
 
         if not (
             has_valid_api_key or (getattr(request, "user", None) and request.user.is_authenticated)
@@ -482,9 +537,9 @@ class OrganizationReleasesBaseEndpoint(OrganizationEndpoint):
         if getattr(request, "user", None) and request.user.id:
             actor_id = "user:%s" % request.user.id
         if getattr(request, "auth", None) and getattr(request.auth, "id", None):
-            actor_id = "apikey:%s" % request.auth.id
+            actor_id = "apikey:%s" % request.auth.id  # type: ignore
         elif getattr(request, "auth", None) and getattr(request.auth, "entity_id", None):
-            actor_id = "apikey:%s" % request.auth.entity_id
+            actor_id = "apikey:%s" % request.auth.entity_id  # type: ignore
         if actor_id is not None:
             requested_project_ids = project_ids
             if requested_project_ids is None:

src/sentry/api/bases/organization_integrations.py

@@ -1,7 +1,11 @@
+from __future__ import annotations
+
+from typing import Any, Dict, Tuple
+
 from django.http import Http404
 from rest_framework.request import Request
 
-from sentry.api.bases.integration import IntegrationEndpoint
+from sentry.api.bases.integration import IntegrationEndpoint, RegionIntegrationEndpoint
 from sentry.api.bases.organization import OrganizationIntegrationsPermission
 from sentry.models import Integration, OrganizationIntegration
 from sentry.services.hybrid_cloud.integration import (
@@ -20,21 +24,6 @@ class OrganizationIntegrationBaseEndpoint(IntegrationEndpoint):
 
     permission_classes = (OrganizationIntegrationsPermission,)
 
-    def convert_args(self, request: Request, organization_slug, integration_id, *args, **kwargs):
-        args, kwargs = super().convert_args(request, organization_slug, *args, **kwargs)
-
-        self.validate_integration_id(integration_id)
-
-        kwargs["integration_id"] = integration_id
-        return (args, kwargs)
-
-    @staticmethod
-    def validate_integration_id(integration_id):
-        try:
-            return int(integration_id)
-        except ValueError:
-            raise Http404
-
     @staticmethod
     def get_organization_integration(
         organization_id: int, integration_id: int
@@ -73,7 +62,7 @@ class OrganizationIntegrationBaseEndpoint(IntegrationEndpoint):
             raise Http404
 
 
-class RegionOrganizationIntegrationBaseEndpoint(OrganizationIntegrationBaseEndpoint):
+class RegionOrganizationIntegrationBaseEndpoint(RegionIntegrationEndpoint):
     """
     OrganizationIntegrationBaseEndpoints expect both Integration and
     OrganizationIntegration DB entries to exist for a given organization and
@@ -82,6 +71,26 @@ class RegionOrganizationIntegrationBaseEndpoint(OrganizationIntegrationBaseEndpo
 
     permission_classes = (OrganizationIntegrationsPermission,)
 
+    def convert_args(
+        self,
+        request: Request,
+        organization_slug: str | None = None,
+        integration_id: str | None = None,
+        *args: Any,
+        **kwargs: Any,
+    ) -> Tuple[Tuple[Any, ...], Dict[str, Any]]:
+        args, kwargs = super().convert_args(request, organization_slug, *args, **kwargs)
+
+        kwargs["integration_id"] = self.validate_integration_id(integration_id or "")
+        return args, kwargs
+
+    @staticmethod
+    def validate_integration_id(integration_id: str) -> int:
+        try:
+            return int(integration_id)
+        except ValueError:
+            raise Http404
+
     @staticmethod
     def get_organization_integration(
         organization_id: int, integration_id: int

src/sentry/api/endpoints/integrations/organization_integrations/details.py

@@ -1,5 +1,7 @@
 from __future__ import annotations
 
+from typing import Any
+
 from django.db import router, transaction
 from django.http import Http404
 from django.views.decorators.cache import never_cache
@@ -16,6 +18,7 @@ from sentry.constants import ObjectStatus
 from sentry.features.helpers import requires_feature
 from sentry.models import OrganizationIntegration, ScheduledDeletion
 from sentry.services.hybrid_cloud.integration import integration_service
+from sentry.services.hybrid_cloud.organization import RpcUserOrganizationContext
 from sentry.shared_integrations.exceptions import IntegrationError
 from sentry.utils.audit import create_audit_entry
 from sentry.web.decorators import set_referrer_policy
@@ -30,8 +33,16 @@ class IntegrationSerializer(serializers.Serializer):
 class OrganizationIntegrationDetailsEndpoint(OrganizationIntegrationBaseEndpoint):
     @set_referrer_policy("strict-origin-when-cross-origin")
     @never_cache
-    def get(self, request: Request, organization, integration_id) -> Response:
-        org_integration = self.get_organization_integration(organization.id, integration_id)
+    def get(
+        self,
+        request: Request,
+        organization_context: RpcUserOrganizationContext,
+        integration_id: int,
+        **kwds: Any,
+    ) -> Response:
+        org_integration = self.get_organization_integration(
+            organization_context.organization.id, integration_id
+        )
 
         return self.respond(
             serialize(
@@ -42,8 +53,14 @@ class OrganizationIntegrationDetailsEndpoint(OrganizationIntegrationBaseEndpoint
     @requires_feature("organizations:integrations-custom-scm")
     @set_referrer_policy("strict-origin-when-cross-origin")
     @never_cache
-    def put(self, request: Request, organization, integration_id) -> Response:
-        integration = self.get_integration(organization.id, integration_id)
+    def put(
+        self,
+        request: Request,
+        organization_context: RpcUserOrganizationContext,
+        integration_id: int,
+        **kwds: Any,
+    ) -> Response:
+        integration = self.get_integration(organization_context.organization.id, integration_id)
 
         if integration.provider != "custom_scm":
             return self.respond({"detail": "Invalid action for this integration"}, status=400)
@@ -62,7 +79,9 @@ class OrganizationIntegrationDetailsEndpoint(OrganizationIntegrationBaseEndpoint
                 update_kwargs["metadata"] = metadata
             integration_service.update_integration(integration_id=integration.id, **update_kwargs)
 
-            org_integration = self.get_organization_integration(organization.id, integration_id)
+            org_integration = self.get_organization_integration(
+                organization_context.organization.id, integration_id
+            )
 
             return self.respond(
                 serialize(
@@ -75,20 +94,26 @@ class OrganizationIntegrationDetailsEndpoint(OrganizationIntegrationBaseEndpoint
 
     @set_referrer_policy("strict-origin-when-cross-origin")
     @never_cache
-    def delete(self, request: Request, organization, integration_id) -> Response:
+    def delete(
+        self,
+        request: Request,
+        organization_context: RpcUserOrganizationContext,
+        integration_id: int,
+        **kwds: Any,
+    ) -> Response:
         # Removing the integration removes the organization
         # integrations and all linked issues.
 
         # NOTE(hybrid-cloud): Deletions require the ORM object, not API versions
         org_integration: OrganizationIntegration | None = OrganizationIntegration.objects.filter(
-            integration_id=integration_id, organization_id=organization.id
+            integration_id=integration_id, organization_id=organization_context.organization.id
         ).first()
         if not org_integration:
             raise Http404
-        integration = self.get_integration(organization.id, integration_id)
+        integration = self.get_integration(organization_context.organization.id, integration_id)
         # do any integration specific deleting steps
         integration_service.get_installation(
-            integration=integration, organization_id=organization.id
+            integration=integration, organization_id=organization_context.organization.id
         ).uninstall()
 
         with transaction.atomic(using=router.db_for_write(OrganizationIntegration)):
@@ -100,7 +125,7 @@ class OrganizationIntegrationDetailsEndpoint(OrganizationIntegrationBaseEndpoint
                 ScheduledDeletion.schedule(org_integration, days=0, actor=request.user)
                 create_audit_entry(
                     request=request,
-                    organization=organization,
+                    organization_id=organization_context.organization.id,
                     target_object=integration.id,
                     event=audit_log.get_event_id("INTEGRATION_REMOVE"),
                     data={"provider": integration.provider, "name": integration.name},
@@ -110,10 +135,16 @@ class OrganizationIntegrationDetailsEndpoint(OrganizationIntegrationBaseEndpoint
 
     @set_referrer_policy("strict-origin-when-cross-origin")
     @never_cache
-    def post(self, request: Request, organization, integration_id) -> Response:
-        integration = self.get_integration(organization.id, integration_id)
+    def post(
+        self,
+        request: Request,
+        organization_context: RpcUserOrganizationContext,
+        integration_id: int,
+        **kwds: Any,
+    ) -> Response:
+        integration = self.get_integration(organization_context.organization.id, integration_id)
         installation = integration_service.get_installation(
-            integration=integration, organization_id=organization.id
+            integration=integration, organization_id=organization_context.organization.id
         )
         try:
             installation.update_organization_config(request.data)

src/sentry/api/endpoints/organization_integration_issues.py

@@ -1,15 +1,24 @@
+from typing import Any
+
 from rest_framework.request import Request
 from rest_framework.response import Response
 
 from sentry.api.base import region_silo_endpoint
 from sentry.api.bases.organization_integrations import RegionOrganizationIntegrationBaseEndpoint
 from sentry.integrations.mixins import IssueSyncMixin
+from sentry.models import Organization
 from sentry.services.hybrid_cloud.integration import integration_service
 
 
 @region_silo_endpoint
 class OrganizationIntegrationIssuesEndpoint(RegionOrganizationIntegrationBaseEndpoint):
-    def put(self, request: Request, organization, integration_id) -> Response:
+    def put(
+        self,
+        request: Request,
+        organization: Organization,
+        integration_id: int,
+        **kwds: Any,
+    ) -> Response:
         """
         Migrate plugin linked issues to integration linked issues
         `````````````````````````````````````````````````````````

src/sentry/api/endpoints/organization_integration_repos.py

@@ -1,4 +1,4 @@
-from typing import Optional, TypedDict
+from typing import Any, Optional, TypedDict
 
 from rest_framework.request import Request
 from rest_framework.response import Response
@@ -8,6 +8,7 @@ from sentry.api.bases.organization_integrations import RegionOrganizationIntegra
 from sentry.auth.exceptions import IdentityNotValid
 from sentry.constants import ObjectStatus
 from sentry.integrations.mixins import RepositoryMixin
+from sentry.models import Organization
 from sentry.services.hybrid_cloud.integration import integration_service
 from sentry.shared_integrations.exceptions import IntegrationError
 
@@ -20,7 +21,13 @@ class IntegrationRepository(TypedDict):
 
 @region_silo_endpoint
 class OrganizationIntegrationReposEndpoint(RegionOrganizationIntegrationBaseEndpoint):
-    def get(self, request: Request, organization, integration_id) -> Response:
+    def get(
+        self,
+        request: Request,
+        organization: Organization,
+        integration_id: int,
+        **kwds: Any,
+    ) -> Response:
         """
         Get the list of repositories available in an integration
         ````````````````````````````````````````````````````````

src/sentry/api/endpoints/organization_integration_serverless_functions.py

@@ -1,3 +1,5 @@
+from typing import Any
+
 from rest_framework import serializers
 from rest_framework.request import Request
 from rest_framework.response import Response
@@ -6,6 +8,7 @@ from sentry.api.base import region_silo_endpoint
 from sentry.api.bases.organization_integrations import RegionOrganizationIntegrationBaseEndpoint
 from sentry.api.serializers.rest_framework.base import CamelSnakeSerializer
 from sentry.integrations.mixins import ServerlessMixin
+from sentry.models import Organization
 from sentry.services.hybrid_cloud.integration import integration_service
 from sentry.shared_integrations.exceptions import IntegrationError
 
@@ -19,7 +22,13 @@ class ServerlessActionSerializer(CamelSnakeSerializer):
 
 @region_silo_endpoint
 class OrganizationIntegrationServerlessFunctionsEndpoint(RegionOrganizationIntegrationBaseEndpoint):
-    def get(self, request: Request, organization, integration_id) -> Response:
+    def get(
+        self,
+        request: Request,
+        organization: Organization,
+        integration_id: int,
+        **kwds: Any,
+    ) -> Response:
         """
         Get the list of repository project path configs in an integration
         """
@@ -39,7 +48,13 @@ class OrganizationIntegrationServerlessFunctionsEndpoint(RegionOrganizationInteg
 
         return self.respond(serverless_functions)
 
-    def post(self, request: Request, organization, integration_id) -> Response:
+    def post(
+        self,
+        request: Request,
+        organization: Organization,
+        integration_id: int,
+        **kwds: Any,
+    ) -> Response:
         integration = self.get_integration(organization.id, integration_id)
         install = integration_service.get_installation(
             integration=integration, organization_id=organization.id

src/sentry/api/endpoints/team_details.py

@@ -11,7 +11,7 @@ from sentry.api.decorators import sudo_required
 from sentry.api.serializers import serialize
 from sentry.api.serializers.models.team import TeamSerializer as ModelTeamSerializer
 from sentry.api.serializers.rest_framework.base import CamelSnakeModelSerializer
-from sentry.models import ScheduledDeletion, Team, TeamStatus
+from sentry.models import RegionScheduledDeletion, Team, TeamStatus
 
 
 class TeamSerializer(CamelSnakeModelSerializer):
@@ -147,7 +147,7 @@ class TeamDetailsEndpoint(TeamEndpoint):
             slug=new_slug, status=TeamStatus.PENDING_DELETION
         )
         if updated:
-            scheduled = ScheduledDeletion.schedule(team, days=0, actor=request.user)
+            scheduled = RegionScheduledDeletion.schedule(team, days=0, actor=request.user)
             self.create_audit_entry(
                 request=request,
                 organization=team.organization,