Should "has_object_permission" call "has_permission" as default instead of returning always "True"?

[Cloves23]

Hi, there.

Short description: The method BasePermission.has_object_permission is returning a inconsistent value.

I was trying to implement a more complex permission check using custom permissions and noticed an inconsistency in the final result. In my case, I have a permission that checks for specific action names and then verifies the user type. The problem occurs when I need to verify object-level permissions. While 'has_permission' returns 'False', 'has_object_permission' returns 'True'. For consistency, wouldn't it be correct to call has_object_permission by default?

My code:

from rest_framework import permissions

@perm_with_attrs("action")
class IsAction(permissions.BasePermission):
    code = "action_not_allowed"
    message = _("Action is not allowed.")
    action: str

    def has_permission(self, request, view):
        return view.action == self.action

high_privileges = ~IsAction("cancel_account") & (
    IsAdminUser | IsTrainerUser | HasDjangoPermission(Customer, ["view"])
)

How I solved it:

from rest_framework import permissions

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

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

@perm_with_attrs("action")
class IsAction(BasePermission):
    ...

How to reproduce

from collections import namedtuple
from rest_framework.permissions import IsAdminUser

User = namedtuple("User", "is_staff")
Request = namedtuple("Request", "user")

req_user = Request(User(False))
assert IsAdminUser().has_permission(req_user, None) is False
assert IsAdminUser().has_object_permission(req_user, None, None) is False  # Break
assert (~IsAdminUser)().has_permission(req_user, None) is True
assert (~IsAdminUser)().has_object_permission(req_user, None, None) is True  # Break

req_staff = Request(User(True))
assert IsAdminUser().has_permission(req_staff, None) is True
assert IsAdminUser().has_object_permission(req_staff, None, None) is True
assert (~IsAdminUser)().has_permission(req_staff, None) is False
assert (~IsAdminUser)().has_object_permission(req_staff, None, None) is False

[browniebroke]

DRF took another approach as described in the docs:

Note: With the exception of DjangoObjectPermissions, the provided permission classes in rest_framework.permissions do not implement the methods necessary to check object permissions.

If you wish to use the provided permission classes in order to check object permissions, you must subclass them and implement the has_object_permission() method described in the Custom permissions section (below).

The base APIView checks for permissions pretty early on:

django-rest-framework/rest_framework/views.py

Line 421 in 2ede857

self.check_permissions(request)

And the generic view checks for object permissions while trying to fetch the individual instance for details endpoints:

django-rest-framework/rest_framework/generics.py

Line 103 in 15c613a

self.check_object_permissions(self.request, obj)

Taking a step back, if you get has_permission() is False, then I think DRF won't even get to the point where has_object_permission() is called, so returning True or defaulting to has_permission doesn't matter unless you bypass the framework...

[Cloves23]

Thank you for the explanation! However, I believe my concern lies in a different aspect. I understand that has_object_permission() is not called if has_permission() returns False, which is expected behavior.

In my case, the issue arises when has_permission() returns True, and has_object_permission() is subsequently called. The problem stems from the default implementation of has_object_permission() in the BasePermission class, which always returns True unless explicitly overridden by derived permission classes.

For example, consider these permissions:

@perm_with_attrs("action")
class IsAction(BasePermission):
    code = "action_not_allowed"
    message = _("Action is not allowed.")
    action: str

    def has_permission(self, request, view):
        return view.action == self.action


class IsReadOnly(BasePermission):
    code = "write_not_allowed"
    message = _("Write is not allowed.")

    def has_permission(self, request, view):
        return request.method in permissions.SAFE_METHODS


@perm_with_attrs("methods")
class IsHTTPMethods(BasePermission):
    code = "method_not_allowed"
    message = _("Method is not allowed.")
    methods: List[str]

    def has_permission(self, request, view):
        method = request.method.lower()
        return any(method == m.lower() for m in self.methods)

When used in composite logic, like this:

high_privileges = ~IsAction("cancel_account") & (
    IsAdminUser
    | IsTrainerUser
    | IsReadOnly & HasDjangoPermissions(["account.view_customer"])
    | ~IsReadOnly
    & (
        IsHTTPMethods(["DELETE"]) & HasDjangoPermissions(["account.delete_customer"])
        | ~IsHTTPMethods(["DELETE"]) & HasDjangoPermissions(["account.change_customer"])
    )
)

The Problem

The has_object_permission() method in BasePermission always returns True unless explicitly overridden by derived classes. This creates an issue where object-level checks are effectively bypassed, even when object-level restrictions are clearly intended.

For instance:

• IsReadOnly does not enforce read-only restrictions at the object level.
• IsHTTPMethods similarly bypasses object-level checks for specific methods.

Suggestion

Wouldn’t it be more appropriate to modify the BasePermission class so that its has_object_permission() calls has_permission() by default? This would ensure consistent behavior across all derived permissions, unless they explicitly override this method.

For example, BasePermission could be modified as follows:

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

This adjustment would propagate the logic of has_permission() to object-level checks by default, ensuring alignment between the two methods and reducing the risk of unintentionally bypassing object-level restrictions.

I’d appreciate your thoughts on whether this approach aligns with the intended behavior or if the alternative best practice for handling these cases is explicitly overwrite.

[browniebroke]

In my case, the issue arises when has_permission() returns True, and has_object_permission() is subsequently called.

How is calling a method that returns True different than returning True directly?

I don't follow your example with complex composite permission, can you distill it down to a more minimal one?