[feature] Added permission class with support for view permissions #249

ManishShah120 · web-flow · commit 70e0ffa25221 · 2021-06-01T11:45:42.000-05:00
Closes #249
diff --git a/README.rst b/README.rst
@@ -600,6 +600,27 @@ Ensure the queryset of your views make use of
 `select_related <https://docs.djangoproject.com/en/3.0/ref/models/querysets/#select-related>`_
 in these cases to avoid generating too many queries.
 
+``DjangoModelPermissions``
+~~~~~~~~~~~~~~~~~~~~~~~~~~
+
+The default ``DjangoModelPermissions`` class doesn't checks for the
+``view`` permission of any object for ``GET`` requests. The extended
+``DjangoModelPermissions`` class overcomes this problem. In order to
+allow ``GET`` requests on any object it checks for the availability 
+of either ``view`` or ``change`` permissions.
+
+Usage example:
+
+.. code-block:: python
+
+    from openwisp_users.api.permissions import DjangoModelPermissions
+    from rest_framework.generics import ListCreateAPIView
+
+    class TemplateListCreateView(ListCreateAPIView):
+        serializer_class = TemplateSerializer
+        permission_classes = (DjangoModelPermissions,)
+        queryset = Template.objects.all()
+
 Django REST Framework Mixins
 ----------------------------
 
diff --git a/openwisp_users/api/permissions.py b/openwisp_users/api/permissions.py
@@ -1,5 +1,8 @@
 from django.utils.translation import gettext_lazy as _
 from rest_framework.permissions import BasePermission
+from rest_framework.permissions import (
+    DjangoModelPermissions as BaseDjangoModelPermissions,
+)
 from swapper import load_model
 
 Organization = load_model('openwisp_users', 'Organization')
@@ -66,3 +69,34 @@ class IsOrganizationOwner(BaseOrganizationPermission):
 
     def validate_membership(self, user, org):
         return org and (user.is_superuser or user.is_owner(org))
+
+
+class DjangoModelPermissions(BaseDjangoModelPermissions):
+    perms_map = {
+        'GET': ['%(app_label)s.view_%(model_name)s'],
+        'OPTIONS': [],
+        'HEAD': ['%(app_label)s.view_%(model_name)s'],
+        'POST': ['%(app_label)s.add_%(model_name)s'],
+        'PUT': ['%(app_label)s.change_%(model_name)s'],
+        'PATCH': ['%(app_label)s.change_%(model_name)s'],
+        'DELETE': ['%(app_label)s.delete_%(model_name)s'],
+    }
+
+    def has_permission(self, request, view):
+        # Workaround to ensure DjangoModelPermissions are not applied
+        # to the root view when using DefaultRouter.
+        if getattr(view, '_ignore_model_permissions', False):
+            return True
+
+        user = request.user
+        if not user or (not user.is_authenticated and self.authenticated_users_only):
+            return False
+
+        queryset = self._queryset(view)
+        perms = self.get_required_permissions(request.method, queryset.model)
+        change_perm = self.get_required_permissions('PUT', queryset.model)
+
+        if request.method == 'GET':
+            return user.has_perms(perms) or user.has_perms(change_perm)
+
+        return user.has_perms(perms)
diff --git a/tests/testapp/__init__.py b/tests/testapp/__init__.py
@@ -14,3 +14,11 @@ def _create_shelf(self, **kwargs):
         s.full_clean()
         s.save()
         return s
+
+    def _create_template(self, **kwargs):
+        options = dict(name='test-template')
+        options.update(kwargs)
+        t = self.template_model(**options)
+        t.full_clean()
+        t.save()
+        return t
diff --git a/tests/testapp/tests/test_permission_classes.py b/tests/testapp/tests/test_permission_classes.py
@@ -1,14 +1,23 @@
+from django.contrib.auth import get_user_model
+from django.contrib.auth.models import Permission
 from django.test import TestCase
 from django.urls import reverse
+from swapper import load_model
 
 from openwisp_users.api.throttling import AuthRateThrottle
 
