From fbeb7f1bdcb60824bb06903678f7285357be274a Mon Sep 17 00:00:00 2001 From: Alexander Tarasov Date: Fri, 4 Oct 2024 17:11:25 +0200 Subject: [PATCH 1/2] fix(dashboards): stricter permission check when dashboards are about all/my projects --- .../api/endpoints/organization_dashboards.py | 25 +++++++- .../test_organization_dashboard_details.py | 59 +++++++++++++++++++ 2 files changed, 81 insertions(+), 3 deletions(-) diff --git a/src/sentry/api/endpoints/organization_dashboards.py b/src/sentry/api/endpoints/organization_dashboards.py index 481c528d320651..e438696d260366 100644 --- a/src/sentry/api/endpoints/organization_dashboards.py +++ b/src/sentry/api/endpoints/organization_dashboards.py @@ -50,9 +50,28 @@ def has_object_permission(self, request: Request, view, obj): 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(): + for project in obj.projects.all(): + if not request.access.has_project_access(project): + return False + return True + + # 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 diff --git a/tests/sentry/api/endpoints/test_organization_dashboard_details.py b/tests/sentry/api/endpoints/test_organization_dashboard_details.py index 5a88a3c168f13c..67948f97ce324a 100644 --- a/tests/sentry/api/endpoints/test_organization_dashboard_details.py +++ b/tests/sentry/api/endpoints/test_organization_dashboard_details.py @@ -384,6 +384,65 @@ def test_disallow_delete_when_no_project_access(self): 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 From 827b72a9085772e7c7bf014d3962b43aa26545c6 Mon Sep 17 00:00:00 2001 From: Alexander Tarasov Date: Mon, 7 Oct 2024 10:07:27 +0200 Subject: [PATCH 2/2] Update src/sentry/api/endpoints/organization_dashboards.py Co-authored-by: George Gritsouk <989898+gggritso@users.noreply.github.com> --- src/sentry/api/endpoints/organization_dashboards.py | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/src/sentry/api/endpoints/organization_dashboards.py b/src/sentry/api/endpoints/organization_dashboards.py index e438696d260366..5cc723e6a8a49b 100644 --- a/src/sentry/api/endpoints/organization_dashboards.py +++ b/src/sentry/api/endpoints/organization_dashboards.py @@ -52,10 +52,7 @@ def has_object_permission(self, request: Request, view, obj): if isinstance(obj, Dashboard): # 1. Dashboard contains certain projects if obj.projects.exists(): - for project in obj.projects.all(): - if not request.access.has_project_access(project): - return False - return True + return request.access.has_projects_access(obj.projects.all()) # 2. Dashboard covers all projects or all my projects