Django-rest-framework: DjangoObjectPermissions behaviour

[Tommos0]

Hi,

I'm having some issues with the default DjangoObjectPermissions. The behaviour currently is not what I would expect. I can update the documentation to be more clear about the current implementation, or if you like I can create a pull request with my proposed fix.

My issue is as follows. If one uses DjangoObjectPermissions, since it extends DjangoModelPermissions, it also checks the basic model permission (DjangoModelPermissions.has_permission). At least I think the documentation should be more clear about this, but actually I think there is a better implementation. I would expect this permission class to ONLY check for the object permissions. Current behaviour can then simply be recreated by using DjangoObjectPermissions & DjangoModelPermissions, but this change would also allow DjangoObjectPermissions | DjangoModelPermissions (which I think is a nice default). The simplest fix would be to just add

    def has_permission(self, request, view):
        return True

to DjangoObjectPermissions.

There is a second issue: because in some cases it raises exceptions (see https://github.com/encode/django-rest-framework/blob/db65282163b91688367095459328de7e9dab94d8/rest_framework/permissions.py#L287-L302), this actually breaks logically combining permission classes (the raise will ignore any other permission class giving positive permission).

I realize this could be a breaking change for many users so perhaps we can create this new permission class with a different name? Let me know what you think, I could create a PR with a change or update the docs to at least explain the current situation.

[Tommos0]

I realize now that this is not how things are implemented, so my proposed solution doesn't work.

I'm using a workaround to get a Object | Model permission like this:

    def has_object_permission(self, request, view, obj):
        if DjangoModelPermissions.has_permission(self, request, view):
            return True
        queryset = self._queryset(view)
        model_cls = queryset.model
        user = request.user
        perms = self.get_required_object_permissions(request.method, model_cls)
        return user.has_perms(perms, obj)

    def has_permission(self, request, view):
        handler = getattr(view, request.method.lower(), None)
        if handler and handler.__name__ == 'list':
            return DjangoModelPermissions.has_permission(self, request, view)
        return True

[johnthagen]

I too found it surprising that DjangoObjectPermissions inherited from DjangoModelPermissions rather than BasePermission, especially given how they are named (it's not DjangoObjectModelPermissions).

Could there be a new Permissions type created that solely performed object permissions? It seems like that would be a useful primitive to include in DRF directly.

[tomchristie]

If someone else wants to implement a third party package for that, then sure. I don’t really see it a strong enough use case for it myself.

[johnthagen]

FWIW, in case this helps someone else, I ended up overriding has_permission to remove the DjangoModelPermissions behavior.

I also added support for view_modelname permissions added in Django 2.1. I'd suggest it be added to DRF, but DRF still supports Django 1.11 (#6230).

from rest_framework.permissions import DjangoObjectPermissions, SAFE_METHODS
from rest_framework.request import Request
from rest_framework.viewsets import GenericViewSet


class IsAuthenticatedDjangoObjectPermissions(DjangoObjectPermissions):
    """Allows access only to authenticated users who have the appropriate object level permissions.
    """

    perms_map = DjangoObjectPermissions.perms_map

    # Add default Django view permissions added in Django 2.1 and not included in DRF
    # DjangoObjectPermissions as of DRF 3.9.4.
    for method in SAFE_METHODS:
        perms_map[method] = ['%(app_label)s.view_%(model_name)s']

    def has_permission(self, request: Request, view: GenericViewSet) -> bool:
        # Override DjangoModelPermissions which DjangoObjectPermissions inherits from.
        # Taken from rest_framework.permissions.IsAuthenticated.
        return bool(request.user and request.user.is_authenticated)

[rpkilby]

6325 will add support for the view permission. It should be merged whenever we're ramping up for the 3.11 release in December, which will add Django 3.0 support and drop support for 2.1 and below.