Custom Deferred truthy value to replace NotImplemented

In Python 3.9, evaluating NotImplemented in a boolean context is deprecated.
So permissions must use a custom truthy value instead to indicate that
`has_permission` defers granting/denying access until object-level permissions
can be checked and vice-versa, `has_object_permission` defers permission to
the view-level permission.

Also, updated docstrings and documentation.
This commit is contained in:
Ben Buchwald 2021-07-19 13:32:27 -04:00
parent a224c6c67b
commit 23aa876efc
3 changed files with 28 additions and 16 deletions

View File

@ -1,4 +1,4 @@
--- ---
source: source:
- permissions.py - permissions.py
--- ---
@ -212,7 +212,8 @@ To implement a custom permission, override `BasePermission` and implement either
* `.has_permission(self, request, view)` * `.has_permission(self, request, view)`
* `.has_object_permission(self, request, view, obj)` * `.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: 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:

View File

@ -8,6 +8,9 @@ from rest_framework import exceptions
SAFE_METHODS = ('GET', 'HEAD', 'OPTIONS') SAFE_METHODS = ('GET', 'HEAD', 'OPTIONS')
Deferred = object()
class OperationHolderMixin: class OperationHolderMixin:
def __and__(self, other): def __and__(self, other):
return OperandHolder(AND, self, other) return OperandHolder(AND, self, other)
@ -57,14 +60,14 @@ class AND:
if not hasperm1: if not hasperm1:
return hasperm1 return hasperm1
hasperm2 = self.op2.has_permission(request, view) 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): def has_object_permission(self, request, view, obj):
hasperm1 = self.op1.has_object_permission(request, view, obj) hasperm1 = self.op1.has_object_permission(request, view, obj)
if not hasperm1: if not hasperm1:
return hasperm1 return hasperm1
hasperm2 = self.op2.has_object_permission(request, view, obj) 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: class OR:
@ -74,19 +77,19 @@ class OR:
def has_permission(self, request, view): def has_permission(self, request, view):
hasperm1 = self.op1.has_permission(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 return hasperm1
hasperm2 = self.op2.has_permission(request, view) hasperm2 = self.op2.has_permission(request, view)
return hasperm2 or hasperm1 return hasperm2 or hasperm1
def has_object_permission(self, request, view, obj): def has_object_permission(self, request, view, obj):
hasperm1 = self.op1.has_object_permission(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) hasperm1 = self.op1.has_permission(request, view)
if hasperm1 and hasperm1 is not NotImplemented: if hasperm1 and hasperm1 is not Deferred:
return hasperm1 return hasperm1
hasperm2 = self.op2.has_object_permission(request, view, obj) hasperm2 = self.op2.has_object_permission(request, view, obj)
if hasperm2 is NotImplemented: if hasperm2 is Deferred:
hasperm2 = self.op2.has_permission(request, view) hasperm2 = self.op2.has_permission(request, view)
return hasperm2 or hasperm1 return hasperm2 or hasperm1
@ -97,11 +100,11 @@ class NOT:
def has_permission(self, request, view): def has_permission(self, request, view):
hasperm = self.op1.has_permission(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): def has_object_permission(self, request, view, obj):
hasperm = self.op1.has_object_permission(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): class BasePermissionMetaclass(OperationHolderMixin, type):
@ -115,15 +118,22 @@ class BasePermission(metaclass=BasePermissionMetaclass):
def has_permission(self, request, view): 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): 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): class AllowAny(BasePermission):

View File

@ -12,6 +12,7 @@ from rest_framework import (
HTTP_HEADER_ENCODING, authentication, generics, permissions, serializers, HTTP_HEADER_ENCODING, authentication, generics, permissions, serializers,
status, views status, views
) )
from rest_framework.permissions import Deferred
from rest_framework.routers import DefaultRouter from rest_framework.routers import DefaultRouter
from rest_framework.test import APIRequestFactory from rest_framework.test import APIRequestFactory
from tests.models import BasicModel from tests.models import BasicModel
@ -688,7 +689,7 @@ class PermissionsCompositionTests(TestCase):
request = factory.get('/1', format='json') request = factory.get('/1', format='json')
request.user = self.user request.user = self.user
composed_perm = ~BasicObjectPerm 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 assert composed_perm().has_object_permission(request, None, None) is True
def test_has_object_permission_not_implemented_false(self): def test_has_object_permission_not_implemented_false(self):
@ -698,7 +699,7 @@ class PermissionsCompositionTests(TestCase):
permissions.IsAdminUser | permissions.IsAdminUser |
BasicObjectPerm 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 assert composed_perm().has_object_permission(request, None, None) is False
def test_has_object_permission_not_implemented_true(self): def test_has_object_permission_not_implemented_true(self):