From ecf375555c99392c62a698fa01cfadccacfebea0 Mon Sep 17 00:00:00 2001
From: Jean-Baptiste <bleme@pm.me>
Date: Wed, 21 Oct 2020 15:22:35 +0200
Subject: [PATCH 1/8] major: Extends queryset to avoid post filtering on view
permission
---
README.md | 2 +
djangoldp/models.py | 2 +-
djangoldp/permissions.py | 25 +++++++++++-
djangoldp/serializers.py | 28 +++++++------
djangoldp/tests/models.py | 12 ++++++
djangoldp/tests/permissions.py | 35 +++++++++++++++++
djangoldp/tests/tests_guardian.py | 42 ++++++++++++++++++--
djangoldp/tests/tests_user_permissions.py | 1 +
djangoldp/views.py | 48 ++++++++++++++---------
9 files changed, 160 insertions(+), 35 deletions(-)
create mode 100644 djangoldp/tests/permissions.py
diff --git a/README.md b/README.md
index cf2760d5..932ae167 100644
--- a/README.md
+++ b/README.md
@@ -337,6 +337,8 @@ This allows you to add permissions for anonymous, logged in user, author ... in
By default `LDPPermissions` is used.
Specific permissin classes can be developed to fit special needs.
+The method `prefilter_query_s_set()` should be override and must add a filter to the query_set removing object without at least the `view` permission
+
## anonymous_perms, user_perms, owner_perms
Those allow you to set permissions from your model's meta.
diff --git a/djangoldp/models.py b/djangoldp/models.py
index 8976d449..7e2a485b 100644
--- a/djangoldp/models.py
+++ b/djangoldp/models.py
@@ -240,7 +240,7 @@ class Model(models.Model):
return None
@classonlymethod
- def get_permission_classes(cls, related_model, default_permissions_classes):
+ def get_permission_classes(cls, related_model, default_permissions_classes) -> LDPPermissions:
'''returns the permission_classes set in the models Meta class'''
return cls.get_meta(related_model, 'permission_classes', default_permissions_classes)
diff --git a/djangoldp/permissions.py b/djangoldp/permissions.py
index c8f71aa8..70e5c1b7 100644
--- a/djangoldp/permissions.py
+++ b/djangoldp/permissions.py
@@ -2,7 +2,9 @@ import time
from django.conf import settings
from django.contrib.auth.models import _user_get_all_permissions
from django.core.exceptions import PermissionDenied
+from django.db.models import QuerySet
from django.db.models.base import ModelBase
+from guardian.shortcuts import get_objects_for_user
from rest_framework.permissions import DjangoObjectPermissions
@@ -33,13 +35,32 @@ class LDPPermissions(DjangoObjectPermissions):
if time.time() - cls.perms_cache['time'] > 5:
cls.invalidate_cache()
+ def prefilter_query_set(self, query_set, request, view, model) -> QuerySet:
+ """
+ Must filter object that user doesn't have at least the `view` permission
+ Should reflect user_permissions
+ :param query_set: the base QuerySet
+ :param request: the request
+ :param view: the view
+ :param model: the model
+ :return: the filtered QuerySet
+ """
+ if self.has_object_permission(request, view, model):
+ value = query_set
+ else:
+ try:
+ permissions_needed = self.get_permissions(request.method, model)
+ value = get_objects_for_user(request.user, permissions_needed, query_set)
+ except:
+ value = []
+ return query_set
+
def user_permissions(self, user, obj_or_model, obj=None):
"""
Filter user permissions for a model class
"""
-
- # this may be a permission for the model class, or an instance
self.refresh_cache()
+ # this may be a permission for the model class, or an instance
if isinstance(obj_or_model, ModelBase):
model = obj_or_model
else:
diff --git a/djangoldp/serializers.py b/djangoldp/serializers.py
index eb6170b6..a7496a46 100644
--- a/djangoldp/serializers.py
+++ b/djangoldp/serializers.py
@@ -14,6 +14,7 @@ from django.urls.resolvers import get_resolver
from django.db.models import QuerySet
from django.utils.datastructures import MultiValueDictKeyError
from django.utils.encoding import uri_to_iri
+from guardian.shortcuts import get_objects_for_user
from rest_framework.exceptions import ValidationError
from rest_framework.fields import SkipField, empty, ReadOnlyField
from rest_framework.fields import get_error_detail, set_value
@@ -96,10 +97,9 @@ class LDListMixin:
parent_model = None
- if isinstance(value, QuerySet):
- value = list(value)
+ permissions_class = Model.get_permission_classes(child_model, [LDPPermissions])[0]()
- if not isinstance(value, Iterable):
+ if not isinstance(value, Iterable) and not isinstance(value, QuerySet):
if not self.id.startswith('http'):
self.id = '{}{}{}'.format(settings.BASE_URL, Model.resource(parent_model), self.id)
@@ -107,7 +107,6 @@ class LDListMixin:
if self.with_cache and self.to_representation_cache.has(cache_key):
return self.to_representation_cache.get(cache_key)
- filtered_values = value
container_permissions = Model.get_permissions(child_model, self.context, ['view', 'add'])
else:
@@ -124,19 +123,26 @@ class LDListMixin:
if self.with_cache and self.to_representation_cache.has(cache_key):
return self.to_representation_cache.get(cache_key)
+ if isinstance(value, QuerySet):
+ # Perf: filter on request if not a subclass of LDPPermissions
+ value = permissions_class.prefilter_query_set(value, self.context['request'], self.context['view'], child_model)
+
# remove objects from the list which I don't have permission to view
- filtered_values = list(
- filter(lambda v: Model.get_permission_classes(v, [LDPPermissions])[0]().has_object_permission(
- self.context['request'], self.context['view'], v), value))
+ # Perf: filtering is uselss in case of LDPPermissions, make sense only for subclass
+ # if type(permissions_class) is not LDPPermissions:
+ # filtered_values = list(
+ # filter(lambda v: permissions_class.has_object_permission(
+ # self.context['request'], self.context['view'], v), value))
container_permissions = Model.get_permissions(child_model, self.context, ['add'])
container_permissions.extend(
Model.get_permissions(parent_model, self.context, ['view']))
self.to_representation_cache.set(self.id, {'@id': self.id,
- '@type': 'ldp:Container',
- 'ldp:contains': super().to_representation(filtered_values),
- 'permissions': container_permissions
- })
+ '@type': 'ldp:Container',
+ 'ldp:contains': super().to_representation(
+ value),
+ 'permissions': container_permissions
+ })
return self.to_representation_cache.get(self.id)
diff --git a/djangoldp/tests/models.py b/djangoldp/tests/models.py
index e3b1f39d..4699baab 100644
--- a/djangoldp/tests/models.py
+++ b/djangoldp/tests/models.py
@@ -9,6 +9,7 @@ from django.utils.datetime_safe import date
from djangoldp.fields import LDPUrlField
from djangoldp.models import Model
from djangoldp.permissions import LDPPermissions
+from djangoldp.tests.permissions import HalfRandomPermissions
class User(AbstractUser, Model):
@@ -125,6 +126,17 @@ class LDPDummy(Model):
owner_perms = ['inherit', 'change', 'delete', 'control']
+class DummyAnonPerm(Model):
+ some = models.CharField(max_length=255, blank=True, null=True)
+ parent = models.ForeignKey(LDPDummy, on_delete=models.DO_NOTHING, related_name="anons", blank=True, null=True)
+
+ class Meta(Model.Meta):
+ anonymous_perms = []
+ authenticated_perms = ['view', 'add']
+ owner_perms = ['inherit', 'change', 'delete', 'control']
+ permission_classes = [HalfRandomPermissions]
+
+
# model used in django-guardian permission tests (no anonymous etc permissions set)
class PermissionlessDummy(Model):
some = models.CharField(max_length=255, blank=True, null=True)
diff --git a/djangoldp/tests/permissions.py b/djangoldp/tests/permissions.py
new file mode 100644
index 00000000..3e9b72d6
--- /dev/null
+++ b/djangoldp/tests/permissions.py
@@ -0,0 +1,35 @@
+from django.db.models import QuerySet
+from django.db.models.base import ModelBase
+
+from djangoldp.permissions import LDPPermissions
+
+
+class HalfRandomPermissions(LDPPermissions):
+
+ def prefilter_query_set(self, query_set: QuerySet, request, view, model) -> QuerySet:
+ if request.user.is_anonymous:
+ return query_set.filter(pk__in=[2, 4, 6, 8])
+ else:
+ return super().prefilter_query_set(query_set, request, view, model)
+
+ def user_permissions(self, user, obj_or_model, obj=None):
+ if isinstance(obj_or_model, ModelBase):
+ model = obj_or_model
+ else:
+ obj = obj_or_model
+ model = obj_or_model.__class__
+
+ perms_cache_key = self.cache_key(model, obj, user)
+ if self.with_cache and perms_cache_key in self.perms_cache:
+ return self.perms_cache[perms_cache_key]
+
+ # start with the permissions set on the object and model
+ perms = set(super().user_permissions(user, obj_or_model, obj))
+
+ if obj is not None and not isinstance(obj, ModelBase) and user.is_anonymous:
+ if obj.pk % 2 == 0:
+ return ['add', 'view']
+ else:
+ return []
+ else:
+ return ['view']
diff --git a/djangoldp/tests/tests_guardian.py b/djangoldp/tests/tests_guardian.py
index 0b51fa35..b47ae1e4 100644
--- a/djangoldp/tests/tests_guardian.py
+++ b/djangoldp/tests/tests_guardian.py
@@ -1,9 +1,9 @@
import json
from django.contrib.auth import get_user_model
from rest_framework.test import APIClient, APITestCase
-from guardian.shortcuts import assign_perm
+from guardian.shortcuts import assign_perm, remove_perm
-from .models import PermissionlessDummy, Dummy
+from .models import PermissionlessDummy, Dummy, LDPDummy, DummyAnonPerm
from djangoldp.permissions import LDPPermissions
@@ -53,7 +53,7 @@ class TestsGuardian(APITestCase):
def test_patch_dummy_permission_granted(self):
self.setUpLoggedInUser()
- self.setUpGuardianDummyWithPerms(['change'])
+ self.setUpGuardianDummyWithPerms(['view', 'change'])
body = {'some': "some_new"}
response = self.client.patch('/permissionless-dummys/{}/'.format(self.dummy.slug), data=json.dumps(body),
content_type='application/ld+json')
@@ -88,3 +88,39 @@ class TestsGuardian(APITestCase):
permissions = LDPPermissions()
result = permissions.user_permissions(self.user, dummy)
self.assertEqual(result.count('view'), 1)
+
+ # test list with and without view perms
+ def test_get_list(self):
+ DummyAnonPerm.objects.create(some='test1')
+ DummyAnonPerm.objects.create(some='test2')
+
+ response = self.client.get('/dummyanonperms/')
+
+ self.assertEqual(response.status_code, 200)
+ self.assertEqual(len(response.data['ldp:contains']), 1)
+
+ self.setUpLoggedInUser()
+
+ response = self.client.get('/dummyanonperms/')
+
+ self.assertEqual(response.status_code, 200)
+ self.assertEqual(len(response.data['ldp:contains']), 2)
+
+ def test_get_sub_list(self):
+ parent = LDPDummy.objects.create(some="test")
+ parent.anons.add(DummyAnonPerm.objects.create(some='test1'))
+ parent.anons.add(DummyAnonPerm.objects.create(some='test2'))
+ parent.save()
+
+ response = self.client.get('/ldpdummys/{}/'.format(parent.pk))
+
+ self.assertEqual(response.status_code, 200)
+ self.assertEqual(len(response.data['anons']['ldp:contains']), 1)
+
+ self.setUpLoggedInUser()
+
+ response = self.client.get('/ldpdummys/{}/'.format(parent.pk))
+
+ self.assertEqual(response.status_code, 200)
+ self.assertEqual(len(response.data['anons']['ldp:contains']), 2)
+
diff --git a/djangoldp/tests/tests_user_permissions.py b/djangoldp/tests/tests_user_permissions.py
index 683b8168..287405b8 100644
--- a/djangoldp/tests/tests_user_permissions.py
+++ b/djangoldp/tests/tests_user_permissions.py
@@ -4,6 +4,7 @@ from .models import JobOffer
import json
+
class TestUserPermissions(APITestCase):
def setUp(self):
diff --git a/djangoldp/views.py b/djangoldp/views.py
index a5f6d990..aa77ae01 100644
--- a/djangoldp/views.py
+++ b/djangoldp/views.py
@@ -1,37 +1,35 @@
import json
-import time
+import logging
+
from django.apps import apps
from django.conf import settings
-
from django.conf.urls import include, re_path
from django.contrib.auth import get_user_model
from django.core.exceptions import FieldDoesNotExist, ObjectDoesNotExist
-from django.urls.resolvers import get_resolver
from django.db import IntegrityError, transaction
from django.http import JsonResponse, Http404
from django.shortcuts import get_object_or_404
+from django.urls.resolvers import get_resolver
from django.utils.decorators import classonlymethod
from django.views import View
from pyld import jsonld
from rest_framework import status
from rest_framework.authentication import SessionAuthentication
-from rest_framework.permissions import AllowAny
from rest_framework.parsers import JSONParser
+from rest_framework.permissions import AllowAny
from rest_framework.renderers import JSONRenderer
from rest_framework.response import Response
from rest_framework.utils import model_meta
from rest_framework.views import APIView
from rest_framework.viewsets import ModelViewSet
+from djangoldp.activities import ActivityPubService
+from djangoldp.activities import ActivityQueueService, as_activitystream
+from djangoldp.activities.errors import ActivityStreamDecodeError, ActivityStreamValidationError
from djangoldp.endpoints.webfinger import WebFingerEndpoint, WebFingerError
+from djangoldp.filters import LocalObjectOnContainerPathBackend
from djangoldp.models import LDPSource, Model, Follower
from djangoldp.permissions import LDPPermissions
-from djangoldp.filters import LocalObjectOnContainerPathBackend
-from djangoldp.activities import ActivityQueueService, as_activitystream
-from djangoldp.activities import ActivityPubService
-from djangoldp.activities.errors import ActivityStreamDecodeError, ActivityStreamValidationError
-import logging
-
logger = logging.getLogger('djangoldp')
get_user_model()._meta.rdf_context = {"get_full_name": "rdfs:label"}
@@ -75,7 +73,7 @@ class InboxView(APIView):
"""
Receive linked data notifications
"""
- permission_classes=[AllowAny,]
+ permission_classes = [AllowAny, ]
def post(self, request, *args, **kwargs):
'''
@@ -94,7 +92,8 @@ class InboxView(APIView):
try:
self._handle_activity(activity, **kwargs)
except IntegrityError:
- return Response({'Unable to save due to an IntegrityError in the receiver model'}, status=status.HTTP_200_OK)
+ return Response({'Unable to save due to an IntegrityError in the receiver model'},
+ status=status.HTTP_200_OK)
# save the activity and return 201
obj = ActivityQueueService._save_sent_activity(activity.to_json(), local_id=request.path_info, success=True,
@@ -126,7 +125,8 @@ class InboxView(APIView):
return self._get_or_create_nested_backlinks(obj, object_model, update)
except IntegrityError as e:
logger.error(str(e))
- logger.warning('received a backlink which you were not able to save because of a constraint on the model field.')
+ logger.warning(
+ 'received a backlink which you were not able to save because of a constraint on the model field.')
raise e
def _get_or_create_nested_backlinks(self, obj, object_model=None, update=False):
@@ -150,7 +150,8 @@ class InboxView(APIView):
item_value = item[1]
item_model = Model.get_subclass_with_rdf_type(item_value['@type'])
if item_model is None:
- raise Http404('unable to store type ' + item_value['@type'] + ', model with this rdf_type not found')
+ raise Http404(
+ 'unable to store type ' + item_value['@type'] + ', model with this rdf_type not found')
# push nested object tuple as a branch
backlink = self._get_or_create_nested_backlinks(item_value, item_model)
@@ -337,6 +338,7 @@ class LDPViewSetGenerator(ModelViewSet):
if view_set is not None:
class LDPNestedCustomViewSet(LDPNestedViewSet, view_set):
pass
+
return LDPNestedCustomViewSet.nested_urls(nested_field, **kwargs)
return LDPNestedViewSet.nested_urls(nested_field, **kwargs)
@@ -433,7 +435,8 @@ class LDPViewSet(LDPViewSetGenerator):
if self.serializer_class is None:
self.serializer_class = LDPSerializer
- return type(LDPSerializer)(self.model._meta.object_name.lower() + name_prefix + 'Serializer', (self.serializer_class,),
+ return type(LDPSerializer)(self.model._meta.object_name.lower() + name_prefix + 'Serializer',
+ (self.serializer_class,),
{'Meta': meta_class})
def is_safe_create(self, user, validated_data, *args, **kwargs):
@@ -479,7 +482,8 @@ class LDPViewSet(LDPViewSetGenerator):
serializer = self.get_write_serializer(data=request.data)
serializer.is_valid(raise_exception=True)
if not self.is_safe_create(request.user, serializer.validated_data):
- return Response({'detail': 'You do not have permission to perform this action'}, status=status.HTTP_403_FORBIDDEN)
+ return Response({'detail': 'You do not have permission to perform this action'},
+ status=status.HTTP_403_FORBIDDEN)
self.perform_create(serializer)
response_serializer = self.get_serializer()
@@ -543,12 +547,16 @@ class LDPViewSet(LDPViewSetGenerator):
def get_queryset(self, *args, **kwargs):
if self.model:
- return self.model.objects.all()
+ query_set = self.model.objects.all()
+ permissions_class = Model.get_permission_classes(self.model, [LDPPermissions])[0]()
+ return permissions_class.prefilter_query_set(query_set, self.request, None, self.model)
else:
return super(LDPView, self).get_queryset(*args, **kwargs)
def dispatch(self, request, *args, **kwargs):
'''overriden dispatch method to append some custom headers'''
+ # pr = cProfile.Profile()
+ # pr.enable()
response = super(LDPViewSet, self).dispatch(request, *args, **kwargs)
response["Access-Control-Allow-Origin"] = request.META.get('HTTP_ORIGIN')
response["Access-Control-Allow-Methods"] = "GET,POST,PUT,PATCH,DELETE"
@@ -566,6 +574,11 @@ class LDPViewSet(LDPViewSetGenerator):
response['User'] = request.user.webid()
except AttributeError:
pass
+ # pr.disable()
+ # s = io.StringIO()
+ # ps = pstats.Stats(pr, stream=s)
+ # ps.print_stats()
+ # print(s.getvalue())
return response
@@ -650,4 +663,3 @@ class WebFingerView(View):
def post(self, request, *args, **kwargs):
return self.on_request(request)
-
--
GitLab
From f6409825ea5359361266434d8fdf07e137fa802b Mon Sep 17 00:00:00 2001
From: Jean-Baptiste <bleme@pm.me>
Date: Mon, 26 Oct 2020 15:27:32 +0100
Subject: [PATCH 2/8] update: avoid costly try/catch on prefilter_query_set
---
djangoldp/permissions.py | 30 ++++++++++++++++++++++--------
djangoldp/tests/permissions.py | 6 +++---
djangoldp/tests/tests_guardian.py | 7 +++++++
3 files changed, 32 insertions(+), 11 deletions(-)
diff --git a/djangoldp/permissions.py b/djangoldp/permissions.py
index 70e5c1b7..f63ecb5f 100644
--- a/djangoldp/permissions.py
+++ b/djangoldp/permissions.py
@@ -45,15 +45,14 @@ class LDPPermissions(DjangoObjectPermissions):
:param model: the model
:return: the filtered QuerySet
"""
- if self.has_object_permission(request, view, model):
- value = query_set
+ if self.has_object_view_permission(request, view, model):
+ return query_set
else:
- try:
+ if not request.user.is_anonymous:
permissions_needed = self.get_permissions(request.method, model)
- value = get_objects_for_user(request.user, permissions_needed, query_set)
- except:
- value = []
- return query_set
+ return get_objects_for_user(request.user, permissions_needed, query_set)
+ else:
+ return model.objects.none()
def user_permissions(self, user, obj_or_model, obj=None):
"""
@@ -192,9 +191,24 @@ class LDPPermissions(DjangoObjectPermissions):
model = obj
user_perms = self.user_permissions(request.user, model, obj)
+ return self.compare_permissions(perms, user_perms)
+
+ def has_object_view_permission(self, request, view, obj):
+ """
+ Access to objects
+ User have view permission on request: Continue
+ User does not have permission: 403
+ """
+ # get permissions required
+ perms = self.get_permissions('GET', obj)
+ model = obj
+ user_perms = self.user_permissions(request.user, model, obj)
+
+ return self.compare_permissions(perms, user_perms)
+
+ def compare_permissions(self, perms, user_perms):
# compare them with the permissions I have
for perm in perms:
if not perm.split('.')[-1].split('_')[0] in user_perms:
return False
-
return True
diff --git a/djangoldp/tests/permissions.py b/djangoldp/tests/permissions.py
index 3e9b72d6..f8ec6d3c 100644
--- a/djangoldp/tests/permissions.py
+++ b/djangoldp/tests/permissions.py
@@ -19,9 +19,9 @@ class HalfRandomPermissions(LDPPermissions):
obj = obj_or_model
model = obj_or_model.__class__
- perms_cache_key = self.cache_key(model, obj, user)
- if self.with_cache and perms_cache_key in self.perms_cache:
- return self.perms_cache[perms_cache_key]
+ # perms_cache_key = self.cache_key(model, obj, user)
+ # if self.with_cache and perms_cache_key in self.perms_cache:
+ # return self.perms_cache[perms_cache_key]
# start with the permissions set on the object and model
perms = set(super().user_permissions(user, obj_or_model, obj))
diff --git a/djangoldp/tests/tests_guardian.py b/djangoldp/tests/tests_guardian.py
index b47ae1e4..6984891d 100644
--- a/djangoldp/tests/tests_guardian.py
+++ b/djangoldp/tests/tests_guardian.py
@@ -1,5 +1,6 @@
import json
from django.contrib.auth import get_user_model
+from djangoldp.serializers import LDListMixin, LDPSerializer
from rest_framework.test import APIClient, APITestCase
from guardian.shortcuts import assign_perm, remove_perm
@@ -11,11 +12,17 @@ class TestsGuardian(APITestCase):
def setUp(self):
self.client = APIClient(enforce_csrf_checks=True)
+ LDPPermissions.invalidate_cache()
+ LDListMixin.to_representation_cache.invalidate_cache()
+ LDPSerializer.to_representation_cache.invalidate_cache()
def setUpLoggedInUser(self):
self.user = get_user_model().objects.create_user(username='john', email='jlennon@beatles.com',
password='glass onion')
self.client.force_authenticate(user=self.user)
+ LDPPermissions.invalidate_cache()
+ LDListMixin.to_representation_cache.invalidate_cache()
+ LDPSerializer.to_representation_cache.invalidate_cache()
# optional setup for testing PermissionlessDummy model with parameterised perms
def setUpGuardianDummyWithPerms(self, perms=[]):
--
GitLab
From 283a1f25401d7a653803e0155b150f4c414c5ad9 Mon Sep 17 00:00:00 2001
From: Calum Mackervoy <c.mackervoy@gmail.com>
Date: Mon, 9 Nov 2020 12:06:18 +0000
Subject: [PATCH 3/8] update: get_permissions now class method, is_owner
convenience function
---
djangoldp/permissions.py | 36 ++++++++++++++++--------------------
1 file changed, 16 insertions(+), 20 deletions(-)
diff --git a/djangoldp/permissions.py b/djangoldp/permissions.py
index f63ecb5f..2087ef26 100644
--- a/djangoldp/permissions.py
+++ b/djangoldp/permissions.py
@@ -9,12 +9,7 @@ from rest_framework.permissions import DjangoObjectPermissions
class LDPPermissions(DjangoObjectPermissions):
- """
- Default permissions
- Anon: None
- Auth: None but inherit from Anon
- Owner: None but inherit from Auth
- """
+ # *DEFAULT* permissions for anon, auth and owner statuses
anonymous_perms = ['view']
authenticated_perms = ['inherit']
owner_perms = ['inherit']
@@ -49,11 +44,17 @@ class LDPPermissions(DjangoObjectPermissions):
return query_set
else:
if not request.user.is_anonymous:
- permissions_needed = self.get_permissions(request.method, model)
+ permissions_needed = LDPPermissions.get_permissions(request.method, model)
return get_objects_for_user(request.user, permissions_needed, query_set)
else:
return model.objects.none()
+ def is_owner(self, user, model, obj):
+ return obj and hasattr(model._meta, 'owner_field') and (
+ getattr(obj, getattr(model._meta, 'owner_field')) == user
+ or (hasattr(user, 'urlid') and getattr(obj, getattr(model._meta, 'owner_field')) == user.urlid)
+ or getattr(obj, getattr(model._meta, 'owner_field')) == user.id)
+
def user_permissions(self, user, obj_or_model, obj=None):
"""
Filter user permissions for a model class
@@ -101,21 +102,15 @@ class LDPPermissions(DjangoObjectPermissions):
# apply anon, owner and auth permissions
if user.is_anonymous:
perms = perms.union(set(anonymous_perms))
-
else:
- if obj and hasattr(model._meta, 'owner_field') and (
- getattr(obj, getattr(model._meta, 'owner_field')) == user
- or (hasattr(user, 'urlid') and getattr(obj, getattr(model._meta, 'owner_field')) == user.urlid)
- or getattr(obj, getattr(model._meta, 'owner_field')) == user.id):
+ if self.is_owner(user, model, obj):
perms = perms.union(set(owner_perms))
-
else:
perms = perms.union(set(authenticated_perms))
self.perms_cache[perms_cache_key] = list(perms)
return self.perms_cache[perms_cache_key]
- # return list(perms)
def filter_user_perms(self, context, obj_or_model, permissions):
# Only used on Model.get_permissions to translate permissions to LDP
@@ -132,7 +127,8 @@ class LDPPermissions(DjangoObjectPermissions):
'DELETE': ['%(app_label)s.delete_%(model_name)s'],
}
- def get_permissions(self, method, obj):
+ @classmethod
+ def get_permissions(cls, method, obj):
"""
Translate perms_map to request
"""
@@ -142,10 +138,10 @@ class LDPPermissions(DjangoObjectPermissions):
}
# Only allows methods that are on perms_map
- if method not in self.perms_map:
+ if method not in cls.perms_map:
raise PermissionDenied
- return [perm % kwargs for perm in self.perms_map[method]]
+ return [perm % kwargs for perm in cls.perms_map[method]]
def has_permission(self, request, view):
"""
@@ -165,7 +161,7 @@ class LDPPermissions(DjangoObjectPermissions):
model = view.model
# get permissions required
- perms = self.get_permissions(request.method, model)
+ perms = LDPPermissions.get_permissions(request.method, model)
user_perms = self.user_permissions(request.user, model, obj)
# compare them with the permissions I have
@@ -187,7 +183,7 @@ class LDPPermissions(DjangoObjectPermissions):
User does not have permission: 403
"""
# get permissions required
- perms = self.get_permissions(request.method, obj)
+ perms = LDPPermissions.get_permissions(request.method, obj)
model = obj
user_perms = self.user_permissions(request.user, model, obj)
@@ -200,7 +196,7 @@ class LDPPermissions(DjangoObjectPermissions):
User does not have permission: 403
"""
# get permissions required
- perms = self.get_permissions('GET', obj)
+ perms = LDPPermissions.get_permissions('GET', obj)
model = obj
user_perms = self.user_permissions(request.user, model, obj)
--
GitLab
From 0da697245c67c6627a84b68bd870c5762310405f Mon Sep 17 00:00:00 2001
From: Calum Mackervoy <c.mackervoy@gmail.com>
Date: Mon, 9 Nov 2020 12:17:05 +0000
Subject: [PATCH 4/8] syntax: cleaning user_perms function
---
djangoldp/permissions.py | 66 ++++++++++++++++++++--------------------
1 file changed, 33 insertions(+), 33 deletions(-)
diff --git a/djangoldp/permissions.py b/djangoldp/permissions.py
index 2087ef26..9a3e2463 100644
--- a/djangoldp/permissions.py
+++ b/djangoldp/permissions.py
@@ -55,6 +55,33 @@ class LDPPermissions(DjangoObjectPermissions):
or (hasattr(user, 'urlid') and getattr(obj, getattr(model._meta, 'owner_field')) == user.urlid)
or getattr(obj, getattr(model._meta, 'owner_field')) == user.id)
+ def _get_cache_key(self, model_name, user, obj):
+ user_key = 'None' if user is None else user.id
+ obj_key = 'None' if obj is None else obj.id
+ return 'User{}{}{}'.format(user_key, model_name, obj_key)
+
+ def _get_model_level_perms(self, model, user, obj):
+ anonymous_perms = getattr(model._meta, 'anonymous_perms', self.anonymous_perms)
+ authenticated_perms = getattr(model._meta, 'authenticated_perms', self.authenticated_perms)
+ owner_perms = getattr(model._meta, 'owner_perms', self.owner_perms)
+
+ # 'inherit' permissions means inherit the permissions from the next level 'down'
+ if 'inherit' in authenticated_perms:
+ authenticated_perms = authenticated_perms + list(set(anonymous_perms) - set(authenticated_perms))
+ if 'inherit' in owner_perms:
+ owner_perms = owner_perms + list(set(authenticated_perms) - set(owner_perms))
+
+ # apply user permissions and return
+ perms = set()
+ if user.is_anonymous:
+ perms = perms.union(set(anonymous_perms))
+ else:
+ if self.is_owner(user, model, obj):
+ perms = perms.union(set(owner_perms))
+ else:
+ perms = perms.union(set(authenticated_perms))
+ return perms
+
def user_permissions(self, user, obj_or_model, obj=None):
"""
Filter user permissions for a model class
@@ -66,47 +93,20 @@ class LDPPermissions(DjangoObjectPermissions):
else:
obj = obj_or_model
model = obj_or_model.__class__
-
model_name = model._meta.model_name
- user_key = 'None' if user is None else user.id
- obj_key = 'None' if obj is None else obj.id
- perms_cache_key = 'User{}{}{}'.format(user_key, model_name, obj_key)
- if self.with_cache and perms_cache_key in self.perms_cache:
- return self.perms_cache[perms_cache_key]
- # Get Anonymous permissions from Model's Meta. If not found use default
- anonymous_perms = getattr(model._meta, 'anonymous_perms', self.anonymous_perms)
-
- # Get Auth permissions from Model's Meta. If not found use default
- authenticated_perms = getattr(model._meta, 'authenticated_perms', self.authenticated_perms)
- # Extend Auth if inherit is given
- if 'inherit' in authenticated_perms:
- authenticated_perms = authenticated_perms + list(set(anonymous_perms) - set(authenticated_perms))
-
- # Get Owner permissions from Model's Meta. If not found use default
- owner_perms = getattr(model._meta, 'owner_perms', self.owner_perms)
- # Extend Owner if inherit is given
- if 'inherit' in owner_perms:
- owner_perms = owner_perms + list(set(authenticated_perms) - set(owner_perms))
+ perms_cache_key = self._get_cache_key(model_name, user, obj)
+ if self.with_cache:
+ if perms_cache_key in self.perms_cache:
+ return self.perms_cache[perms_cache_key]
# return permissions - using set to avoid duplicates
- # apply Django-Guardian (object-level) permissions
- perms = set()
+ perms = self._get_model_level_perms(model, user, obj)
if obj is not None and not user.is_anonymous:
# get permissions from all backends and then remove model name from the permissions
- model_name = model._meta.model_name
forbidden_string = "_" + model_name
- perms = set([p.replace(forbidden_string, '') for p in _user_get_all_permissions(user, obj)])
-
- # apply anon, owner and auth permissions
- if user.is_anonymous:
- perms = perms.union(set(anonymous_perms))
- else:
- if self.is_owner(user, model, obj):
- perms = perms.union(set(owner_perms))
- else:
- perms = perms.union(set(authenticated_perms))
+ perms = perms.union(set([p.replace(forbidden_string, '') for p in _user_get_all_permissions(user, obj)]))
self.perms_cache[perms_cache_key] = list(perms)
--
GitLab
From 2118afa5b1dbe34b466a4a0779acfa758a342a54 Mon Sep 17 00:00:00 2001
From: Calum Mackervoy <c.mackervoy@gmail.com>
Date: Tue, 10 Nov 2020 07:34:01 +0000
Subject: [PATCH 5/8] update: prefilter moved to LDPViewSet.filter_queryset
---
README.md | 2 -
djangoldp/permissions.py | 63 ++++++++---------------
djangoldp/serializers.py | 12 -----
djangoldp/tests/models.py | 12 +----
djangoldp/tests/tests_guardian.py | 62 +++++++++++-----------
djangoldp/tests/tests_user_permissions.py | 29 ++++++++++-
djangoldp/views.py | 26 +++++-----
7 files changed, 94 insertions(+), 112 deletions(-)
diff --git a/README.md b/README.md
index 932ae167..cf2760d5 100644
--- a/README.md
+++ b/README.md
@@ -337,8 +337,6 @@ This allows you to add permissions for anonymous, logged in user, author ... in
By default `LDPPermissions` is used.
Specific permissin classes can be developed to fit special needs.
-The method `prefilter_query_s_set()` should be override and must add a filter to the query_set removing object without at least the `view` permission
-
## anonymous_perms, user_perms, owner_perms
Those allow you to set permissions from your model's meta.
diff --git a/djangoldp/permissions.py b/djangoldp/permissions.py
index 9a3e2463..68a251cf 100644
--- a/djangoldp/permissions.py
+++ b/djangoldp/permissions.py
@@ -2,9 +2,7 @@ import time
from django.conf import settings
from django.contrib.auth.models import _user_get_all_permissions
from django.core.exceptions import PermissionDenied
-from django.db.models import QuerySet
from django.db.models.base import ModelBase
-from guardian.shortcuts import get_objects_for_user
from rest_framework.permissions import DjangoObjectPermissions
@@ -30,26 +28,8 @@ class LDPPermissions(DjangoObjectPermissions):
if time.time() - cls.perms_cache['time'] > 5:
cls.invalidate_cache()
- def prefilter_query_set(self, query_set, request, view, model) -> QuerySet:
- """
- Must filter object that user doesn't have at least the `view` permission
- Should reflect user_permissions
- :param query_set: the base QuerySet
- :param request: the request
- :param view: the view
- :param model: the model
- :return: the filtered QuerySet
- """
- if self.has_object_view_permission(request, view, model):
- return query_set
- else:
- if not request.user.is_anonymous:
- permissions_needed = LDPPermissions.get_permissions(request.method, model)
- return get_objects_for_user(request.user, permissions_needed, query_set)
- else:
- return model.objects.none()
-
- def is_owner(self, user, model, obj):
+ @classmethod
+ def is_owner(cls, user, model, obj):
return obj and hasattr(model._meta, 'owner_field') and (
getattr(obj, getattr(model._meta, 'owner_field')) == user
or (hasattr(user, 'urlid') and getattr(obj, getattr(model._meta, 'owner_field')) == user.urlid)
@@ -60,10 +40,12 @@ class LDPPermissions(DjangoObjectPermissions):
obj_key = 'None' if obj is None else obj.id
return 'User{}{}{}'.format(user_key, model_name, obj_key)
- def _get_model_level_perms(self, model, user, obj):
- anonymous_perms = getattr(model._meta, 'anonymous_perms', self.anonymous_perms)
- authenticated_perms = getattr(model._meta, 'authenticated_perms', self.authenticated_perms)
- owner_perms = getattr(model._meta, 'owner_perms', self.owner_perms)
+ @classmethod
+ def get_model_level_perms(cls, model, user, obj=None):
+ '''Auxiliary function returns the model-level anon-auth-owner permissions for a given, model, user and object'''
+ anonymous_perms = getattr(model._meta, 'anonymous_perms', cls.anonymous_perms)
+ authenticated_perms = getattr(model._meta, 'authenticated_perms', cls.authenticated_perms)
+ owner_perms = getattr(model._meta, 'owner_perms', cls.owner_perms)
# 'inherit' permissions means inherit the permissions from the next level 'down'
if 'inherit' in authenticated_perms:
@@ -76,7 +58,7 @@ class LDPPermissions(DjangoObjectPermissions):
if user.is_anonymous:
perms = perms.union(set(anonymous_perms))
else:
- if self.is_owner(user, model, obj):
+ if cls.is_owner(user, model, obj):
perms = perms.union(set(owner_perms))
else:
perms = perms.union(set(authenticated_perms))
@@ -101,7 +83,7 @@ class LDPPermissions(DjangoObjectPermissions):
return self.perms_cache[perms_cache_key]
# return permissions - using set to avoid duplicates
- perms = self._get_model_level_perms(model, user, obj)
+ perms = self.get_model_level_perms(model, user, obj)
if obj is not None and not user.is_anonymous:
# get permissions from all backends and then remove model name from the permissions
@@ -187,21 +169,20 @@ class LDPPermissions(DjangoObjectPermissions):
model = obj
user_perms = self.user_permissions(request.user, model, obj)
- return self.compare_permissions(perms, user_perms)
+ return LDPPermissions.compare_permissions(perms, user_perms)
- def has_object_view_permission(self, request, view, obj):
- """
- Access to objects
- User have view permission on request: Continue
- User does not have permission: 403
- """
- # get permissions required
- perms = LDPPermissions.get_permissions('GET', obj)
- model = obj
- user_perms = self.user_permissions(request.user, model, obj)
-
- return self.compare_permissions(perms, user_perms)
+ @classmethod
+ def has_model_view_permission(cls, request, model):
+ '''
+ shortcut to compare the requested user's permissions on the model-level
+ :return: True or False
+ '''
+ # compare required permissions with those I have (on the model)
+ perms = LDPPermissions.get_permissions('GET', model)
+ user_perms = LDPPermissions.get_model_level_perms(model, request.user)
+ return cls.compare_permissions(perms, user_perms)
+ @classmethod
def compare_permissions(self, perms, user_perms):
# compare them with the permissions I have
for perm in perms:
diff --git a/djangoldp/serializers.py b/djangoldp/serializers.py
index a7496a46..ecd1b3d6 100644
--- a/djangoldp/serializers.py
+++ b/djangoldp/serializers.py
@@ -97,8 +97,6 @@ class LDListMixin:
parent_model = None
- permissions_class = Model.get_permission_classes(child_model, [LDPPermissions])[0]()
-
if not isinstance(value, Iterable) and not isinstance(value, QuerySet):
if not self.id.startswith('http'):
self.id = '{}{}{}'.format(settings.BASE_URL, Model.resource(parent_model), self.id)
@@ -123,16 +121,6 @@ class LDListMixin:
if self.with_cache and self.to_representation_cache.has(cache_key):
return self.to_representation_cache.get(cache_key)
- if isinstance(value, QuerySet):
- # Perf: filter on request if not a subclass of LDPPermissions
- value = permissions_class.prefilter_query_set(value, self.context['request'], self.context['view'], child_model)
-
- # remove objects from the list which I don't have permission to view
- # Perf: filtering is uselss in case of LDPPermissions, make sense only for subclass
- # if type(permissions_class) is not LDPPermissions:
- # filtered_values = list(
- # filter(lambda v: permissions_class.has_object_permission(
- # self.context['request'], self.context['view'], v), value))
container_permissions = Model.get_permissions(child_model, self.context, ['add'])
container_permissions.extend(
Model.get_permissions(parent_model, self.context, ['view']))
diff --git a/djangoldp/tests/models.py b/djangoldp/tests/models.py
index 4699baab..b576ceb2 100644
--- a/djangoldp/tests/models.py
+++ b/djangoldp/tests/models.py
@@ -126,21 +126,11 @@ class LDPDummy(Model):
owner_perms = ['inherit', 'change', 'delete', 'control']
-class DummyAnonPerm(Model):
- some = models.CharField(max_length=255, blank=True, null=True)
- parent = models.ForeignKey(LDPDummy, on_delete=models.DO_NOTHING, related_name="anons", blank=True, null=True)
-
- class Meta(Model.Meta):
- anonymous_perms = []
- authenticated_perms = ['view', 'add']
- owner_perms = ['inherit', 'change', 'delete', 'control']
- permission_classes = [HalfRandomPermissions]
-
-
# model used in django-guardian permission tests (no anonymous etc permissions set)
class PermissionlessDummy(Model):
some = models.CharField(max_length=255, blank=True, null=True)
slug = models.SlugField(blank=True, null=True, unique=True)
+ parent = models.ForeignKey(LDPDummy, on_delete=models.DO_NOTHING, related_name="anons", blank=True, null=True)
class Meta(Model.Meta):
anonymous_perms = []
diff --git a/djangoldp/tests/tests_guardian.py b/djangoldp/tests/tests_guardian.py
index 6984891d..1ceb9b2a 100644
--- a/djangoldp/tests/tests_guardian.py
+++ b/djangoldp/tests/tests_guardian.py
@@ -1,10 +1,11 @@
import json
+import uuid
from django.contrib.auth import get_user_model
from djangoldp.serializers import LDListMixin, LDPSerializer
from rest_framework.test import APIClient, APITestCase
-from guardian.shortcuts import assign_perm, remove_perm
+from guardian.shortcuts import assign_perm
-from .models import PermissionlessDummy, Dummy, LDPDummy, DummyAnonPerm
+from .models import PermissionlessDummy, Dummy, LDPDummy
from djangoldp.permissions import LDPPermissions
@@ -24,13 +25,20 @@ class TestsGuardian(APITestCase):
LDListMixin.to_representation_cache.invalidate_cache()
LDPSerializer.to_representation_cache.invalidate_cache()
- # optional setup for testing PermissionlessDummy model with parameterised perms
- def setUpGuardianDummyWithPerms(self, perms=[]):
- self.dummy = PermissionlessDummy.objects.create(some='test', slug='test')
+ def _get_dummy_with_perms(self, perms=None, parent=None):
+ if perms is None:
+ perms = []
+ dummy = PermissionlessDummy.objects.create(some='test', slug=uuid.uuid4(), parent=parent)
model_name = PermissionlessDummy._meta.model_name
for perm in perms:
- assign_perm(perm + '_' + model_name, self.user, self.dummy)
+ assign_perm(perm + '_' + model_name, self.user, dummy)
+
+ return dummy
+
+ # optional setup for testing PermissionlessDummy model with parameterised perms
+ def setUpGuardianDummyWithPerms(self, perms=None, parent=None):
+ self.dummy = self._get_dummy_with_perms(perms, parent)
# test that dummy with no permissions set returns no results
def test_get_dummy_no_permissions(self):
@@ -96,38 +104,26 @@ class TestsGuardian(APITestCase):
result = permissions.user_permissions(self.user, dummy)
self.assertEqual(result.count('view'), 1)
- # test list with and without view perms
- def test_get_list(self):
- DummyAnonPerm.objects.create(some='test1')
- DummyAnonPerm.objects.create(some='test2')
-
- response = self.client.get('/dummyanonperms/')
-
+ # TODO: https://git.startinblox.com/djangoldp-packages/djangoldp/issues/297
+ '''def test_list_dummy_exception(self):
+ self.setUpLoggedInUser()
+ # I have permission on a permissionless dummy, but not in general
+ dummy_a = self._get_dummy_with_perms()
+ dummy_b = self._get_dummy_with_perms(['view'])
+ response = self.client.get('/permissionless-dummys/')
self.assertEqual(response.status_code, 200)
self.assertEqual(len(response.data['ldp:contains']), 1)
+ self.assertNotIn(response.data['ldp:contains'], dummy_a.urlid)
+ self.assertIn(response.data['ldp:contains'], dummy_b.urlid)'''
+ # TODO:
+ '''def test_list_dummy_exception_nested(self):
self.setUpLoggedInUser()
-
- response = self.client.get('/dummyanonperms/')
-
- self.assertEqual(response.status_code, 200)
- self.assertEqual(len(response.data['ldp:contains']), 2)
-
- def test_get_sub_list(self):
parent = LDPDummy.objects.create(some="test")
- parent.anons.add(DummyAnonPerm.objects.create(some='test1'))
- parent.anons.add(DummyAnonPerm.objects.create(some='test2'))
- parent.save()
-
- response = self.client.get('/ldpdummys/{}/'.format(parent.pk))
-
- self.assertEqual(response.status_code, 200)
- self.assertEqual(len(response.data['anons']['ldp:contains']), 1)
-
- self.setUpLoggedInUser()
-
+ # two dummies, one I have permission to view and one I don't
+ dummy_a = self._get_dummy_with_perms(parent=parent)
+ dummy_b = self._get_dummy_with_perms(['view'], parent)
response = self.client.get('/ldpdummys/{}/'.format(parent.pk))
-
self.assertEqual(response.status_code, 200)
- self.assertEqual(len(response.data['anons']['ldp:contains']), 2)
+ self.assertEqual(len(response.data['anons']['ldp:contains']), 1)'''
diff --git a/djangoldp/tests/tests_user_permissions.py b/djangoldp/tests/tests_user_permissions.py
index 287405b8..6f9fe195 100644
--- a/djangoldp/tests/tests_user_permissions.py
+++ b/djangoldp/tests/tests_user_permissions.py
@@ -1,6 +1,7 @@
from django.contrib.auth import get_user_model
+from django.contrib.auth.models import Permission, Group
from rest_framework.test import APIClient, APITestCase
-from .models import JobOffer
+from .models import JobOffer, LDPDummy, PermissionlessDummy
import json
@@ -13,10 +14,36 @@ class TestUserPermissions(APITestCase):
self.client.force_authenticate(user=self.user)
self.job = JobOffer.objects.create(title="job", slug="slug1")
+ def setUpGroup(self):
+ self.group = Group.objects.create(name='Test')
+ view_perm = Permission.objects.get(codename='view_permissionlessdummy')
+ self.group.permissions.add(view_perm)
+ self.group.save()
+
+ # list - simple
def test_get_for_authenticated_user(self):
response = self.client.get('/job-offers/')
self.assertEqual(response.status_code, 200)
+ # list - I do not have permission from the model, but I do have permission via a Group I am assigned
+ # TODO: https://git.startinblox.com/djangoldp-packages/djangoldp/issues/291
+ '''def test_group_list_access(self):
+ self.setUpGroup()
+
+ response = self.client.get('/permissionless-dummys/')
+ self.assertEqual(response.status_code, 403)
+
+ self.user.groups.add(self.group)
+ self.user.save()
+ response = self.client.get('/permissionless-dummys/')
+ self.assertEqual(response.status_code, 200)'''
+
+ # repeat of the above test on nested field
+ '''def test_group_list_access_nested(self):
+ self.setUpGroup()
+ parent = LDPDummy.objects.create()
+ dummy = PermissionlessDummy.objects.create(parent=parent)'''
+
def test_get_1_for_authenticated_user(self):
response = self.client.get('/job-offers/{}/'.format(self.job.slug))
self.assertEqual(response.status_code, 200)
diff --git a/djangoldp/views.py b/djangoldp/views.py
index aa77ae01..6c01c35c 100644
--- a/djangoldp/views.py
+++ b/djangoldp/views.py
@@ -22,6 +22,7 @@ from rest_framework.response import Response
from rest_framework.utils import model_meta
from rest_framework.views import APIView
from rest_framework.viewsets import ModelViewSet
+from guardian.shortcuts import get_objects_for_user
from djangoldp.activities import ActivityPubService
from djangoldp.activities import ActivityQueueService, as_activitystream
@@ -546,17 +547,23 @@ class LDPViewSet(LDPViewSetGenerator):
serializer.save(**kwargs)
def get_queryset(self, *args, **kwargs):
- if self.model:
- query_set = self.model.objects.all()
- permissions_class = Model.get_permission_classes(self.model, [LDPPermissions])[0]()
- return permissions_class.prefilter_query_set(query_set, self.request, None, self.model)
+ return self.model.objects.all()
+
+ def filter_queryset(self, queryset):
+ queryset = super(LDPViewSet, self).filter_queryset(queryset)
+
+ # compares the requirement for GET, with what the user has on the MODEL
+ if LDPPermissions.has_model_view_permission(self.request, self.model):
+ return queryset
else:
- return super(LDPView, self).get_queryset(*args, **kwargs)
+ if not self.request.user.is_anonymous:
+ permissions_needed = LDPPermissions.get_permissions(self.request.method, self.model)
+ return get_objects_for_user(self.request.user, permissions_needed, queryset)
+ else:
+ return self.model.objects.none()
def dispatch(self, request, *args, **kwargs):
'''overriden dispatch method to append some custom headers'''
- # pr = cProfile.Profile()
- # pr.enable()
response = super(LDPViewSet, self).dispatch(request, *args, **kwargs)
response["Access-Control-Allow-Origin"] = request.META.get('HTTP_ORIGIN')
response["Access-Control-Allow-Methods"] = "GET,POST,PUT,PATCH,DELETE"
@@ -574,11 +581,6 @@ class LDPViewSet(LDPViewSetGenerator):
response['User'] = request.user.webid()
except AttributeError:
pass
- # pr.disable()
- # s = io.StringIO()
- # ps = pstats.Stats(pr, stream=s)
- # ps.print_stats()
- # print(s.getvalue())
return response
--
GitLab
From 15046c1bbe2060d979c8fb4f1a2f7b3ff9847d5e Mon Sep 17 00:00:00 2001
From: Calum Mackervoy <c.mackervoy@gmail.com>
Date: Wed, 11 Nov 2020 13:43:41 +0000
Subject: [PATCH 6/8] minor: switched to filter_backends based system
---
README.md | 14 ++++---
djangoldp/filters.py | 26 +++++++++++-
djangoldp/models.py | 26 ++++++++++++
djangoldp/permissions.py | 4 ++
djangoldp/serializers.py | 6 ++-
djangoldp/tests/tests_guardian.py | 15 +++++--
djangoldp/tests/tests_user_permissions.py | 2 +-
djangoldp/views.py | 51 ++---------------------
8 files changed, 84 insertions(+), 60 deletions(-)
diff --git a/README.md b/README.md
index cf2760d5..442a2ecc 100644
--- a/README.md
+++ b/README.md
@@ -253,6 +253,8 @@ In the following example, besides the urls `/members/` and `/members/<pk>/`, two
## Filter Backends
+### LocalObjectFilterBackend: Excluding distant resources
+
To achieve federation, DjangoLDP includes links to objects from federated servers and stores these as local objects (see 1.0 - Models). In some situations, you will want to exclude these from the queryset of a custom view
To provide for this need, there is defined in `djangoldp.filters` a FilterBackend which can be included in custom viewsets to restrict the queryset to only objects which were created locally:
@@ -327,17 +329,17 @@ class MyModel(models.Model):
Now when an instance of `MyModel` is saved, its `author_user` property will be set to the **profile** of the authenticated user.
-## permissions
-
-Django-Guardian is used by default to support object-level permissions. Custom permissions can be added to your model using this attribute. See the [Django-Guardian documentation](https://django-guardian.readthedocs.io/en/stable/userguide/assign.html) for more information
-
-## permissions_classes
+### permissions_classes
This allows you to add permissions for anonymous, logged in user, author ... in the url:
By default `LDPPermissions` is used.
Specific permissin classes can be developed to fit special needs.
-## anonymous_perms, user_perms, owner_perms
+For developing custom permissions classes using `LDPPermissions`, please see the DjangoLDP [guide on permisssions](https://git.startinblox.com/djangoldp-packages/djangoldp/wikis/guides/custom-permissions)
+
+Django-Guardian is used by default to support object-level permissions. Custom permissions can be added to your model using this attribute. See the [Django-Guardian documentation](https://django-guardian.readthedocs.io/en/stable/userguide/assign.html) for more information
+
+### anonymous_perms, user_perms, owner_perms
Those allow you to set permissions from your model's meta.
diff --git a/djangoldp/filters.py b/djangoldp/filters.py
index 59afd6b9..c002770a 100644
--- a/djangoldp/filters.py
+++ b/djangoldp/filters.py
@@ -1,5 +1,25 @@
+from guardian.shortcuts import get_objects_for_user
from rest_framework.filters import BaseFilterBackend
-from djangoldp.models import Model
+
+
+class LDPPermissionsFilterBackend(BaseFilterBackend):
+ """
+ Default FilterBackend for LDPPermissions. If user does not have model-level permissions, filters by
+ Django-Guardian's get_objects_for_user
+ """
+ def filter_queryset(self, request, queryset, view):
+ from djangoldp.permissions import LDPPermissions
+
+ # compares the requirement for GET, with what the user has on the MODEL
+ if LDPPermissions.has_model_view_permission(request, view.model):
+ return queryset
+ else:
+ if not request.user.is_anonymous:
+ permissions_needed = LDPPermissions.get_permissions(request.method, view.model)
+ queryset = get_objects_for_user(request.user, permissions_needed, queryset)
+ return queryset
+ else:
+ return view.model.objects.none()
class LocalObjectFilterBackend(BaseFilterBackend):
@@ -8,6 +28,8 @@ class LocalObjectFilterBackend(BaseFilterBackend):
For querysets which should only include local objects
"""
def filter_queryset(self, request, queryset, view):
+ from djangoldp.models import Model
+
internal_ids = [x.pk for x in queryset if not Model.is_external(x)]
return queryset.filter(pk__in=internal_ids)
@@ -18,6 +40,8 @@ class LocalObjectOnContainerPathBackend(LocalObjectFilterBackend):
is the model container path
"""
def filter_queryset(self, request, queryset, view):
+ from djangoldp.models import Model
+
if issubclass(view.model, Model) and request.path_info == view.model.get_container_path():
return super(LocalObjectOnContainerPathBackend, self).filter_queryset(request, queryset, view)
return queryset
diff --git a/djangoldp/models.py b/djangoldp/models.py
index 7e2a485b..2c44edb1 100644
--- a/djangoldp/models.py
+++ b/djangoldp/models.py
@@ -59,6 +59,32 @@ class Model(models.Model):
def __init__(self, *args, **kwargs):
super(Model, self).__init__(*args, **kwargs)
+ @classmethod
+ def filter_backends(cls):
+ '''constructs a list of filter_backends configured on the permissions classes applied to this model'''
+ filtered_classes = [p for p in cls.get_permission_classes(cls, [LDPPermissions]) if
+ hasattr(p, 'filter_backends') and p.filter_backends is not None]
+ filter_backends = list()
+ for p in filtered_classes:
+ filter_backends = list(set(filter_backends).union(set(p.filter_backends)))
+ return filter_backends
+
+ @classmethod
+ def get_queryset(cls, request, view, queryset=None, model=None):
+ '''
+ when serializing as a child of another resource (my model has a many-to-one relationship with some parent),
+ get_queryset is used to obtain the resources which should be displayed. This allows us to exclude those objects
+ which I do not have permission to view in an automatically generated serializer
+ '''
+ if queryset is None:
+ queryset = cls.objects.all()
+ # this is a hack - sorry! https://git.startinblox.com/djangoldp-packages/djangoldp/issues/301/
+ if model is not None:
+ view.model = model
+ for backend in list(cls.filter_backends()):
+ queryset = backend().filter_queryset(request, queryset, view)
+ return queryset
+
@classmethod
def get_view_set(cls):
'''returns the view_set defined in the model Meta or the LDPViewSet class'''
diff --git a/djangoldp/permissions.py b/djangoldp/permissions.py
index 68a251cf..3f9a2d61 100644
--- a/djangoldp/permissions.py
+++ b/djangoldp/permissions.py
@@ -4,6 +4,7 @@ from django.contrib.auth.models import _user_get_all_permissions
from django.core.exceptions import PermissionDenied
from django.db.models.base import ModelBase
from rest_framework.permissions import DjangoObjectPermissions
+from djangoldp.filters import LDPPermissionsFilterBackend
class LDPPermissions(DjangoObjectPermissions):
@@ -11,6 +12,9 @@ class LDPPermissions(DjangoObjectPermissions):
anonymous_perms = ['view']
authenticated_perms = ['inherit']
owner_perms = ['inherit']
+ # filter backends associated with the permissions class. This will be used to filter queryset in the (auto-generated)
+ # view for a model, and in the serializing nested fields
+ filter_backends = [LDPPermissionsFilterBackend]
perms_cache = {
'time': time.time()
diff --git a/djangoldp/serializers.py b/djangoldp/serializers.py
index ecd1b3d6..e37e3358 100644
--- a/djangoldp/serializers.py
+++ b/djangoldp/serializers.py
@@ -14,7 +14,6 @@ from django.urls.resolvers import get_resolver
from django.db.models import QuerySet
from django.utils.datastructures import MultiValueDictKeyError
from django.utils.encoding import uri_to_iri
-from guardian.shortcuts import get_objects_for_user
from rest_framework.exceptions import ValidationError
from rest_framework.fields import SkipField, empty, ReadOnlyField
from rest_framework.fields import get_error_detail, set_value
@@ -121,6 +120,11 @@ class LDListMixin:
if self.with_cache and self.to_representation_cache.has(cache_key):
return self.to_representation_cache.get(cache_key)
+ # filter the queryset automatically based on child model permissions classes (filter_backends)
+ if isinstance(value, QuerySet) and hasattr(child_model, 'get_queryset'):
+ value = child_model.get_queryset(self.context['request'], self.context['view'], queryset=value,
+ model=child_model)
+
container_permissions = Model.get_permissions(child_model, self.context, ['add'])
container_permissions.extend(
Model.get_permissions(parent_model, self.context, ['view']))
diff --git a/djangoldp/tests/tests_guardian.py b/djangoldp/tests/tests_guardian.py
index 1ceb9b2a..c66369f3 100644
--- a/djangoldp/tests/tests_guardian.py
+++ b/djangoldp/tests/tests_guardian.py
@@ -116,8 +116,17 @@ class TestsGuardian(APITestCase):
self.assertNotIn(response.data['ldp:contains'], dummy_a.urlid)
self.assertIn(response.data['ldp:contains'], dummy_b.urlid)'''
- # TODO:
- '''def test_list_dummy_exception_nested(self):
+ def test_list_dummy_exception_nested_view(self):
+ self.setUpLoggedInUser()
+ parent = LDPDummy.objects.create(some="test")
+ # two dummies, one I have permission to view and one I don't
+ dummy_a = self._get_dummy_with_perms(parent=parent)
+ dummy_b = self._get_dummy_with_perms(['view'], parent)
+ response = self.client.get('/ldpdummys/{}/anons/'.format(parent.pk))
+ self.assertEqual(response.status_code, 200)
+ self.assertEqual(len(response.data['ldp:contains']), 1)
+
+ def test_list_dummy_exception_nested_serializer(self):
self.setUpLoggedInUser()
parent = LDPDummy.objects.create(some="test")
# two dummies, one I have permission to view and one I don't
@@ -125,5 +134,5 @@ class TestsGuardian(APITestCase):
dummy_b = self._get_dummy_with_perms(['view'], parent)
response = self.client.get('/ldpdummys/{}/'.format(parent.pk))
self.assertEqual(response.status_code, 200)
- self.assertEqual(len(response.data['anons']['ldp:contains']), 1)'''
+ self.assertEqual(len(response.data['anons']['ldp:contains']), 1)
diff --git a/djangoldp/tests/tests_user_permissions.py b/djangoldp/tests/tests_user_permissions.py
index 6f9fe195..2f61261c 100644
--- a/djangoldp/tests/tests_user_permissions.py
+++ b/djangoldp/tests/tests_user_permissions.py
@@ -38,7 +38,7 @@ class TestUserPermissions(APITestCase):
response = self.client.get('/permissionless-dummys/')
self.assertEqual(response.status_code, 200)'''
- # repeat of the above test on nested field
+ # TODO: repeat of the above test on nested field
'''def test_group_list_access_nested(self):
self.setUpGroup()
parent = LDPDummy.objects.create()
diff --git a/djangoldp/views.py b/djangoldp/views.py
index 6c01c35c..61ee5e05 100644
--- a/djangoldp/views.py
+++ b/djangoldp/views.py
@@ -22,7 +22,6 @@ from rest_framework.response import Response
from rest_framework.utils import model_meta
from rest_framework.views import APIView
from rest_framework.viewsets import ModelViewSet
-from guardian.shortcuts import get_objects_for_user
from djangoldp.activities import ActivityPubService
from djangoldp.activities import ActivityQueueService, as_activitystream
@@ -393,9 +392,9 @@ class LDPViewSet(LDPViewSetGenerator):
# attach filter backends based on permissions classes, to reduce the queryset based on these permissions
# https://www.django-rest-framework.org/api-guide/filtering/#generic-filtering
if self.permission_classes:
- for p in self.permission_classes:
- if hasattr(p, 'filter_class') and p.filter_class:
- self.filter_backends.append(p.filter_class)
+ filtered_classes = [p for p in self.permission_classes if hasattr(p, 'filter_backends') and p.filter_backends is not None]
+ for p in filtered_classes:
+ self.filter_backends = list(set(self.filter_backends).union(set(p.filter_backends)))
self.serializer_class = self.build_read_serializer()
self.write_serializer_class = self.build_write_serializer()
@@ -448,37 +447,6 @@ class LDPViewSet(LDPViewSetGenerator):
'''
return True
- # def list(self, request, *args, **kwargs):
- # t1 = time.time()
- # queryset = self.get_queryset()
- # t2 = time.time()
- # print('got queryset in ' + str(t2 - t1))
- #
- # t1 = time.time()
- # queryset = self.filter_queryset(queryset)
- # t2 = time.time()
- # print('filtered queryset in ' + str(t2 - t1))
- #
- # t1 = time.time()
- # page = self.paginate_queryset(queryset)
- # t2 = time.time()
- # print('paginated queryset in ' + str(t2-t1))
- # if page is not None:
- # t1 = time.time()
- # serializer = self.get_serializer(page, many=True)
- # paginated_response = self.get_paginated_response(serializer.data)
- # t2 = time.time()
- # print('paginated response in ' + str(t2-t1))
- #
- # return paginated_response
- #
- # t1 = time.time()
- # serializer = self.get_serializer(queryset, many=True)
- # response = Response(serializer.data)
- # t2 = time.time()
- # print('regular response in ' + str(t2-t1))
- # return response
-
def create(self, request, *args, **kwargs):
serializer = self.get_write_serializer(data=request.data)
serializer.is_valid(raise_exception=True)
@@ -549,19 +517,6 @@ class LDPViewSet(LDPViewSetGenerator):
def get_queryset(self, *args, **kwargs):
return self.model.objects.all()
- def filter_queryset(self, queryset):
- queryset = super(LDPViewSet, self).filter_queryset(queryset)
-
- # compares the requirement for GET, with what the user has on the MODEL
- if LDPPermissions.has_model_view_permission(self.request, self.model):
- return queryset
- else:
- if not self.request.user.is_anonymous:
- permissions_needed = LDPPermissions.get_permissions(self.request.method, self.model)
- return get_objects_for_user(self.request.user, permissions_needed, queryset)
- else:
- return self.model.objects.none()
-
def dispatch(self, request, *args, **kwargs):
'''overriden dispatch method to append some custom headers'''
response = super(LDPViewSet, self).dispatch(request, *args, **kwargs)
--
GitLab
From b403007593b85d0c426b12acd2e413657f76dc3d Mon Sep 17 00:00:00 2001
From: Calum Mackervoy <c.mackervoy@gmail.com>
Date: Thu, 12 Nov 2020 15:09:18 +0000
Subject: [PATCH 7/8] update: adapted for new cache
---
djangoldp/serializers.py | 3 ---
djangoldp/tests/tests_guardian.py | 8 ++++----
2 files changed, 4 insertions(+), 7 deletions(-)
diff --git a/djangoldp/serializers.py b/djangoldp/serializers.py
index 2ce691e0..28a8ad51 100644
--- a/djangoldp/serializers.py
+++ b/djangoldp/serializers.py
@@ -105,9 +105,6 @@ class LDListMixin:
cache_vary = str(self.context['request'].user)
- if isinstance(value, QuerySet):
- value = list(value)
-
if not isinstance(value, Iterable):
if not self.id.startswith('http'):
self.id = '{}{}{}'.format(settings.BASE_URL, Model.resource(parent_model), self.id)
diff --git a/djangoldp/tests/tests_guardian.py b/djangoldp/tests/tests_guardian.py
index c66369f3..79a770fd 100644
--- a/djangoldp/tests/tests_guardian.py
+++ b/djangoldp/tests/tests_guardian.py
@@ -14,16 +14,16 @@ class TestsGuardian(APITestCase):
def setUp(self):
self.client = APIClient(enforce_csrf_checks=True)
LDPPermissions.invalidate_cache()
- LDListMixin.to_representation_cache.invalidate_cache()
- LDPSerializer.to_representation_cache.invalidate_cache()
+ LDListMixin.to_representation_cache.reset()
+ LDPSerializer.to_representation_cache.reset()
def setUpLoggedInUser(self):
self.user = get_user_model().objects.create_user(username='john', email='jlennon@beatles.com',
password='glass onion')
self.client.force_authenticate(user=self.user)
LDPPermissions.invalidate_cache()
- LDListMixin.to_representation_cache.invalidate_cache()
- LDPSerializer.to_representation_cache.invalidate_cache()
+ LDListMixin.to_representation_cache.reset()
+ LDPSerializer.to_representation_cache.reset()
def _get_dummy_with_perms(self, perms=None, parent=None):
if perms is None:
--
GitLab
From d62014924ff5446f2ffd26247cba4c8ebb4455da Mon Sep 17 00:00:00 2001
From: Calum Mackervoy <c.mackervoy@gmail.com>
Date: Thu, 12 Nov 2020 15:48:41 +0000
Subject: [PATCH 8/8] update: using ObjectPermissionsFilter
---
djangoldp/filters.py | 15 ++++++---------
djangoldp/serializers.py | 2 +-
djangoldp/tests/settings_default.py | 1 +
setup.cfg | 1 +
4 files changed, 9 insertions(+), 10 deletions(-)
diff --git a/djangoldp/filters.py b/djangoldp/filters.py
index c002770a..b9c9bd01 100644
--- a/djangoldp/filters.py
+++ b/djangoldp/filters.py
@@ -1,8 +1,8 @@
-from guardian.shortcuts import get_objects_for_user
from rest_framework.filters import BaseFilterBackend
+from rest_framework_guardian.filters import ObjectPermissionsFilter
-class LDPPermissionsFilterBackend(BaseFilterBackend):
+class LDPPermissionsFilterBackend(ObjectPermissionsFilter):
"""
Default FilterBackend for LDPPermissions. If user does not have model-level permissions, filters by
Django-Guardian's get_objects_for_user
@@ -13,13 +13,10 @@ class LDPPermissionsFilterBackend(BaseFilterBackend):
# compares the requirement for GET, with what the user has on the MODEL
if LDPPermissions.has_model_view_permission(request, view.model):
return queryset
- else:
- if not request.user.is_anonymous:
- permissions_needed = LDPPermissions.get_permissions(request.method, view.model)
- queryset = get_objects_for_user(request.user, permissions_needed, queryset)
- return queryset
- else:
- return view.model.objects.none()
+ if not request.user.is_anonymous:
+ return super().filter_queryset(request, queryset, view)
+ # user is anonymous without anonymous permissions
+ return view.model.objects.none()
class LocalObjectFilterBackend(BaseFilterBackend):
diff --git a/djangoldp/serializers.py b/djangoldp/serializers.py
index 28a8ad51..89af7347 100644
--- a/djangoldp/serializers.py
+++ b/djangoldp/serializers.py
@@ -105,7 +105,7 @@ class LDListMixin:
cache_vary = str(self.context['request'].user)
- if not isinstance(value, Iterable):
+ if not isinstance(value, Iterable) and not isinstance(value, QuerySet):
if not self.id.startswith('http'):
self.id = '{}{}{}'.format(settings.BASE_URL, Model.resource(parent_model), self.id)
diff --git a/djangoldp/tests/settings_default.py b/djangoldp/tests/settings_default.py
index 3686e777..2634324c 100644
--- a/djangoldp/tests/settings_default.py
+++ b/djangoldp/tests/settings_default.py
@@ -93,3 +93,4 @@ LDP_RDF_CONTEXT={
}
SEND_BACKLINKS=False
GUARDIAN_AUTO_PREFETCH = True
+SERIALIZER_CACHE = False
diff --git a/setup.cfg b/setup.cfg
index 45f79b19..ae8396a8 100644
--- a/setup.cfg
+++ b/setup.cfg
@@ -19,6 +19,7 @@ install_requires =
pyld==1.0.5
django-guardian==2.3.0
django-brotli
+ djangorestframework-guardian
[options.extras_require]
dev =
--
GitLab