diff --git a/docs/api-guide/permissions.md b/docs/api-guide/permissions.md index 6912c375c2..5cb9acd229 100644 --- a/docs/api-guide/permissions.md +++ b/docs/api-guide/permissions.md @@ -1,4 +1,4 @@ ---- +--- source: - permissions.py --- @@ -212,7 +212,8 @@ To implement a custom permission, override `BasePermission` and implement either * `.has_permission(self, request, view)` * `.has_object_permission(self, request, view, obj)` -The methods should return `True` if the request should be granted access, and `False` otherwise. +The methods should return `True` if the request should be granted access, and `False` if it should be denied. Most permission classes will only need to implement one of these methods. The base class implementations return a special truthy value called `Deferred` which is used to make and/or/not composition work correctly where `has_permission` should always +succeed in order to let object permissions be checked and `has_object_permission` should defer to the view-level permission. If you need to test if a request is a read operation or a write operation, you should check the request method against the constant `SAFE_METHODS`, which is a tuple containing `'GET'`, `'OPTIONS'` and `'HEAD'`. For example: diff --git a/rest_framework/permissions.py b/rest_framework/permissions.py index 3a8c580646..676e055437 100644 --- a/rest_framework/permissions.py +++ b/rest_framework/permissions.py @@ -8,6 +8,9 @@ SAFE_METHODS = ('GET', 'HEAD', 'OPTIONS') +Deferred = object() + + class OperationHolderMixin: def __and__(self, other): return OperandHolder(AND, self, other) @@ -53,16 +56,18 @@ def __init__(self, op1, op2): self.op2 = op2 def has_permission(self, request, view): - return ( - self.op1.has_permission(request, view) and - self.op2.has_permission(request, view) - ) + hasperm1 = self.op1.has_permission(request, view) + if not hasperm1: + return hasperm1 + hasperm2 = self.op2.has_permission(request, view) + return hasperm1 if hasperm2 is Deferred else hasperm2 def has_object_permission(self, request, view, obj): - return ( - self.op1.has_object_permission(request, view, obj) and - self.op2.has_object_permission(request, view, obj) - ) + hasperm1 = self.op1.has_object_permission(request, view, obj) + if not hasperm1: + return hasperm1 + hasperm2 = self.op2.has_object_permission(request, view, obj) + return hasperm1 if hasperm2 is Deferred else hasperm2 class OR: @@ -71,16 +76,22 @@ def __init__(self, op1, op2): self.op2 = op2 def has_permission(self, request, view): - return ( - self.op1.has_permission(request, view) or - self.op2.has_permission(request, view) - ) + hasperm1 = self.op1.has_permission(request, view) + if hasperm1 and hasperm1 is not Deferred: + return hasperm1 + hasperm2 = self.op2.has_permission(request, view) + return hasperm2 or hasperm1 def has_object_permission(self, request, view, obj): - return ( - self.op1.has_object_permission(request, view, obj) or - self.op2.has_object_permission(request, view, obj) - ) + hasperm1 = self.op1.has_object_permission(request, view, obj) + if hasperm1 is Deferred: + hasperm1 = self.op1.has_permission(request, view) + if hasperm1 and hasperm1 is not Deferred: + return hasperm1 + hasperm2 = self.op2.has_object_permission(request, view, obj) + if hasperm2 is Deferred: + hasperm2 = self.op2.has_permission(request, view) + return hasperm2 or hasperm1 class NOT: @@ -88,10 +99,12 @@ def __init__(self, op1): self.op1 = op1 def has_permission(self, request, view): - return not self.op1.has_permission(request, view) + hasperm = self.op1.has_permission(request, view) + return hasperm if hasperm is Deferred else not hasperm def has_object_permission(self, request, view, obj): - return not self.op1.has_object_permission(request, view, obj) + hasperm = self.op1.has_object_permission(request, view, obj) + return hasperm if hasperm is Deferred else not hasperm class BasePermissionMetaclass(OperationHolderMixin, type): @@ -105,15 +118,22 @@ class BasePermission(metaclass=BasePermissionMetaclass): def has_permission(self, request, view): """ - Return `True` if permission is granted, `False` otherwise. + Return `True` if permission is granted, `False` if permission is denied, + and `Deferred` if object permissions must be checked. """ - return True + return Deferred def has_object_permission(self, request, view, obj): """ - Return `True` if permission is granted, `False` otherwise. + Return `True` if permission is granted, `False` if permission is denied, + and `Deferred` if object permissions aren't implemented and should be + granted or denied based on `has_permission` as necessary. Returning + `Deferred` is more efficient than simply calling `has_permission` + because it prevents calling `has_permission` redundantly in most cases + where `has_permission` was already checked before calling + `has_object_perimssion`. """ - return True + return Deferred class AllowAny(BasePermission): @@ -220,7 +240,7 @@ 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 + return super().has_permission(request, view) if not request.user or ( not request.user.is_authenticated and self.authenticated_users_only): diff --git a/tests/test_permissions.py b/tests/test_permissions.py index 4e6cae4b81..dda424fc1f 100644 --- a/tests/test_permissions.py +++ b/tests/test_permissions.py @@ -12,6 +12,7 @@ HTTP_HEADER_ENCODING, authentication, generics, permissions, serializers, status, views ) +from rest_framework.permissions import Deferred from rest_framework.routers import DefaultRouter from rest_framework.test import APIRequestFactory from tests.models import BasicModel @@ -528,6 +529,12 @@ def setUp(self): self.password ) self.client.login(username=self.username, password=self.password) + self.admin_user = User.objects.create_user( + 'paul', + 'mccartney@thebeatles.com', + 'password', + is_staff=True, + ) def test_and_false(self): request = factory.get('/1', format='json') @@ -677,3 +684,30 @@ def test_object_and_lazyness(self): assert hasperm is False assert mock_deny.call_count == 1 mock_allow.assert_not_called() + + def test_has_permission_not_implemented(self): + request = factory.get('/1', format='json') + request.user = self.user + composed_perm = ~BasicObjectPerm + assert composed_perm().has_permission(request, None) is Deferred + assert composed_perm().has_object_permission(request, None, None) is True + + def test_has_object_permission_not_implemented_false(self): + request = factory.get('/1', format='json') + request.user = self.user + composed_perm = ( + permissions.IsAdminUser | + BasicObjectPerm + ) + assert composed_perm().has_permission(request, None) is Deferred + assert composed_perm().has_object_permission(request, None, None) is False + + def test_has_object_permission_not_implemented_true(self): + request = factory.get('/1', format='json') + request.user = self.admin_user + composed_perm = ( + permissions.IsAdminUser | + BasicObjectPerm + ) + assert composed_perm().has_permission(request, None) is True + assert composed_perm().has_object_permission(request, None, None) is True