fix: prevent overwrite of hidden relations in patch

README.md

@@ -19,26 +19,23 @@ specific rules as to who can write posts.
 DGAP makes it easy to implement the blog once, and make the permissions,
 visibilities, and validations custom to each deployment.
 
-
 ## Concepts
 
 DGAP provides you with three configuration settings: Visibilities, Permissions,
 and Validations.
 
-* The **visibilities** are run when getting data from the API. They define, on a
+- The **visibilities** are run when getting data from the API. They define, on a
   per-user base, who can see which data.
-* The **validations** are run on create and update operations, so
+- The **validations** are run on create and update operations, so
   data can be checked and modified before the update takes place.
-* The **permissions** then define what a user can do with a given (visible) piece of
+- The **permissions** then define what a user can do with a given (visible) piece of
   data.
 
-
 ## Installation for app developers
 
 If you want to integrate DGAP into your app, these are the steps you need.
 Install DGAP (and add to your requirements files etc) first.
 
-
 ```bash
 pip install django-generic-api-permissions
 ```
@@ -76,15 +73,21 @@ class MyModelViewset(VisibilityViewMixin, ModelViewSet):
 Data leaks in REST Framework may happen if you use includes (or even only references) from a model the user may see to something that should be hidden.
 To avoid such leaks, make sure to use a subclassed related field (either by creating your own using the provided `VisibilityRelatedFieldMixin`, or by using one of the provided types. See example below).
 
+Similar problems can occur when PATCHing existing relationships where some are hidden through the visibilities.
+The hidden relations will be overwritten by the latest update.
+To prevent that include the `VisibilitySerializerMixin` in your serializer where you use `ManyToMany` relations.
+Preferably after the `ValidatorMixin` so that validations first run and only afterwards it considers PATCHing the relation.
+
 ```python
 # serializers.py
 from rest_framework.viewsets import ModelSerializer
-from generic_permissions.visibilities import VisibilityPrimaryKeyRelatedField
-class MyModelSerializers(ModelSerializer):
+from generic_permissions.visibilities import VisibilityPrimaryKeyRelatedField, VisibilitySerializerMixin
+class MyModelSerializers(VisibilitySerializerMixin, ModelSerializer):
     serializer_related_field = VisibilityPrimaryKeyRelatedField
 ```
 
-A few subclassed fields are provided:
+A few subclassed fields are provided for different types of `RelatedField`:
+
 - `VisibilityPrimaryKeyRelatedField`
 - `VisibilityResourceRelatedField`
 - `VisibilitySerializerMethodResourceRelatedField`
@@ -110,12 +113,13 @@ class MyModelViewset(PermissionViewMixin, VisibilityViewMixin, ModelViewSet):
     serializer_class = ...
     queryset = ...
 ```
-You may use only one of the two mixins, or both, depending on your needs.
 
+You may use only one of the two mixins, or both, depending on your needs.
 
 ### Validation subsystem
 
 Last, for the validation system, you extend your **serializer** with a mixin:
+
 ```python
 # serializers.py
 from rest_framework.serializers import ModelSerializer
@@ -124,14 +128,14 @@ from generic_permissions.serializers import PermissionSerializerMixin
 from generic_permissions.validation import ValidatorMixin
 
 from myapp import models
+
 class MyModelSerializer(ValidatorMixin, ModelSerializer):
     # my field definitions...
     class Meta:
         model = models.MyModel
         fields = "__all__"
 ```
 
-
 ## Usage - for people deploying a DGAP-equipped app
 
 Say you have an blog you want to deploy that uses DGAP. You want public blog
@@ -142,6 +146,7 @@ accordingly.
 ### Visibilities
 
 First, let's define the visibility class:
+
 ```python
 # my_custom_visibilities.py
 from generic_permissions.visibilities import filter_queryset_for
@@ -192,7 +197,6 @@ class ResultingVisibility(Union):
     visibility_classes = [MyFirstVisibility, MySecondVisibility]
 ```
 
-
 ### Permissions
 
 Permission classes define who may perform which data mutation. They can be configured
@@ -202,10 +206,11 @@ To write custom permission classes, you create a simple class, and decorate the
 methods that define the permissions accordingly.
 
 There are two types of methods in the permissions system:
-* `permission_for`: Marks methods that define generic access permissions for a
+
+- `permission_for`: Marks methods that define generic access permissions for a
   given model. They are always checked first.
   Those methods will receive one positional argument, namely the `request` object
-* `object_permission_for`: Define whether access to a specific object shall be
+- `object_permission_for`: Define whether access to a specific object shall be
   granted. This called for all other operations **except** creation.
   These methods will receive two positional arguments: First, the `request`
   object, and second, the model instance that is being accessed in the request.
@@ -221,7 +226,7 @@ that to avoid code duplication.
 You can find more information about the `request` object in the
 [Django documentation](https://docs.djangoproject.com/en/3.1/ref/request-response/#httprequest-objects)
 
-``` python
+```python
 from generic_permissions.permissions import permission_for, object_permission_for
 from my_app.models import Post, Comment
 
@@ -230,7 +235,7 @@ class OnlyAuthenticated:
     def has_permission_default(self, request):
         # No permission is granted for any non-authenticated users
         return request.user.is_authenticated
-        
+
 class BlogPermissions:
     @permission_for(Comment)
     def has_permission_for_comment(self, request):
@@ -247,12 +252,12 @@ class BlogPermissions:
 ```
 
 The following pre-defined classes are available:
