Skip to content

Commit

Permalink
🐛(backend) fix invitations API endpoint access rights
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
sampaccoud committed Oct 22, 2024
1 parent 7fc59ed commit 3762dcc
Show file tree
Hide file tree
Showing 9 changed files with 545 additions and 349 deletions.
3 changes: 2 additions & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ and this project adheres to

## Fixed

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


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

## Changed

- ♻️(frontend) More multi theme friendly #325
- ♻️(frontend) more multi theme friendly #325
- ♻️ Bootstrap frontend #257
- ♻️ Add username in email #314

Expand Down
35 changes: 35 additions & 0 deletions src/backend/core/api/permissions.py
Original file line number Diff line number Diff line change
@@ -1,9 +1,12 @@
"""Permission handlers for the impress core app."""

from django.core import exceptions
from django.db.models import Q

from rest_framework import permissions

from core.models import DocumentAccess, RoleChoices

ACTION_FOR_METHOD_TO_PERMISSION = {
"versions_detail": {"DELETE": "versions_destroy", "GET": "versions_retrieve"}
}
Expand Down Expand Up @@ -59,6 +62,38 @@ def has_object_permission(self, request, view, obj):
return False


class CanCreateInvitationPermission(permissions.BasePermission):
"""
Custom permission class to handle permission checks for managing invitations.
"""

def has_permission(self, request, view):
user = request.user

# Ensure the user is authenticated
if not (bool(request.auth) or request.user.is_authenticated):
return False

# Apply permission checks only for creation (POST requests)
if view.action != "create":
return True

# Check if resource_id is passed in the context
try:
document_id = view.kwargs["resource_id"]
except KeyError as exc:
raise exceptions.ValidationError(
"You must set a document ID in kwargs to manage document invitations."
) from exc

# Check if the user has access to manage invitations (Owner/Admin roles)
return DocumentAccess.objects.filter(
Q(user=user) | Q(team__in=user.teams),
document=document_id,
role__in=[RoleChoices.OWNER, RoleChoices.ADMIN],
).exists()


class AccessPermission(permissions.BasePermission):
"""Permission class for access objects."""

Expand Down
52 changes: 20 additions & 32 deletions src/backend/core/api/serializers.py
Original file line number Diff line number Diff line change
Expand Up @@ -328,48 +328,36 @@ def get_abilities(self, invitation) -> dict:
return {}

def validate(self, attrs):
"""Validate and restrict invitation to new user based on email."""

"""Validate invitation data."""
request = self.context.get("request")
user = getattr(request, "user", None)
role = attrs.get("role")

try:
document_id = self.context["resource_id"]
except KeyError as exc:
raise exceptions.ValidationError(
"You must set a document ID in kwargs to create a new document invitation."
) from exc
attrs["document_id"] = self.context["resource_id"]

if not user and user.is_authenticated:
raise exceptions.PermissionDenied(
"Anonymous users are not allowed to create invitations."
)
# Only set the issuer if the instance is being created
if self.instance is None:
attrs["issuer"] = user

if not models.DocumentAccess.objects.filter(
Q(user=user) | Q(team__in=user.teams),
document=document_id,
role__in=[models.RoleChoices.OWNER, models.RoleChoices.ADMIN],
).exists():
raise exceptions.PermissionDenied(
"You are not allowed to manage invitations for this document."
)
return attrs

if (
role == models.RoleChoices.OWNER
and not models.DocumentAccess.objects.filter(
def validate_role(self, role):
"""Custom validation for the role field."""
request = self.context.get("request")
user = getattr(request, "user", None)
document_id = self.context["resource_id"]

# If the role is OWNER, check if the user has OWNER access
if role == models.RoleChoices.OWNER:
if not models.DocumentAccess.objects.filter(
Q(user=user) | Q(team__in=user.teams),
document=document_id,
role=models.RoleChoices.OWNER,
).exists()
):
raise exceptions.PermissionDenied(
"Only owners of a document can invite other users as owners."
)
).exists():
raise serializers.ValidationError(
"Only owners of a document can invite other users as owners."
)

attrs["document_id"] = document_id
attrs["issuer"] = user
return attrs
return role


class VersionFilterSerializer(serializers.Serializer):
Expand Down
17 changes: 13 additions & 4 deletions src/backend/core/api/viewsets.py
Original file line number Diff line number Diff line change
Expand Up @@ -807,7 +807,10 @@ class InvitationViewset(

lookup_field = "id"
pagination_class = Pagination
permission_classes = [permissions.IsAuthenticated, permissions.AccessPermission]
permission_classes = [
permissions.CanCreateInvitationPermission,
permissions.AccessPermission,
]
queryset = (
models.Invitation.objects.all()
.select_related("document")
Expand Down Expand Up @@ -842,10 +845,16 @@ def get_queryset(self):
)

queryset = (
# The logged-in user should be part of a document to see its accesses
# The logged-in user should be administrator or owner to see its accesses
queryset.filter(
Q(document__accesses__user=user)
| Q(document__accesses__team__in=teams),
Q(
document__accesses__user=user,
document__accesses__role__in=models.PRIVILEGED_ROLES,
)
| Q(
document__accesses__team__in=teams,
document__accesses__role__in=models.PRIVILEGED_ROLES,
),
)
# Abilities are computed based on logged-in user's role and
# the user role on each document access
Expand Down
24 changes: 11 additions & 13 deletions src/backend/core/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,9 @@ class RoleChoices(models.TextChoices):
OWNER = "owner", _("Owner")


PRIVILEGED_ROLES = [RoleChoices.ADMIN, RoleChoices.OWNER]


class LinkReachChoices(models.TextChoices):
"""Defines types of access for links"""

Expand Down Expand Up @@ -514,6 +517,7 @@ def get_abilities(self, user):
"destroy": RoleChoices.OWNER in roles,
"link_configuration": is_owner_or_admin,
"manage_accesses": is_owner_or_admin,
"invite_owner": RoleChoices.OWNER in roles,
"partial_update": is_owner_or_admin or is_editor,
"retrieve": can_get,
"update": is_owner_or_admin or is_editor,
Expand Down Expand Up @@ -880,8 +884,6 @@ def is_expired(self):

def get_abilities(self, user):
"""Compute and return abilities for a given user."""
can_delete = False
can_update = False
roles = []

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

can_delete = bool(
set(roles).intersection({RoleChoices.OWNER, RoleChoices.ADMIN})
)

can_update = bool(
set(roles).intersection({RoleChoices.OWNER, RoleChoices.ADMIN})
)
is_admin_or_owner = bool(
set(roles).intersection({RoleChoices.OWNER, RoleChoices.ADMIN})
)

return {
"destroy": can_delete,
"update": can_update,
"partial_update": can_update,
"retrieve": bool(roles),
"destroy": is_admin_or_owner,
"update": is_admin_or_owner,
"partial_update": is_admin_or_owner,
"retrieve": is_admin_or_owner,
}
Loading

0 comments on commit 3762dcc

Please sign in to comment.