Skip to content

Commit d85d20f

Browse files
committed
✨(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 64c312b commit d85d20f

File tree

4 files changed

+112
-82
lines changed

4 files changed

+112
-82
lines changed

src/backend/core/api/serializers.py

+8-4
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

+23-13
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,6 @@
77

88
from django.conf import settings
99
from django.contrib.postgres.aggregates import ArrayAgg
10-
from django.contrib.postgres.fields import ArrayField
1110
from django.contrib.postgres.search import TrigramSimilarity
1211
from django.core.exceptions import ValidationError
1312
from django.core.files.storage import default_storage
@@ -1333,12 +1332,10 @@ class DocumentAccessViewSet(
13331332
queryset = models.DocumentAccess.objects.select_related("user").all()
13341333
resource_field_name = "document"
13351334
serializer_class = serializers.DocumentAccessSerializer
1336-
is_current_user_owner_or_admin = False
13371335

13381336
def list(self, request, *args, **kwargs):
13391337
"""Return accesses for the current document with filters and annotations."""
13401338
user = self.request.user
1341-
queryset = self.filter_queryset(self.get_queryset())
13421339

13431340
try:
13441341
document = models.Document.objects.get(pk=self.kwargs["resource_id"])
@@ -1349,22 +1346,35 @@ def list(self, request, *args, **kwargs):
13491346
if not roles:
13501347
return drf.response.Response([])
13511348

1352-
is_owner_or_admin = bool(roles.intersection(set(models.PRIVILEGED_ROLES)))
1353-
self.is_current_user_owner_or_admin = is_owner_or_admin
1354-
if not is_owner_or_admin:
1349+
ancestors = (
1350+
(document.get_ancestors() | models.Document.objects.filter(pk=document.pk))
1351+
.filter(ancestors_deleted_at__isnull=True)
1352+
.order_by("path")
1353+
)
1354+
highest_readable = ancestors.readable_per_se(user).only("depth").first()
1355+
1356+
if highest_readable is None:
1357+
return drf.response.Response([])
1358+
1359+
queryset = self.get_queryset()
1360+
queryset = queryset.filter(
1361+
document__in=ancestors.filter(depth__gte=highest_readable.depth)
1362+
)
1363+
1364+
is_privileged = bool(roles.intersection(set(models.PRIVILEGED_ROLES)))
1365+
if is_privileged:
1366+
serializer_class = serializers.DocumentAccessSerializer
1367+
else:
13551368
# Return only the document's privileged accesses
13561369
queryset = queryset.filter(role__in=models.PRIVILEGED_ROLES)
1370+
serializer_class = serializers.DocumentAccessLightSerializer
13571371

13581372
queryset = queryset.distinct()
1359-
serializer = self.get_serializer(queryset, many=True)
1373+
serializer = serializer_class(
1374+
queryset, many=True, context=self.get_serializer_context()
1375+
)
13601376
return drf.response.Response(serializer.data)
13611377

1362-
def get_serializer_class(self):
1363-
if self.action == "list" and not self.is_current_user_owner_or_admin:
1364-
return serializers.DocumentAccessLightSerializer
1365-
1366-
return super().get_serializer_class()
1367-
13681378
def perform_create(self, serializer):
13691379
"""Add a new access to the document and send an email to the new added user."""
13701380
access = serializer.save()

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

+78-65
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

+3
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)