Skip to content

Commit 40785f3

Browse files
sampaccoudPanchoutNathan
authored andcommitted
✨(backend) we want to display ancestors accesses on a document share
The document accesses a user have on a document's ancestors also apply to this document. The frontend needs to list them as "inherited" so we need to add them to the list. Adding a "document_id" field on the output will allow the frontend to differentiate between inherited and direct accesses on a document.
1 parent 149f711 commit 40785f3

File tree

5 files changed

+113
-82
lines changed

5 files changed

+113
-82
lines changed

CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -85,6 +85,7 @@ and this project adheres to
8585

8686
## Added
8787

88+
- ✨(backend) include ancestors accesses on document accesses list view # 846
8889
- ✨(backend) add ancestors links definitions to document abilities #846
8990
- 🚸(backend) make document search on title accent-insensitive #874
9091
- 🚩 add homepage feature flag #861

src/backend/core/api/serializers.py

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -97,7 +97,7 @@ def validate(self, attrs):
9797

9898
if not self.Meta.model.objects.filter( # pylint: disable=no-member
9999
Q(user=user) | Q(team__in=user.teams),
100-
role__in=[models.RoleChoices.OWNER, models.RoleChoices.ADMIN],
100+
role__in=models.PRIVILEGED_ROLES,
101101
**{self.Meta.resource_field_name: resource_id}, # pylint: disable=no-member
102102
).exists():
103103
raise exceptions.PermissionDenied(
@@ -124,6 +124,10 @@ def validate(self, attrs):
124124
class DocumentAccessSerializer(BaseAccessSerializer):
125125
"""Serialize document accesses."""
126126

127+
document_id = serializers.PrimaryKeyRelatedField(
128+
read_only=True,
129+
source="document",
130+
)
127131
user_id = serializers.PrimaryKeyRelatedField(
128132
queryset=models.User.objects.all(),
129133
write_only=True,
@@ -136,11 +140,11 @@ class DocumentAccessSerializer(BaseAccessSerializer):
136140
class Meta:
137141
model = models.DocumentAccess
138142
resource_field_name = "document"
139-
fields = ["id", "user", "user_id", "team", "role", "abilities"]
140-
read_only_fields = ["id", "abilities"]
143+
fields = ["id", "document_id", "user", "user_id", "team", "role", "abilities"]
144+
read_only_fields = ["id", "document_id", "abilities"]
141145

142146

143-
class DocumentAccessLightSerializer(DocumentAccessSerializer):
147+
class DocumentAccessLightSerializer(BaseAccessSerializer):
144148
"""Serialize document accesses with limited fields."""
145149

146150
user = UserLightSerializer(read_only=True)

src/backend/core/api/viewsets.py

Lines changed: 23 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,6 @@
88

99
from django.conf import settings
1010
from django.contrib.postgres.aggregates import ArrayAgg
11-
from django.contrib.postgres.fields import ArrayField
1211
from django.contrib.postgres.search import TrigramSimilarity
1312
from django.core.cache import cache
1413
from django.core.exceptions import ValidationError
@@ -1432,12 +1431,10 @@ class DocumentAccessViewSet(
14321431
queryset = models.DocumentAccess.objects.select_related("user").all()
14331432
resource_field_name = "document"
14341433
serializer_class = serializers.DocumentAccessSerializer
1435-
is_current_user_owner_or_admin = False
14361434

14371435
def list(self, request, *args, **kwargs):
14381436
"""Return accesses for the current document with filters and annotations."""
14391437
user = self.request.user
1440-
queryset = self.filter_queryset(self.get_queryset())
14411438

14421439
try:
14431440
document = models.Document.objects.get(pk=self.kwargs["resource_id"])
@@ -1448,22 +1445,35 @@ def list(self, request, *args, **kwargs):
14481445
if not roles:
14491446
return drf.response.Response([])
14501447

1451-
is_owner_or_admin = bool(roles.intersection(set(models.PRIVILEGED_ROLES)))
1452-
self.is_current_user_owner_or_admin = is_owner_or_admin
1453-
if not is_owner_or_admin:
1448+
ancestors = (
1449+
(document.get_ancestors() | models.Document.objects.filter(pk=document.pk))
1450+
.filter(ancestors_deleted_at__isnull=True)
1451+
.order_by("path")
1452+
)
1453+
highest_readable = ancestors.readable_per_se(user).only("depth").first()
1454+
1455+
if highest_readable is None:
1456+
return drf.response.Response([])
1457+
1458+
queryset = self.get_queryset()
1459+
queryset = queryset.filter(
1460+
document__in=ancestors.filter(depth__gte=highest_readable.depth)
1461+
)
1462+
1463+
is_privileged = bool(roles.intersection(set(models.PRIVILEGED_ROLES)))
1464+
if is_privileged:
1465+
serializer_class = serializers.DocumentAccessSerializer
1466+
else:
14541467
# Return only the document's privileged accesses
14551468
queryset = queryset.filter(role__in=models.PRIVILEGED_ROLES)
1469+
serializer_class = serializers.DocumentAccessLightSerializer
14561470

14571471
queryset = queryset.distinct()
1458-
serializer = self.get_serializer(queryset, many=True)
1472+
serializer = serializer_class(
1473+
queryset, many=True, context=self.get_serializer_context()
1474+
)
14591475
return drf.response.Response(serializer.data)
14601476

1461-
def get_serializer_class(self):
1462-
if self.action == "list" and not self.is_current_user_owner_or_admin:
1463-
return serializers.DocumentAccessLightSerializer
1464-
1465-
return super().get_serializer_class()
1466-
14671477
def perform_create(self, serializer):
14681478
"""Add a new access to the document and send an email to the new added user."""
14691479
access = serializer.save()

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

Lines changed: 78 additions & 65 deletions
Original file line numberDiff line numberDiff line change
@@ -76,22 +76,30 @@ def test_api_document_accesses_list_authenticated_related_non_privileged(
7676
via, role, mock_user_teams
7777
):
7878
"""
79-
Authenticated users should be able to list document accesses for a document
80-
to which they are directly related, whatever their role in the document.
79+
Authenticated users with no privileged role should only be able to list document
80+
accesses associated with privileged roles for a document, including from ancestors.
8181
"""
8282
user = factories.UserFactory()
83-
8483
client = APIClient()
8584
client.force_login(user)
8685

87-
owner = factories.UserFactory()
88-
accesses = []
89-
90-
document_access = factories.UserDocumentAccessFactory(
91-
user=owner, role=models.RoleChoices.OWNER
86+
# Create documents structured as a tree
87+
unreadable_ancestor = factories.DocumentFactory(link_reach="restricted")
88+
# make all documents below the grand parent readable without a specific access for the user
89+
grand_parent = factories.DocumentFactory(
90+
parent=unreadable_ancestor, link_reach="authenticated"
9291
)
93-
accesses.append(document_access)
94-
document = document_access.document
92+
parent = factories.DocumentFactory(parent=grand_parent)
93+
document = factories.DocumentFactory(parent=parent)
94+
child = factories.DocumentFactory(parent=document)
95+
96+
# Create accesses related to each document
97+
factories.UserDocumentAccessFactory(document=unreadable_ancestor)
98+
grand_parent_access = factories.UserDocumentAccessFactory(document=grand_parent)
99+
parent_access = factories.UserDocumentAccessFactory(document=parent)
100+
document_access = factories.UserDocumentAccessFactory(document=document)
101+
factories.UserDocumentAccessFactory(document=child)
102+
95103
if via == USER:
96104
models.DocumentAccess.objects.create(
97105
document=document,
@@ -108,8 +116,6 @@ def test_api_document_accesses_list_authenticated_related_non_privileged(
108116

109117
access1 = factories.TeamDocumentAccessFactory(document=document)
110118
access2 = factories.UserDocumentAccessFactory(document=document)
111-
accesses.append(access1)
112-
accesses.append(access2)
113119

114120
# Accesses for other documents to which the user is related should not be listed either
115121
other_access = factories.UserDocumentAccessFactory(user=user)
@@ -119,13 +125,16 @@ def test_api_document_accesses_list_authenticated_related_non_privileged(
119125
f"/api/v1.0/documents/{document.id!s}/accesses/",
120126
)
121127

122-
# Return only privileged roles
123-
privileged_accesses = [
124-
access for access in accesses if access.role in models.PRIVILEGED_ROLES
125-
]
126128
assert response.status_code == 200
127129
content = response.json()
130+
131+
# Make sure only privileged roles are returned
132+
accesses = [grand_parent_access, parent_access, document_access, access1, access2]
133+
privileged_accesses = [
134+
acc for acc in accesses if acc.role in models.PRIVILEGED_ROLES
135+
]
128136
assert len(content) == len(privileged_accesses)
137+
129138
assert sorted(content, key=lambda x: x["id"]) == sorted(
130139
[
131140
{
@@ -147,33 +156,39 @@ def test_api_document_accesses_list_authenticated_related_non_privileged(
147156
key=lambda x: x["id"],
148157
)
149158

150-
for access in content:
151-
assert access["role"] in models.PRIVILEGED_ROLES
152-
153159

154160
@pytest.mark.parametrize("via", VIA)
155-
@pytest.mark.parametrize("role", models.PRIVILEGED_ROLES)
156-
def test_api_document_accesses_list_authenticated_related_privileged_roles(
161+
@pytest.mark.parametrize(
162+
"role", [role for role in models.RoleChoices if role in models.PRIVILEGED_ROLES]
163+
)
164+
def test_api_document_accesses_list_authenticated_related_privileged(
157165
via, role, mock_user_teams
158166
):
159167
"""
160-
Authenticated users should be able to list document accesses for a document
161-
to which they are directly related, whatever their role in the document.
168+
Authenticated users with a privileged role should be able to list all
169+
document accesses whatever the role, including from ancestors.
162170
"""
163171
user = factories.UserFactory()
164-
165172
client = APIClient()
166173
client.force_login(user)
167174

168-
owner = factories.UserFactory()
169-
accesses = []
170-
171-
document_access = factories.UserDocumentAccessFactory(
172-
user=owner, role=models.RoleChoices.OWNER
175+
# Create documents structured as a tree
176+
unreadable_ancestor = factories.DocumentFactory(link_reach="restricted")
177+
# make all documents below the grand parent readable without a specific access for the user
178+
grand_parent = factories.DocumentFactory(
179+
parent=unreadable_ancestor, link_reach="authenticated"
173180
)
174-
accesses.append(document_access)
175-
document = document_access.document
176-
user_access = None
181+
parent = factories.DocumentFactory(parent=grand_parent)
182+
document = factories.DocumentFactory(parent=parent)
183+
child = factories.DocumentFactory(parent=document)
184+
185+
# Create accesses related to each document
186+
factories.UserDocumentAccessFactory(document=unreadable_ancestor)
187+
grand_parent_access = factories.UserDocumentAccessFactory(document=grand_parent)
188+
parent_access = factories.UserDocumentAccessFactory(document=parent)
189+
document_access = factories.UserDocumentAccessFactory(document=document)
190+
factories.UserDocumentAccessFactory(document=child)
191+
177192
if via == USER:
178193
user_access = models.DocumentAccess.objects.create(
179194
document=document,
@@ -187,11 +202,11 @@ def test_api_document_accesses_list_authenticated_related_privileged_roles(
187202
team="lasuite",
188203
role=role,
189204
)
205+
else:
206+
raise RuntimeError()
190207

191208
access1 = factories.TeamDocumentAccessFactory(document=document)
192209
access2 = factories.UserDocumentAccessFactory(document=document)
193-
accesses.append(access1)
194-
accesses.append(access2)
195210

196211
# Accesses for other documents to which the user is related should not be listed either
197212
other_access = factories.UserDocumentAccessFactory(user=user)
@@ -201,42 +216,39 @@ def test_api_document_accesses_list_authenticated_related_privileged_roles(
201216
f"/api/v1.0/documents/{document.id!s}/accesses/",
202217
)
203218

204-
access2_user = serializers.UserSerializer(instance=access2.user).data
205-
base_user = serializers.UserSerializer(instance=user).data
206-
207219
assert response.status_code == 200
208220
content = response.json()
209-
assert len(content) == 4
221+
222+
# Make sure all expected accesses are returned
223+
accesses = [
224+
user_access,
225+
grand_parent_access,
226+
parent_access,
227+
document_access,
228+
access1,
229+
access2,
230+
]
231+
assert len(content) == 6
232+
210233
assert sorted(content, key=lambda x: x["id"]) == sorted(
211234
[
212235
{
213-
"id": str(user_access.id),
214-
"user": base_user if via == "user" else None,
215-
"team": "lasuite" if via == "team" else "",
216-
"role": user_access.role,
217-
"abilities": user_access.get_abilities(user),
218-
},
219-
{
220-
"id": str(access1.id),
221-
"user": None,
222-
"team": access1.team,
223-
"role": access1.role,
224-
"abilities": access1.get_abilities(user),
225-
},
226-
{
227-
"id": str(access2.id),
228-
"user": access2_user,
229-
"team": "",
230-
"role": access2.role,
231-
"abilities": access2.get_abilities(user),
232-
},
233-
{
234-
"id": str(document_access.id),
235-
"user": serializers.UserSerializer(instance=owner).data,
236-
"team": "",
237-
"role": models.RoleChoices.OWNER,
238-
"abilities": document_access.get_abilities(user),
239-
},
236+
"id": str(access.id),
237+
"document_id": str(access.document_id),
238+
"user": {
239+
"id": str(access.user.id),
240+
"email": access.user.email,
241+
"language": access.user.language,
242+
"full_name": access.user.full_name,
243+
"short_name": access.user.short_name,
244+
}
245+
if access.user
246+
else None,
247+
"team": access.team,
248+
"role": access.role,
249+
"abilities": access.get_abilities(user),
250+
}
251+
for access in accesses
240252
],
241253
key=lambda x: x["id"],
242254
)
@@ -331,6 +343,7 @@ def test_api_document_accesses_retrieve_authenticated_related(
331343
assert response.status_code == 200
332344
assert response.json() == {
333345
"id": str(access.id),
346+
"document_id": str(access.document_id),
334347
"user": access_user,
335348
"team": "",
336349
"role": access.role,

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

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -165,6 +165,7 @@ def test_api_document_accesses_create_authenticated_administrator(via, mock_user
165165
other_user = serializers.UserSerializer(instance=other_user).data
166166
assert response.json() == {
167167
"abilities": new_document_access.get_abilities(user),
168+
"document_id": str(new_document_access.document_id),
168169
"id": str(new_document_access.id),
169170
"team": "",
170171
"role": role,
@@ -222,6 +223,7 @@ def test_api_document_accesses_create_authenticated_owner(via, mock_user_teams):
222223
new_document_access = models.DocumentAccess.objects.filter(user=other_user).get()
223224
other_user = serializers.UserSerializer(instance=other_user).data
224225
assert response.json() == {
226+
"document_id": str(new_document_access.document_id),
225227
"id": str(new_document_access.id),
226228
"user": other_user,
227229
"team": "",
@@ -286,6 +288,7 @@ def test_api_document_accesses_create_email_in_receivers_language(via, mock_user
286288
).get()
287289
other_user_data = serializers.UserSerializer(instance=other_user).data
288290
assert response.json() == {
291+
"document_id": str(new_document_access.document_id),
289292
"id": str(new_document_access.id),
290293
"user": other_user_data,
291294
"team": "",

0 commit comments

Comments
 (0)