Change semantic of OR of two permission classes (#7522)

* Change semantic of OR of two permission classes

The original semantic of OR is defined as: the request pass either of the two has_permission() check, and pass either of the two has_object_permission() check, which could lead to situations that a request passes has_permission() but fails on has_object_permission() of Permission Class A, fails has_permission() but passes has_object_permission() of Permission Class B, passes the OR permission check. This should not be the desired permission check semantic in applications, because such a request should fail on either Permission Class (on Django object permission) alone, but passes the OR or the two.

My code fix this by changing the semantic so that the request has to pass either class's has_permission() and has_object_permission() to get the Django object permission of the OR check.

* Update rest_framework/permissions.py

* Update setup.cfg

Co-authored-by: Mark Yu <markyu98@outlook.com>
Co-authored-by: Tom Christie <tom@tomchristie.com>
This commit is contained in:
David Smith 2022-09-21 12:19:33 +01:00 committed by GitHub
parent 354ae73ffb
commit 4aea8dd65a
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
3 changed files with 20 additions and 4 deletions

View File

@ -78,8 +78,11 @@ class OR:
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)
self.op1.has_permission(request, view)
and self.op1.has_object_permission(request, view, obj)
) or (
self.op2.has_permission(request, view)
and self.op2.has_object_permission(request, view, obj)
)

View File

@ -5,7 +5,7 @@ license_files = LICENSE.md
addopts=--tb=short --strict-markers -ra
[flake8]
ignore = E501,W504
ignore = E501,W503,W504
banned-modules = json = use from rest_framework.utils import json!
[isort]

View File

@ -635,7 +635,7 @@ class PermissionsCompositionTests(TestCase):
composed_perm = (permissions.IsAuthenticated | permissions.AllowAny)
hasperm = composed_perm().has_object_permission(request, None, None)
assert hasperm is True
assert mock_deny.call_count == 1
assert mock_deny.call_count == 0
assert mock_allow.call_count == 1
def test_and_lazyness(self):
@ -677,3 +677,16 @@ class PermissionsCompositionTests(TestCase):
assert hasperm is False
assert mock_deny.call_count == 1
mock_allow.assert_not_called()
def test_unimplemented_has_object_permission(self):
"test for issue 6402 https://github.com/encode/django-rest-framework/issues/6402"
request = factory.get('/1', format='json')
request.user = AnonymousUser()
class IsAuthenticatedUserOwner(permissions.IsAuthenticated):
def has_object_permission(self, request, view, obj):
return True
composed_perm = (IsAuthenticatedUserOwner | permissions.IsAdminUser)
hasperm = composed_perm().has_object_permission(request, None, None)
assert hasperm is False