mirror of
https://github.com/suitenumerique/drive.git
synced 2026-08-17 20:15:40 +02:00
♻️(backend) limit accesses list to the last access for a user
We don't need the whole accesses list for all the users on the list endpoint. We want to return the last access, meaning the access on the deepest item in the items tree.
This commit is contained in:
@@ -4,6 +4,7 @@
|
||||
import json
|
||||
import logging
|
||||
import re
|
||||
from collections import defaultdict
|
||||
from urllib.parse import unquote, urlparse
|
||||
|
||||
from django.conf import settings
|
||||
@@ -1415,30 +1416,36 @@ class ItemAccessViewSet(
|
||||
)
|
||||
ancestor_items = list(ancestors_qs.order_by("path"))
|
||||
|
||||
queryset = self.get_queryset().filter(item__in=ancestors_qs)
|
||||
accesses_queryset = self.get_queryset().filter(item__in=ancestors_qs)
|
||||
if role not in PRIVILEGED_ROLES:
|
||||
# Restrict the queryset to only privileged roles
|
||||
# to not leak information to non-privileged users
|
||||
queryset = queryset.filter(role__in=PRIVILEGED_ROLES)
|
||||
|
||||
accesses = list(queryset.order_by("item__path", "created_at"))
|
||||
accesses_queryset = accesses_queryset.filter(role__in=PRIVILEGED_ROLES)
|
||||
|
||||
accesses = list(accesses_queryset.order_by("item__path", "created_at"))
|
||||
|
||||
request_target_keys = {f"user:{user.id}"}
|
||||
request_target_keys.update(
|
||||
f"team:{team}" for team in getattr(user, "teams", [])
|
||||
)
|
||||
|
||||
max_role_by_target = {}
|
||||
user_roles_by_path = {}
|
||||
accesses_by_target = {}
|
||||
accesses_by_target = defaultdict(list)
|
||||
for access in accesses:
|
||||
previous_max_role_ancestor = max_role_by_target.get(access.target_key, {})
|
||||
previous_role = previous_max_role_ancestor.get("role")
|
||||
previous = max_role_by_target.get(access.target_key)
|
||||
previous_role = previous["role"] if previous else None
|
||||
access.max_ancestors_role = previous_role
|
||||
access.max_ancestors_role_item_id = previous_max_role_ancestor.get(
|
||||
"item_id"
|
||||
access.max_ancestors_role_item_id = (
|
||||
previous["item_id"] if previous else None
|
||||
)
|
||||
|
||||
max_role_by_target[access.target_key] = {
|
||||
"role": models.RoleChoices.max(previous_role, access.role),
|
||||
"item_id": access.item_id,
|
||||
}
|
||||
|
||||
accesses_by_target.setdefault(access.target_key, []).append(access)
|
||||
accesses_by_target[access.target_key].append(access)
|
||||
|
||||
if access.target_key in request_target_keys:
|
||||
path = str(access.item.path)
|
||||
@@ -1451,22 +1458,23 @@ class ItemAccessViewSet(
|
||||
for ancestor in ancestor_items:
|
||||
path = str(ancestor.path)
|
||||
parent_path = str(ancestor.path[:-1]) if ancestor.depth > 1 else ""
|
||||
ancestors_role = user_max_role_by_path.get(parent_path)
|
||||
user_ancestor_role_by_path[path] = ancestors_role
|
||||
ancestor_role = user_max_role_by_path.get(parent_path)
|
||||
user_ancestor_role_by_path[path] = ancestor_role
|
||||
user_max_role_by_path[path] = models.RoleChoices.max(
|
||||
ancestors_role, user_roles_by_path.get(path)
|
||||
ancestor_role, user_roles_by_path.get(path)
|
||||
)
|
||||
|
||||
selected_access_ids = set()
|
||||
for target_accesses in accesses_by_target.values():
|
||||
target_accesses.sort(
|
||||
selected_access_ids = {
|
||||
max(
|
||||
target_accesses,
|
||||
key=lambda item_access: (
|
||||
item_access.item.depth if item_access.item_id else 0,
|
||||
item_access.created_at,
|
||||
),
|
||||
reverse=True,
|
||||
)
|
||||
selected_access_ids.update(access.pk for access in target_accesses[:2])
|
||||
).pk
|
||||
for target_accesses in accesses_by_target.values()
|
||||
if target_accesses
|
||||
}
|
||||
|
||||
# serialize and return the response
|
||||
context = self.get_serializer_context()
|
||||
|
||||
@@ -325,16 +325,15 @@ def test_api_item_accesses_retrieve_set_role_to_child():
|
||||
assert result_dict[str(parent_access.id)] == []
|
||||
|
||||
# Add an access for the other user on the parent
|
||||
parent_access_other_user = factories.UserItemAccessFactory(
|
||||
item=parent, user=other_user, role="editor"
|
||||
)
|
||||
factories.UserItemAccessFactory(item=parent, user=other_user, role="editor")
|
||||
|
||||
response = client.get(f"/api/v1.0/items/{item.id!s}/accesses/")
|
||||
|
||||
assert response.status_code == 200
|
||||
content = response.json()
|
||||
assert len(content) == 3
|
||||
assert len(content) == 2
|
||||
|
||||
# the new added access is not present in the result so the list should be the same
|
||||
result_dict = {
|
||||
result["id"]: result["abilities"]["set_role_to"] for result in content
|
||||
}
|
||||
@@ -344,12 +343,6 @@ def test_api_item_accesses_retrieve_set_role_to_child():
|
||||
"owner",
|
||||
]
|
||||
assert result_dict[str(parent_access.id)] == []
|
||||
assert result_dict[str(parent_access_other_user.id)] == [
|
||||
"reader",
|
||||
"editor",
|
||||
"administrator",
|
||||
"owner",
|
||||
]
|
||||
|
||||
|
||||
@pytest.mark.parametrize(
|
||||
@@ -359,7 +352,6 @@ def test_api_item_accesses_retrieve_set_role_to_child():
|
||||
["administrator", "reader", "reader", "reader"],
|
||||
[
|
||||
["reader", "editor", "administrator"],
|
||||
[],
|
||||
["reader", "editor", "administrator"],
|
||||
],
|
||||
],
|
||||
@@ -367,7 +359,6 @@ def test_api_item_accesses_retrieve_set_role_to_child():
|
||||
["owner", "reader", "reader", "reader"],
|
||||
[
|
||||
["reader", "editor", "administrator", "owner"],
|
||||
[],
|
||||
["reader", "editor", "administrator", "owner"],
|
||||
],
|
||||
],
|
||||
@@ -375,7 +366,6 @@ def test_api_item_accesses_retrieve_set_role_to_child():
|
||||
["owner", "reader", "reader", "owner"],
|
||||
[
|
||||
["reader", "editor", "administrator", "owner"],
|
||||
[],
|
||||
["reader", "editor", "administrator", "owner"],
|
||||
],
|
||||
],
|
||||
@@ -402,9 +392,9 @@ def test_api_item_accesses_list_authenticated_related_same_user(roles, results):
|
||||
# Create accesses for another user
|
||||
other_user = factories.UserFactory()
|
||||
factories.UserItemAccessFactory(item=grand_parent, user=other_user, role=roles[1])
|
||||
factories.UserItemAccessFactory(item=parent, user=other_user, role=roles[2])
|
||||
accesses = [
|
||||
factories.UserItemAccessFactory(item=item, user=user, role=roles[0]),
|
||||
factories.UserItemAccessFactory(item=parent, user=other_user, role=roles[2]),
|
||||
factories.UserItemAccessFactory(item=item, user=other_user, role=roles[3]),
|
||||
]
|
||||
|
||||
@@ -412,7 +402,7 @@ def test_api_item_accesses_list_authenticated_related_same_user(roles, results):
|
||||
|
||||
assert response.status_code == 200
|
||||
content = response.json()
|
||||
assert len(content) == 3
|
||||
assert len(content) == 2
|
||||
|
||||
for result in content:
|
||||
assert (
|
||||
@@ -434,7 +424,6 @@ def test_api_item_accesses_list_authenticated_related_same_user(roles, results):
|
||||
["administrator", "reader", "reader", "reader"],
|
||||
[
|
||||
["reader", "editor", "administrator"],
|
||||
[],
|
||||
["reader", "editor", "administrator"],
|
||||
],
|
||||
],
|
||||
@@ -442,7 +431,6 @@ def test_api_item_accesses_list_authenticated_related_same_user(roles, results):
|
||||
["owner", "reader", "reader", "reader"],
|
||||
[
|
||||
["reader", "editor", "administrator", "owner"],
|
||||
[],
|
||||
["reader", "editor", "administrator", "owner"],
|
||||
],
|
||||
],
|
||||
@@ -450,7 +438,6 @@ def test_api_item_accesses_list_authenticated_related_same_user(roles, results):
|
||||
["owner", "reader", "reader", "owner"],
|
||||
[
|
||||
["reader", "editor", "administrator", "owner"],
|
||||
[],
|
||||
["reader", "editor", "administrator", "owner"],
|
||||
],
|
||||
],
|
||||
@@ -458,7 +445,6 @@ def test_api_item_accesses_list_authenticated_related_same_user(roles, results):
|
||||
["reader", "reader", "reader", "owner"],
|
||||
[
|
||||
["reader", "editor", "administrator", "owner"],
|
||||
[],
|
||||
["reader", "editor", "administrator", "owner"],
|
||||
],
|
||||
],
|
||||
@@ -467,7 +453,6 @@ def test_api_item_accesses_list_authenticated_related_same_user(roles, results):
|
||||
[
|
||||
["reader", "editor", "administrator"],
|
||||
[],
|
||||
[],
|
||||
],
|
||||
],
|
||||
[
|
||||
@@ -475,7 +460,6 @@ def test_api_item_accesses_list_authenticated_related_same_user(roles, results):
|
||||
[
|
||||
["reader", "editor", "administrator"],
|
||||
[],
|
||||
[],
|
||||
],
|
||||
],
|
||||
],
|
||||
@@ -502,10 +486,10 @@ def test_api_item_accesses_list_authenticated_related_same_team(
|
||||
|
||||
mock_user_teams.return_value = ["lasuite", "unknown"]
|
||||
factories.TeamItemAccessFactory(item=grand_parent, team="lasuite", role=roles[1])
|
||||
factories.TeamItemAccessFactory(item=parent, team="lasuite", role=roles[2])
|
||||
accesses = [
|
||||
factories.UserItemAccessFactory(item=item, user=user, role=roles[0]),
|
||||
# Create accesses for a team
|
||||
factories.TeamItemAccessFactory(item=parent, team="lasuite", role=roles[2]),
|
||||
factories.TeamItemAccessFactory(item=item, team="lasuite", role=roles[3]),
|
||||
]
|
||||
|
||||
@@ -513,7 +497,7 @@ def test_api_item_accesses_list_authenticated_related_same_team(
|
||||
|
||||
assert response.status_code == 200
|
||||
content = response.json()
|
||||
assert len(content) == 3
|
||||
assert len(content) == 2
|
||||
|
||||
for result in content:
|
||||
assert (
|
||||
@@ -1523,7 +1507,7 @@ def test_api_item_accesses_explicit():
|
||||
item = factories.ItemFactory(parent=parent, type=models.ItemTypeChoices.FOLDER)
|
||||
|
||||
# explicit access on root.
|
||||
root_access = factories.UserItemAccessFactory(item=root, user=user, role="editor")
|
||||
factories.UserItemAccessFactory(item=root, user=user, role="editor")
|
||||
# explicit access on item.
|
||||
item_access = factories.UserItemAccessFactory(item=item, user=user, role="owner")
|
||||
other_admin_access = factories.UserItemAccessFactory(
|
||||
@@ -1537,34 +1521,6 @@ def test_api_item_accesses_explicit():
|
||||
assert response.status_code == 200
|
||||
content = response.json()
|
||||
assert content == [
|
||||
{
|
||||
"id": str(root_access.id),
|
||||
"item": {
|
||||
"id": str(root.id),
|
||||
"path": str(root.path),
|
||||
"depth": root.depth,
|
||||
},
|
||||
"user": {
|
||||
"id": str(user.id),
|
||||
"email": user.email,
|
||||
"full_name": user.full_name,
|
||||
"short_name": user.short_name,
|
||||
"language": user.language,
|
||||
},
|
||||
"team": "",
|
||||
"role": "editor",
|
||||
"abilities": {
|
||||
"destroy": False,
|
||||
"update": False,
|
||||
"partial_update": False,
|
||||
"retrieve": True,
|
||||
"set_role_to": [],
|
||||
},
|
||||
"max_ancestors_role": None,
|
||||
"max_ancestors_role_item_id": None,
|
||||
"max_role": "editor",
|
||||
"is_explicit": False,
|
||||
},
|
||||
{
|
||||
"id": str(other_admin_access.id),
|
||||
"item": {
|
||||
@@ -1657,9 +1613,7 @@ def test_api_item_accesses_explicit():
|
||||
owner_access = factories.UserItemAccessFactory(
|
||||
item=root, user=other_owner, role="owner"
|
||||
)
|
||||
parent_access = factories.UserItemAccessFactory(
|
||||
item=parent, user=user, role="administrator"
|
||||
)
|
||||
factories.UserItemAccessFactory(item=parent, user=user, role="administrator")
|
||||
|
||||
response = client.get(f"/api/v1.0/items/{item.id!s}/accesses/")
|
||||
assert response.status_code == 200
|
||||
@@ -1749,34 +1703,6 @@ def test_api_item_accesses_explicit():
|
||||
"max_role": "owner",
|
||||
"is_explicit": False,
|
||||
},
|
||||
{
|
||||
"id": str(parent_access.id),
|
||||
"item": {
|
||||
"id": str(parent.id),
|
||||
"path": str(parent.path),
|
||||
"depth": parent.depth,
|
||||
},
|
||||
"user": {
|
||||
"id": str(user.id),
|
||||
"email": user.email,
|
||||
"full_name": user.full_name,
|
||||
"short_name": user.short_name,
|
||||
"language": user.language,
|
||||
},
|
||||
"team": "",
|
||||
"role": "administrator",
|
||||
"abilities": {
|
||||
"destroy": True,
|
||||
"update": False,
|
||||
"partial_update": False,
|
||||
"retrieve": True,
|
||||
"set_role_to": [],
|
||||
},
|
||||
"max_ancestors_role": "editor",
|
||||
"max_ancestors_role_item_id": str(root.id),
|
||||
"max_role": "administrator",
|
||||
"is_explicit": False,
|
||||
},
|
||||
{
|
||||
"id": str(item_access.id),
|
||||
"item": {
|
||||
|
||||
Reference in New Issue
Block a user