Skip to content

Commit 0f0f812

Browse files
committed
🐛(backend) fix invitations API endpoint access rights
Only users who have the rights to manage accesses on the document should be allowed to see and manipulate invitations. Other users can see access rights on the document but only when the corresponding user/team has actually been granted access. We added a parameter in document abilities so the frontend knows when the logged-in user can invite another user with the owner role or not.
1 parent 7fc59ed commit 0f0f812

File tree

9 files changed

+545
-349
lines changed

9 files changed

+545
-349
lines changed

CHANGELOG.md

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@ and this project adheres to
1616

1717
## Fixed
1818

19+
- 🐛(backend) require right to manage document accesses to see invitations #369
1920
- 🐛(frontend) add default toolbar buttons #355
2021

2122

@@ -29,7 +30,7 @@ and this project adheres to
2930

3031
## Changed
3132

32-
- ♻️(frontend) More multi theme friendly #325
33+
- ♻️(frontend) more multi theme friendly #325
3334
- ♻️ Bootstrap frontend #257
3435
- ♻️ Add username in email #314
3536

src/backend/core/api/permissions.py

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,12 @@
11
"""Permission handlers for the impress core app."""
22

33
from django.core import exceptions
4+
from django.db.models import Q
45

56
from rest_framework import permissions
67

8+
from core.models import DocumentAccess, RoleChoices
9+
710
ACTION_FOR_METHOD_TO_PERMISSION = {
811
"versions_detail": {"DELETE": "versions_destroy", "GET": "versions_retrieve"}
912
}
@@ -59,6 +62,38 @@ def has_object_permission(self, request, view, obj):
5962
return False
6063

6164

65+
class CanCreateInvitationPermission(permissions.BasePermission):
66+
"""
67+
Custom permission class to handle permission checks for managing invitations.
68+
"""
69+
70+
def has_permission(self, request, view):
71+
user = request.user
72+
73+
# Ensure the user is authenticated
74+
if not (bool(request.auth) or request.user.is_authenticated):
75+
return False
76+
77+
# Apply permission checks only for creation (POST requests)
78+
if view.action != "create":
79+
return True
80+
81+
# Check if resource_id is passed in the context
82+
try:
83+
document_id = view.kwargs["resource_id"]
84+
except KeyError as exc:
85+
raise exceptions.ValidationError(
86+
"You must set a document ID in kwargs to manage document invitations."
87+
) from exc
88+
89+
# Check if the user has access to manage invitations (Owner/Admin roles)
90+
return DocumentAccess.objects.filter(
91+
Q(user=user) | Q(team__in=user.teams),
92+
document=document_id,
93+
role__in=[RoleChoices.OWNER, RoleChoices.ADMIN],
94+
).exists()
95+
96+
6297
class AccessPermission(permissions.BasePermission):
6398
"""Permission class for access objects."""
6499

src/backend/core/api/serializers.py

Lines changed: 20 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -328,48 +328,36 @@ def get_abilities(self, invitation) -> dict:
328328
return {}
329329

330330
def validate(self, attrs):
331-
"""Validate and restrict invitation to new user based on email."""
332-
331+
"""Validate invitation data."""
333332
request = self.context.get("request")
334333
user = getattr(request, "user", None)
335-
role = attrs.get("role")
336334

337-
try:
338-
document_id = self.context["resource_id"]
339-
except KeyError as exc:
340-
raise exceptions.ValidationError(
341-
"You must set a document ID in kwargs to create a new document invitation."
342-
) from exc
335+
attrs["document_id"] = self.context["resource_id"]
343336

344-
if not user and user.is_authenticated:
345-
raise exceptions.PermissionDenied(
346-
"Anonymous users are not allowed to create invitations."
347-
)
337+
# Only set the issuer if the instance is being created
338+
if self.instance is None:
339+
attrs["issuer"] = user
348340

349-
if not models.DocumentAccess.objects.filter(
350-
Q(user=user) | Q(team__in=user.teams),
351-
document=document_id,
352-
role__in=[models.RoleChoices.OWNER, models.RoleChoices.ADMIN],
353-
).exists():
354-
raise exceptions.PermissionDenied(
355-
"You are not allowed to manage invitations for this document."
356-
)
341+
return attrs
357342

