Skip to content

Commit 2be4e3a

Browse files
committed
♻️(backend) rely on set_role_to from DocumentAskForAccess abilities
Like in other abilities, we compute a set_role_to property on the abilities. This set_role_to contains all the roles lower or equal than the current user role. We rely on this propoerty to validate the accept endpoint and it will be used by the front allpication to built the role select list.
1 parent bf68a5a commit 2be4e3a

File tree

3 files changed

+30
-22
lines changed

3 files changed

+30
-22
lines changed

src/backend/core/api/viewsets.py

Lines changed: 3 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -2162,13 +2162,10 @@ def accept(self, request, *args, **kwargs):
21622162
serializer = serializers.RoleSerializer(data=request.data)
21632163
serializer.is_valid(raise_exception=True)
21642164

2165-
document = self.get_document_or_404()
2166-
user_role = document.get_role(request.user)
2167-
target_role = serializer.validated_data.get("role")
2165+
target_role = serializer.validated_data.get("role", document_ask_for_access.role)
2166+
abilities = document_ask_for_access.get_abilities(request.user)
21682167

2169-
if models.RoleChoices.get_priority(user_role) < models.RoleChoices.get_priority(
2170-
target_role
2171-
):
2168+
if target_role not in abilities["set_role_to"]:
21722169
return drf.response.Response(
21732170
{"detail": "You cannot accept a role higher than your own."},
21742171
status=drf.status.HTTP_400_BAD_REQUEST,

src/backend/core/models.py

Lines changed: 8 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -1205,30 +1205,22 @@ def __str__(self):
12051205

12061206
def get_abilities(self, user):
12071207
"""Compute and return abilities for a given user."""
1208-
roles = []
1208+
user_role = self.document.get_role(user)
1209+
is_admin_or_owner = user_role in PRIVILEGED_ROLES
12091210

1210-
if user.is_authenticated:
1211-
teams = user.teams
1212-
try:
1213-
roles = self.user_roles or []
1214-
except AttributeError:
1215-
try:
1216-
roles = self.document.accesses.filter(
1217-
models.Q(user=user) | models.Q(team__in=teams),
1218-
).values_list("role", flat=True)
1219-
except (self._meta.model.DoesNotExist, IndexError):
1220-
roles = []
1221-
1222-
is_admin_or_owner = bool(
1223-
set(roles).intersection({RoleChoices.OWNER, RoleChoices.ADMIN})
1224-
)
1211+
set_role_to = [
1212+
role
1213+
for role in RoleChoices.values
1214+
if RoleChoices.get_priority(role) <= RoleChoices.get_priority(user_role)
1215+
]
12251216

12261217
return {
12271218
"destroy": is_admin_or_owner,
12281219
"update": is_admin_or_owner,
12291220
"partial_update": is_admin_or_owner,
12301221
"retrieve": is_admin_or_owner,
12311222
"accept": is_admin_or_owner,
1223+
"set_role_to": set_role_to,
12321224
}
12331225

12341226
def accept(self, role=None):

src/backend/core/tests/documents/test_api_documents_ask_for_access.py

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -287,6 +287,7 @@ def test_api_documents_ask_for_access_list_authenticated_own_request():
287287
"update": False,
288288
"partial_update": False,
289289
"retrieve": False,
290+
"set_role_to": [],
290291
},
291292
}
292293
],
@@ -356,6 +357,15 @@ def test_api_documents_ask_for_access_list_owner_or_admin(role):
356357

357358
response = client.get(f"/api/v1.0/documents/{document.id}/ask-for-access/")
358359
assert response.status_code == 200
360+
361+
expected_set_role_to = [
362+
RoleChoices.READER,
363+
RoleChoices.EDITOR,
364+
RoleChoices.ADMIN,
365+
]
366+
if role == RoleChoices.OWNER:
367+
expected_set_role_to.append(RoleChoices.OWNER)
368+
359369
assert response.json() == {
360370
"count": 3,
361371
"next": None,
@@ -375,6 +385,7 @@ def test_api_documents_ask_for_access_list_owner_or_admin(role):
375385
"update": True,
376386
"partial_update": True,
377387
"retrieve": True,
388+
"set_role_to": expected_set_role_to,
378389
},
379390
}
380391
for document_ask_for_access in document_ask_for_accesses
@@ -467,6 +478,13 @@ def test_api_documents_ask_for_access_retrieve_owner_or_admin(role):
467478
f"/api/v1.0/documents/{document.id}/ask-for-access/{document_ask_for_access.id}/"
468479
)
469480
assert response.status_code == 200
481+
expected_set_role_to = [
482+
RoleChoices.READER,
483+
RoleChoices.EDITOR,
484+
RoleChoices.ADMIN,
485+
]
486+
if role == RoleChoices.OWNER:
487+
expected_set_role_to.append(RoleChoices.OWNER)
470488
assert response.json() == {
471489
"id": str(document_ask_for_access.id),
472490
"document": str(document.id),
@@ -481,6 +499,7 @@ def test_api_documents_ask_for_access_retrieve_owner_or_admin(role):
481499
"update": True,
482500
"partial_update": True,
483501
"retrieve": True,
502+
"set_role_to": expected_set_role_to,
484503
},
485504
}
486505

0 commit comments

Comments
 (0)