🚸(backend) validate document access roles on submit
The frontend should only let users choose override roles that make sense.
This commit is contained in:
@@ -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)
|
||||
|
||||
@@ -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,
|
||||
|
||||
@@ -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
|
||||
|
||||
|
||||
Reference in New Issue
Block a user