🚸(backend) validate document access roles on submit

The frontend should only let users choose override roles that make sense.

Author: sampaccoud

src/backend/core/api/serializers.py

@@ -339,12 +339,53 @@ def get_max_ancestors_role(self, instance):
  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)

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,