From e391a6cb1151d9acae4720d1a45ef15caaca35d7 Mon Sep 17 00:00:00 2001
From: Calum Mackervoy <c.mackervoy@gmail.com>
Date: Fri, 28 Feb 2020 15:30:11 +0000
Subject: [PATCH] Bugfiexs with user_permissions
---
djangoldp/permissions.py | 18 +++++++++---------
djangoldp/tests/tests_guardian.py | 14 +++++++++++++-
2 files changed, 22 insertions(+), 10 deletions(-)
diff --git a/djangoldp/permissions.py b/djangoldp/permissions.py
index 30c2a340..2e4ed672 100644
--- a/djangoldp/permissions.py
+++ b/djangoldp/permissions.py
@@ -1,7 +1,7 @@
from django.core.exceptions import PermissionDenied
from django.db.models.base import ModelBase
from rest_framework.permissions import DjangoObjectPermissions
-from guardian.shortcuts import get_perms
+from guardian.shortcuts import get_user_perms
class LDPPermissions(DjangoObjectPermissions):
@@ -42,33 +42,33 @@ class LDPPermissions(DjangoObjectPermissions):
if 'inherit' in owner_perms:
owner_perms = owner_perms + list(set(authenticated_perms) - set(owner_perms))
- # return permissions
+ # return permissions - using set to avoid duplicates
# apply Django-Guardian (object-level) permissions
- perms = []
+ perms = set()
if obj is not None and not user.is_anonymous:
- guardian_perms = get_perms(user, obj)
+ guardian_perms = get_user_perms(user, obj)
model_name = model._meta.model_name
# remove model name from the permissions
forbidden_string = "_" + model_name
- perms = [p.replace(forbidden_string, '') for p in guardian_perms]
+ perms = set([p.replace(forbidden_string, '') for p in guardian_perms])
# apply anon, owner and auth permissions
if user.is_anonymous:
- perms = perms + anonymous_perms
+ 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):
- perms = perms + owner_perms
+ perms = perms.union(set(owner_perms))
else:
- perms = perms + authenticated_perms
+ perms = perms.union(set(authenticated_perms))
- return perms
+ return list(perms)
def filter_user_perms(self, user, obj_or_model, permissions):
# Only used on Model.get_permissions to translate permissions to LDP
diff --git a/djangoldp/tests/tests_guardian.py b/djangoldp/tests/tests_guardian.py
index 04bc2bb0..2be9d4c9 100644
--- a/djangoldp/tests/tests_guardian.py
+++ b/djangoldp/tests/tests_guardian.py
@@ -3,7 +3,7 @@ from django.contrib.auth import get_user_model
from rest_framework.test import APIClient, APITestCase
from guardian.shortcuts import assign_perm
-from .models import PermissionlessDummy
+from .models import PermissionlessDummy, Dummy
from djangoldp.permissions import LDPPermissions
@@ -90,3 +90,15 @@ class TestsGuardian(APITestCase):
permissions = LDPPermissions()
result = permissions.user_permissions(self.user, self.dummy)
self.assertIn('custom_permission', result)
+
+ # test that duplicate permissions aren't returned
+ def test_no_duplicate_permissions(self):
+ self.setUpLoggedInUser()
+ dummy = Dummy.objects.create(some='test', slug='test')
+ model_name = Dummy._meta.model_name
+
+ assign_perm('view_' + model_name, self.user, dummy)
+
+ permissions = LDPPermissions()
+ result = permissions.user_permissions(self.user, dummy)
+ self.assertEqual(result.count('view'), 1)
--
GitLab