fix(notification-actions): allow org and team admins to modify (#53797)

src/sentry/api/endpoints/notifications/notification_actions_details.py

@@ -2,6 +2,7 @@ import logging
 
 from django.db.models import Q
 from rest_framework import status
+from rest_framework.exceptions import PermissionDenied
 from rest_framework.request import Request
 from rest_framework.response import Response
 
@@ -34,7 +35,21 @@ class NotificationActionsDetailsEndpoint(OrganizationEndpoint):
     def convert_args(self, request: Request, action_id: int, *args, **kwargs):
         parsed_args, parsed_kwargs = super().convert_args(request, *args, **kwargs)
         organization = parsed_kwargs["organization"]
+        # projects where the user has access
         accessible_projects = self.get_projects(request, organization, project_ids={-1})
+        # projects where the user has project membership
+        projects = self.get_projects(request, organization)
+
+        # team admins and regular org members don't have project:write on an org level
+        if not request.access.has_scope("project:write"):
+            # team admins will have project:write scoped to their projects, members will not
+            team_admin_has_access = all(
+                [request.access.has_project_scope(project, "project:write") for project in projects]
+            )
+            # all() returns True for empty list, so include a check for it
+            if not team_admin_has_access or not projects:
+                raise PermissionDenied
+
         try:
             # It must either have no project affiliation, or be accessible to the user...
             action = (

src/sentry/api/endpoints/notifications/notification_actions_index.py

@@ -3,6 +3,7 @@ from typing import Dict
 
 from django.db.models import Q
 from rest_framework import status
+from rest_framework.exceptions import PermissionDenied
 from rest_framework.request import Request
 from rest_framework.response import Response
 
@@ -21,9 +22,21 @@ logger = logging.getLogger(__name__)
 class NotificationActionsPermission(OrganizationPermission):
     scope_map = {
         "GET": ["org:read", "org:write", "org:admin"],
-        "POST": ["org:write", "org:admin"],
-        "PUT": ["org:write", "org:admin"],
-        "DELETE": ["org:write", "org:admin"],
+        "POST": [
+            "org:read",
+            "org:write",
+            "org:admin",
+        ],
+        "PUT": [
+            "org:read",
+            "org:write",
+            "org:admin",
+        ],
+        "DELETE": [
+            "org:read",
+            "org:write",
+            "org:admin",
+        ],
     }
 
 
@@ -68,11 +81,29 @@ class NotificationActionsIndexEndpoint(OrganizationEndpoint):
         )
 
     def post(self, request: Request, organization: Organization) -> Response:
+        # team admins and regular org members don't have project:write on an org level
+        if not request.access.has_scope("project:write"):
+            # check if user has access to create notification actions for all requested projects
+            requested_projects = request.data.get("projects", [])
+            projects = self.get_projects(request, organization)
+            project_slugs = [project.slug for project in projects]
+            missing_access_projects = set(requested_projects).difference(set(project_slugs))
+
+            if missing_access_projects:
+                raise PermissionDenied(
+                    detail="You do not have permission to create notification actions for projects "
+                    + str(list(missing_access_projects))
+                )
+            # team admins will have project:write scoped to their projects, members will not
+            team_admin_has_access = all(
+                [request.access.has_project_scope(project, "project:write") for project in projects]
+            )
+            # all() returns True for empty list, so include a check for it
+            if not team_admin_has_access or not projects:
+                raise PermissionDenied
+
         serializer = NotificationActionSerializer(
-            context={
-                "access": request.access,
-                "organization": organization,
-            },
+            context={"access": request.access, "organization": organization},
             data=request.data,
         )
         if not serializer.is_valid():

tests/sentry/api/endpoints/notifications/test_notification_actions_details.py

@@ -13,6 +13,7 @@ from sentry.models.notificationaction import (
     NotificationAction,
     NotificationActionProject,
 )
+from sentry.models.organizationmemberteam import OrganizationMemberTeam
 from sentry.silo import SiloMode
 from sentry.testutils.cases import APITestCase
 from sentry.testutils.helpers.slack import install_slack
@@ -214,7 +215,7 @@ class NotificationActionsDetailsEndpointTest(APITestCase):
 
     @patch.dict(NotificationAction._registry, {})
     @responses.activate
-    def test_post_with_slack_validation(self):
+    def test_put_with_slack_validation(self):
         class MockActionRegistration(ActionRegistration):
             pass
 
@@ -262,7 +263,7 @@ class NotificationActionsDetailsEndpointTest(APITestCase):
         assert response.data["targetIdentifier"] == channel_id
 
     @patch.dict(NotificationAction._registry, {})
-    def test_PUT_with_pagerduty_validation(self):
+    def test_put_with_pagerduty_validation(self):
         class MockActionRegistration(ActionRegistration):
             pass
 
@@ -355,6 +356,40 @@ class NotificationActionsDetailsEndpointTest(APITestCase):
         # Relation table has been updated
         assert not NotificationActionProject.objects.filter(action_id=self.notif_action.id).exists()
 
+    @patch.dict(NotificationAction._registry, {})
+    def test_put_org_member(self):
+        user = self.create_user()
+        self.create_member(organization=self.organization, user=user, teams=[self.team])
+        self.login_as(user)
+
+        data = {**self.base_data}
+        self.get_error_response(
+            self.organization.slug,
+            self.notif_action.id,
+            status_code=status.HTTP_403_FORBIDDEN,
+            method="PUT",
+            **data,
+        )
+
+    @patch.dict(NotificationAction._registry, {})
+    def test_put_org_admin(self):
+        user = self.create_user()
+        self.create_member(organization=self.organization, user=user, role="admin")
+        self.login_as(user)
+
+        self.test_put_simple()
+
+    @patch.dict(NotificationAction._registry, {})
+    def test_put_team_admin(self):
+        user = self.create_user()
+        member = self.create_member(organization=self.organization, user=user, role="member")
+        OrganizationMemberTeam.objects.create(
+            team=self.team, organizationmember=member, role="admin"
+        )
+        self.login_as(user)
+
+        self.test_put_simple()
+
     def test_delete_invalid_action(self):
         self.get_error_response(
             self.organization.slug,
@@ -381,15 +416,38 @@ class NotificationActionsDetailsEndpointTest(APITestCase):
         )
         assert not NotificationAction.objects.filter(id=self.notif_action.id).exists()
 
-    def test_delete_success_as_manager(self):
+    def test_delete_manager(self):
         user = self.create_user()
         self.create_member(user=user, organization=self.organization, role="manager")
         self.login_as(user)
-        assert NotificationAction.objects.filter(id=self.notif_action.id).exists()
-        self.get_success_response(
+
+        self.test_delete_simple()
+
+    def test_delete_org_member(self):
+        user = self.create_user()
+        self.create_member(user=user, organization=self.organization)
+        self.login_as(user)
+
+        self.get_error_response(
             self.organization.slug,
             self.notif_action.id,
-            status_code=status.HTTP_204_NO_CONTENT,
+            status_code=status.HTTP_403_FORBIDDEN,
             method="DELETE",
         )
-        assert not NotificationAction.objects.filter(id=self.notif_action.id).exists()
+
+    def test_delete_org_admin(self):
+        user = self.create_user()
+        self.create_member(user=user, organization=self.organization, role="admin")
+        self.login_as(user)
+
+        self.test_delete_simple()
+
+    def test_delete_team_admin(self):
+        user = self.create_user()
+        member = self.create_member(organization=self.organization, user=user, role="member")
+        OrganizationMemberTeam.objects.create(
+            team=self.team, organizationmember=member, role="admin"
+        )
+        self.login_as(user)
+
+        self.test_delete_simple()

tests/sentry/api/endpoints/notifications/test_notification_actions_index.py

@@ -13,6 +13,7 @@ from sentry.models.notificationaction import (
     NotificationAction,
     NotificationActionProject,
 )
+from sentry.models.organizationmemberteam import OrganizationMemberTeam
 from sentry.silo import SiloMode
 from sentry.testutils.cases import APITestCase
 from sentry.testutils.helpers.slack import install_slack
@@ -228,6 +229,21 @@ class NotificationActionsIndexEndpointTest(APITestCase):
             **data,
         )
 
+    def test_post_org_member(self):
+        user = self.create_user("hornet@hk.com")
+        self.create_member(user=user, organization=self.organization, teams=[self.team])
+        self.login_as(user)
+        data = {
+            **self.base_data,
+            "projects": [p.slug for p in self.projects],
+        }
+        self.get_error_response(
+            self.organization.slug,
+            status_code=status.HTTP_403_FORBIDDEN,
+            method="POST",
+            **data,
+        )
+
     @patch.dict(NotificationAction._registry, {})
     def test_post_raises_validation_from_registry(self):
         error_message = "oops-idea-installed"
@@ -389,3 +405,63 @@ class NotificationActionsIndexEndpointTest(APITestCase):
         # Relation table has been updated
         notif_action_projects = NotificationActionProject.objects.filter(action_id=notif_action.id)
         assert len(notif_action_projects) == len(self.projects)
+
+    @patch.dict(NotificationAction._registry, {})
+    def test_post_org_admin(self):
+        user = self.create_user()
+        self.create_member(organization=self.organization, user=user, role="admin")
+        self.login_as(user)
+
+        self.test_post_simple()
+
+    @patch.dict(NotificationAction._registry, {})
+    def test_post_team_admin__success(self):
+        user = self.create_user()
+        member = self.create_member(organization=self.organization, user=user, role="member")
+        OrganizationMemberTeam.objects.create(
+            team=self.team, organizationmember=member, role="admin"
+        )
+        self.login_as(user)
+
+        self.test_post_simple()
+
+    @patch.dict(NotificationAction._registry, {})
+    def test_post_team_admin__missing_access(self):
+        user = self.create_user()
+        member = self.create_member(organization=self.organization, user=user, role="member")
+        OrganizationMemberTeam.objects.create(
+            team=self.team, organizationmember=member, role="admin"
+        )
+        self.login_as(user)
+
+        non_admin_project = self.create_project(
+            organization=self.organization, teams=[self.create_team()]
+        )
+
+        class MockActionRegistration(ActionRegistration):
+            validate_action = MagicMock()
+
+        registration = MockActionRegistration
+        NotificationAction.register_action(
+            trigger_type=ActionTrigger.get_value(self.base_data["triggerType"]),
+            service_type=ActionService.get_value(self.base_data["serviceType"]),
+            target_type=ActionTarget.get_value(self.base_data["targetType"]),
+        )(registration)
+
+        data = {
+            **self.base_data,
+            "projects": [p.slug for p in self.projects] + [non_admin_project.slug],
+        }
+
+        assert not registration.validate_action.called
+        response = self.get_error_response(
+            self.organization.slug,
+            status_code=status.HTTP_403_FORBIDDEN,
+            method="POST",
+            **data,
+        )
+
+        assert (
+            "You do not have permission to create notification actions for projects"
+            in response.data["detail"]
+        )