-
Notifications
You must be signed in to change notification settings - Fork 79
Restricted access #756
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Restricted access #756
Changes from all commits
6980008
2b17686
f9af57b
42e5cb9
5e2bbb2
f630208
9296c1d
bfc7c21
cecd976
c7bc135
7b140c7
64a0984
88e9913
6290c61
4cec906
ddb8730
2b52158
e69f5d3
166a1a2
39757b8
b851c8c
f97b33f
0c1802d
47a544b
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -2,6 +2,8 @@ | |
|
|
||
| # pylint: disable=no-name-in-module | ||
|
|
||
| from __future__ import annotations | ||
|
|
||
| import json | ||
| import logging | ||
| from datetime import timedelta | ||
|
|
@@ -13,7 +15,7 @@ | |
| from django.utils.translation import gettext_lazy as _ | ||
|
|
||
| from lasuite.drf.models.choices import LinkReachChoices, get_equivalent_link_definition | ||
| from rest_framework import serializers | ||
| from rest_framework import exceptions, serializers | ||
|
|
||
| from core import models | ||
| from core.api import utils | ||
|
|
@@ -248,6 +250,7 @@ class Meta: | |
| "is_favorite", | ||
| "link_role", | ||
| "link_reach", | ||
| "is_restricted", | ||
| "nb_accesses", | ||
| "numchild", | ||
| "numchild_folder", | ||
|
|
@@ -280,6 +283,7 @@ class Meta: | |
| "creator", | ||
| "depth", | ||
| "is_favorite", | ||
| "is_restricted", | ||
| "link_role", | ||
| "link_reach", | ||
| "nb_accesses", | ||
|
|
@@ -478,6 +482,7 @@ class Meta: | |
| "is_favorite", | ||
| "link_role", | ||
| "link_reach", | ||
| "is_restricted", | ||
| "nb_accesses", | ||
| "numchild", | ||
| "numchild_folder", | ||
|
|
@@ -534,7 +539,17 @@ def create(self, validated_data): | |
| raise NotImplementedError("Create method can not be used.") | ||
|
|
||
| def update(self, instance, validated_data): | ||
| """Validate that the title is unique in the current path.""" | ||
| """Update an item, handling restriction and title uniqueness.""" | ||
| is_restricted = validated_data.pop("is_restricted", None) | ||
| if is_restricted is not None and is_restricted != instance.is_restricted: | ||
| user = self.context["request"].user | ||
| if not instance.get_abilities(user).get("restrict"): | ||
| raise exceptions.PermissionDenied() | ||
| if is_restricted: | ||
| instance.activate_restriction(user) | ||
| else: | ||
| instance.deactivate_restriction() | ||
|
|
||
| if validated_data.get("title") and instance.title != validated_data.get("title"): | ||
| if instance.depth > 1: | ||
| validated_data["title"] = instance.manage_unique_title(validated_data.get("title")) | ||
|
|
@@ -581,6 +596,7 @@ class Meta: | |
| "creator", | ||
| "depth", | ||
| "is_favorite", | ||
| "is_restricted", | ||
| "link_role", | ||
| "link_reach", | ||
| "nb_accesses", | ||
|
|
@@ -691,6 +707,12 @@ def validate(self, attrs): | |
| code="item_create_folder_title_required", | ||
| ) | ||
|
|
||
| if attrs.get("is_restricted") and attrs["type"] != models.ItemTypeChoices.FOLDER: | ||
| raise serializers.ValidationError( | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think this constraint should be defined on the model directly |
||
| {"is_restricted": _("Only folders can be restricted.")}, | ||
| code="item_create_restricted_only_on_folders", | ||
| ) | ||
|
|
||
| return super().validate(attrs) | ||
|
|
||
| def get_policy(self, item): | ||
|
|
@@ -748,15 +770,8 @@ class Meta: | |
| "link_reach", | ||
| ] | ||
|
|
||
| def validate(self, attrs): | ||
| """Validate that link_role and link_reach are compatible using get_select_options.""" | ||
| link_reach = attrs.get("link_reach") | ||
| link_role = attrs.get("link_role") | ||
|
|
||
| if not link_reach: | ||
| raise serializers.ValidationError({"link_reach": _("This field is required.")}) | ||
|
|
||
| # Get available options based on ancestors' link definition | ||
| def _validate_against_ancestors(self, link_reach: str, link_role: str) -> None: | ||
| """Validate the link definition against the options allowed by ancestors.""" | ||
| available_options = LinkReachChoices.get_select_options( | ||
| **self.instance.ancestors_link_definition | ||
| ) | ||
|
|
@@ -788,12 +803,34 @@ def validate(self, attrs): | |
| raise serializers.ValidationError( | ||
| { | ||
| "link_role": ( | ||
| f"Link role '{link_role}' is not allowed for link reach '{link_reach}'. " | ||
| f"Allowed roles: {allowed_roles_str}" | ||
| f"Link role '{link_role}' is not allowed for link reach " | ||
| f"'{link_reach}'. Allowed roles: {allowed_roles_str}" | ||
| ) | ||
| } | ||
| ) | ||
|
|
||
| def validate(self, attrs: dict) -> dict: | ||
| """Validate that link_role and link_reach are compatible using get_select_options.""" | ||
| link_reach = attrs.get("link_reach") | ||
| link_role = attrs.get("link_role") | ||
|
|
||
| if not link_reach: | ||
| raise serializers.ValidationError({"link_reach": _("This field is required.")}) | ||
|
|
||
| # Restricted folders are independent of parent link configuration | ||
| if self.instance.is_restricted: | ||
| if link_reach == LinkReachChoices.RESTRICTED and link_role is not None: | ||
| raise serializers.ValidationError( | ||
| { | ||
| "link_role": ( | ||
| "Cannot set link_role when link_reach is 'restricted'. " | ||
| "Link role must be null for restricted reach." | ||
| ) | ||
| } | ||
| ) | ||
| else: | ||
| self._validate_against_ancestors(link_reach, link_role) | ||
|
|
||
| return attrs | ||
|
|
||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -47,6 +47,7 @@ | |
|
|
||
| from core import enums, models | ||
| from core.entitlements import get_entitlements_backend | ||
| from core.permissions import get_permissions_backend | ||
| from core.services.item_exports import build_zip_stream, export_descendants | ||
| from core.services.sdk_relay import SDKRelayManager | ||
| from core.services.search_indexers import ( | ||
|
|
@@ -679,8 +680,13 @@ def perform_create(self, serializer): | |
| ) | ||
|
|
||
| def perform_destroy(self, instance): | ||
| """Override to implement a soft delete instead of dumping the record in database.""" | ||
| instance.soft_delete() | ||
| """Override to implement a soft delete or uproot for restricted folders.""" | ||
| abilities = instance.get_abilities(self.request.user) | ||
| # Parent owner with no access to restricted child can only displace, not delete | ||
| if abilities["destroy"] and not abilities["hard_delete"] and instance.is_restricted: | ||
| instance.uproot() | ||
| else: | ||
| instance.soft_delete() | ||
|
|
||
| def perform_update(self, serializer): | ||
| """Override to check if a file is renamed in order to rename file on storage.""" | ||
|
|
@@ -1124,11 +1130,15 @@ def children(self, request, *args, **kwargs): | |
| # Apply ordering only now that everything is filtered and annotated | ||
| queryset = ItemOrdering().filter_queryset(self.request, queryset, self) | ||
|
|
||
| # Pre-compute number of accesses | ||
| # Pre-compute number of accesses; the parent's count does not apply | ||
| # to restricted children which cut inheritance | ||
| item_nb_accesses = item.nb_accesses | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think this should be renamed to |
||
| direct_accesses_count = Coalesce(db.Count("accesses", distinct=True), 0) | ||
| queryset = queryset.annotate( | ||
| _nb_accesses=db.Value(item_nb_accesses) | ||
| + Coalesce(db.Count("accesses", distinct=True), 0), | ||
| _nb_accesses=db.Case( | ||
| db.When(is_restricted=True, then=direct_accesses_count), | ||
| default=db.Value(item_nb_accesses) + direct_accesses_count, | ||
| ), | ||
| ) | ||
|
|
||
| # Pass ancestors' links paths mapping to the serializer as a context variable | ||
|
|
@@ -1407,7 +1417,7 @@ def search(self, request, *args, **kwargs): | |
|
|
||
| # Without the indexer, the "title" filtering is kept | ||
| queryset = filterset.filter_queryset(queryset) | ||
| queryset = queryset.annotate_user_roles(user) | ||
| queryset = get_permissions_backend().visible(queryset, user) | ||
| queryset = queryset.annotate_with_numchild() | ||
|
|
||
| page = self.paginate_queryset(queryset) | ||
|
|
@@ -1466,7 +1476,7 @@ def link_configuration(self, request, *args, **kwargs): | |
| if models.LinkReachChoices.get_priority( | ||
| item.link_reach | ||
| ) >= models.LinkReachChoices.get_priority(previous_link_reach): | ||
| item.descendants().update(link_reach=None) | ||
| get_permissions_backend().propagation_scope(item).update(link_reach=None) | ||
|
|
||
| return drf.response.Response(serializer.data, status=drf.status.HTTP_200_OK) | ||
|
|
||
|
|
@@ -1620,7 +1630,7 @@ def export(self, request, *args, **kwargs): | |
| """ | ||
| folder = self.get_object() | ||
|
|
||
| descendants = export_descendants(folder) | ||
| descendants = export_descendants(folder, user=request.user) | ||
| zip_stream = build_zip_stream(descendants) | ||
|
|
||
| encoded_name = quote(f"{folder.title}.zip", safe="") | ||
|
|
@@ -1842,9 +1852,12 @@ def list(self, request, *args, **kwargs): | |
| if not role: | ||
| return drf.response.Response([]) | ||
|
|
||
| # Get all accesses from ancestors (including current item) | ||
| ancestors_qs = models.Item.objects.filter( | ||
| path__ancestors=self.item.path, ancestors_deleted_at__isnull=True | ||
| # Get all accesses from ancestors (including current item), stopping | ||
| # at the deepest restricted folder which cuts inheritance | ||
| ancestors_qs = ( | ||
| get_permissions_backend() | ||
| .inheritance_scope(self.item) | ||
| .filter(ancestors_deleted_at__isnull=True) | ||
| ) | ||
| accesses_qs = self.get_queryset().filter(item__in=ancestors_qs) | ||
| if role not in PRIVILEGED_ROLES: | ||
|
|
@@ -1967,25 +1980,27 @@ def perform_create(self, serializer): | |
| ) | ||
|
|
||
| # Look for the max ancestors role of the item for the current user. | ||
| ancestor_qs = (self.item.ancestors() | models.Item.objects.filter(pk=self.item.pk)).filter( | ||
| ancestors_deleted_at__isnull=True | ||
| ) | ||
| ancestors_roles = models.ItemAccess.objects.filter( | ||
| item__in=ancestor_qs, user=serializer.validated_data.get("user") | ||
| ).values_list("role", flat=True) | ||
| max_ancestors_role = models.RoleChoices.max(*ancestors_roles) | ||
|
|
||
| if models.RoleChoices.get_priority(max_ancestors_role) >= models.RoleChoices.get_priority( | ||
| role | ||
| ): | ||
| raise drf.exceptions.ValidationError( | ||
| { | ||
| "role": ( | ||
| f"The role {role} you are trying to assign is lower or equal" | ||
| f" than the max ancestors role {max_ancestors_role}." | ||
| ), | ||
| } | ||
| ) | ||
| # Restricted folders cut inheritance: only check the item itself. | ||
| if not self.item.is_restricted: | ||
| ancestor_qs = ( | ||
| self.item.ancestors() | models.Item.objects.filter(pk=self.item.pk) | ||
| ).filter(ancestors_deleted_at__isnull=True) | ||
| ancestors_roles = models.ItemAccess.objects.filter( | ||
| item__in=ancestor_qs, user=serializer.validated_data.get("user") | ||
| ).values_list("role", flat=True) | ||
| max_ancestors_role = models.RoleChoices.max(*ancestors_roles) | ||
|
|
||
| if models.RoleChoices.get_priority( | ||
| max_ancestors_role | ||
| ) >= models.RoleChoices.get_priority(role): | ||
| raise drf.exceptions.ValidationError( | ||
| { | ||
| "role": ( | ||
| f"The role {role} you are trying to assign is lower or equal" | ||
| f" than the max ancestors role {max_ancestors_role}." | ||
| ), | ||
| } | ||
| ) | ||
|
|
||
| access = serializer.save(item_id=self.kwargs["resource_id"]) | ||
| self._syncronize_descendants_accesses(access) | ||
|
|
@@ -2028,7 +2043,11 @@ def _syncronize_descendants_accesses(self, access): | |
| Syncronize the accesses of the descendants of the item | ||
| by removing accesses with roles lower than the current user's role. | ||
| """ | ||
| descendants = self.item.descendants().filter(ancestors_deleted_at__isnull=True) | ||
| descendants = ( | ||
| get_permissions_backend() | ||
| .propagation_scope(self.item) | ||
| .filter(ancestors_deleted_at__isnull=True) | ||
| ) | ||
|
|
||
| condition_filter = db.Q() | ||
| if access.user: | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,27 @@ | ||
| # Generated by Django 5.2.14 on 2026-06-23 09:58 | ||
|
|
||
| import django.contrib.postgres.indexes | ||
| from django.db import migrations, models | ||
|
|
||
|
|
||
| class Migration(migrations.Migration): | ||
|
|
||
| dependencies = [ | ||
| ('core', '0024_alter_item_upload_state'), | ||
| ] | ||
|
|
||
| operations = [ | ||
| migrations.AddField( | ||
| model_name='item', | ||
| name='is_restricted', | ||
| field=models.BooleanField(default=False), | ||
| ), | ||
| migrations.AddConstraint( | ||
| model_name='item', | ||
| constraint=models.CheckConstraint(condition=models.Q(('is_restricted', False), ('type', 'folder'), _connector='OR'), name='check_is_restricted_only_on_folders'), | ||
| ), | ||
| migrations.AddIndex( | ||
| model_name='item', | ||
| index=django.contrib.postgres.indexes.GistIndex(condition=models.Q(('is_restricted', True)), fields=['path'], name='drive_item_restricted_path_ix'), | ||
| ), | ||
| ] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is going to create confusion and break things. Instead you should forbid deletion by a user if there is a restricted folder on which s.he does not have deletion rights...
Uprooting the restricted folder can be suggested but should be done explicitly... Maybe it means that owners should see restricted folders (greyed) and be able to move them out of their folders even if they have no rights on them...