+from ..models import Template
 from .mixins import TestMultitenancyMixin
 
+User = get_user_model()
+Group = load_model('openwisp_users', 'Group')
+OrganizationUser = load_model('openwisp_users', 'OrganizationUser')
+
 
 class TestPermissionClasses(TestMultitenancyMixin, TestCase):
     def setUp(self):
         AuthRateThrottle.rate = 0
+        self.template_model = Template
         self.member_url = reverse('test_api_member_view')
         self.manager_url = reverse('test_api_manager_view')
         self.owner_url = reverse('test_api_owner_view')
@@ -117,3 +126,106 @@ def test_organization_field_with_errored_parent(self):
         with self.assertRaises(AttributeError) as error:
             self.client.get(reverse('test_error_field_view'), **auth)
         self.assertIn('Organization not found', str(error.exception))
+
+    def _get_auth_template(self, user, org1):
+        OrganizationUser.objects.create(user=user, organization=org1, is_admin=True)
+        self.client.force_login(user)
+        token = self._obtain_auth_token(user)
+        auth = dict(HTTP_AUTHORIZATION=f'Bearer {token}')
+        t1 = self._create_template(organization=org1)
+        return (auth, t1)
+
+    def test_view_permission_with_operator(self):
+        user = self._get_user()
+        operator_group = Group.objects.filter(name='Operator')
+        user.groups.set(operator_group)
+        org1 = self._get_org()
+        auth, t1 = self._get_auth_template(user, org1)
+        with self.subTest('Get Template List'):
+            response = self.client.get(reverse('test_template_list'), **auth)
+            self.assertEqual(response.status_code, 403)
+        with self.subTest('Get Template Detail'):
+            response = self.client.get(
+                reverse('test_template_detail', args=[t1.pk]), **auth
+            )
+            self.assertEqual(response.status_code, 403)
+
+    def test_view_permission_with_administrator(self):
+        user = self._get_user()
+        administrator_group = Group.objects.get(name='Administrator')
+        change_perm = Permission.objects.get(codename='change_template')
+        administrator_group.permissions.add(change_perm)
+        user.groups.add(administrator_group)
+        org1 = self._get_org()
+        auth, t1 = self._get_auth_template(user, org1)
+        with self.subTest('Get Template List'):
+            response = self.client.get(reverse('test_template_list'), **auth)
+            self.assertEqual(response.status_code, 200)
+        with self.subTest('Get Template Detail'):
+            response = self.client.get(
+                reverse('test_template_detail', args=[t1.pk]), **auth
+            )
+            self.assertEqual(response.status_code, 200)
+        permissions = administrator_group.permissions.values_list('codename', flat=True)
+        self.assertFalse('view_template' in permissions)
+        self.assertTrue('change_template' in permissions)
+
+    def test_view_permission_with_operator_having_view_perm(self):
+        user = self._get_user()
+        operator_group = Group.objects.get(name='Operator')
+        view_perm = Permission.objects.get(codename='view_template')
+        operator_group.permissions.add(view_perm)
+        user.groups.add(operator_group)
+        org1 = self._get_org()
+        auth, t1 = self._get_auth_template(user, org1)
+        with self.subTest('Get Template List'):
+            response = self.client.get(reverse('test_template_list'), **auth)
+            self.assertEqual(response.status_code, 200)
+        with self.subTest('Get Template Detail'):
+            response = self.client.get(
+                reverse('test_template_detail', args=[t1.pk]), **auth
+            )
+            self.assertEqual(response.status_code, 200)
+        with self.subTest('Change Template Detail'):
+            data = {'name': 'change-template'}
+            response = self.client.patch(
+                reverse('test_template_detail', args=[t1.pk]), data, **auth
+            )
+            self.assertEqual(response.status_code, 403)
+        with self.subTest('Delete Template'):
+            response = self.client.delete(
+                reverse('test_template_detail', args=[t1.pk]), **auth
+            )
+            self.assertEqual(response.status_code, 403)
+
+    def test_view_django_model_permission_with_view_perm(self):
+        user = self._get_user()
+        user_permissions = Permission.objects.filter(codename='view_template')
+        user.user_permissions.add(*user_permissions)
+        user.organizations_dict  # force caching
+        org1 = self._get_org()
+        auth, t1 = self._get_auth_template(user, org1)
+        with self.subTest('Get Template List'):
+            response = self.client.get(reverse('test_template_list'), **auth)
+            self.assertEqual(response.status_code, 200)
+        with self.subTest('Get Template Detail'):
+            response = self.client.get(
+                reverse('test_template_detail', args=[t1.pk]), **auth
+            )
+            self.assertEqual(response.status_code, 200)
+
+    def test_view_django_model_permission_with_change_perm(self):
+        user = self._get_user()
+        user_permissions = Permission.objects.filter(codename='change_template')
+        user.user_permissions.add(*user_permissions)
+        user.organizations_dict  # force caching
+        org1 = self._get_org()
+        auth, t1 = self._get_auth_template(user, org1)
+        with self.subTest('Get Template List'):
+            response = self.client.get(reverse('test_template_list'), **auth)
+            self.assertEqual(response.status_code, 200)
+        with self.subTest('Get Template Detail'):
+            response = self.client.get(
+                reverse('test_template_detail', args=[t1.pk]), **auth
+            )
+            self.assertEqual(response.status_code, 200)
diff --git a/tests/testapp/urls.py b/tests/testapp/urls.py
@@ -48,4 +48,5 @@
         name='test_shelf_list_unauthorized_view',
     ),
     path('template/', views.template_list, name='test_template_list',),
