diff --git a/CHANGELOG.md b/CHANGELOG.md index 9f2d99219..4db20e219 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -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 diff --git a/src/backend/core/api/permissions.py b/src/backend/core/api/permissions.py index 75a2d288b..445a2c16e 100644 --- a/src/backend/core/api/permissions.py +++ b/src/backend/core/api/permissions.py @@ -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 @@ 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.""" diff --git a/src/backend/core/api/serializers.py b/src/backend/core/api/serializers.py index 2f498b263..2947e3b8f 100644 --- a/src/backend/core/api/serializers.py +++ b/src/backend/core/api/serializers.py @@ -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): diff --git a/src/backend/core/api/viewsets.py b/src/backend/core/api/viewsets.py index 669396860..98560eb84 100644 --- a/src/backend/core/api/viewsets.py +++ b/src/backend/core/api/viewsets.py @@ -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 @@ 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 diff --git a/src/backend/core/models.py b/src/backend/core/models.py index 3697568be..6be54d649 100644 --- a/src/backend/core/models.py +++ b/src/backend/core/models.py @@ -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 @@ 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, @@ -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: @@ -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, } diff --git a/src/backend/core/tests/documents/test_api_document_invitations.py b/src/backend/core/tests/documents/test_api_document_invitations.py index f103b4f81..1b9e61688 100644 --- a/src/backend/core/tests/documents/test_api_document_invitations.py +++ b/src/backend/core/tests/documents/test_api_document_invitations.py @@ -3,21 +3,302 @@ """ import random -import time +from datetime import timedelta +from unittest import mock from django.core import mail +from django.utils import timezone import pytest -from rest_framework import status from rest_framework.test import APIClient from core import factories, models +from core.api import serializers from core.tests.conftest import TEAM, USER, VIA pytestmark = pytest.mark.django_db -def test_api_document_invitations__create__anonymous(): +# List + + +def test_api_document_invitations_list_anonymous_user(): + """Anonymous users should not be able to list invitations.""" + invitation = factories.InvitationFactory() + response = APIClient().get( + f"/api/v1.0/documents/{invitation.document.id!s}/invitations/" + ) + assert response.status_code == 401 + + +@pytest.mark.parametrize("via", VIA) +@pytest.mark.parametrize("role", ["owner", "administrator"]) +def test_api_document_invitations_list_authenticated_privileged( + role, via, mock_user_teams, django_assert_num_queries +): + """ + Authenticated users should be able to list invitations for documents to which they are + related with administrator or owner privilege, including invitations issued by other users. + """ + user = factories.UserFactory() + other_user = factories.UserFactory() + document = factories.DocumentFactory() + if via == USER: + factories.UserDocumentAccessFactory(document=document, user=user, role=role) + elif via == TEAM: + mock_user_teams.return_value = ["lasuite", "unknown"] + factories.TeamDocumentAccessFactory( + document=document, team="lasuite", role=role + ) + + invitation = factories.InvitationFactory(document=document, issuer=user) + other_invitations = factories.InvitationFactory.create_batch( + 2, document=document, issuer=other_user + ) + + # invitations from other documents should not be listed + other_document = factories.DocumentFactory() + factories.InvitationFactory.create_batch(2, document=other_document) + + client = APIClient() + client.force_login(user) + with django_assert_num_queries(3): + response = client.get( + f"/api/v1.0/documents/{document.id!s}/invitations/", + ) + assert response.status_code == 200 + assert response.json()["count"] == 3 + assert sorted(response.json()["results"], key=lambda x: x["created_at"]) == sorted( + [ + { + "id": str(i.id), + "created_at": i.created_at.isoformat().replace("+00:00", "Z"), + "email": str(i.email), + "document": str(document.id), + "role": i.role, + "issuer": str(i.issuer.id), + "is_expired": False, + "abilities": { + "destroy": role in ["administrator", "owner"], + "update": role in ["administrator", "owner"], + "partial_update": role in ["administrator", "owner"], + "retrieve": True, + }, + } + for i in [invitation, *other_invitations] + ], + key=lambda x: x["created_at"], + ) + + +@pytest.mark.parametrize("via", VIA) +@pytest.mark.parametrize("role", ["reader", "editor"]) +def test_api_document_invitations_list_authenticated_unprivileged( + role, via, mock_user_teams, django_assert_num_queries +): + """ + Authenticated users should not be able to list invitations for documents to which they are + related with reader or editor role, including invitations issued by other users. + """ + user = factories.UserFactory() + other_user = factories.UserFactory() + document = factories.DocumentFactory() + if via == USER: + factories.UserDocumentAccessFactory(document=document, user=user, role=role) + elif via == TEAM: + mock_user_teams.return_value = ["lasuite", "unknown"] + factories.TeamDocumentAccessFactory( + document=document, team="lasuite", role=role + ) + + factories.InvitationFactory(document=document, issuer=user) + factories.InvitationFactory.create_batch(2, document=document, issuer=other_user) + + # invitations from other documents should not be listed + other_document = factories.DocumentFactory() + factories.InvitationFactory.create_batch(2, document=other_document) + + client = APIClient() + client.force_login(user) + with django_assert_num_queries(2): + response = client.get( + f"/api/v1.0/documents/{document.id!s}/invitations/", + ) + assert response.status_code == 200 + assert response.json()["count"] == 0 + + +def test_api_document_invitations_list_expired_invitations_still_listed(): + """ + Expired invitations are still listed. + """ + user = factories.UserFactory() + other_user = factories.UserFactory() + + document = factories.DocumentFactory( + users=[(user, "administrator"), (other_user, "owner")] + ) + + expired_invitation = factories.InvitationFactory( + document=document, + role="reader", + issuer=user, + ) + + client = APIClient() + client.force_login(user) + + # mock timezone.now to accelerate validation expiration + 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 + + response = client.get( + f"/api/v1.0/documents/{document.id!s}/invitations/", + ) + + assert response.status_code == 200 + assert response.json()["count"] == 1 + assert sorted(response.json()["results"], key=lambda x: x["created_at"]) == sorted( + [ + { + "id": str(expired_invitation.id), + "created_at": expired_invitation.created_at.isoformat().replace( + "+00:00", "Z" + ), + "email": str(expired_invitation.email), + "document": str(document.id), + "role": expired_invitation.role, + "issuer": str(expired_invitation.issuer.id), + "is_expired": True, + "abilities": { + "destroy": True, + "update": True, + "partial_update": True, + "retrieve": True, + }, + }, + ], + key=lambda x: x["created_at"], + ) + + +# Retrieve + + +def test_api_document_invitations_retrieve_anonymous_user(): + """ + Anonymous users should not be able to retrieve invitations. + """ + + invitation = factories.InvitationFactory() + response = APIClient().get( + f"/api/v1.0/documents/{invitation.document.id!s}/invitations/{invitation.id!s}/", + ) + + assert response.status_code == 401 + + +def test_api_document_invitations_retrieve_unrelated_user(): + """ + Authenticated unrelated users should not be able to retrieve invitations. + """ + user = factories.UserFactory() + invitation = factories.InvitationFactory() + + client = APIClient() + client.force_login(user) + response = client.get( + f"/api/v1.0/documents/{invitation.document.id!s}/invitations/{invitation.id!s}/", + ) + + assert response.status_code == 403 + + +@pytest.mark.parametrize("via", VIA) +@pytest.mark.parametrize("role", ["administrator", "owner"]) +def test_api_document_invitations_retrieve_document_privileged( + role, via, mock_user_teams +): + """ + Authenticated users related to the document should be able to retrieve invitations + provided they are administrators or owners of the document. + """ + user = factories.UserFactory() + invitation = factories.InvitationFactory() + + if via == USER: + factories.UserDocumentAccessFactory( + document=invitation.document, user=user, role=role + ) + elif via == TEAM: + mock_user_teams.return_value = ["lasuite", "unknown"] + factories.TeamDocumentAccessFactory( + document=invitation.document, team="lasuite", role=role + ) + + client = APIClient() + client.force_login(user) + + response = client.get( + f"/api/v1.0/documents/{invitation.document.id!s}/invitations/{invitation.id!s}/", + ) + + assert response.status_code == 200 + assert response.json() == { + "id": str(invitation.id), + "created_at": invitation.created_at.isoformat().replace("+00:00", "Z"), + "email": invitation.email, + "document": str(invitation.document.id), + "role": str(invitation.role), + "issuer": str(invitation.issuer.id), + "is_expired": False, + "abilities": { + "destroy": True, + "update": True, + "partial_update": True, + "retrieve": True, + }, + } + + +@pytest.mark.parametrize("via", VIA) +@pytest.mark.parametrize("role", ["reader", "editor"]) +def test_api_document_invitations_retrieve_document_unprivileged( + role, via, mock_user_teams +): + """ + Authenticated users related to the document should not be able to retrieve invitations + if they are simply reader or editor of the document. + """ + user = factories.UserFactory() + invitation = factories.InvitationFactory() + + if via == USER: + factories.UserDocumentAccessFactory( + document=invitation.document, user=user, role=role + ) + elif via == TEAM: + mock_user_teams.return_value = ["lasuite", "unknown"] + factories.TeamDocumentAccessFactory( + document=invitation.document, team="lasuite", role=role + ) + + client = APIClient() + client.force_login(user) + + response = client.get( + f"/api/v1.0/documents/{invitation.document.id!s}/invitations/{invitation.id!s}/", + ) + + assert response.status_code == 403 + assert response.content + + +# Create + + +def test_api_document_invitations_create_anonymous(): """Anonymous users should not be able to create invitations.""" document = factories.DocumentFactory() invitation_values = { @@ -26,18 +307,18 @@ def test_api_document_invitations__create__anonymous(): } response = APIClient().post( - f"/api/v1.0/documents/{document.id}/invitations/", + f"/api/v1.0/documents/{document.id!s}/invitations/", invitation_values, format="json", ) - assert response.status_code == status.HTTP_401_UNAUTHORIZED + assert response.status_code == 401 assert response.json() == { "detail": "Authentication credentials were not provided." } -def test_api_document_invitations__create__authenticated_outsider(): +def test_api_document_invitations_create_authenticated_outsider(): """Users outside of document should not be permitted to invite to document.""" user = factories.UserFactory() document = factories.DocumentFactory() @@ -48,39 +329,40 @@ def test_api_document_invitations__create__authenticated_outsider(): client = APIClient() client.force_login(user) + response = client.post( - f"/api/v1.0/documents/{document.id}/invitations/", + f"/api/v1.0/documents/{document.id!s}/invitations/", invitation_values, format="json", ) - assert response.status_code == status.HTTP_403_FORBIDDEN + assert response.status_code == 403 @pytest.mark.parametrize( - "inviting,invited,is_allowed", + "inviting,invited,response_code", ( - ["reader", "reader", False], - ["reader", "editor", False], - ["reader", "administrator", False], - ["reader", "owner", False], - ["editor", "reader", False], - ["editor", "editor", False], - ["editor", "administrator", False], - ["editor", "owner", False], - ["administrator", "reader", True], - ["administrator", "editor", True], - ["administrator", "administrator", True], - ["administrator", "owner", False], - ["owner", "reader", True], - ["owner", "editor", True], - ["owner", "administrator", True], - ["owner", "owner", True], + ["reader", "reader", 403], + ["reader", "editor", 403], + ["reader", "administrator", 403], + ["reader", "owner", 403], + ["editor", "reader", 403], + ["editor", "editor", 403], + ["editor", "administrator", 403], + ["editor", "owner", 403], + ["administrator", "reader", 201], + ["administrator", "editor", 201], + ["administrator", "administrator", 201], + ["administrator", "owner", 400], + ["owner", "reader", 201], + ["owner", "editor", 201], + ["owner", "administrator", 201], + ["owner", "owner", 201], ), ) @pytest.mark.parametrize("via", VIA) -def test_api_document_invitations__create__privileged_members( - via, inviting, invited, is_allowed, mock_user_teams +def test_api_document_invitations_create_privileged_members( + via, inviting, invited, response_code, mock_user_teams ): """ Only owners and administrators should be able to invite new users. @@ -106,12 +388,14 @@ def test_api_document_invitations__create__privileged_members( client = APIClient() client.force_login(user) response = client.post( - f"/api/v1.0/documents/{document.id}/invitations/", + f"/api/v1.0/documents/{document.id!s}/invitations/", invitation_values, format="json", ) - if is_allowed: - assert response.status_code == status.HTTP_201_CREATED + + assert response.status_code == response_code + + if response_code == 201: assert models.Invitation.objects.count() == 1 assert len(mail.outbox) == 1 @@ -123,11 +407,17 @@ def test_api_document_invitations__create__privileged_members( in email_content ) else: - assert response.status_code == status.HTTP_403_FORBIDDEN assert models.Invitation.objects.exists() is False + if response_code == 400: + assert response.json() == { + "role": [ + "Only owners of a document can invite other users as owners.", + ], + } -def test_api_document_invitations__create__email_from_content_language(): + +def test_api_document_invitations_create_email_from_content_language(): """ The email generated is from the language set in the Content-Language header """ @@ -144,14 +434,15 @@ def test_api_document_invitations__create__email_from_content_language(): client = APIClient() client.force_login(user) + response = client.post( - f"/api/v1.0/documents/{document.id}/invitations/", + f"/api/v1.0/documents/{document.id!s}/invitations/", invitation_values, format="json", headers={"Content-Language": "fr-fr"}, ) - assert response.status_code == status.HTTP_201_CREATED + assert response.status_code == 201 assert response.json()["email"] == "guest@example.com" assert models.Invitation.objects.count() == 1 assert len(mail.outbox) == 1 @@ -167,7 +458,7 @@ def test_api_document_invitations__create__email_from_content_language(): ) -def test_api_document_invitations__create__email_from_content_language_not_supported(): +def test_api_document_invitations_create_email_from_content_language_not_supported(): """ If the language from the Content-Language is not supported it will display the default language, English. @@ -185,14 +476,15 @@ def test_api_document_invitations__create__email_from_content_language_not_suppo client = APIClient() client.force_login(user) + response = client.post( - f"/api/v1.0/documents/{document.id}/invitations/", + f"/api/v1.0/documents/{document.id!s}/invitations/", invitation_values, format="json", headers={"Content-Language": "not-supported"}, ) - assert response.status_code == status.HTTP_201_CREATED + assert response.status_code == 201 assert response.json()["email"] == "guest@example.com" assert models.Invitation.objects.count() == 1 assert len(mail.outbox) == 1 @@ -208,7 +500,7 @@ def test_api_document_invitations__create__email_from_content_language_not_suppo ) -def test_api_document_invitations__create__email__full_name_empty(): +def test_api_document_invitations_create_email_full_name_empty(): """ If the full name of the user is empty, it will display the email address. """ @@ -225,14 +517,15 @@ def test_api_document_invitations__create__email__full_name_empty(): client = APIClient() client.force_login(user) + response = client.post( - f"/api/v1.0/documents/{document.id}/invitations/", + f"/api/v1.0/documents/{document.id!s}/invitations/", invitation_values, format="json", headers={"Content-Language": "not-supported"}, ) - assert response.status_code == status.HTTP_201_CREATED + assert response.status_code == 201 assert response.json()["email"] == "guest@example.com" assert models.Invitation.objects.count() == 1 assert len(mail.outbox) == 1 @@ -249,7 +542,7 @@ def test_api_document_invitations__create__email__full_name_empty(): ) -def test_api_document_invitations__create__issuer_and_document_override(): +def test_api_document_invitations_create_issuer_and_document_override(): """It should not be possible to set the "document" and "issuer" fields.""" user = factories.UserFactory() document = factories.DocumentFactory(users=[(user, "owner")]) @@ -263,19 +556,20 @@ def test_api_document_invitations__create__issuer_and_document_override(): client = APIClient() client.force_login(user) + response = client.post( - f"/api/v1.0/documents/{document.id}/invitations/", + f"/api/v1.0/documents/{document.id!s}/invitations/", invitation_values, format="json", ) - assert response.status_code == status.HTTP_201_CREATED + assert response.status_code == 201 # document and issuer automatically set assert response.json()["document"] == str(document.id) assert response.json()["issuer"] == str(user.id) -def test_api_document_invitations__create__cannot_duplicate_invitation(): +def test_api_document_invitations_create_cannot_duplicate_invitation(): """An email should not be invited multiple times to the same document.""" existing_invitation = factories.InvitationFactory() document = existing_invitation.document @@ -294,19 +588,20 @@ def test_api_document_invitations__create__cannot_duplicate_invitation(): client = APIClient() client.force_login(user) + response = client.post( - f"/api/v1.0/documents/{document.id}/invitations/", + f"/api/v1.0/documents/{document.id!s}/invitations/", invitation_values, format="json", ) - assert response.status_code == status.HTTP_400_BAD_REQUEST + assert response.status_code == 400 assert response.json() == [ "Document invitation with this Email address and Document already exists." ] -def test_api_document_invitations__create__cannot_invite_existing_users(): +def test_api_document_invitations_create_cannot_invite_existing_users(): """ It should not be possible to invite already existing users. """ @@ -322,174 +617,84 @@ def test_api_document_invitations__create__cannot_invite_existing_users(): client = APIClient() client.force_login(user) + response = client.post( - f"/api/v1.0/documents/{document.id}/invitations/", + f"/api/v1.0/documents/{document.id!s}/invitations/", invitation_values, format="json", ) - assert response.status_code == status.HTTP_400_BAD_REQUEST + assert response.status_code == 400 assert response.json() == ["This email is already associated to a registered user."] -def test_api_document_invitations__list__anonymous_user(): - """Anonymous users should not be able to list invitations.""" - document = factories.DocumentFactory() - response = APIClient().get(f"/api/v1.0/documents/{document.id}/invitations/") - assert response.status_code == status.HTTP_401_UNAUTHORIZED +# Update @pytest.mark.parametrize("via", VIA) -def test_api_document_invitations__list__authenticated( - via, mock_user_teams, django_assert_num_queries +@pytest.mark.parametrize("role", ["administrator", "owner"]) +def test_api_document_invitations_update_authenticated_privileged_any_field_except_role( + role, via, mock_user_teams ): """ - Authenticated users should be able to list invitations for documents to which they are - related, whatever the role and including invitations issued by other users. + Authenticated user can update invitations if they are administrator or owner of the document. """ user = factories.UserFactory() - other_user = factories.UserFactory() - document = factories.DocumentFactory() - role = random.choice(models.RoleChoices.choices)[0] + invitation = factories.InvitationFactory() + if via == USER: - factories.UserDocumentAccessFactory(document=document, user=user, role=role) + factories.UserDocumentAccessFactory( + document=invitation.document, user=user, role=role + ) elif via == TEAM: mock_user_teams.return_value = ["lasuite", "unknown"] factories.TeamDocumentAccessFactory( - document=document, team="lasuite", role=role + document=invitation.document, team="lasuite", role=role ) - invitation = factories.InvitationFactory( - document=document, role="administrator", issuer=user - ) - other_invitations = factories.InvitationFactory.create_batch( - 2, document=document, role="reader", issuer=other_user - ) - - # invitations from other documents should not be listed - other_document = factories.DocumentFactory() - factories.InvitationFactory.create_batch(2, document=other_document, role="reader") + old_invitation_values = serializers.InvitationSerializer(instance=invitation).data + new_invitation_values = serializers.InvitationSerializer( + instance=factories.InvitationFactory() + ).data + # The update of a role is tested in the next test + del new_invitation_values["role"] client = APIClient() client.force_login(user) - with django_assert_num_queries(3): - response = client.get( - f"/api/v1.0/documents/{document.id}/invitations/", - ) - assert response.status_code == status.HTTP_200_OK - assert response.json()["count"] == 3 - assert sorted(response.json()["results"], key=lambda x: x["created_at"]) == sorted( - [ - { - "id": str(i.id), - "created_at": i.created_at.isoformat().replace("+00:00", "Z"), - "email": str(i.email), - "document": str(document.id), - "role": i.role, - "issuer": str(i.issuer.id), - "is_expired": False, - "abilities": { - "destroy": role in ["administrator", "owner"], - "update": role in ["administrator", "owner"], - "partial_update": role in ["administrator", "owner"], - "retrieve": True, - }, - } - for i in [invitation, *other_invitations] - ], - key=lambda x: x["created_at"], - ) - -def test_api_document_invitations__list__expired_invitations_still_listed(settings): - """ - Expired invitations are still listed. - """ - user = factories.UserFactory() - other_user = factories.UserFactory() - - document = factories.DocumentFactory( - users=[(user, "administrator"), (other_user, "owner")] + url = ( + f"/api/v1.0/documents/{invitation.document.id!s}/invitations/{invitation.id!s}/" ) + response = client.put(url, new_invitation_values, format="json") - # override settings to accelerate validation expiration - settings.INVITATION_VALIDITY_DURATION = 1 # second - expired_invitation = factories.InvitationFactory( - document=document, - role="reader", - issuer=user, - ) - time.sleep(1) + assert response.status_code == 200 - client = APIClient() - client.force_login(user) - response = client.get( - f"/api/v1.0/documents/{document.id}/invitations/", - ) - assert response.status_code == status.HTTP_200_OK - assert response.json()["count"] == 1 - assert sorted(response.json()["results"], key=lambda x: x["created_at"]) == sorted( - [ - { - "id": str(expired_invitation.id), - "created_at": expired_invitation.created_at.isoformat().replace( - "+00:00", "Z" - ), - "email": str(expired_invitation.email), - "document": str(document.id), - "role": expired_invitation.role, - "issuer": str(expired_invitation.issuer.id), - "is_expired": True, - "abilities": { - "destroy": True, - "update": True, - "partial_update": True, - "retrieve": True, - }, - }, - ], - key=lambda x: x["created_at"], - ) - - -def test_api_document_invitations__retrieve__anonymous_user(): - """ - Anonymous users should not be able to retrieve invitations. - """ - - invitation = factories.InvitationFactory() - response = APIClient().get( - f"/api/v1.0/documents/{invitation.document.id}/invitations/{invitation.id}/", - ) - - assert response.status_code == status.HTTP_401_UNAUTHORIZED - - -def test_api_document_invitations__retrieve__unrelated_user(): - """ - Authenticated unrelated users should not be able to retrieve invitations. - """ - user = factories.UserFactory() - invitation = factories.InvitationFactory() - - client = APIClient() - client.force_login(user) - response = client.get( - f"/api/v1.0/documents/{invitation.document.id!s}/invitations/{invitation.id!s}/", - ) + invitation.refresh_from_db() + invitation_values = serializers.InvitationSerializer(instance=invitation).data - assert response.status_code == status.HTTP_403_FORBIDDEN + for key, value in invitation_values.items(): + if key == "email": + assert value == new_invitation_values[key] + elif key == "updated_at": + assert value > old_invitation_values[key] + else: + assert value == old_invitation_values[key] @pytest.mark.parametrize("via", VIA) -def test_api_document_invitations__retrieve__document_member(via, mock_user_teams): +@pytest.mark.parametrize("role_set", models.RoleChoices.values) +@pytest.mark.parametrize("role", ["administrator", "owner"]) +def test_api_document_invitations_update_authenticated_privileged_role( + role, role_set, via, mock_user_teams +): """ - Authenticated users related to the document should be able to retrieve invitations - whatever their role in the document. + Authenticated user can update invitations if they are administrator or owner of the document, + but only owners can set the invitation role to the "owner" role. """ user = factories.UserFactory() invitation = factories.InvitationFactory() - role = random.choice(models.RoleChoices.choices)[0] + old_role = invitation.role + if via == USER: factories.UserDocumentAccessFactory( document=invitation.document, user=user, role=role @@ -500,146 +705,90 @@ def test_api_document_invitations__retrieve__document_member(via, mock_user_team document=invitation.document, team="lasuite", role=role ) - client = APIClient() - client.force_login(user) - response = client.get( - f"/api/v1.0/documents/{invitation.document.id}/invitations/{invitation.id}/", - ) - - assert response.status_code == status.HTTP_200_OK - assert response.json() == { - "id": str(invitation.id), - "created_at": invitation.created_at.isoformat().replace("+00:00", "Z"), - "email": invitation.email, - "document": str(invitation.document.id), - "role": str(invitation.role), - "issuer": str(invitation.issuer.id), - "is_expired": False, - "abilities": { - "destroy": role in ["administrator", "owner"], - "update": role in ["administrator", "owner"], - "partial_update": role in ["administrator", "owner"], - "retrieve": True, - }, - } - - -@pytest.mark.parametrize("via", VIA) -def test_api_document_invitations__put_authenticated(via, mock_user_teams): - """ - Authenticated user can put invitations. - """ - user = factories.UserFactory() - invitation = factories.InvitationFactory() - if via == USER: - factories.UserDocumentAccessFactory( - document=invitation.document, user=user, role="owner" - ) - elif via == TEAM: - mock_user_teams.return_value = ["lasuite", "unknown"] - factories.TeamDocumentAccessFactory( - document=invitation.document, team="lasuite", role="owner" - ) + new_invitation_values = serializers.InvitationSerializer(instance=invitation).data + new_invitation_values["role"] = role_set client = APIClient() client.force_login(user) - url = f"/api/v1.0/documents/{invitation.document.id}/invitations/{invitation.id}/" - response = client.patch(url, {"email": "test@test.test"}, format="json") - assert response.status_code == status.HTTP_200_OK + url = ( + f"/api/v1.0/documents/{invitation.document.id!s}/invitations/{invitation.id!s}/" + ) + response = client.put(url, new_invitation_values, format="json") invitation.refresh_from_db() - assert invitation.email == "test@test.test" + + if role_set == "owner" and role != "owner": + assert response.status_code == 400 + assert invitation.role == old_role + assert response.json() == { + "role": [ + "Only owners of a document can invite other users as owners.", + ], + } + else: + assert response.status_code == 200 + assert invitation.role == role_set @pytest.mark.parametrize("via", VIA) -def test_api_document_invitations__patch_authenticated(via, mock_user_teams): +@pytest.mark.parametrize("role", ["reader", "editor"]) +def test_api_document_invitations_update_authenticated_unprivileged( + role, via, mock_user_teams +): """ - Authenticated user can patch invitations. + Authenticated user should not be allowed to update invitations if they are + simple reader or editor of the document. """ user = factories.UserFactory() - invitation = factories.InvitationFactory(role="owner") + invitation = factories.InvitationFactory() + if via == USER: factories.UserDocumentAccessFactory( - document=invitation.document, user=user, role="owner" + document=invitation.document, user=user, role=role ) elif via == TEAM: mock_user_teams.return_value = ["lasuite", "unknown"] factories.TeamDocumentAccessFactory( - document=invitation.document, team="lasuite", role="owner" + document=invitation.document, team="lasuite", role=role ) - assert invitation.role == "owner" + old_invitation_values = serializers.InvitationSerializer(instance=invitation).data + new_invitation_values = serializers.InvitationSerializer( + instance=factories.InvitationFactory() + ).data client = APIClient() client.force_login(user) - url = f"/api/v1.0/documents/{invitation.document.id}/invitations/{invitation.id}/" - response = client.patch( - url, - {"role": "reader"}, - format="json", + + url = ( + f"/api/v1.0/documents/{invitation.document.id!s}/invitations/{invitation.id!s}/" ) + response = client.put(url, new_invitation_values, format="json") - assert response.status_code == status.HTTP_200_OK + assert response.status_code == 403 invitation.refresh_from_db() - assert invitation.role == "reader" + invitation_values = serializers.InvitationSerializer(instance=invitation).data + for key, value in invitation_values.items(): + assert value == old_invitation_values[key] -@pytest.mark.parametrize("via", VIA) -@pytest.mark.parametrize( - "method", - ["put", "patch"], -) -@pytest.mark.parametrize( - "role", - ["editor", "reader"], -) -def test_api_document_invitations__update__forbidden__not_authenticated( - method, via, role, mock_user_teams -): - """ - Update of invitations is currently forbidden. - """ - user = factories.UserFactory(with_owned_document=True) - invitation = factories.InvitationFactory() - if via == USER: - factories.UserDocumentAccessFactory( - document=invitation.document, user=user, role=role - ) - elif via == TEAM: - mock_user_teams.return_value = ["lasuite", "unknown"] - factories.TeamDocumentAccessFactory( - document=invitation.document, team="lasuite", role=role - ) - - client = APIClient() - client.force_login(user) - url = f"/api/v1.0/documents/{invitation.document.id}/invitations/{invitation.id}/" - - response = client.put(url) - if method == "patch": - response = client.patch(url) - - assert response.status_code == status.HTTP_403_FORBIDDEN - assert ( - response.json()["detail"] - == "You do not have permission to perform this action." - ) +# Delete -def test_api_document_invitations__delete__anonymous(): +def test_api_document_invitations_delete_anonymous(): """Anonymous user should not be able to delete invitations.""" invitation = factories.InvitationFactory() response = APIClient().delete( - f"/api/v1.0/documents/{invitation.document.id}/invitations/{invitation.id}/", + f"/api/v1.0/documents/{invitation.document.id!s}/invitations/{invitation.id!s}/", ) - assert response.status_code == status.HTTP_401_UNAUTHORIZED + assert response.status_code == 401 -def test_api_document_invitations__delete__authenticated_outsider(): +def test_api_document_invitations_delete_authenticated_outsider(): """Members unrelated to a document should not be allowed to cancel invitations.""" user = factories.UserFactory(with_owned_document=True) @@ -648,17 +797,16 @@ def test_api_document_invitations__delete__authenticated_outsider(): client = APIClient() client.force_login(user) + response = client.delete( - f"/api/v1.0/documents/{document.id}/invitations/{invitation.id}/", + f"/api/v1.0/documents/{document.id!s}/invitations/{invitation.id!s}/", ) - assert response.status_code == status.HTTP_403_FORBIDDEN + assert response.status_code == 403 @pytest.mark.parametrize("via", VIA) @pytest.mark.parametrize("role", ["owner", "administrator"]) -def test_api_document_invitations__delete__privileged_members( - role, via, mock_user_teams -): +def test_api_document_invitations_delete_privileged_members(role, via, mock_user_teams): """Privileged member should be able to cancel invitation.""" user = factories.UserFactory() document = factories.DocumentFactory() @@ -674,10 +822,11 @@ def test_api_document_invitations__delete__privileged_members( client = APIClient() client.force_login(user) + response = client.delete( - f"/api/v1.0/documents/{document.id}/invitations/{invitation.id}/", + f"/api/v1.0/documents/{document.id!s}/invitations/{invitation.id!s}/", ) - assert response.status_code == status.HTTP_204_NO_CONTENT + assert response.status_code == 204 @pytest.mark.parametrize("role", ["reader", "editor"]) @@ -698,10 +847,11 @@ def test_api_document_invitations_delete_readers_or_editors(via, role, mock_user client = APIClient() client.force_login(user) + response = client.delete( - f"/api/v1.0/documents/{document.id}/invitations/{invitation.id}/", + f"/api/v1.0/documents/{document.id!s}/invitations/{invitation.id!s}/", ) - assert response.status_code == status.HTTP_403_FORBIDDEN + assert response.status_code == 403 assert ( response.json()["detail"] == "You do not have permission to perform this action." diff --git a/src/backend/core/tests/documents/test_api_documents_retrieve.py b/src/backend/core/tests/documents/test_api_documents_retrieve.py index 63d058b0a..220b5213a 100644 --- a/src/backend/core/tests/documents/test_api_documents_retrieve.py +++ b/src/backend/core/tests/documents/test_api_documents_retrieve.py @@ -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, diff --git a/src/backend/core/tests/test_models_documents.py b/src/backend/core/tests/test_models_documents.py index 813087361..d034f0110 100644 --- a/src/backend/core/tests/test_models_documents.py +++ b/src/backend/core/tests/test_models_documents.py @@ -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, diff --git a/src/backend/core/tests/test_models_invitations.py b/src/backend/core/tests/test_models_invitations.py index bfb3fdf5b..4ae4bfc75 100644 --- a/src/backend/core/tests/test_models_invitations.py +++ b/src/backend/core/tests/test_models_invitations.py @@ -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, }