358-
if (
359-
role == models.RoleChoices.OWNER
360-
and not models.DocumentAccess.objects.filter(
343+
def validate_role(self, role):
344+
"""Custom validation for the role field."""
345+
request = self.context.get("request")
346+
user = getattr(request, "user", None)
347+
document_id = self.context["resource_id"]
348+
349+
# If the role is OWNER, check if the user has OWNER access
350+
if role == models.RoleChoices.OWNER:
351+
if not models.DocumentAccess.objects.filter(
361352
Q(user=user) | Q(team__in=user.teams),
362353
document=document_id,
363354
role=models.RoleChoices.OWNER,
364-
).exists()
365-
):
366-
raise exceptions.PermissionDenied(
367-
"Only owners of a document can invite other users as owners."
368-
)
355+
).exists():
356+
raise serializers.ValidationError(
357+
"Only owners of a document can invite other users as owners."
358+
)
369359

370-
attrs["document_id"] = document_id
371-
attrs["issuer"] = user
372-
return attrs
360+
return role
373361

374362

375363
class VersionFilterSerializer(serializers.Serializer):

src/backend/core/api/viewsets.py

Lines changed: 13 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -807,7 +807,10 @@ class InvitationViewset(
807807

808808
lookup_field = "id"
809809
pagination_class = Pagination
810-
permission_classes = [permissions.IsAuthenticated, permissions.AccessPermission]
810+
permission_classes = [
811+
permissions.CanCreateInvitationPermission,
812+
permissions.AccessPermission,
813+
]
811814
queryset = (
812815
models.Invitation.objects.all()
813816
.select_related("document")
@@ -842,10 +845,16 @@ def get_queryset(self):
842845
)
843846

844847
queryset = (
845-
# The logged-in user should be part of a document to see its accesses
848+
# The logged-in user should be administrator or owner to see its accesses
846849
queryset.filter(
847-
Q(document__accesses__user=user)
848-
| Q(document__accesses__team__in=teams),
850+
Q(
851+
document__accesses__user=user,
852+
document__accesses__role__in=models.PRIVILEGED_ROLES,
853+
)
854+
| Q(
855+
document__accesses__team__in=teams,
856+
document__accesses__role__in=models.PRIVILEGED_ROLES,
857+
),
849858
)
850859
# Abilities are computed based on logged-in user's role and
851860
# the user role on each document access

src/backend/core/models.py

Lines changed: 11 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -72,6 +72,9 @@ class RoleChoices(models.TextChoices):
7272
OWNER = "owner", _("Owner")
7373

7474

75+
PRIVILEGED_ROLES = [RoleChoices.ADMIN, RoleChoices.OWNER]
76+
77+
7578
class LinkReachChoices(models.TextChoices):
7679
"""Defines types of access for links"""
7780

@@ -514,6 +517,7 @@ def get_abilities(self, user):
514517
"destroy": RoleChoices.OWNER in roles,
515518
"link_configuration": is_owner_or_admin,
516519
"manage_accesses": is_owner_or_admin,
520+
"invite_owner": RoleChoices.OWNER in roles,
517521
"partial_update": is_owner_or_admin or is_editor,
518522
"retrieve": can_get,
519523
"update": is_owner_or_admin or is_editor,
@@ -880,8 +884,6 @@ def is_expired(self):
880884

881885
def get_abilities(self, user):
882886
"""Compute and return abilities for a given user."""
883-
can_delete = False
884-
can_update = False
885887
roles = []
886888

887889
if user.is_authenticated:
@@ -896,17 +898,13 @@ def get_abilities(self, user):
896898
except (self._meta.model.DoesNotExist, IndexError):
897899
roles = []
898900

899-
can_delete = bool(
900-
set(roles).intersection({RoleChoices.OWNER, RoleChoices.ADMIN})
901-
)
902-
903-
can_update = bool(
904-
set(roles).intersection({RoleChoices.OWNER, RoleChoices.ADMIN})
905-
)
901+
is_admin_or_owner = bool(
902+
set(roles).intersection({RoleChoices.OWNER, RoleChoices.ADMIN})
903+
)
906904

907905
return {
908-
"destroy": can_delete,
909-
"update": can_update,
910-
"partial_update": can_update,
911-
"retrieve": bool(roles),
906+
"destroy": is_admin_or_owner,
907+
"update": is_admin_or_owner,
908+
"partial_update": is_admin_or_owner,
909+
"retrieve": is_admin_or_owner,
912910
}

0 commit comments

Comments
 (0)