+    path('template/<int:pk>/', views.template_detail, name='test_template_detail',),
 ]
diff --git a/tests/testapp/views.py b/tests/testapp/views.py
@@ -1,5 +1,9 @@
 import swapper
-from rest_framework.generics import ListAPIView, ListCreateAPIView
+from rest_framework.generics import (
+    ListAPIView,
+    ListCreateAPIView,
+    RetrieveUpdateDestroyAPIView,
+)
 from rest_framework.response import Response
 from rest_framework.views import APIView
 
@@ -14,6 +18,7 @@
 )
 from openwisp_users.api.permissions import (
     BaseOrganizationPermission,
+    DjangoModelPermissions,
     IsOrganizationManager,
     IsOrganizationMember,
     IsOrganizationOwner,
@@ -166,10 +171,23 @@ def get_queryset(self):
         return shelf.book_set.all()
 
 
-class TemplateListCreateView(ListCreateAPIView):
+class TemplateListCreateView(FilterByOrganizationManaged, ListCreateAPIView):
     serializer_class = TemplateSerializer
     authentication_classes = (BearerAuthentication,)
-    permission_classes = (IsOrganizationMember,)
+    permission_classes = (
+        IsOrganizationMember,
+        DjangoModelPermissions,
+    )
+    queryset = Template.objects.all()
+
+
+class TemplateDetailView(FilterByOrganizationManaged, RetrieveUpdateDestroyAPIView):
+    serializer_class = TemplateSerializer
+    authentication_classes = (BearerAuthentication,)
+    permission_classes = (
+        IsOrganizationMember,
+        DjangoModelPermissions,
+    )
     queryset = Template.objects.all()
 
 
@@ -188,3 +206,4 @@ class TemplateListCreateView(ListCreateAPIView):
 shelf_list_manager_view = ShelfListManagerView.as_view()
 shelf_list_owner_view = ShelfListOwnerView.as_view()
 template_list = TemplateListCreateView.as_view()
+template_detail = TemplateDetailView.as_view()

Original file line number	Diff line number	Diff line change
`@@ -48,4 +48,5 @@`
`48`	`48`	`name='test_shelf_list_unauthorized_view',`
`49`	`49`	`),`
`50`	`50`	`path('template/', views.template_list, name='test_template_list',),`
	`51`	`+ path('template/<int:pk>/', views.template_detail, name='test_template_detail',),`
`51`	`52`	`]`