Skip to content

Commit 215e7f7

Browse files
harshithaduraiharshithadurai
andauthored
ref(dashboards): Modify how permissions are handled for editing/deleting dashboards (#80684)
Modify how permissions are handled for editing/deleting dashboards: Previously, users without access to some projects were restricted from editing/deleting dashboards. (Change made here #78615). This PR removes that restriction, and enables restricting edit perms to specific teams. --------- Co-authored-by: harshithadurai <[email protected]>
1 parent 0661d41 commit 215e7f7

File tree

4 files changed

+246
-40
lines changed

4 files changed

+246
-40
lines changed

src/sentry/api/endpoints/organization_dashboard_details.py

Lines changed: 1 addition & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,6 @@
33
from django.db.models import F
44
from django.utils import timezone
55
from drf_spectacular.utils import extend_schema
6-
from rest_framework.permissions import BasePermission
76
from rest_framework.request import Request
87
from rest_framework.response import Response
98

@@ -31,32 +30,9 @@
3130
READ_FEATURE = "organizations:dashboards-basic"
3231

3332

34-
class DashboardPermissions(BasePermission):
35-
"""
36-
Django Permissions Class for managing Dashboard Edit
37-
permissions defined in the DashboardPermissions Model
38-
"""
39-
40-
scope_map = {
41-
"GET": ["org:read", "org:write", "org:admin"],
42-
"POST": ["org:read", "org:write", "org:admin"],
43-
"PUT": ["org:read", "org:write", "org:admin"],
44-
"DELETE": ["org:read", "org:write", "org:admin"],
45-
}
46-
47-
def has_object_permission(self, request: Request, view, obj):
48-
if isinstance(obj, Dashboard) and features.has(
49-
"organizations:dashboards-edit-access", obj.organization, actor=request.user
50-
):
51-
# Check if user has permissions to edit dashboard
52-
if hasattr(obj, "permissions"):
53-
return obj.permissions.has_edit_permissions(request.user.id)
54-
return True
55-
56-
5733
class OrganizationDashboardBase(OrganizationEndpoint):
5834
owner = ApiOwner.PERFORMANCE
59-
permission_classes = (OrganizationDashboardsPermission, DashboardPermissions)
35+
permission_classes = (OrganizationDashboardsPermission,)
6036

6137
def convert_args(
6238
self, request: Request, organization_id_or_slug, dashboard_id, *args, **kwargs

src/sentry/api/endpoints/organization_dashboards.py

Lines changed: 29 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -48,25 +48,40 @@ def has_object_permission(self, request: Request, view, obj):
4848
return super().has_object_permission(request, view, obj)
4949

5050
if isinstance(obj, Dashboard):
51-
# 1. Dashboard contains certain projects
52-
if obj.projects.exists():
53-
return request.access.has_projects_access(obj.projects.all())
51+
if features.has(
52+
"organizations:dashboards-edit-access", obj.organization, actor=request.user
53+
):
54+
# allow for Managers and Owners
55+
if request.access.has_scope("org:write"):
56+
return True
57+
58+
# check if user is restricted from editing dashboard
59+
if hasattr(obj, "permissions"):
60+
return obj.permissions.has_edit_permissions(request.user.id)
61+
62+
# if no permissions are assigned, it is considered accessible to all users
63+
return True
5464

55-
# 2. Dashboard covers all projects or all my projects
65+
else:
66+
# 1. Dashboard contains certain projects
67+
if obj.projects.exists():
68+
return request.access.has_projects_access(obj.projects.all())
5669

57-
# allow when Open Membership
58-
if obj.organization.flags.allow_joinleave:
59-
return True
70+
# 2. Dashboard covers all projects or all my projects
6071

61-
# allow for Managers and Owners
62-
if request.access.has_scope("org:write"):
63-
return True
72+
# allow when Open Membership
73+
if obj.organization.flags.allow_joinleave:
74+
return True
6475

65-
# allow for creator
66-
if request.user.id == obj.created_by_id:
67-
return True
76+
# allow for Managers and Owners
77+
if request.access.has_scope("org:write"):
78+
return True
79+
80+
# allow for creator
81+
if request.user.id == obj.created_by_id:
82+
return True
6883

69-
return False
84+
return False
7085

7186
return True
7287

src/sentry/api/serializers/rest_framework/dashboard.py

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -567,7 +567,9 @@ def validate(self, data):
567567
permissions = data.get("permissions")
568568
if permissions and self.instance:
569569
currentUser = self.context["request"].user
570-
if self.instance.created_by_id != currentUser.id:
570+
# managers and owners
571+
has_write_access = self.context["request"].access.has_scope("org:write")
572+
if self.instance.created_by_id != currentUser.id and not has_write_access:
571573
raise serializers.ValidationError(
572574
"Only the Dashboard Creator may modify Dashboard Edit Access"
573575
)

tests/sentry/api/endpoints/test_organization_dashboard_details.py

Lines changed: 213 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -513,6 +513,108 @@ def test_disallow_delete_my_projects_dashboard_when_no_open_membership(self):
513513
response = self.do_request("delete", self.url(dashboard.id))
514514
assert response.status_code == 204
515515

516+
def test_allow_delete_when_no_project_access(self):
517+
# disable Open Membership
518+
self.organization.flags.allow_joinleave = False
519+
self.organization.save()
520+
521+
# assign a project to a dashboard
522+
self.dashboard.projects.set([self.project])
523+
524+
# user has no access to the above project
525+
user_no_team = self.create_user(is_superuser=False)
526+
self.create_member(
527+
user=user_no_team, organization=self.organization, role="member", teams=[]
528+
)
529+
self.login_as(user_no_team)
530+
531+
with self.feature({"organizations:dashboards-edit-access": True}):
532+
response = self.do_request("delete", self.url(self.dashboard.id))
533+
assert response.status_code == 204
534+
535+
def test_allow_delete_all_projects_dashboard_when_no_open_membership(self):
536+
# disable Open Membership
537+
self.organization.flags.allow_joinleave = False
538+
self.organization.save()
539+
540+
dashboard = Dashboard.objects.create(
541+
title="Dashboard For All Projects",
542+
created_by_id=self.user.id,
543+
organization=self.organization,
544+
filters={"all_projects": True},
545+
)
546+
547+
# user has no access to all the projects
548+
user_no_team = self.create_user(is_superuser=False)
549+
self.create_member(
550+
user=user_no_team, organization=self.organization, role="member", teams=[]
551+
)
552+
self.login_as(user_no_team)
553+
554+
with self.feature({"organizations:dashboards-edit-access": True}):
555+
response = self.do_request("delete", self.url(dashboard.id))
556+
assert response.status_code == 204
557+
558+
def test_allow_delete_my_projects_dashboard_when_no_open_membership(self):
559+
# disable Open Membership
560+
self.organization.flags.allow_joinleave = False
561+
self.organization.save()
562+
563+
dashboard = Dashboard.objects.create(
564+
title="Dashboard For My Projects",
565+
created_by_id=self.user.id,
566+
organization=self.organization,
567+
# no 'filter' field means the dashboard covers all available projects
568+
)
569+
570+
# user has no access to all the projects
571+
user_no_team = self.create_user(is_superuser=False)
572+
self.create_member(
573+
user=user_no_team, organization=self.organization, role="member", teams=[]
574+
)
575+
self.login_as(user_no_team)
576+
577+
with self.feature({"organizations:dashboards-edit-access": True}):
578+
response = self.do_request("delete", self.url(dashboard.id))
579+
assert response.status_code == 204
580+
581+
def test_disallow_delete_when_no_project_access_and_no_edit_perms(self):
582+
# disable Open Membership
583+
self.organization.flags.allow_joinleave = False
584+
self.organization.save()
585+
586+
# assign a project to a dashboard
587+
self.dashboard.projects.set([self.project])
588+
589+
# user has no access to the above project
590+
user_no_team = self.create_user(is_superuser=False)
591+
self.create_member(
592+
user=user_no_team, organization=self.organization, role="member", teams=[]
593+
)
594+
self.login_as(user_no_team)
595+
596+
with self.feature({"organizations:dashboards-edit-access": True}):
597+
response = self.do_request("delete", self.url(self.dashboard.id))
598+
assert response.status_code == 204
599+
600+
def test_allow_delete_as_superuser_but_no_edit_perms(self):
601+
self.create_user(id=12333)
602+
dashboard = Dashboard.objects.create(
603+
id=67,
604+
title="Dashboard With Dataset Source",
605+
created_by_id=12333,
606+
organization=self.organization,
607+
)
608+
DashboardPermissions.objects.create(is_editable_by_everyone=False, dashboard=dashboard)
609+
610+
# Create and login as superuser
611+
superuser = self.create_user(is_superuser=True)
612+
self.login_as(user=superuser, superuser=True)
613+
614+
with self.feature({"organizations:dashboards-edit-access": True}):
615+
response = self.do_request("delete", self.url(dashboard.id))
616+
assert response.status_code == 204, response.content
617+
516618
def test_dashboard_does_not_exist(self):
517619
response = self.do_request("delete", self.url(1234567890))
518620
assert response.status_code == 404
@@ -731,6 +833,102 @@ def test_disallow_put_when_no_project_access(self):
731833
assert response.status_code == 403, response.data
732834
assert response.data == {"detail": "You do not have permission to perform this action."}
733835

836+
def test_allow_put_when_no_project_access(self):
837+
# disable Open Membership
838+
self.organization.flags.allow_joinleave = False
839+
self.organization.save()
840+
841+
# assign a project to a dashboard
842+
self.dashboard.projects.set([self.project])
843+
844+
# user has no access to the above project
845+
user_no_team = self.create_user(is_superuser=False)
846+
self.create_member(
847+
user=user_no_team, organization=self.organization, role="member", teams=[]
848+
)
849+
self.login_as(user_no_team)
850+
851+
with self.feature({"organizations:dashboards-edit-access": True}):
852+
response = self.do_request(
853+
"put", self.url(self.dashboard.id), data={"title": "Dashboard Hello"}
854+
)
855+
assert response.status_code == 200, response.data
856+
857+
def test_disallow_put_when_no_project_access_and_no_edit_perms(self):
858+
# set dashboard edit perms to be editable only by creator
859+
self.dashboard.permissions = DashboardPermissions.objects.create(
860+
is_editable_by_everyone=False, dashboard=self.dashboard
861+
)
862+
863+
# disable Open Membership
864+
self.organization.flags.allow_joinleave = False
865+
self.organization.save()
866+
867+
# assign a project to a dashboard
868+
self.dashboard.projects.set([self.project])
869+
870+
# user has no access to the above project
871+
user_no_team = self.create_user(is_superuser=False)
872+
self.create_member(
873+
user=user_no_team, organization=self.organization, role="member", teams=[]
874+
)
875+
self.login_as(user_no_team)
876+
877+
with self.feature({"organizations:dashboards-edit-access": True}):
878+
response = self.do_request(
879+
"put", self.url(self.dashboard.id), data={"title": "Dashboard Hello"}
880+
)
881+
assert response.status_code == 403, response.data
882+
assert response.data == {"detail": "You do not have permission to perform this action."}
883+
884+
def test_disallow_put_when_has_project_access_and_no_edit_perms(self):
885+
# set dashboard edit perms to be editable only by creator
886+
self.dashboard.permissions = DashboardPermissions.objects.create(
887+
is_editable_by_everyone=False, dashboard=self.dashboard
888+
)
889+
890+
# disable Open Membership
891+
self.organization.flags.allow_joinleave = False
892+
self.organization.save()
893+
894+
# assign a project to a dashboard
895+
self.dashboard.projects.set([self.project])
896+
897+
# user has access to the above project
898+
user = self.create_user(id=3456)
899+
team = self.create_team(organization=self.organization)
900+
self.create_member(user=user, organization=self.organization, teams=[team])
901+
self.project.add_team(team)
902+
self.login_as(user)
903+
904+
with self.feature({"organizations:dashboards-edit-access": True}):
905+
response = self.do_request(
906+
"put", self.url(self.dashboard.id), data={"title": "Dashboard Hello"}
907+
)
908+
assert response.status_code == 403, response.data
909+
assert response.data == {"detail": "You do not have permission to perform this action."}
910+
911+
def test_allow_put_as_superuser_but_no_edit_perms(self):
912+
self.create_user(id=12333)
913+
dashboard = Dashboard.objects.create(
914+
id=67,
915+
title="Dashboard With Dataset Source",
916+
created_by_id=12333,
917+
organization=self.organization,
918+
)
919+
DashboardPermissions.objects.create(is_editable_by_everyone=False, dashboard=dashboard)
920+
921+
# Create and login as superuser
922+
superuser = self.create_user(is_superuser=True)
923+
self.login_as(user=superuser, superuser=True)
924+
925+
with self.feature({"organizations:dashboards-edit-access": True}):
926+
response = self.do_request(
927+
"put", self.url(dashboard.id), data={"title": "New Dashboard 9"}
928+
)
929+
assert response.status_code == 200, response.content
930+
assert response.data["title"] == "New Dashboard 9"
931+
734932
def test_add_widget(self):
735933
data: dict[str, Any] = {
736934
"title": "First dashboard",
@@ -2136,6 +2334,21 @@ def test_user_tries_to_update_dashboard_edit_perms(self):
21362334
in response.content.decode()
21372335
)
21382336

2337+
def test_manager_or_owner_can_update_dashboard_edit_perms(self):
2338+
DashboardPermissions.objects.create(is_editable_by_everyone=False, dashboard=self.dashboard)
2339+
2340+
user = self.create_user(id=28193)
2341+
self.create_member(user=user, organization=self.organization, role="manager")
2342+
self.login_as(user)
2343+
2344+
with self.feature({"organizations:dashboards-edit-access": True}):
2345+
response = self.do_request(
2346+
"put",
2347+
self.url(self.dashboard.id),
2348+
data={"permissions": {"is_editable_by_everyone": False}},
2349+
)
2350+
assert response.status_code == 200
2351+
21392352
def test_update_dashboard_permissions_with_new_teams(self):
21402353
mock_project = self.create_project()
21412354
permission = DashboardPermissions.objects.create(

0 commit comments

Comments
 (0)