studio/docs
Archived
2
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

🐛(backend) fix invitations API endpoint access rights

Browse Source 
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.
This commit is contained in:
Samuel Paccoud - DINUM
2024-10-22 00:28:16 +02:00
committed by Samuel Paccoud
parent 7fc59ed497
commit 0f0f812059
9 changed files with 701 additions and 505 deletions
Download Patch File Download Diff File Expand all files Collapse all files

3
CHANGELOG.md
View File

@@ -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
@@ -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

35
src/backend/core/api/permissions.py
View File

@@ -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"}
}
@@ -59,6 +62,38 @@ class IsOwnedOrPublic(IsAuthenticated):
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."""

52
src/backend/core/api/serializers.py
View File

@@ -328,48 +328,36 @@ class InvitationSerializer(serializers.ModelSerializer):
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):

17
src/backend/core/api/viewsets.py
View File

@@ -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")
@@ -842,10 +845,16 @@ class InvitationViewset(
)
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

24
src/backend/core/models.py
View File

@@ -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"""
@@ -514,6 +517,7 @@ class Document(BaseModel):
"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,
@@ -880,8 +884,6 @@ class Invitation(BaseModel):
def get_abilities(self, user):
"""Compute and return abilities for a given user."""
can_delete = False
can_update = False
roles = []
if user.is_authenticated:
@@ -896,17 +898,13 @@ class Invitation(BaseModel):
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,
}

1040
src/backend/core/tests/documents/test_api_document_invitations.py
View File

File diff suppressed because it is too large Load Diff

2
src/backend/core/tests/documents/test_api_documents_retrieve.py
View File

@@ -25,6 +25,7 @@ def test_api_documents_retrieve_anonymous_public():
"ai_translate": document.link_role == "editor",
"attachment_upload": document.link_role == "editor",
"destroy": False,
"invite_owner": False,
"link_configuration": False,
"manage_accesses": False,
"partial_update": document.link_role == "editor",
@@ -82,6 +83,7 @@ def test_api_documents_retrieve_authenticated_unrelated_public_or_authenticated(
"attachment_upload": document.link_role == "editor",
"link_configuration": False,
"destroy": False,
"invite_owner": False,
"manage_accesses": False,
"partial_update": document.link_role == "editor",
"retrieve": True,

8
src/backend/core/tests/test_models_documents.py
View File

@@ -88,6 +88,7 @@ def test_models_documents_get_abilities_forbidden(is_authenticated, reach, role)
"attachment_upload": False,
"link_configuration": False,
"destroy": False,
"invite_owner": False,
"manage_accesses": False,
"partial_update": False,
"retrieve": False,
@@ -120,6 +121,7 @@ def test_models_documents_get_abilities_reader(is_authenticated, reach):
"attachment_upload": False,
"destroy": False,
"link_configuration": False,
"invite_owner": False,
"manage_accesses": False,
"partial_update": False,
"retrieve": True,
@@ -152,6 +154,7 @@ def test_models_documents_get_abilities_editor(is_authenticated, reach):
"attachment_upload": True,
"destroy": False,
"link_configuration": False,
"invite_owner": False,
"manage_accesses": False,
"partial_update": True,
"retrieve": True,
@@ -173,6 +176,7 @@ def test_models_documents_get_abilities_owner():
"attachment_upload": True,
"destroy": True,
"link_configuration": True,
"invite_owner": True,
"manage_accesses": True,
"partial_update": True,
"retrieve": True,
@@ -193,6 +197,7 @@ def test_models_documents_get_abilities_administrator():
"attachment_upload": True,
"destroy": False,
"link_configuration": True,
"invite_owner": False,
"manage_accesses": True,
"partial_update": True,
"retrieve": True,
@@ -216,6 +221,7 @@ def test_models_documents_get_abilities_editor_user(django_assert_num_queries):
"attachment_upload": True,
"destroy": False,
"link_configuration": False,
"invite_owner": False,
"manage_accesses": False,
"partial_update": True,
"retrieve": True,
@@ -241,6 +247,7 @@ def test_models_documents_get_abilities_reader_user(django_assert_num_queries):
"attachment_upload": False,
"destroy": False,
"link_configuration": False,
"invite_owner": False,
"manage_accesses": False,
"partial_update": False,
"retrieve": True,
@@ -267,6 +274,7 @@ def test_models_documents_get_abilities_preset_role(django_assert_num_queries):
"attachment_upload": False,
"destroy": False,
"link_configuration": False,
"invite_owner": False,
"manage_accesses": False,
"partial_update": False,
"retrieve": True,

25
src/backend/core/tests/test_models_invitations.py
View File

@@ -2,10 +2,12 @@
Unit tests for the Invitation model
"""
import time
from datetime import timedelta
from unittest import mock
from django.contrib.auth.models import AnonymousUser
from django.core import exceptions
from django.utils import timezone
import pytest
from faker import Faker
@@ -60,7 +62,7 @@ def test_models_invitations_role_among_choices():
factories.InvitationFactory(role="boss")
def test_models_invitations__is_expired(settings):
def test_models_invitations_is_expired():
"""
The 'is_expired' property should return False until validity duration
is exceeded and True afterwards.
@@ -68,13 +70,16 @@ def test_models_invitations__is_expired(settings):
expired_invitation = factories.InvitationFactory()
assert expired_invitation.is_expired is False
settings.INVITATION_VALIDITY_DURATION = 1
time.sleep(1)
not_late = timezone.now() + timedelta(seconds=604799)
with mock.patch("django.utils.timezone.now", return_value=not_late):
assert expired_invitation.is_expired is False
assert expired_invitation.is_expired is True
too_late = timezone.now() + timedelta(seconds=604800) # 7 days
with mock.patch("django.utils.timezone.now", return_value=too_late):
assert expired_invitation.is_expired is True
def test_models_invitation__new_user__convert_invitations_to_accesses():
def test_models_invitationd_new_userd_convert_invitations_to_accesses():
"""
Upon creating a new user, invitations linked to the email
should be converted to accesses and then deleted.
@@ -109,7 +114,7 @@ def test_models_invitation__new_user__convert_invitations_to_accesses():
).exists() # the other invitation remains
def test_models_invitation__new_user__filter_expired_invitations():
def test_models_invitationd_new_user_filter_expired_invitations():
"""
Upon creating a new identity, valid invitations should be converted into accesses
and expired invitations should remain unchanged.
@@ -140,7 +145,7 @@ def test_models_invitation__new_user__filter_expired_invitations():
@pytest.mark.parametrize("num_invitations, num_queries", [(0, 3), (1, 6), (20, 6)])
def test_models_invitation__new_user__user_creation_constant_num_queries(
def test_models_invitationd_new_userd_user_creation_constant_num_queries(
django_assert_num_queries, num_invitations, num_queries
):
"""
@@ -235,7 +240,7 @@ def test_models_document_invitations_get_abilities_reader(via, mock_user_teams):
assert abilities == {
"destroy": False,
"retrieve": True,
"retrieve": False,
"partial_update": False,
"update": False,
}
@@ -260,7 +265,7 @@ def test_models_document_invitations_get_abilities_editor(via, mock_user_teams):
assert abilities == {
"destroy": False,
"retrieve": True,
"retrieve": False,
"partial_update": False,
"update": False,
}