✨(api) allow updating link configuration for a document
We open a specific endpoint to update documents link configuration because it makes it more secure and simple to limit access rights to administrators/owners whereas other document fields like title and content can be edited by anonymous or authenticated users with much less access rights.
This commit is contained in:
committed by
Samuel Paccoud
parent
f5c4106547
commit
1e432cfdc2
@@ -164,6 +164,20 @@ class DocumentSerializer(BaseResourceSerializer):
|
|||||||
]
|
]
|
||||||
|
|
||||||
|
|
||||||
|
class LinkDocumentSerializer(BaseResourceSerializer):
|
||||||
|
"""
|
||||||
|
Serialize link configuration for documents.
|
||||||
|
We expose it separately from document in order to simplify and secure access control.
|
||||||
|
"""
|
||||||
|
|
||||||
|
class Meta:
|
||||||
|
model = models.Document
|
||||||
|
fields = [
|
||||||
|
"link_role",
|
||||||
|
"link_reach",
|
||||||
|
]
|
||||||
|
|
||||||
|
|
||||||
# Suppress the warning about not implementing `create` and `update` methods
|
# Suppress the warning about not implementing `create` and `update` methods
|
||||||
# since we don't use a model and only rely on the serializer for validation
|
# since we don't use a model and only rely on the serializer for validation
|
||||||
# pylint: disable=abstract-method
|
# pylint: disable=abstract-method
|
||||||
|
|||||||
@@ -451,6 +451,24 @@ class DocumentViewSet(
|
|||||||
}
|
}
|
||||||
)
|
)
|
||||||
|
|
||||||
|
@decorators.action(detail=True, methods=["put"], url_path="link-configuration")
|
||||||
|
def link_configuration(self, request, *args, **kwargs):
|
||||||
|
"""Update link configuration with specific rights (cf get_abilities)."""
|
||||||
|
# Check permissions first
|
||||||
|
document = self.get_object()
|
||||||
|
|
||||||
|
# Deserialize and validate the data
|
||||||
|
serializer = serializers.LinkDocumentSerializer(
|
||||||
|
document, data=request.data, partial=True
|
||||||
|
)
|
||||||
|
if not serializer.is_valid():
|
||||||
|
return drf_response.Response(
|
||||||
|
serializer.errors, status=status.HTTP_400_BAD_REQUEST
|
||||||
|
)
|
||||||
|
|
||||||
|
serializer.save()
|
||||||
|
return drf_response.Response(serializer.data, status=status.HTTP_200_OK)
|
||||||
|
|
||||||
@decorators.action(detail=True, methods=["post"], url_path="attachment-upload")
|
@decorators.action(detail=True, methods=["post"], url_path="attachment-upload")
|
||||||
def attachment_upload(self, request, *args, **kwargs):
|
def attachment_upload(self, request, *args, **kwargs):
|
||||||
"""Upload a file related to a given document"""
|
"""Upload a file related to a given document"""
|
||||||
|
|||||||
@@ -512,6 +512,7 @@ class Document(BaseModel):
|
|||||||
return {
|
return {
|
||||||
"attachment_upload": is_owner_or_admin or is_editor,
|
"attachment_upload": is_owner_or_admin or is_editor,
|
||||||
"destroy": RoleChoices.OWNER in roles,
|
"destroy": RoleChoices.OWNER in roles,
|
||||||
|
"link_configuration": is_owner_or_admin,
|
||||||
"manage_accesses": is_owner_or_admin,
|
"manage_accesses": is_owner_or_admin,
|
||||||
"partial_update": is_owner_or_admin or is_editor,
|
"partial_update": is_owner_or_admin or is_editor,
|
||||||
"retrieve": can_get,
|
"retrieve": can_get,
|
||||||
|
|||||||
@@ -0,0 +1,152 @@
|
|||||||
|
"""Tests for link configuration of documents on API endpoint"""
|
||||||
|
|
||||||
|
import pytest
|
||||||
|
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
|
||||||
|
|
||||||
|
|
||||||
|
@pytest.mark.parametrize("role", models.LinkRoleChoices.values)
|
||||||
|
@pytest.mark.parametrize("reach", models.LinkReachChoices.values)
|
||||||
|
def test_api_documents_link_configuration_update_anonymous(reach, role):
|
||||||
|
"""Anonymous users should not be allowed to update a link configuration."""
|
||||||
|
document = factories.DocumentFactory(link_reach=reach, link_role=role)
|
||||||
|
old_document_values = serializers.LinkDocumentSerializer(instance=document).data
|
||||||
|
|
||||||
|
new_document_values = serializers.LinkDocumentSerializer(
|
||||||
|
instance=factories.DocumentFactory()
|
||||||
|
).data
|
||||||
|
response = APIClient().put(
|
||||||
|
f"/api/v1.0/documents/{document.id!s}/link-configuration/",
|
||||||
|
new_document_values,
|
||||||
|
format="json",
|
||||||
|
)
|
||||||
|
assert response.status_code == 401
|
||||||
|
assert response.json() == {
|
||||||
|
"detail": "Authentication credentials were not provided."
|
||||||
|
}
|
||||||
|
|
||||||
|
document.refresh_from_db()
|
||||||
|
document_values = serializers.LinkDocumentSerializer(instance=document).data
|
||||||
|
assert document_values == old_document_values
|
||||||
|
|
||||||
|
|
||||||
|
@pytest.mark.parametrize("role", models.LinkRoleChoices.values)
|
||||||
|
@pytest.mark.parametrize("reach", models.LinkReachChoices.values)
|
||||||
|
def test_api_documents_link_configuration_update_authenticated_unrelated(reach, role):
|
||||||
|
"""
|
||||||
|
Authenticated users should not be allowed to update the link configuration for
|
||||||
|
a document to which they are not related.
|
||||||
|
"""
|
||||||
|
user = factories.UserFactory()
|
||||||
|
|
||||||
|
client = APIClient()
|
||||||
|
client.force_login(user)
|
||||||
|
|
||||||
|
document = factories.DocumentFactory(link_reach=reach, link_role=role)
|
||||||
|
old_document_values = serializers.LinkDocumentSerializer(instance=document).data
|
||||||
|
|
||||||
|
new_document_values = serializers.LinkDocumentSerializer(
|
||||||
|
instance=factories.DocumentFactory()
|
||||||
|
).data
|
||||||
|
response = client.put(
|
||||||
|
f"/api/v1.0/documents/{document.id!s}/link-configuration/",
|
||||||
|
new_document_values,
|
||||||
|
format="json",
|
||||||
|
)
|
||||||
|
|
||||||
|
assert response.status_code == 403
|
||||||
|
assert response.json() == {
|
||||||
|
"detail": "You do not have permission to perform this action."
|
||||||
|
}
|
||||||
|
|
||||||
|
document.refresh_from_db()
|
||||||
|
document_values = serializers.LinkDocumentSerializer(instance=document).data
|
||||||
|
assert document_values == old_document_values
|
||||||
|
|
||||||
|
|
||||||
|
@pytest.mark.parametrize("role", ["editor", "reader"])
|
||||||
|
@pytest.mark.parametrize("via", VIA)
|
||||||
|
def test_api_documents_link_configuration_update_authenticated_related_forbidden(
|
||||||
|
via, role, mock_user_teams
|
||||||
|
):
|
||||||
|
"""
|
||||||
|
Users who are readers or editors of a document should not be allowed to update
|
||||||
|
the link configuration.
|
||||||
|
"""
|
||||||
|
user = factories.UserFactory()
|
||||||
|
|
||||||
|
client = APIClient()
|
||||||
|
client.force_login(user)
|
||||||
|
|
||||||
|
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
|
||||||
|
)
|
||||||
|
|
||||||
|
old_document_values = serializers.LinkDocumentSerializer(instance=document).data
|
||||||
|
|
||||||
|
new_document_values = serializers.LinkDocumentSerializer(
|
||||||
|
instance=factories.DocumentFactory()
|
||||||
|
).data
|
||||||
|
response = client.put(
|
||||||
|
f"/api/v1.0/documents/{document.id!s}/link-configuration/",
|
||||||
|
new_document_values,
|
||||||
|
format="json",
|
||||||
|
)
|
||||||
|
|
||||||
|
assert response.status_code == 403
|
||||||
|
assert response.json() == {
|
||||||
|
"detail": "You do not have permission to perform this action."
|
||||||
|
}
|
||||||
|
|
||||||
|
document.refresh_from_db()
|
||||||
|
document_values = serializers.LinkDocumentSerializer(instance=document).data
|
||||||
|
assert document_values == old_document_values
|
||||||
|
|
||||||
|
|
||||||
|
@pytest.mark.parametrize("role", ["administrator", "owner"])
|
||||||
|
@pytest.mark.parametrize("via", VIA)
|
||||||
|
def test_api_documents_link_configuration_update_authenticated_related_success(
|
||||||
|
via, role, mock_user_teams
|
||||||
|
):
|
||||||
|
"""
|
||||||
|
A user who is administrator or owner of a document should be allowed to update
|
||||||
|
the link configuration.
|
||||||
|
"""
|
||||||
|
user = factories.UserFactory()
|
||||||
|
|
||||||
|
client = APIClient()
|
||||||
|
client.force_login(user)
|
||||||
|
|
||||||
|
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
|
||||||
|
)
|
||||||
|
|
||||||
|
new_document_values = serializers.LinkDocumentSerializer(
|
||||||
|
instance=factories.DocumentFactory()
|
||||||
|
).data
|
||||||
|
response = client.put(
|
||||||
|
f"/api/v1.0/documents/{document.id!s}/link-configuration/",
|
||||||
|
new_document_values,
|
||||||
|
format="json",
|
||||||
|
)
|
||||||
|
assert response.status_code == 200
|
||||||
|
|
||||||
|
document = models.Document.objects.get(pk=document.pk)
|
||||||
|
document_values = serializers.LinkDocumentSerializer(instance=document).data
|
||||||
|
for key, value in document_values.items():
|
||||||
|
assert value == new_document_values[key]
|
||||||
@@ -23,6 +23,7 @@ def test_api_documents_retrieve_anonymous_public():
|
|||||||
"abilities": {
|
"abilities": {
|
||||||
"attachment_upload": document.link_role == "editor",
|
"attachment_upload": document.link_role == "editor",
|
||||||
"destroy": False,
|
"destroy": False,
|
||||||
|
"link_configuration": False,
|
||||||
"manage_accesses": False,
|
"manage_accesses": False,
|
||||||
"partial_update": document.link_role == "editor",
|
"partial_update": document.link_role == "editor",
|
||||||
"retrieve": True,
|
"retrieve": True,
|
||||||
@@ -75,6 +76,7 @@ def test_api_documents_retrieve_authenticated_unrelated_public_or_authenticated(
|
|||||||
"id": str(document.id),
|
"id": str(document.id),
|
||||||
"abilities": {
|
"abilities": {
|
||||||
"attachment_upload": document.link_role == "editor",
|
"attachment_upload": document.link_role == "editor",
|
||||||
|
"link_configuration": False,
|
||||||
"destroy": False,
|
"destroy": False,
|
||||||
"manage_accesses": False,
|
"manage_accesses": False,
|
||||||
"partial_update": document.link_role == "editor",
|
"partial_update": document.link_role == "editor",
|
||||||
|
|||||||
@@ -78,6 +78,7 @@ def test_models_documents_get_abilities_forbidden(is_authenticated, reach, role)
|
|||||||
abilities = document.get_abilities(user)
|
abilities = document.get_abilities(user)
|
||||||
assert abilities == {
|
assert abilities == {
|
||||||
"attachment_upload": False,
|
"attachment_upload": False,
|
||||||
|
"link_configuration": False,
|
||||||
"destroy": False,
|
"destroy": False,
|
||||||
"manage_accesses": False,
|
"manage_accesses": False,
|
||||||
"partial_update": False,
|
"partial_update": False,
|
||||||
@@ -108,6 +109,7 @@ def test_models_documents_get_abilities_reader(is_authenticated, reach):
|
|||||||
assert abilities == {
|
assert abilities == {
|
||||||
"attachment_upload": False,
|
"attachment_upload": False,
|
||||||
"destroy": False,
|
"destroy": False,
|
||||||
|
"link_configuration": False,
|
||||||
"manage_accesses": False,
|
"manage_accesses": False,
|
||||||
"partial_update": False,
|
"partial_update": False,
|
||||||
"retrieve": True,
|
"retrieve": True,
|
||||||
@@ -137,6 +139,7 @@ def test_models_documents_get_abilities_editor(is_authenticated, reach):
|
|||||||
assert abilities == {
|
assert abilities == {
|
||||||
"attachment_upload": True,
|
"attachment_upload": True,
|
||||||
"destroy": False,
|
"destroy": False,
|
||||||
|
"link_configuration": False,
|
||||||
"manage_accesses": False,
|
"manage_accesses": False,
|
||||||
"partial_update": True,
|
"partial_update": True,
|
||||||
"retrieve": True,
|
"retrieve": True,
|
||||||
@@ -155,6 +158,7 @@ def test_models_documents_get_abilities_owner():
|
|||||||
assert abilities == {
|
assert abilities == {
|
||||||
"attachment_upload": True,
|
"attachment_upload": True,
|
||||||
"destroy": True,
|
"destroy": True,
|
||||||
|
"link_configuration": True,
|
||||||
"manage_accesses": True,
|
"manage_accesses": True,
|
||||||
"partial_update": True,
|
"partial_update": True,
|
||||||
"retrieve": True,
|
"retrieve": True,
|
||||||
@@ -172,6 +176,7 @@ def test_models_documents_get_abilities_administrator():
|
|||||||
assert abilities == {
|
assert abilities == {
|
||||||
"attachment_upload": True,
|
"attachment_upload": True,
|
||||||
"destroy": False,
|
"destroy": False,
|
||||||
|
"link_configuration": True,
|
||||||
"manage_accesses": True,
|
"manage_accesses": True,
|
||||||
"partial_update": True,
|
"partial_update": True,
|
||||||
"retrieve": True,
|
"retrieve": True,
|
||||||
@@ -192,6 +197,7 @@ def test_models_documents_get_abilities_editor_user(django_assert_num_queries):
|
|||||||
assert abilities == {
|
assert abilities == {
|
||||||
"attachment_upload": True,
|
"attachment_upload": True,
|
||||||
"destroy": False,
|
"destroy": False,
|
||||||
|
"link_configuration": False,
|
||||||
"manage_accesses": False,
|
"manage_accesses": False,
|
||||||
"partial_update": True,
|
"partial_update": True,
|
||||||
"retrieve": True,
|
"retrieve": True,
|
||||||
@@ -214,6 +220,7 @@ def test_models_documents_get_abilities_reader_user(django_assert_num_queries):
|
|||||||
assert abilities == {
|
assert abilities == {
|
||||||
"attachment_upload": False,
|
"attachment_upload": False,
|
||||||
"destroy": False,
|
"destroy": False,
|
||||||
|
"link_configuration": False,
|
||||||
"manage_accesses": False,
|
"manage_accesses": False,
|
||||||
"partial_update": False,
|
"partial_update": False,
|
||||||
"retrieve": True,
|
"retrieve": True,
|
||||||
@@ -237,6 +244,7 @@ def test_models_documents_get_abilities_preset_role(django_assert_num_queries):
|
|||||||
assert abilities == {
|
assert abilities == {
|
||||||
"attachment_upload": False,
|
"attachment_upload": False,
|
||||||
"destroy": False,
|
"destroy": False,
|
||||||
|
"link_configuration": False,
|
||||||
"manage_accesses": False,
|
"manage_accesses": False,
|
||||||
"partial_update": False,
|
"partial_update": False,
|
||||||
"retrieve": True,
|
"retrieve": True,
|
||||||
|
|||||||
Reference in New Issue
Block a user