Browse Source

fix(dashboards): stricter permission check when dashboards cover all/my projects (#78615)

When Open Membership is disabled, it is expected to have more granular
access to certain objects that are associated with projects. First
version of project-level access on dashboards was implemented in
https://github.com/getsentry/sentry/pull/70228

However, dashboards that cover "All Projects" or "My Projects" do not
have explicit project ids, therefore we need to do a different check.
After this PR, we will allow access to such dashboards only in these
cases:
* if Open Membership is enabled;
* if actor is a Manager/Owner (having `org:write` scope);
* if actor is the original creator of a dashboard.

---------

Co-authored-by: George Gritsouk <989898+gggritso@users.noreply.github.com>
Alexander Tarasov 5 months ago
parent
commit
e282378456

+ 19 - 3
src/sentry/api/endpoints/organization_dashboards.py

@@ -50,9 +50,25 @@ class OrganizationDashboardsPermission(OrganizationPermission):
             return super().has_object_permission(request, view, obj)
 
         if isinstance(obj, Dashboard):
-            for project in obj.projects.all():
-                if not request.access.has_project_access(project):
-                    return False
+            # 1. Dashboard contains certain projects
+            if obj.projects.exists():
+                return request.access.has_projects_access(obj.projects.all())
+
+            # 2. Dashboard covers all projects or all my projects
+
+            # allow when Open Membership
+            if obj.organization.flags.allow_joinleave:
+                return True
+
+            # allow for Managers and Owners
+            if request.access.has_scope("org:write"):
+                return True
+
+            # allow for creator
+            if request.user.id == obj.created_by_id:
+                return True
+
+            return False
 
         return True
 

+ 59 - 0
tests/sentry/api/endpoints/test_organization_dashboard_details.py

@@ -384,6 +384,65 @@ class OrganizationDashboardDetailsDeleteTest(OrganizationDashboardDetailsTestCas
         assert response.status_code == 403
         assert response.data == {"detail": "You do not have permission to perform this action."}
 
+    def test_disallow_delete_all_projects_dashboard_when_no_open_membership(self):
+        # disable Open Membership
+        self.organization.flags.allow_joinleave = False
+        self.organization.save()
+
+        dashboard = Dashboard.objects.create(
+            title="Dashboard For All Projects",
+            created_by_id=self.user.id,
+            organization=self.organization,
+            filters={"all_projects": True},
+        )
+
+        # user has no access to all the projects
+        user_no_team = self.create_user(is_superuser=False)
+        self.create_member(
+            user=user_no_team, organization=self.organization, role="member", teams=[]
+        )
+        self.login_as(user_no_team)
+
+        response = self.do_request("delete", self.url(dashboard.id))
+        assert response.status_code == 403
+        assert response.data == {"detail": "You do not have permission to perform this action."}
+
+        # owner is allowed to delete
+        self.owner = self.create_member(
+            user=self.create_user(), organization=self.organization, role="owner"
+        )
+        self.login_as(self.owner)
+        response = self.do_request("delete", self.url(dashboard.id))
+        assert response.status_code == 204
+
+    def test_disallow_delete_my_projects_dashboard_when_no_open_membership(self):
+        # disable Open Membership
+        self.organization.flags.allow_joinleave = False
+        self.organization.save()
+
+        dashboard = Dashboard.objects.create(
+            title="Dashboard For My Projects",
+            created_by_id=self.user.id,
+            organization=self.organization,
+            # no 'filter' field means the dashboard covers all available projects
+        )
+
+        # user has no access to all the projects
+        user_no_team = self.create_user(is_superuser=False)
+        self.create_member(
+            user=user_no_team, organization=self.organization, role="member", teams=[]
+        )
+        self.login_as(user_no_team)
+
+        response = self.do_request("delete", self.url(dashboard.id))
+        assert response.status_code == 403
+        assert response.data == {"detail": "You do not have permission to perform this action."}
+
+        # creator is allowed to delete
+        self.login_as(self.user)
+        response = self.do_request("delete", self.url(dashboard.id))
+        assert response.status_code == 204
+
     def test_dashboard_does_not_exist(self):
         response = self.do_request("delete", self.url(1234567890))
         assert response.status_code == 404