diff --git a/README.rst b/README.rst index a023afd26..3b8a52b0e 100644 --- a/README.rst +++ b/README.rst @@ -600,6 +600,27 @@ Ensure the queryset of your views make use of `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 index 8979b6427..04c4c6ff0 100644 --- 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 index ec5cb8398..c364e18d0 100644 --- 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 index 89b7b3548..f69cda56c 100644 --- 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 index 625bd022a..6a7202ff2 100644 --- 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//', views.template_detail, name='test_template_detail',), ] diff --git a/tests/testapp/views.py b/tests/testapp/views.py index 716ce61db..4c84c9921 100644 --- 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()