ref(hc): Remove get_orgs() (#54635)

user.get_orgs() is problematic, because the Organization objects
associated to a user via OrganizationMember can only ever be *per
region*, and not contain all organizations a user belongs to globally.

This PR attempts to use separate means of getting the same question
answered but with more specific implementations, so we can extract the
`user.get_orgs()` which is ambiguous.

For instance, `Organization.get_for_user_ids` can only mean *within a
region scope*. but `user_service.get_organizations` will include global
organizations.

---------

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

pyproject.toml

@@ -405,7 +405,6 @@ module = [
     "sentry.discover.endpoints.serializers",
     "sentry.eventstore.models",
     "sentry.features.handler",
-    "sentry.features.helpers",
     "sentry.features.manager",
     "sentry.filestore.gcs",
     "sentry.filestore.s3",

src/sentry/api/bases/sentryapps.py

@@ -16,11 +16,14 @@ from sentry.api.permissions import SentryPermission
 from sentry.auth.superuser import is_active_superuser
 from sentry.coreapi import APIError
 from sentry.middleware.stats import add_request_metric_tags
-from sentry.models import Organization
 from sentry.models.integrations.sentry_app import SentryApp
 from sentry.models.organization import OrganizationStatus
 from sentry.services.hybrid_cloud.app import RpcSentryApp, app_service
-from sentry.services.hybrid_cloud.organization import organization_service
+from sentry.services.hybrid_cloud.organization import (
+    RpcUserOrganizationContext,
+    organization_service,
+)
+from sentry.services.hybrid_cloud.user import RpcUser
 from sentry.services.hybrid_cloud.user.service import user_service
 from sentry.utils.sdk import configure_scope
 from sentry.utils.strings import to_single_line_str
@@ -77,17 +80,17 @@ class SentryAppsPermission(SentryPermission):
         "POST": ("org:write", "org:admin"),
     }
 
-    def has_object_permission(self, request: Request, view, organization):
+    def has_object_permission(self, request: Request, view, context: RpcUserOrganizationContext):
         if not hasattr(request, "user") or not request.user:
             return False
 
-        self.determine_access(request, organization)
+        self.determine_access(request, context)
 
         if is_active_superuser(request):
             return True
 
         # User must be a part of the Org they're trying to create the app in.
-        if organization not in request.user.get_orgs():
+        if context.organization.status != OrganizationStatus.ACTIVE or not context.member:
             raise Http404
 
         return ensure_scoped_permission(request, self.scope_map.get(request.method))
@@ -109,27 +112,36 @@ class SentryAppsBaseEndpoint(IntegrationPlatformEndpoint):
             raise ValidationError({"organization": to_single_line_str(error_message)})
         return organization_slug
 
-    def _get_organization_for_superuser(self, organization_slug):
-        try:
-            return Organization.objects.get(slug=organization_slug)
-        except Organization.DoesNotExist:
+    def _get_organization_for_superuser(
+        self, user: RpcUser, organization_slug: str
+    ) -> RpcUserOrganizationContext:
+        context = organization_service.get_organization_by_slug(
+            slug=organization_slug, only_visible=False, user_id=user.id
+        )
+
+        if context is None:
             error_message = f"Organization '{organization_slug}' does not exist."
             raise ValidationError({"organization": to_single_line_str(error_message)})
 
-    def _get_organization_for_user(self, user, organization_slug):
-        try:
-            return user.get_orgs().get(slug=organization_slug)
-        except Organization.DoesNotExist:
+        return context
+
+    def _get_organization_for_user(
+        self, user: RpcUser, organization_slug: str
+    ) -> RpcUserOrganizationContext:
+        context = organization_service.get_organization_by_slug(
+            slug=organization_slug, only_visible=True, user_id=user.id
+        )
+        if context is None or context.member is None:
             error_message = f"User does not belong to the '{organization_slug}' organization."
             raise PermissionDenied(to_single_line_str(error_message))
+        return context
 
-    def _get_organization(self, request: Request):
+    def _get_org_context(self, request: Request) -> RpcUserOrganizationContext:
         organization_slug = self._get_organization_slug(request)
         if is_active_superuser(request):
-            return self._get_organization_for_superuser(organization_slug)
+            return self._get_organization_for_superuser(request.user, organization_slug)
         else:
-            user = request.user
-            return self._get_organization_for_user(user, organization_slug)
+            return self._get_organization_for_user(request.user, organization_slug)
 
     def convert_args(self, request: Request, *args, **kwargs):
         """
@@ -154,9 +166,9 @@ class SentryAppsBaseEndpoint(IntegrationPlatformEndpoint):
         if not request.json_body:
             return (args, kwargs)
 
-        organization = self._get_organization(request)
-        self.check_object_permissions(request, organization)
-        kwargs["organization"] = organization
+        context = self._get_org_context(request)
+        self.check_object_permissions(request, context)
+        kwargs["organization"] = context.organization
 
         return (args, kwargs)
 

src/sentry/api/bases/user.py

@@ -1,3 +1,5 @@
+from __future__ import annotations
+
 from rest_framework.request import Request
 
 from sentry.api.base import Endpoint
@@ -6,10 +8,11 @@ from sentry.api.permissions import SentryPermission
 from sentry.auth.superuser import is_active_superuser
 from sentry.auth.system import is_system_auth
 from sentry.models import Organization, OrganizationStatus, User
+from sentry.services.hybrid_cloud.user import RpcUser
 
 
 class UserPermission(SentryPermission):
-    def has_object_permission(self, request: Request, view, user=None):
+    def has_object_permission(self, request: Request, view, user: User | RpcUser | None = None):
         if user is None:
             user = request.user
         if request.user.id == user.id:

src/sentry/api/endpoints/integrations/sentry_apps/index.py

@@ -14,6 +14,7 @@ from sentry.auth.superuser import is_active_superuser
 from sentry.constants import SentryAppStatus
 from sentry.models import SentryApp
 from sentry.sentry_apps.apps import SentryAppCreator
+from sentry.services.hybrid_cloud.user.service import user_service
 from sentry.utils import json
 
 logger = logging.getLogger(__name__)
@@ -30,11 +31,25 @@ class SentryAppsEndpoint(SentryAppsBaseEndpoint):
         elif status == "unpublished":
             queryset = SentryApp.objects.filter(status=SentryAppStatus.UNPUBLISHED)
             if not is_active_superuser(request):
-                queryset = queryset.filter(owner_id__in=[o.id for o in request.user.get_orgs()])
+                queryset = queryset.filter(
+                    owner_id__in=[
+                        o.id
+                        for o in user_service.get_organizations(
+                            user_id=request.user.id, only_visible=True
+                        )
+                    ]
+                )
         elif status == "internal":
             queryset = SentryApp.objects.filter(status=SentryAppStatus.INTERNAL)
             if not is_active_superuser(request):
-                queryset = queryset.filter(owner_id__in=[o.id for o in request.user.get_orgs()])
+                queryset = queryset.filter(
+                    owner_id__in=[
+                        o.id
+                        for o in user_service.get_organizations(
+                            user_id=request.user.id, only_visible=True
+                        )
+                    ]
+                )
         else:
             if is_active_superuser(request):
                 queryset = SentryApp.objects.all()

src/sentry/api/endpoints/user_notification_fine_tuning.py

@@ -1,3 +1,5 @@
+from typing import Any, Mapping
+
 from django.db import router, transaction
 from rest_framework import status
 from rest_framework.request import Request
@@ -13,6 +15,7 @@ from sentry.notifications.utils.legacy_mappings import (
     get_option_value_from_int,
     get_type_from_fine_tuning_key,
 )
+from sentry.services.hybrid_cloud.user.service import user_service
 from sentry.types.integrations import ExternalProviders
 
 INVALID_EMAIL_MSG = (
@@ -80,7 +83,8 @@ class UserNotificationFineTuningEndpoint(UserEndpoint):
 
         # Validate that all of the IDs are integers.
         try:
-            ids_to_update = {int(i) for i in request.data.keys()}
+            for k in request.data.keys():
+                int(k)
         except ValueError:
             return Response(
                 {"detail": "Invalid id value provided. Id values should be integers."},
@@ -89,28 +93,11 @@ class UserNotificationFineTuningEndpoint(UserEndpoint):
 
         # Make sure that the IDs we are going to update are a subset of the
         # user's list of organizations or projects.
-        parents = (
-            user.get_orgs() if notification_type == FineTuningAPIKey.DEPLOY else user.get_projects()
-        )
-        parent_ids = {parent.id for parent in parents}
-        if not ids_to_update.issubset(parent_ids):
-            bad_ids = ids_to_update - parent_ids
-            return Response(
-                {
-                    "detail": "At least one of the requested projects is not \
-                    available to this user, because the user does not belong \
-                    to the necessary teams. (ids of unavailable projects: %s)"
-                    % bad_ids
-                },
-                status=status.HTTP_403_FORBIDDEN,
-            )
 
         if notification_type == FineTuningAPIKey.EMAIL:
             return self._handle_put_emails(user, request.data)
 
-        return self._handle_put_notification_settings(
-            user, notification_type, parents, request.data
-        )
+        return self._handle_put_notification_settings(user, notification_type, request.data)
 
     @staticmethod
     def _handle_put_reports(user, data):
@@ -122,7 +109,7 @@ class UserNotificationFineTuningEndpoint(UserEndpoint):
         value = set(user_option.value or [])
 
         # The set of IDs of the organizations of which the user is a member.
-        org_ids = {organization.id for organization in user.get_orgs()}
+        org_ids = {o.id for o in user_service.get_organizations(user_id=user.id, only_visible=True)}
         for org_id, enabled in data.items():
             org_id = int(org_id)
             # We want "0" to be falsey
@@ -167,26 +154,26 @@ class UserNotificationFineTuningEndpoint(UserEndpoint):
         return Response(status=status.HTTP_204_NO_CONTENT)
 
     @staticmethod
-    def _handle_put_notification_settings(user, notification_type: FineTuningAPIKey, parents, data):
+    def _handle_put_notification_settings(
+        user, notification_type: FineTuningAPIKey, data: Mapping[str, Any]
+    ):
         with transaction.atomic(using=router.db_for_write(NotificationSetting)):
-            for parent in parents:
-                # We fetched every available parent but only care about the ones in `request.data`.
-                if str(parent.id) not in data:
-                    continue
+            for setting_obj_id_str, value_str in data.items():
+                setting_obj_id_int = int(setting_obj_id_str)
 
                 # This partitioning always does the same thing because notification_type stays constant.
                 project_option, organization_option = (
-                    (None, parent)
+                    (None, setting_obj_id_int)
                     if notification_type == FineTuningAPIKey.DEPLOY
-                    else (parent, None)
+                    else (setting_obj_id_int, None)
                 )
 
                 type = get_type_from_fine_tuning_key(notification_type)
-                value = int(data[str(parent.id)])
+                value_int = int(value_str)
                 NotificationSetting.objects.update_settings(
                     ExternalProviders.EMAIL,
                     type,
-                    get_option_value_from_int(type, value),
+                    get_option_value_from_int(type, value_int),
                     user_id=user.id,
                     project=project_option,
                     organization=organization_option,

src/sentry/api/endpoints/user_organizations.py

@@ -1,17 +1,45 @@
+from __future__ import annotations
+
 from django.db.models import Q
 from rest_framework.request import Request
 from rest_framework.response import Response
+from typing_extensions import override
 
-from sentry.api.base import control_silo_endpoint
-from sentry.api.bases.user import UserEndpoint
+from sentry.api.base import Endpoint, region_silo_endpoint
+from sentry.api.bases.user import UserPermission
+from sentry.api.exceptions import ResourceDoesNotExist
 from sentry.api.paginator import OffsetPaginator
 from sentry.api.serializers import serialize
+from sentry.models import Organization, User
+from sentry.services.hybrid_cloud.user import RpcUser
+from sentry.services.hybrid_cloud.user.service import user_service
+
+
+@region_silo_endpoint
+class UserOrganizationsEndpoint(Endpoint):
+    permission_classes = (UserPermission,)
+
+    @override
+    def convert_args(self, request: Request, user_id: str | None = None, *args, **kwargs):
+        user: RpcUser | User | None = None
+
+        if user_id == "me":
+            if not request.user.is_authenticated:
+                raise ResourceDoesNotExist
+            user = request.user
+        elif user_id is not None:
+            user = user_service.get_user(user_id=int(user_id))
+
+        if not user:
+            raise ResourceDoesNotExist
+
+        self.check_object_permissions(request, user)
 
+        kwargs["user"] = user
+        return args, kwargs
 
-@control_silo_endpoint
-class UserOrganizationsEndpoint(UserEndpoint):
-    def get(self, request: Request, user) -> Response:
-        queryset = user.get_orgs()
+    def get(self, request: Request, user: RpcUser) -> Response:
+        queryset = Organization.objects.get_for_user_ids({user.id})
 
         query = request.GET.get("query")
         if query:

src/sentry/api/serializers/models/sentry_app.py

@@ -82,9 +82,7 @@ class SentryAppSerializer(Serializer):
         user_org_ids = attrs["user_org_ids"]
 
         if owner:
-            if (env.request and is_active_superuser(env.request)) or (
-                hasattr(user, "get_orgs") and owner.id in user_org_ids
-            ):
+            if (env.request and is_active_superuser(env.request)) or owner.id in user_org_ids:
                 client_secret = (
                     obj.application.client_secret if obj.show_auth_info(access) else MASKED_VALUE
                 )

src/sentry/features/helpers.py

@@ -1,4 +1,4 @@
-from typing import TYPE_CHECKING, Any, Callable, Optional, Sequence
+from typing import TYPE_CHECKING, Any, Callable, Sequence
 
 from rest_framework.request import Request
 from rest_framework.response import Response
@@ -18,16 +18,11 @@ def any_organization_has_feature(
     return any([features.has(feature, organization, **kwargs) for organization in organizations])
 
 
-def requires_feature(
-    feature: str, any_org: Optional[bool] = None
-) -> Callable[[EndpointFunc], EndpointFunc]:
+def requires_feature(feature: str) -> Callable[[EndpointFunc], EndpointFunc]:
     """
     Require a feature flag to access an endpoint.
 
-    If ``any_org`` is ``True``, this will check all of the request User's
-    Organizations for the flag. If any are flagged in, the endpoint is accessible.
-
-    Without ``any_org=True``, the endpoint must resolve an Organization via
+    The endpoint must resolve an Organization via
     ``convert_args`` (and therefor be in ``kwargs``). The resolved Org must have
     the passed feature.
 
@@ -41,24 +36,13 @@ def requires_feature(
 
     def decorator(func: EndpointFunc) -> EndpointFunc:
         def wrapped(self: Any, request: Request, *args: Any, **kwargs: Any) -> Response:
-            # The endpoint is accessible if any of the User's Orgs have the feature
-            # flag enabled.
-            if any_org:
-                if not any_organization_has_feature(
-                    feature, request.user.get_orgs(), actor=request.user
-                ):
-                    return Response(status=404)
-
-                return func(self, request, *args, **kwargs)
-            # The Org in scope for the request must have the feature flag enabled.
-            else:
-                if "organization" not in kwargs:
-                    return Response(status=404)
-
-                if not features.has(feature, kwargs["organization"], actor=request.user):
-                    return Response(status=404)
-
-                return func(self, request, *args, **kwargs)
+            if "organization" not in kwargs:
+                return Response(status=404)
+
+            if not features.has(feature, kwargs["organization"], actor=request.user):
+                return Response(status=404)
+
+            return func(self, request, *args, **kwargs)
 
         return wrapped
 

src/sentry/models/user.py

@@ -356,16 +356,6 @@ class User(BaseModel, AbstractBaseUser):
         if request is not None:
             request.session["_nonce"] = self.session_nonce
 
-    def get_orgs(self):
-        from sentry.models import Organization
-
-        return Organization.objects.get_for_user_ids({self.id})
-
-    def get_projects(self):
-        from sentry.models import Project
-
-        return Project.objects.get_for_user_ids({self.id})
-
     def get_orgs_require_2fa(self):
         from sentry.models import Organization, OrganizationStatus
 

tests/sentry/api/endpoints/test_user_notification_fine_tuning.py

@@ -60,7 +60,9 @@ class UserNotificationFineTuningTest(UserNotificationFineTuningTestBase):
     method = "put"
 
     def test_update_invalid_project(self):
-        self.get_error_response("me", "alerts", status_code=403, **{"123": 1})
+        # We do not validate project / organization ids!  Due to the silo split we accept these at face value rather than fan out
+        # across all silos to validate them.
+        self.get_response("me", "alerts", **{"123": 1})
 
     def test_invalid_id_value(self):
         self.get_error_response("me", "alerts", status_code=400, **{"nope": 1})
@@ -271,9 +273,6 @@ class UserNotificationFineTuningTest(UserNotificationFineTuningTestBase):
             user=self.user, organization_id=new_org.id, key="reports"
         ).exists()
 
-        data = {str(new_project.id): 1}
-        self.get_error_response("me", "alerts", status_code=403, **data)
-
         value = NotificationSetting.objects.get_settings(
             ExternalProviders.EMAIL,
             NotificationSettingTypes.ISSUE_ALERTS,