caxap / rest_condition

Complex permissions flow for django-rest-framework (http://django-rest-framework.org).
MIT License
281 stars 29 forks source link

has_object_permission doesn't get called #12

Open alexseitsinger opened 6 years ago

alexseitsinger commented 6 years ago

In my viewset I have the following: permission_classes = [ Or(And(IsReadOnlyRequest, IsAuthenticated), And(IsPutRequest, IsObjectUser)) ]

Where IsObjectUser has: def has_object_permission(self, request, view, obj): return obj.user == request.user

The has_object_permission method is never invoked when wrapped inside the conditional statement. However, If I use the IsObjectUser as a plain permission class in permission_classes list, it works fine. This isn't suitable for me because I need them for each method, not for the whole viewset, always.

If this is bug, I would love to see it fixed. Any suggestions or workarounds are appreciated.

dabrowne commented 6 years ago

You will find that the outer Or is evaluating to True and short circuiting (not evaluating the second argument) because And(IsReadOnlyRequest, IsAuthenticated).has_object_permission returns True.

This is because IsReadOnlyRequest and IsAuthenticated both inherit from BasePermission which just returns True for has_object_permission. This is a gotcha when combining permissions with an or, which doesn't normally happen with the default DRF behavior.

To get your desired behavior, you will need to subclass IsReadOnlyRequest and IsAuthenticated so that their has_object_permission returns a meaningful result. I normally just do this:

class IsAuthenticated(permissions.IsAuthenticated):
    def has_object_permission(self, request, view, obj):
        return self.has_permission(request, view)