🐛(backend) fix creating/updating document accesses for teams

This use case was forgotten when the support for team accesses
was added. We add tests to stabilize the feature and its security.

Author: sampaccoud

src/backend/core/api/viewsets.py

@@ -1475,14 +1475,15 @@ def perform_create(self, serializer):
 
         access = serializer.save(document_id=self.kwargs["resource_id"])
 
-        access.document.send_invitation_email(
-            access.user.email,
-            access.role,
-            self.request.user,
-            access.user.language
-            or self.request.user.language
-            or settings.LANGUAGE_CODE,
-        )
+        if access.user:
+            access.document.send_invitation_email(
+                access.user.email,
+                access.role,
+                self.request.user,
+                access.user.language
+                or self.request.user.language
+                or settings.LANGUAGE_CODE,
+            )
 
     def perform_update(self, serializer):
         """Update an access to the document and notify the collaboration server."""

src/backend/core/tests/documents/test_api_document_accesses.py

@@ -654,7 +654,9 @@ def test_api_document_accesses_update_authenticated_reader_or_editor(
 
 
 @pytest.mark.parametrize("via", VIA)
+@pytest.mark.parametrize("create_for", VIA)
 def test_api_document_accesses_update_administrator_except_owner(
+    create_for,
     via,
     mock_user_teams,
     mock_reset_connections,  # pylint: disable=redefined-outer-name
@@ -687,9 +689,12 @@ def test_api_document_accesses_update_administrator_except_owner(
 
     new_values = {
         "id": uuid4(),
-        "user_id": factories.UserFactory().id,
         "role": random.choice(["administrator", "editor", "reader"]),
     }
+    if create_for == USER:
+        new_values["user_id"] = factories.UserFactory().id
+    elif create_for == TEAM:
+        new_values["team"] = "new-team"
 
     for field, value in new_values.items():
         new_data = {**old_values, field: value}
@@ -825,7 +830,9 @@ def test_api_document_accesses_update_administrator_to_owner(
 
 
 @pytest.mark.parametrize("via", VIA)
+@pytest.mark.parametrize("create_for", VIA)
 def test_api_document_accesses_update_owner(
+    create_for,
     via,
     mock_user_teams,
     mock_reset_connections,  # pylint: disable=redefined-outer-name
@@ -856,9 +863,12 @@ def test_api_document_accesses_update_owner(
 
     new_values = {
         "id": uuid4(),
-        "user_id": factories.UserFactory().id,
         "role": random.choice(models.RoleChoices.values),
     }
+    if create_for == USER:
+        new_values["user_id"] = factories.UserFactory().id
+    elif create_for == TEAM:
+        new_values["team"] = "new-team"
 
     for field, value in new_values.items():
         new_data = {**old_values, field: value}

src/backend/core/tests/documents/test_api_document_accesses_create.py

@@ -105,7 +105,7 @@ def test_api_document_accesses_create_authenticated_reader_or_editor(
 
 @pytest.mark.parametrize("depth", [1, 2, 3])
 @pytest.mark.parametrize("via", VIA)
-def test_api_document_accesses_create_authenticated_administrator(
+def test_api_document_accesses_create_authenticated_administrator_share_to_user(
     via, depth, mock_user_teams
 ):
     """
@@ -195,7 +195,90 @@ def test_api_document_accesses_create_authenticated_administrator(
 
 @pytest.mark.parametrize("depth", [1, 2, 3])
 @pytest.mark.parametrize("via", VIA)
-def test_api_document_accesses_create_authenticated_owner(via, depth, mock_user_teams):
+def test_api_document_accesses_create_authenticated_administrator_share_to_team(
+    via, depth, mock_user_teams
+):
+    """
+    Administrators of a document (direct or by heritage) should be able to create
+    document accesses 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)
+    client = APIClient()
+    client.force_login(user)
+
+    documents = []
+    for i in range(depth):
+        parent = documents[i - 1] if i > 0 else None
+        documents.append(factories.DocumentFactory(parent=parent))
+
+    if via == USER:
+        factories.UserDocumentAccessFactory(
+            document=documents[0], user=user, role="administrator"
+        )
+    elif via == TEAM:
+        mock_user_teams.return_value = ["lasuite", "unknown"]
+        factories.TeamDocumentAccessFactory(
+            document=documents[0], team="lasuite", role="administrator"
+        )
+
+    other_user = factories.UserFactory(language="en-us")
+    document = documents[-1]
+    response = client.post(
+        f"/api/v1.0/documents/{document.id!s}/accesses/",
+        {
+            "team": "new-team",
+            "role": "owner",
+        },
+        format="json",
+    )
+
+    assert response.status_code == 403
+    assert response.json() == {
+        "detail": "Only owners of a document can assign other users as owners."
+    }
+
+    # It should be allowed to create a lower access
+    role = random.choice(
+        [role[0] for role in models.RoleChoices.choices if role[0] != "owner"]
+    )
+
+    assert len(mail.outbox) == 0
+
+    response = client.post(
+        f"/api/v1.0/documents/{document.id!s}/accesses/",
+        {
+            "team": "new-team",
+            "role": role,
+        },
+        format="json",
+    )
+
+    assert response.status_code == 201
+    assert models.DocumentAccess.objects.filter(team="new-team").count() == 1
+    new_document_access = models.DocumentAccess.objects.filter(team="new-team").get()
+    other_user = serializers.UserSerializer(instance=other_user).data
+    assert response.json() == {
+        "abilities": new_document_access.get_abilities(user),
+        "document": {
+            "id": str(new_document_access.document_id),
+            "depth": new_document_access.document.depth,
+            "path": new_document_access.document.path,
+        },
+        "id": str(new_document_access.id),
+        "user": None,
+        "team": "new-team",
+        "role": role,
+        "max_ancestors_role": None,
+    }
+    assert len(mail.outbox) == 0
+
+
+@pytest.mark.parametrize("depth", [1, 2, 3])
+@pytest.mark.parametrize("via", VIA)
+def test_api_document_accesses_create_authenticated_owner_share_to_user(
+    via, depth, mock_user_teams
+):
     """
     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.
@@ -264,6 +347,70 @@ def test_api_document_accesses_create_authenticated_owner(via, depth, mock_user_
     assert "docs/" + str(document.id) + "/" in email_content
 
 
+@pytest.mark.parametrize("depth", [1, 2, 3])
+@pytest.mark.parametrize("via", VIA)
+def test_api_document_accesses_create_authenticated_owner_share_to_team(
+    via, depth, mock_user_teams
+):
+    """
+    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.
+    """
+    user = factories.UserFactory()
+
+    client = APIClient()
+    client.force_login(user)
+
+    documents = []
+    for i in range(depth):
+        parent = documents[i - 1] if i > 0 else None
+        documents.append(factories.DocumentFactory(parent=parent))
+
+    if via == USER:
+        factories.UserDocumentAccessFactory(
+            document=documents[0], user=user, role="owner"
+        )
+    elif via == TEAM:
+        mock_user_teams.return_value = ["lasuite", "unknown"]
+        factories.TeamDocumentAccessFactory(
+            document=documents[0], team="lasuite", role="owner"
+        )
+
+    other_user = factories.UserFactory(language="en-us")
+    document = documents[-1]
+    role = random.choice([role[0] for role in models.RoleChoices.choices])
+
+    assert len(mail.outbox) == 0
+
+    response = client.post(
+        f"/api/v1.0/documents/{document.id!s}/accesses/",
+        {
+            "team": "new-team",
+            "role": role,
+        },
+        format="json",
+    )
+
+    assert response.status_code == 201
+    assert models.DocumentAccess.objects.filter(team="new-team").count() == 1
+    new_document_access = models.DocumentAccess.objects.filter(team="new-team").get()
+    other_user = serializers.UserSerializer(instance=other_user).data
+    assert response.json() == {
+        "document": {
+            "id": str(new_document_access.document_id),
+            "path": new_document_access.document.path,
+            "depth": new_document_access.document.depth,
+        },
+        "id": str(new_document_access.id),
+        "user": None,
+        "team": "new-team",
+        "role": role,
+        "max_ancestors_role": None,
+        "abilities": new_document_access.get_abilities(user),
+    }
+    assert len(mail.outbox) == 0
+
+
 @pytest.mark.parametrize("via", VIA)
 def test_api_document_accesses_create_email_in_receivers_language(via, mock_user_teams):
     """