chore(staff): Let staff access org details endpoint (#65138)

We override this in getsentry and replace the permission class, so I
need one PR in each repo for this one endpoint.
Added a comment referencing the override so other people don't get
confused.

getsentry changes are in
https://github.com/getsentry/getsentry/pull/12936

src/sentry/api/endpoints/organization_details.py

@@ -26,6 +26,7 @@ from sentry.api.serializers.models.organization import (
     BaseOrganizationSerializer,
     TrustedRelaySerializer,
 )
+from sentry.auth.staff import is_active_staff
 from sentry.constants import (
     ACCOUNT_RATE_LIMIT_DEFAULT,
     AI_SUGGESTED_SOLUTION,
@@ -532,6 +533,7 @@ def post_org_pending_deletion(
         send_delete_confirmation(delete_confirmation_args)
 
 
+# NOTE: We override the permission class of this endpoint in getsentry with the OrganizationDetailsPermission class
 @region_silo_endpoint
 class OrganizationDetailsEndpoint(OrganizationEndpoint):
     publish_status = {
@@ -553,9 +555,9 @@ class OrganizationDetailsEndpoint(OrganizationEndpoint):
         :param string detailed: Specify '0' to retrieve details without projects and teams.
         :auth: required
         """
-
         serializer = org_serializers.OrganizationSerializer
-        if request.access.has_scope("org:read"):
+
+        if request.access.has_scope("org:read") or is_active_staff(request):
             is_detailed = request.GET.get("detailed", "1") != "0"
 
             serializer = org_serializers.DetailedOrganizationSerializer
@@ -583,6 +585,7 @@ class OrganizationDetailsEndpoint(OrganizationEndpoint):
         """
         from sentry import features
 
+        # We don't need to check for staff here b/c the _admin portal uses another endpoint to update orgs
         if request.access.has_scope("org:admin"):
             serializer_cls = OwnerOrganizationSerializer
         else:

src/sentry/api/permissions.py

@@ -81,6 +81,11 @@ class StaffPermissionMixin:
     def has_object_permission(self, request, *args, **kwargs):
         return super().has_object_permission(request, *args, **kwargs) or is_active_staff(request)
 
+    def is_not_2fa_compliant(self, request, *args, **kwargs) -> bool:
+        return super().is_not_2fa_compliant(request, *args, **kwargs) and not is_active_staff(
+            request
+        )
+
 
 # NOTE(schew2381): This is a temporary permission that does NOT perform an OR
 # between SuperuserPermission and StaffPermission. Instead, it uses StaffPermission

tests/sentry/api/bases/test_organization.py

@@ -228,17 +228,28 @@ class OrganizationAndStaffPermissionTest(PermissionBaseTestCase):
         assert not self.has_object_perm("GET", self.org, user=user)
 
     def test_superuser(self):
-        user = self.create_user(is_superuser=True)
-        assert self.has_object_perm("GET", self.org, user=user, is_superuser=True)
+        superuser = self.create_user(is_superuser=True)
+        assert self.has_object_perm("GET", self.org, user=superuser, is_superuser=True)
 
     @mock.patch("sentry.api.permissions.is_active_staff", wraps=is_active_staff)
     def test_staff(self, mock_is_active_staff):
-        user = self.create_user(is_staff=True)
+        staff_user = self.create_user(is_staff=True)
 
-        assert self.has_object_perm("GET", self.org, user=user, is_staff=True)
+        assert self.has_object_perm("GET", self.org, user=staff_user, is_staff=True)
         # ensure we fail the scope check and call is_active_staff
         assert mock_is_active_staff.call_count == 3
 
+    @mock.patch("sentry.api.permissions.is_active_staff", wraps=is_active_staff)
+    def test_staff_passes_2FA(self, mock_is_active_staff):
+        staff_user = self.create_user(is_staff=True)
+        request = self.make_request(user=staff_user, is_staff=True)
+        permission = self.permission_cls()
+        self.org.flags.require_2fa = True
+        self.org.save()
+
+        assert not permission.is_not_2fa_compliant(request=request, organization=self.org)
+        assert mock_is_active_staff.call_count == 1
+
 
 class BaseOrganizationEndpointTest(TestCase):
     @cached_property

tests/sentry/api/endpoints/test_organization_details.py

@@ -197,15 +197,14 @@ class OrganizationDetailsTest(OrganizationDetailsTestBase):
             assert "orgRoleList" not in response.data
 
     def test_as_superuser(self):
-        self.user = self.create_user("super@example.org", is_superuser=True)
-        org = self.create_organization(owner=self.user)
-        team = self.create_team(name="appy", organization=org)
+        self.superuser = self.create_user(is_superuser=True)
+        team = self.create_team(name="appy", organization=self.organization)
 
-        self.login_as(user=self.user)
+        self.login_as(user=self.superuser, superuser=True)
         for i in range(5):
-            self.create_project(organization=org, teams=[team])
+            self.create_project(organization=self.organization, teams=[team])
 
-        response = self.get_success_response(org.slug)
+        response = self.get_success_response(self.organization.slug)
         assert len(response.data["projects"]) == 5
         assert len(response.data["teams"]) == 1
 
@@ -275,16 +274,12 @@ class OrganizationDetailsTest(OrganizationDetailsTestBase):
         assert response.data["hasAuthProvider"] is True
 
     def test_is_dynamically_sampled(self):
-        self.user = self.create_user("super@example.org", is_superuser=True)
-        org = self.create_organization(owner=self.user)
-        self.login_as(user=self.user)
-
         with self.feature({"organizations:dynamic-sampling": True}):
             with patch(
                 "sentry.dynamic_sampling.rules.base.quotas.get_blended_sample_rate",
                 return_value=0.5,
             ):
-                response = self.get_success_response(org.slug)
+                response = self.get_success_response(self.organization.slug)
                 assert response.data["isDynamicallySampled"]
 
         with self.feature({"organizations:dynamic-sampling": True}):
@@ -292,7 +287,7 @@ class OrganizationDetailsTest(OrganizationDetailsTestBase):
                 "sentry.dynamic_sampling.rules.base.quotas.get_blended_sample_rate",
                 return_value=1.0,
             ):
-                response = self.get_success_response(org.slug)
+                response = self.get_success_response(self.organization.slug)
                 assert not response.data["isDynamicallySampled"]
 
         with self.feature({"organizations:dynamic-sampling": True}):
@@ -300,7 +295,7 @@ class OrganizationDetailsTest(OrganizationDetailsTestBase):
                 "sentry.dynamic_sampling.rules.base.quotas.get_blended_sample_rate",
                 return_value=None,
             ):
-                response = self.get_success_response(org.slug)
+                response = self.get_success_response(self.organization.slug)
                 assert not response.data["isDynamicallySampled"]
 
         with self.feature({"organizations:dynamic-sampling": False}):
@@ -308,7 +303,7 @@ class OrganizationDetailsTest(OrganizationDetailsTestBase):
                 "sentry.dynamic_sampling.rules.base.quotas.get_blended_sample_rate",
                 return_value=None,
             ):
-                response = self.get_success_response(org.slug)
+                response = self.get_success_response(self.organization.slug)
                 assert not response.data["isDynamicallySampled"]
 
     def test_sensitive_fields_too_long(self):
@@ -1178,8 +1173,8 @@ class OrganizationSettings2FATest(TwoFactorAPITestCase):
         self.get_error_response(self.org_2fa.slug, status_code=401)
 
     def test_superuser_can_access_org_details(self):
-        user = self.create_user(is_superuser=True)
-        self.login_as(user, superuser=True)
+        superuser = self.create_user(is_superuser=True)
+        self.login_as(superuser, superuser=True)
         self.get_success_response(self.org_2fa.slug)