diff --git a/src/backend/core/api/serializers.py b/src/backend/core/api/serializers.py index 2403c102..269c08ea 100644 --- a/src/backend/core/api/serializers.py +++ b/src/backend/core/api/serializers.py @@ -339,12 +339,53 @@ class DocumentAccessSerializer(serializers.ModelSerializer): return getattr(instance, "max_ancestors_role", None) def get_max_role(self, instance): - """Return max_ancestors_role if annotated; else None.""" + """Return max role.""" return choices.RoleChoices.max( getattr(instance, "max_ancestors_role", None), instance.role, ) + def validate(self, attrs): + """ + Ensure the selected role is greater than or equal to the maximum role + found in any ancestor document for the same user/team. + """ + + if self.instance: + document = self.instance.document + user = self.instance.user + team = self.instance.team + else: + document_id = self.context["resource_id"] + document = models.Document.objects.get(id=document_id) + team = attrs.get("team") + user = attrs.get("user") + + ancestors = document.get_ancestors().filter(ancestors_deleted_at__isnull=True) + access_qs = models.DocumentAccess.objects.filter(document__in=ancestors) + + if user: + access_qs = access_qs.filter(user=user) + if team: + access_qs = access_qs.filter(team=team) + + ancestor_roles = access_qs.values_list("role", flat=True) + inherited_role = choices.RoleChoices.max(*ancestor_roles) + + role = attrs.get("role") + get_priority = choices.RoleChoices.get_priority + if inherited_role and get_priority(inherited_role) >= get_priority(role): + raise serializers.ValidationError( + { + "role": ( + "Role overrides must be greater than the inherited role: " + f"{inherited_role}/{role}" + ) + } + ) + + return super().validate(attrs) + def update(self, instance, validated_data): """Make "user" field readonly but only on update.""" validated_data.pop("team", None) diff --git a/src/backend/core/tests/documents/test_api_document_accesses.py b/src/backend/core/tests/documents/test_api_document_accesses.py index 9f0c4917..92c4798e 100644 --- a/src/backend/core/tests/documents/test_api_document_accesses.py +++ b/src/backend/core/tests/documents/test_api_document_accesses.py @@ -140,9 +140,9 @@ def test_api_document_accesses_list_authenticated_related_non_privileged( { "id": str(access.id), "document": { + "depth": access.document.depth, "id": str(access.document_id), "path": access.document.path, - "depth": access.document.depth, }, "user": { "full_name": access.user.full_name, @@ -240,9 +240,9 @@ def test_api_document_accesses_list_authenticated_related_privileged( { "id": str(access.id), "document": { + "depth": access.document.depth, "id": str(access.document_id), "path": access.document.path, - "depth": access.document.depth, }, "user": { "id": str(access.user.id), @@ -611,14 +611,15 @@ def test_api_document_accesses_retrieve_authenticated_related( "id": str(access.id), "abilities": access.get_abilities(user), "document": { + "depth": access.document.depth, "id": str(access.document_id), "path": access.document.path, - "depth": access.document.depth, }, "user": access_user, "team": "", "role": access.role, "max_ancestors_role": None, + "max_role": access.role, } @@ -963,6 +964,119 @@ def test_api_document_accesses_update_owner( assert updated_values == old_values +@pytest.mark.parametrize("new_override_role", choices.RoleChoices.values) +@pytest.mark.parametrize("parent_role", choices.RoleChoices.values) +def test_api_document_accesses_update_higher_role_to_user( + parent_role, + new_override_role, + mock_reset_connections, # pylint: disable=redefined-outer-name +): + """ + It should not be allowed to update the role of a document access override + for a user with a role lower or equal to the inherited role. + """ + user, other_user = factories.UserFactory.create_batch(2) + + client = APIClient() + client.force_login(user) + + parent = factories.DocumentFactory( + users=[[user, "owner"], [other_user, parent_role]] + ) + document = factories.DocumentFactory(parent=parent) + + override_role = random.choice(choices.RoleChoices.values) + access = factories.UserDocumentAccessFactory( + document=document, user=other_user, role=override_role + ) + + get_priority = choices.RoleChoices.get_priority + if get_priority(new_override_role) > get_priority(parent_role): + with mock_reset_connections(document.id, str(access.user_id)): + response = client.put( + f"/api/v1.0/documents/{document.id!s}/accesses/{access.id!s}/", + data={"role": new_override_role}, + format="json", + ) + + assert response.status_code == 200 + access.refresh_from_db() + assert access.role == new_override_role + else: + response = client.put( + f"/api/v1.0/documents/{document.id!s}/accesses/{access.id!s}/", + data={"role": new_override_role}, + format="json", + ) + assert response.status_code == 400 + access.refresh_from_db() + assert access.role == override_role + assert response.json() == { + "role": [ + "Role overrides must be greater than the inherited role: " + f"{parent_role}/{new_override_role}" + ], + } + + +@pytest.mark.skip( + reason="Pending fix on https://github.com/suitenumerique/docs/issues/969" +) +@pytest.mark.parametrize("new_override_role", choices.RoleChoices.values) +@pytest.mark.parametrize("parent_role", choices.RoleChoices.values) +def test_api_document_accesses_update_higher_role_to_team( + parent_role, + new_override_role, + mock_reset_connections, # pylint: disable=redefined-outer-name +): + """ + It should not be allowed to update the role of a document access override + for a team with a role lower or equal to the inherited role. + """ + user = factories.UserFactory() + + client = APIClient() + client.force_login(user) + + parent = factories.DocumentFactory( + users=[[user, "owner"]], teams=[["lasuite", parent_role]] + ) + document = factories.DocumentFactory(parent=parent) + + override_role = random.choice(choices.RoleChoices.values) + access = factories.TeamDocumentAccessFactory( + document=document, team="lasuite", role=override_role + ) + + get_priority = choices.RoleChoices.get_priority + if get_priority(new_override_role) > get_priority(parent_role): + with mock_reset_connections(document.id, str(access.user_id)): + response = client.put( + f"/api/v1.0/documents/{document.id!s}/accesses/{access.id!s}/", + data={"role": new_override_role}, + format="json", + ) + + assert response.status_code == 200 + access.refresh_from_db() + assert access.role == new_override_role + else: + response = client.put( + f"/api/v1.0/documents/{document.id!s}/accesses/{access.id!s}/", + data={"role": new_override_role}, + format="json", + ) + assert response.status_code == 400 + access.refresh_from_db() + assert access.role == override_role + assert response.json() == { + "role": [ + "Role overrides must be greater than the inherited role: " + f"{parent_role}/{new_override_role}" + ], + } + + @pytest.mark.parametrize("via", VIA) def test_api_document_accesses_update_owner_self_root( via, diff --git a/src/backend/core/tests/documents/test_api_document_accesses_create.py b/src/backend/core/tests/documents/test_api_document_accesses_create.py index 3c1e1b93..f883be94 100644 --- a/src/backend/core/tests/documents/test_api_document_accesses_create.py +++ b/src/backend/core/tests/documents/test_api_document_accesses_create.py @@ -9,7 +9,7 @@ from django.core import mail import pytest from rest_framework.test import APIClient -from core import factories, models +from core import choices, factories, models from core.api import serializers from core.tests.conftest import TEAM, USER, VIA @@ -29,7 +29,7 @@ def test_api_document_accesses_create_anonymous(): { "user_id": str(other_user.id), "document": str(document.id), - "role": random.choice(models.RoleChoices.values), + "role": random.choice(choices.RoleChoices.values), }, format="json", ) @@ -88,7 +88,7 @@ def test_api_document_accesses_create_authenticated_reader_or_editor( other_user = factories.UserFactory() - for new_role in [role[0] for role in models.RoleChoices.choices]: + for new_role in choices.RoleChoices.values: response = client.post( f"/api/v1.0/documents/{document.id!s}/accesses/", { @@ -110,7 +110,7 @@ def test_api_document_accesses_create_authenticated_administrator_share_to_user( ): """ Administrators of a document (direct or by heritage) should be able to create - document accesses except for the "owner" role. + document accesses for a user except for the "owner" role. An email should be sent to the accesses to notify them of the adding. """ user = factories.UserFactory(with_owned_document=True) @@ -150,7 +150,7 @@ def test_api_document_accesses_create_authenticated_administrator_share_to_user( # It should be allowed to create a lower access role = random.choice( - [role[0] for role in models.RoleChoices.choices if role[0] != "owner"] + [role for role in choices.RoleChoices.values if role != "owner"] ) assert len(mail.outbox) == 0 @@ -201,7 +201,7 @@ def test_api_document_accesses_create_authenticated_administrator_share_to_team( ): """ Administrators of a document (direct or by heritage) should be able to create - document accesses except for the "owner" role. + document accesses for a team except for the "owner" role. An email should be sent to the accesses to notify them of the adding. """ user = factories.UserFactory(with_owned_document=True) @@ -241,7 +241,7 @@ def test_api_document_accesses_create_authenticated_administrator_share_to_team( # It should be allowed to create a lower access role = random.choice( - [role[0] for role in models.RoleChoices.choices if role[0] != "owner"] + [role for role in choices.RoleChoices.values if role != "owner"] ) assert len(mail.outbox) == 0 @@ -283,7 +283,8 @@ def test_api_document_accesses_create_authenticated_owner_share_to_user( ): """ Owners of a document (direct or by heritage) should be able to create document accesses - whatever the role. An email should be sent to the accesses to notify them of the adding. + for a user, whatever the role. An email should be sent to the accesses to notify them + of the adding. """ user = factories.UserFactory() @@ -307,7 +308,7 @@ def test_api_document_accesses_create_authenticated_owner_share_to_user( other_user = factories.UserFactory(language="en-us") document = documents[-1] - role = random.choice([role[0] for role in models.RoleChoices.choices]) + role = random.choice(choices.RoleChoices.values) assert len(mail.outbox) == 0 @@ -357,7 +358,8 @@ def test_api_document_accesses_create_authenticated_owner_share_to_team( ): """ Owners of a document (direct or by heritage) should be able to create document accesses - whatever the role. An email should be sent to the accesses to notify them of the adding. + for a team whatever the role. An email should be sent to the accesses to notify them of + the adding. """ user = factories.UserFactory() @@ -381,7 +383,7 @@ def test_api_document_accesses_create_authenticated_owner_share_to_team( other_user = factories.UserFactory(language="en-us") document = documents[-1] - role = random.choice([role[0] for role in models.RoleChoices.choices]) + role = random.choice(choices.RoleChoices.values) assert len(mail.outbox) == 0 @@ -415,6 +417,121 @@ def test_api_document_accesses_create_authenticated_owner_share_to_team( assert len(mail.outbox) == 0 +@pytest.mark.parametrize("override_role", choices.RoleChoices.values) +@pytest.mark.parametrize("parent_role", choices.RoleChoices.values) +def test_api_document_accesses_create_authenticated_higher_role_to_user( + parent_role, override_role +): + """ + It should not be allowed to create a document access override for a user + with a role lower or equal to the inherited role. + """ + user, other_user = factories.UserFactory.create_batch(2) + + client = APIClient() + client.force_login(user) + + parent = factories.DocumentFactory( + users=([user, "owner"], [other_user, parent_role]) + ) + document = factories.DocumentFactory(parent=parent) + + response = client.post( + f"/api/v1.0/documents/{document.id!s}/accesses/", + { + "user_id": str(other_user.id), + "role": override_role, + }, + format="json", + ) + + get_priority = choices.RoleChoices.get_priority + if get_priority(override_role) > get_priority(parent_role): + assert response.status_code == 201 + assert models.DocumentAccess.objects.filter(user=other_user).count() == 2 + else: + assert response.status_code == 400 + assert response.json() == { + "role": [ + "Role overrides must be greater than the inherited role: " + f"{parent_role}/{override_role}" + ], + } + assert models.DocumentAccess.objects.filter(user=other_user).count() == 1 + + +@pytest.mark.parametrize("override_role", choices.RoleChoices.values) +@pytest.mark.parametrize("parent_role", choices.RoleChoices.values) +def test_api_document_accesses_create_authenticated_higher_role_to_team( + parent_role, override_role, mock_user_teams +): + """ + It should not be allowed to create a document access override for a team + with a role lower or equal to the inherited role. + """ + user = factories.UserFactory() + + client = APIClient() + client.force_login(user) + + mock_user_teams.return_value = ["lasuite", "unknown"] + + parent = factories.DocumentFactory( + users=[[user, "owner"]], teams=[["lasuite", parent_role]] + ) + document = factories.DocumentFactory(parent=parent) + + response = client.post( + f"/api/v1.0/documents/{document.id!s}/accesses/", + { + "team": "lasuite", + "role": override_role, + }, + format="json", + ) + + get_priority = choices.RoleChoices.get_priority + if get_priority(override_role) > get_priority(parent_role): + assert response.status_code == 201 + assert models.DocumentAccess.objects.filter(team="lasuite").count() == 2 + else: + assert response.status_code == 400 + assert response.json() == { + "role": [ + "Role overrides must be greater than the inherited role: " + f"{parent_role}/{override_role}" + ], + } + assert models.DocumentAccess.objects.filter(team="lasuite").count() == 1 + + +def test_api_document_accesses_create_authenticated_user_and_team(): + """Trying to create a document access with a user and a team should return a 400 error.""" + user = factories.UserFactory() + + client = APIClient() + client.force_login(user) + + document = factories.DocumentFactory(users=[[user, "owner"]]) + other_user = factories.UserFactory(language="en-us") + + response = client.post( + f"/api/v1.0/documents/{document.id!s}/accesses/", + { + "user_id": str(other_user.id), + "team": "lasuite", + "role": "reader", + }, + format="json", + ) + + assert response.status_code == 400 + assert response.json() == { + "__all__": ["Either user or team must be set, not both."] + } + assert models.DocumentAccess.objects.count() == 1 + + @pytest.mark.parametrize("via", VIA) def test_api_document_accesses_create_email_in_receivers_language(via, mock_user_teams): """ @@ -434,7 +551,7 @@ def test_api_document_accesses_create_email_in_receivers_language(via, mock_user document=document, team="lasuite", role="owner" ) - role = random.choice([role[0] for role in models.RoleChoices.choices]) + role = random.choice(choices.RoleChoices.values) assert len(mail.outbox) == 0