From 94fbfcb6fdc8f9755a9f005bd005b00be3309ff8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fr=C3=A9d=C3=A9ric=20Massart?= Date: Mon, 25 Feb 2019 20:47:02 +0800 Subject: [PATCH] Added lazy evaluation to composed permissions. (#6463) Refs #6402. --- rest_framework/compat.py | 11 +++++ rest_framework/permissions.py | 8 ++-- tests/test_permissions.py | 87 ++++++++++++++++++++++++++++++++++- 3 files changed, 101 insertions(+), 5 deletions(-) diff --git a/rest_framework/compat.py b/rest_framework/compat.py index 59217c587..9422e6ad5 100644 --- a/rest_framework/compat.py +++ b/rest_framework/compat.py @@ -5,6 +5,8 @@ versions of Django/Python, and compatibility wrappers around optional packages. from __future__ import unicode_literals +import sys + from django.conf import settings from django.core import validators from django.utils import six @@ -34,6 +36,11 @@ try: except ImportError: ProhibitNullCharactersValidator = None +try: + from unittest import mock +except ImportError: + mock = None + def get_original_route(urlpattern): """ @@ -314,3 +321,7 @@ class MinLengthValidator(CustomValidatorMessage, validators.MinLengthValidator): class MaxLengthValidator(CustomValidatorMessage, validators.MaxLengthValidator): pass + + +# Version Constants. +PY36 = sys.version_info >= (3, 6) diff --git a/rest_framework/permissions.py b/rest_framework/permissions.py index ac616e202..69432d79a 100644 --- a/rest_framework/permissions.py +++ b/rest_framework/permissions.py @@ -44,13 +44,13 @@ class AND: def has_permission(self, request, view): return ( - self.op1.has_permission(request, view) & + self.op1.has_permission(request, view) and self.op2.has_permission(request, view) ) def has_object_permission(self, request, view, obj): return ( - self.op1.has_object_permission(request, view, obj) & + self.op1.has_object_permission(request, view, obj) and self.op2.has_object_permission(request, view, obj) ) @@ -62,13 +62,13 @@ class OR: def has_permission(self, request, view): return ( - self.op1.has_permission(request, view) | + self.op1.has_permission(request, view) or self.op2.has_permission(request, view) ) def has_object_permission(self, request, view, obj): return ( - self.op1.has_object_permission(request, view, obj) | + self.op1.has_object_permission(request, view, obj) or self.op2.has_object_permission(request, view, obj) ) diff --git a/tests/test_permissions.py b/tests/test_permissions.py index 381ec448c..f9d53430f 100644 --- a/tests/test_permissions.py +++ b/tests/test_permissions.py @@ -5,6 +5,7 @@ import unittest import warnings import django +import pytest from django.contrib.auth.models import AnonymousUser, Group, Permission, User from django.db import models from django.test import TestCase @@ -14,7 +15,7 @@ from rest_framework import ( HTTP_HEADER_ENCODING, authentication, generics, permissions, serializers, status, views ) -from rest_framework.compat import is_guardian_installed +from rest_framework.compat import PY36, is_guardian_installed, mock from rest_framework.filters import DjangoObjectPermissionsFilter from rest_framework.routers import DefaultRouter from rest_framework.test import APIRequestFactory @@ -600,3 +601,87 @@ class PermissionsCompositionTests(TestCase): permissions.IsAuthenticated ) assert composed_perm().has_permission(request, None) is True + + @pytest.mark.skipif(not PY36, reason="assert_called_once() not available") + def test_or_lazyness(self): + request = factory.get('/1', format='json') + request.user = AnonymousUser() + + with mock.patch.object(permissions.AllowAny, 'has_permission', return_value=True) as mock_allow: + with mock.patch.object(permissions.IsAuthenticated, 'has_permission', return_value=False) as mock_deny: + composed_perm = (permissions.AllowAny | permissions.IsAuthenticated) + hasperm = composed_perm().has_permission(request, None) + self.assertIs(hasperm, True) + mock_allow.assert_called_once() + mock_deny.assert_not_called() + + with mock.patch.object(permissions.AllowAny, 'has_permission', return_value=True) as mock_allow: + with mock.patch.object(permissions.IsAuthenticated, 'has_permission', return_value=False) as mock_deny: + composed_perm = (permissions.IsAuthenticated | permissions.AllowAny) + hasperm = composed_perm().has_permission(request, None) + self.assertIs(hasperm, True) + mock_deny.assert_called_once() + mock_allow.assert_called_once() + + @pytest.mark.skipif(not PY36, reason="assert_called_once() not available") + def test_object_or_lazyness(self): + request = factory.get('/1', format='json') + request.user = AnonymousUser() + + with mock.patch.object(permissions.AllowAny, 'has_object_permission', return_value=True) as mock_allow: + with mock.patch.object(permissions.IsAuthenticated, 'has_object_permission', return_value=False) as mock_deny: + composed_perm = (permissions.AllowAny | permissions.IsAuthenticated) + hasperm = composed_perm().has_object_permission(request, None, None) + self.assertIs(hasperm, True) + mock_allow.assert_called_once() + mock_deny.assert_not_called() + + with mock.patch.object(permissions.AllowAny, 'has_object_permission', return_value=True) as mock_allow: + with mock.patch.object(permissions.IsAuthenticated, 'has_object_permission', return_value=False) as mock_deny: + composed_perm = (permissions.IsAuthenticated | permissions.AllowAny) + hasperm = composed_perm().has_object_permission(request, None, None) + self.assertIs(hasperm, True) + mock_deny.assert_called_once() + mock_allow.assert_called_once() + + @pytest.mark.skipif(not PY36, reason="assert_called_once() not available") + def test_and_lazyness(self): + request = factory.get('/1', format='json') + request.user = AnonymousUser() + + with mock.patch.object(permissions.AllowAny, 'has_permission', return_value=True) as mock_allow: + with mock.patch.object(permissions.IsAuthenticated, 'has_permission', return_value=False) as mock_deny: + composed_perm = (permissions.AllowAny & permissions.IsAuthenticated) + hasperm = composed_perm().has_permission(request, None) + self.assertIs(hasperm, False) + mock_allow.assert_called_once() + mock_deny.assert_called_once() + + with mock.patch.object(permissions.AllowAny, 'has_permission', return_value=True) as mock_allow: + with mock.patch.object(permissions.IsAuthenticated, 'has_permission', return_value=False) as mock_deny: + composed_perm = (permissions.IsAuthenticated & permissions.AllowAny) + hasperm = composed_perm().has_permission(request, None) + self.assertIs(hasperm, False) + mock_allow.assert_not_called() + mock_deny.assert_called_once() + + @pytest.mark.skipif(not PY36, reason="assert_called_once() not available") + def test_object_and_lazyness(self): + request = factory.get('/1', format='json') + request.user = AnonymousUser() + + with mock.patch.object(permissions.AllowAny, 'has_object_permission', return_value=True) as mock_allow: + with mock.patch.object(permissions.IsAuthenticated, 'has_object_permission', return_value=False) as mock_deny: + composed_perm = (permissions.AllowAny & permissions.IsAuthenticated) + hasperm = composed_perm().has_object_permission(request, None, None) + self.assertIs(hasperm, False) + mock_allow.assert_called_once() + mock_deny.assert_called_once() + + with mock.patch.object(permissions.AllowAny, 'has_object_permission', return_value=True) as mock_allow: + with mock.patch.object(permissions.IsAuthenticated, 'has_object_permission', return_value=False) as mock_deny: + composed_perm = (permissions.IsAuthenticated & permissions.AllowAny) + hasperm = composed_perm().has_object_permission(request, None, None) + self.assertIs(hasperm, False) + mock_allow.assert_not_called() + mock_deny.assert_called_once()