Skip to content

Commit 16d1af9

Browse files
committed
Ensure Django{Model,Object}Permissions don't hide exceptions.
Quietly catching `AttributeError` and `TypeError` when calling `get_queryset()` is rather insidious, as those exceptions get caught no matter where they might happen in the call stack.
1 parent 511c9b8 commit 16d1af9

File tree

2 files changed

+39
-8
lines changed

2 files changed

+39
-8
lines changed

rest_framework/permissions.py

+8-8
Original file line numberDiff line numberDiff line change
@@ -112,15 +112,15 @@ def has_permission(self, request, view):
112112
if getattr(view, '_ignore_model_permissions', False):
113113
return True
114114

115-
try:
115+
if hasattr(view, 'get_queryset'):
116116
queryset = view.get_queryset()
117-
except AttributeError:
117+
else:
118118
queryset = getattr(view, 'queryset', None)
119119

120120
assert queryset is not None, (
121121
'Cannot apply DjangoModelPermissions on a view that '
122-
'does not have `.queryset` property or overrides the '
123-
'`.get_queryset()` method.')
122+
'does not set `.queryset` or have `.get_queryset()` method.'
123+
)
124124

125125
perms = self.get_required_permissions(request.method, queryset.model)
126126

@@ -169,15 +169,15 @@ def get_required_object_permissions(self, method, model_cls):
169169
return [perm % kwargs for perm in self.perms_map[method]]
170170

171171
def has_object_permission(self, request, view, obj):
172-
try:
172+
if hasattr(view, 'get_queryset'):
173173
queryset = view.get_queryset()
174-
except AttributeError:
174+
else:
175175
queryset = getattr(view, 'queryset', None)
176176

177177
assert queryset is not None, (
178178
'Cannot apply DjangoObjectPermissions on a view that '
179-
'does not have `.queryset` property or overrides the '
180-
'`.get_queryset()` method.')
179+
'does not set `.queryset` or have `.get_queryset()` method.'
180+
)
181181

182182
model_cls = queryset.model
183183
user = request.user

tests/test_permissions.py

+31
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
import base64
44
import unittest
55

6+
import pytest
67
from django.contrib.auth.models import Group, Permission, User
78
from django.core.urlresolvers import ResolverMatch
89
from django.db import models
@@ -14,6 +15,9 @@
1415
)
1516
from rest_framework.compat import guardian
1617
from rest_framework.filters import DjangoObjectPermissionsFilter
18+
from rest_framework.permissions import (
19+
DjangoModelPermissions, DjangoObjectPermissions
20+
)
1721
from rest_framework.routers import DefaultRouter
1822
from rest_framework.test import APIRequestFactory
1923
from tests.models import BasicModel
@@ -468,3 +472,30 @@ def test_permission_denied_for_object_with_custom_detail(self):
468472
detail = response.data.get('detail')
469473
self.assertEqual(response.status_code, status.HTTP_403_FORBIDDEN)
470474
self.assertEqual(detail, self.custom_message)
475+
476+
477+
class CheckAttributeError(AttributeError):
478+
pass
479+
480+
481+
class GetQuerysetRaises(generics.ListAPIView):
482+
def get_queryset(self):
483+
raise CheckAttributeError("Something terrible occurred deep down in the call stack")
484+
485+
486+
class DjangoObjectPermissionsList(GetQuerysetRaises):
487+
permission_classes = (DjangoObjectPermissions,)
488+
489+
490+
class DjangoModelPermissionsList(GetQuerysetRaises):
491+
permission_classes = (DjangoModelPermissions,)
492+
493+
494+
def test_DjangoObjectPermissions_hiding():
495+
with pytest.raises(CheckAttributeError):
496+
DjangoObjectPermissionsList().check_object_permissions(request=None, obj=None)
497+
498+
499+
def test_DjangoModelPermissions_hiding():
500+
with pytest.raises(CheckAttributeError):
501+
DjangoModelPermissionsList().check_permissions(request=None)

0 commit comments

Comments
 (0)