Permissions not treated as units

[xlotlu]

Because C.has_permission and C.has_object_permission both run self.evaluate_permissions() in a disconnected manner, this leads to the following logical error when OR-ing permissions:

Given

    A = hpA & hopA, and
    B = hpB & hopB,

A || B should really get evaluated unitarily, as

    (hpA & hopA) || (hpB & hopB),

but instead gets evaluated as

    (hpA || hpB) & (hopA || hopB)

which is incorrect, and leads to the following problem:

Consider

class A(BasePermission):
    def has_permission(self, request, view):
        return True

    def has_object_permission(self, request, view, obj):
        return False

class B(BasePermission):
    def has_permission(self, request, view):
        return False

    def has_object_permission(self, request, view, obj):
        return True

C(A) | C(B) should always return false for object-level access, but it returns true.

I saw that in #1 you dismissed maintaining state during the initial has_permission run as something downstream developers should implement, but it really needs to be done in rest_condition for logical correctness. If maintaining state, in the case above "the whole of B" would get marked as False during the initial phase, and B.has_object_permission would never get to run.

On an unrelated note, this would also be the stepping stone for dealing sensibly with mixing permissions with different combinations of has_permissions / has_object_permissions (both #1 and #2). When the method is missing (presuming one didn't inherit from BasePermission), it should be a no-op -- basically behaving like it returned True when AND-ed, and False when OR-ed.

[ignus2]

I worked around the issue mentioned here as well as in #2 by separating general and object-level permissions completely. By using a class something like this:

class CombinedCondition(BasePermission):
    """
    Extension for rest_condition to be able to separate the general and object-level paths of permission checks.
    """

    def __init__(self, gen_perm_class, obj_perm_class):
        self.gen_perm = gen_perm_class()
        self.obj_perm = obj_perm_class()
        super().__init__()

    def __call__(self):
        return self

    def has_permission(self, request, view):
        return self.gen_perm.has_permission(request, view)

    def has_object_permission(self, request, view, obj):
        return self.obj_perm.has_object_permission(request, view, obj)


CC = CombinedCondition

you can do:

permission_classes=[CC(
    IsAuthenticated,  # General-permissions
    Or(IsAdminUserObjectLevel, IsObjectCreator, IsObjectEditor) # Object-level permissions
)]

The first set of permissions only check has_permission(), while the second only check has_object_permission().
At least for me, this makes it perfectly clear what is happening.

Hope it helps someone.