diff --git a/docs/api-guide/permissions.md b/docs/api-guide/permissions.md index 6912c375c..5cb9acd22 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 974715ae1..676e05543 100644 --- a/rest_framework/permissions.py +++ b/rest_framework/permissions.py @@ -8,6 +8,9 @@ from rest_framework import exceptions SAFE_METHODS = ('GET', 'HEAD', 'OPTIONS') +Deferred = object() + + class OperationHolderMixin: def __and__(self, other): return OperandHolder(AND, self, other) @@ -57,14 +60,14 @@ class AND: if not hasperm1: return hasperm1 hasperm2 = self.op2.has_permission(request, view) - return hasperm1 if hasperm2 is NotImplemented else hasperm2 + return hasperm1 if hasperm2 is Deferred else hasperm2 def has_object_permission(self, 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 NotImplemented else hasperm2 + return hasperm1 if hasperm2 is Deferred else hasperm2 class OR: @@ -74,19 +77,19 @@ class OR: def has_permission(self, request, view): hasperm1 = self.op1.has_permission(request, view) - if hasperm1 and hasperm1 is not NotImplemented: + 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): hasperm1 = self.op1.has_object_permission(request, view, obj) - if hasperm1 is NotImplemented: + if hasperm1 is Deferred: hasperm1 = self.op1.has_permission(request, view) - if hasperm1 and hasperm1 is not NotImplemented: + if hasperm1 and hasperm1 is not Deferred: return hasperm1 hasperm2 = self.op2.has_object_permission(request, view, obj) - if hasperm2 is NotImplemented: + if hasperm2 is Deferred: hasperm2 = self.op2.has_permission(request, view) return hasperm2 or hasperm1 @@ -97,11 +100,11 @@ class NOT: def has_permission(self, request, view): hasperm = self.op1.has_permission(request, view) - return hasperm if hasperm is NotImplemented else not hasperm + return hasperm if hasperm is Deferred else not hasperm def has_object_permission(self, request, view, obj): hasperm = self.op1.has_object_permission(request, view, obj) - return hasperm if hasperm is NotImplemented else not hasperm + return hasperm if hasperm is Deferred else not hasperm class BasePermissionMetaclass(OperationHolderMixin, type): @@ -115,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 NotImplemented + 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 NotImplemented + return Deferred class AllowAny(BasePermission): diff --git a/tests/test_permissions.py b/tests/test_permissions.py index 33bc6cf4a..dda424fc1 100644 --- a/tests/test_permissions.py +++ b/tests/test_permissions.py @@ -12,6 +12,7 @@ from rest_framework import ( 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 @@ -688,7 +689,7 @@ class PermissionsCompositionTests(TestCase): request = factory.get('/1', format='json') request.user = self.user composed_perm = ~BasicObjectPerm - assert composed_perm().has_permission(request, None) is NotImplemented + 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): @@ -698,7 +699,7 @@ class PermissionsCompositionTests(TestCase): permissions.IsAdminUser | BasicObjectPerm ) - assert composed_perm().has_permission(request, None) is NotImplemented + 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):