Skip to content

Commit cd6979b

Browse files
committed
fix(permissions): make sure to first check the base permissions
Currently we first check the generic permissions and the the views actual base permission. This doesn't make sense as we should first check e.g. whether a user is even authenticated before checking more specific permissions.
1 parent a5ab7d0 commit cd6979b

File tree

3 files changed

+37
-3
lines changed

3 files changed

+37
-3
lines changed

generic_permissions/permissions.py

+2-2
Original file line numberDiff line numberDiff line change
@@ -23,9 +23,9 @@ def check_permissions(self, request):
2323
"""
2424
Overwrite default implementation to check DGAP permissions.
2525
"""
26+
super().check_permissions(request)
2627
if request.method != "GET":
2728
self._check_permissions(request)
28-
super().check_permissions(request)
2929

3030
def _check_object_permissions(self, request, instance):
3131
"""
@@ -43,9 +43,9 @@ def check_object_permissions(self, request, instance):
4343
"""
4444
Overwrite default implementation to check DGAP object permissions.
4545
"""
46+
super().check_object_permissions(request, instance)
4647
if request.method != "GET":
4748
self._check_object_permissions(request, instance)
48-
super().check_object_permissions(request, instance)
4949

5050

5151
class BasePermission:

tests/test_permissions.py

+20-1
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
import pytest
22
from django.core.exceptions import ImproperlyConfigured, PermissionDenied
33
from django.urls import reverse
4+
from rest_framework.exceptions import PermissionDenied as DRFPermissionDenied
45
from rest_framework.status import (
56
HTTP_200_OK,
67
HTTP_201_CREATED,
@@ -14,7 +15,7 @@
1415
object_permission_for,
1516
permission_for,
1617
)
17-
from tests.views import Test1ViewSet, Test2ViewSet
18+
from tests.views import Test1ViewSet, Test2ViewSet, TestBaseViewSet
1819

1920
from .models import Model1, Model2
2021

@@ -215,3 +216,21 @@ def has_object_permission_for_both_mutations(self, request, instance):
215216
Test1ViewSet(request=request, format_kwarg="json").check_object_permissions(
216217
request, tm1
217218
)
219+
220+
221+
def test_base_permission(db, rf):
222+
ObjectPermissionsConfig.register_handler_class(DenyAll)
223+
PermissionsConfig.register_handler_class(DenyAll)
224+
225+
request = rf.patch("")
226+
request.authenticators = None
227+
request.data = {"text": "foo"}
228+
229+
# Make sure that the permission error is from DRF, not from the generic permissions
230+
with pytest.raises(DRFPermissionDenied):
231+
TestBaseViewSet(request=request, format_kwarg="json").check_permissions(request)
232+
233+
with pytest.raises(DRFPermissionDenied):
234+
TestBaseViewSet(request=request, format_kwarg="json").check_object_permissions(
235+
request, Model1.objects.create()
236+
)

tests/views.py

+15
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
from rest_framework.permissions import BasePermission
12
from rest_framework.viewsets import ModelViewSet
23

34
from generic_permissions.permissions import PermissionViewMixin
@@ -6,6 +7,14 @@
67
from . import models, serializers
78

89

10+
class BaseDenyAll(BasePermission):
11+
def has_permission(self, request, view):
12+
return False
13+
14+
def has_object_permission(self, request, view, obj):
15+
return False
16+
17+
918
class Test1ViewSet(PermissionViewMixin, VisibilityViewMixin, ModelViewSet):
1019
serializer_class = serializers.TestModel1Serializer
1120
queryset = models.Model1.objects.all()
@@ -14,3 +23,9 @@ class Test1ViewSet(PermissionViewMixin, VisibilityViewMixin, ModelViewSet):
1423
class Test2ViewSet(PermissionViewMixin, VisibilityViewMixin, ModelViewSet):
1524
serializer_class = serializers.TestModel2Serializer
1625
queryset = models.Model2.objects.all()
26+
27+
28+
class TestBaseViewSet(PermissionViewMixin, VisibilityViewMixin, ModelViewSet):
29+
permission_classes = [BaseDenyAll]
30+
serializer_class = serializers.TestModel1Serializer
31+
queryset = models.Model1.objects.all()

0 commit comments

Comments
 (0)