-* `generic_permissions.permissions.AllowAny`: allow any users to perform any mutation (default)
-* `generic_permissions.permissions.DenyAll`: deny all operations to any object.
+
+- `generic_permissions.permissions.AllowAny`: allow any users to perform any mutation (default)
+- `generic_permissions.permissions.DenyAll`: deny all operations to any object.
   You can use this as a base class for your permissions - as long as you don't
   allow something, it will be denied.
 
-
 ### Data validation
 
 Once the permission to access or modify an object is granted, you may want to
@@ -285,4 +290,4 @@ the method returns a `dict` with a compatible structure. You may also
 to succeed.
 The second parameter, `context`, is a `dict` containing the DRF context: Access
 `context['request']` to get the request (if validation depends on the user,
-for example). 
+for example).

generic_permissions/visibilities.py

@@ -60,6 +60,33 @@ def get_attribute(self, instance):
         return queryset
 
 
+class VisibilitySerializerMixin:
+    def validate(self, *args, **kwargs):
+        validated_data = super().validate(*args, **kwargs)
+
+        if not self.instance:
+            return validated_data
+
+        # Incoming patches can have a subset of all relations, so we need to
+        # keep the existing relations which are not included in the request.
+        for key, field in self.fields.items():
+            if (
+                type(field) is not VisibilityManyRelatedField
+                or key not in validated_data
+            ):
+                continue
+
+            # Find the relations which the request can include.
+            queryset = getattr(self.instance, key).all()
+            for handler in VisibilitiesConfig.get_handlers(queryset.model):
+                queryset = handler(queryset, self.context)
+
+            # Add remaining relations which can not be included in the request.
+            validated_data[key] += getattr(self.instance, key).exclude(pk__in=queryset)
+
+        return validated_data
+
+
 class VisibilityRelatedFieldMixin:
     @classmethod
     def many_init(cls, *args, **kwargs):

tests/serializers.py

@@ -1,12 +1,17 @@
 from rest_framework import serializers
 
 from generic_permissions.validation import ValidatorMixin
-from generic_permissions.visibilities import VisibilityPrimaryKeyRelatedField
+from generic_permissions.visibilities import (
+    VisibilityPrimaryKeyRelatedField,
+    VisibilitySerializerMixin,
+)
 
 from . import models
 
 
-class TestModel1Serializer(ValidatorMixin, serializers.ModelSerializer):
+class TestModel1Serializer(
+    ValidatorMixin, VisibilitySerializerMixin, serializers.ModelSerializer
+):
     serializer_related_field = VisibilityPrimaryKeyRelatedField
     explicit = VisibilityPrimaryKeyRelatedField(
         queryset=models.Model1.objects.all(), many=True, source="many"
@@ -17,7 +22,9 @@ class Meta:
         fields = "__all__"
 
 
-class TestModel2Serializer(ValidatorMixin, serializers.ModelSerializer):
+class TestModel2Serializer(
+    ValidatorMixin, VisibilitySerializerMixin, serializers.ModelSerializer
+):
     class Meta:
         model = models.Model2
         fields = "__all__"

tests/test_visibilities.py

@@ -246,3 +246,38 @@ def filter_queryset_for_document(self, queryset, request):
         assert len(result["explicit"]) == 1
         assert model2.pk in result["many"]
         assert model2.pk in result["explicit"]
+
+
+def test_visibility_relation_patch(db, admin_user, admin_client):
+    class TestVisibility:
+        @filter_queryset_for(Model2)
+        def filter_queryset_for_document(self, queryset, request):
+            return queryset.exclude(text="apple")
+
+    VisibilitiesConfig.clear_handlers()
+    VisibilitiesConfig.register_handler_class(TestVisibility)
+
+    model1 = Model1.objects.create(text="pear")
+    Model2.objects.create(text="hidden")
+    model2 = Model2.objects.create(text="apple")
+    model3 = Model2.objects.create(text="melon")
+    model4 = Model2.objects.create(text="orange")
+    model1.many.add(model2)
+    model1.many.add(model3)
+
+    data = {"many": [model4.pk]}
+
+    url = reverse("model1-detail", args=[model1.pk])
+    response = admin_client.patch(url, data)
+
+    assert response.status_code == HTTP_200_OK
+    result = response.json()
+
+    assert result["text"] == "pear"
+    assert len(result["many"]) == 1
+    assert model4.pk in result["many"]
+
+    model1.refresh_from_db()
+    assert model1.many.all().count() == 2
+    assert model1.many.filter(pk=model2.pk).exists()
+    assert model1.many.filter(pk=model4.pk).exists()