drf permissions

[nux17]

Permissions are not chained, eg when trying to POST for create at an endpoint like 'item/1/subitem', nothing checks if the item 1 belongs to the user.

[alanjds]

Is this not a configuration for the subitem ViewSet?

I mean, is better to explicitly set it on the SubitemViewSet instead of trying to bubble the ItemViewSet stuff, is not?

[nux17]

I don't know, but it seems logical.

Let's say you're on an API managing your house, and access house objects is permission based. Your neighbor house ID is 2. You can't access /api/house/2 with GET put able to add a window to his house through POST at /api/house/2/window/?

Is it me or a real problem?

[alanjds]

In my opinion, is a real problem, already solved by setting permission handling on the HouseView AND on the WindowView. Manually.

If one can think some kind of decorator or even a mixin to inherit permissions from the parent resource, I will be happy to review it.

[voblivion]

@alanjds @include_parent_permission(MyParentView) then in this decorator simply override permission classes with those of given parent view... it does the trick.

[alanjds]

Sorry for the loooong delay here. Is this decorator already implemented somewhere or a proposal?

[voblivion]

I can't remember but I think it's not implemented yet. You can write it easily and even make a PR ?

[alanjds]

I am not in position to write new code right now, specially code that I will not use soon. Sorry. But will happily review any PR arriving.

[anx-ckreuzberger]

I just noticed this too. The problem is even worse, as you could access the windows of a house that do not exist: GET /api/house/5/window/ would return an empty array, although house(id=5) does not exist (I would have expected a 404, just as you would get when accessing /api/house/5/).

I guess it would even be possible to make a POST on something like that, though the Django ORM should figure out that something is wrong anyway and throw an error (hopefully 4xx).

In my opinion, we would not only need a decorator that inherits the permissions, but also a way to verify that the parent object actually exists.

(I usually use object level permissions, so for me permissions are handled in the QuerySet rather than DRF)

[anx-ckreuzberger]

I just found a very convenient way of fixing the issue for me. In your ViewSet, you can overwrite the initial method:

class WindowViewSet(ModelViewSet):
    serializer_class = WindowSerializer

    def initial(self, request, *args, **kwargs):
        get_object_or_404(House.objects.all(), pk=kwargs['house_pk'])
        return super(RelationViewSet, self).initial(request, args, kwargs)

The initial method is called on every request towards the REST API (GET, POST, PUT, ...), and is called within a try/except block, which handles the Http404 raised by get_object_or_404.

If you use some kind of object level permissions, instead of House.object.all() you could call House.objects.viewable_by_current_user(), where viewable_by_current_user is a QuerySet method you need to write by yourself (or that is provided by the framework you are using).

[alanjds]

This looks nice as a class decorator to augment the WindowViewSet.initial

I guess more discussion is needed to decide if this kind of decorator should be automatically applied by the nested routers, but @anx-ckreuzberger seems a good start

[anx-ckreuzberger]

I'd really just offer the possibility to use the decorator (and explain in the readme how to do it). In addition, I'd suggest to make sure people know the difference between the get_object_or_404 approach vs. the "inherit" permission approach.

I guess we could have two decorators / options, one for overwriting "initial" method, and one for inheriting (not sure if this is the correct term) the permission_classes from the parent view?

Maybe we could have them named like this:

• @verify_parent_object_exists(House.objects.all())
• @inherit_permission_classes(HouseViewSet) (which should just clone the permission_classes object of HouseViewSet)

[claytondaley]

I don't object to offering decorators since they ensure there's only one "right" way to get the job done. However, I'd like point out that there's a case against both of the proposed behaviors (parent existence, parent permissions):

• Enforcing parent permissions by default would prohibit a POST to facebook.com/post/{id}/comments. I imagine there are many other examples of this kind of behavior including like, subscribe and attend.
• I have a serializer that creates a related object if it does not exist. The full representation of that related object is included in the payload. It's counter-intuitive, but there's no logical reason why the related object couldn't be the "parent" in the URL structure.

I think the first bullet is strong enough to argue against permissions checking by default. The second bullet is weak enough that I'm +1 on existence checking by default.

[alanjds]

I do have some resource that creates itself when "tried" too, but I do this on the model level. They are not Django ORM models and the checking API is different. Then do like permission-passing by default, but not existance checking.

At the end, I am getting the conclusion that no enforcement should occur by default, but should exist as an optional easy-to-use way.

[ambsw-technology]

Some more food for thought. The "right" way to handle permissions in DRF is a BasePermission object. We recently created one to centralize DRF authorization. Recognizing the importance of Permission objects, one idea was to have the child inhert the permissions of the parent.

As you can see in rest_framework/permissions.py, these classes have two methods to choose from:

    def has_permission(self, request, view):
        """Return `True` if permission is granted, `False` otherwise."""
        return True

    def has_object_permission(self, request, view, obj):
        """Return `True` if permission is granted, `False` otherwise."""
        return True

As I see it, a key problem with this approach is that has_object_permission expects a specific type of object. Either:

• a HousePermission expects a House and a WindowPermission expects a Window or
• A general-purpose Permission object uses isinstance to runs the check appropriate to the instance passed

If the child inherits the parent,

• The HousePermission is run against a Window
• The general-purpose Permission object is run twice on Window (i.e. never checking House)

I definitely think BasePermission objects are the right idea, but inheritance doesn't seem to be the appropriate mechanism.

[BenjaminHabert]

I am trying to develop something along those lines at the moment. Here are a few thoughts. Let's suppose the model Comment is nested under the model User.

• GET /users/<id>/comments

The permission that will be checked by Django here is has_permission(request). However if we rely on the "parent", what should probably be checked is has_object_permission(user, request) where user is the one targeted in the <id>.

This is not convenient as it requires finding the "owner" from the request query parameters..

• GET /users/<id>/comments/<id>

This case is probably easier, has_object_permission(comment, request) will be called and should probably defer to has_object_permission(user, request). In this situation it is easy to find the user instance from the comment instance.

[ambsw-technology]

We ended up with BasePermission object that used plugins to decentralize authorization decisions. Aside the relevant view, we created a plugin that did the appropriate object-level permissions checking. If we needed different permissions for the same Model, we used a ModelProxy (but not that there are some quirks with automatic permission creation for a ModelProxy).

In the particular plugin, we made authorization decisions explicit. For example, with a /post/<id>/comment/ endpoint (which differs slightly from Benjamin's example), you might require READ access to the Post in order to comment on it. Our Comment permissions object would explicitly check the user's permission to the Post. In most cases, a value like Post is explicit in the create statement (or available on the instance).

[dmartin]

I've come across what seems to be a fairly nice solution for checking nested permissions, at least for a simple use case:

Going with the house/window example from earlier in the thread, say we have routes such as: /houses/<id>/ /houses/<house_id>/windows/ /houses/<house_id>/windows/<id>/

In this case, we only want users to be able to access a house's windows if they own that house. Additionally, we can prove ownership of a house via the ability to access the detail route of a house without raising an exception.

Therefore, we can utilize all the existing error handling and permission checking of the house ViewSet "for free" by firing off a synthetic GET /houses/<id>/ request before dispatching the window request:

class HouseViewSet(ModelViewSet):
    lookup_url_kwarg = "id"
    queryset = House.objects.filter(owner=request.user)
    serializer_class = HouseSerializer
    permission_classes = [CustomHousePermissions]

class WindowViewSet(ModelViewSet):
    serializer_class = WindowSerializer

    def dispatch(self, request, *args, **kwargs):
        parent_view = HouseViewSet.as_view({"get": "retrieve"})
        original_method = request.method
        request.method = "GET"
        parent_kwargs = {"id": kwargs["house_id"]}

        parent_response = parent_view(request, *args, **parent_kwargs)
        if parent_response.exception:
            return parent_response

        request.method = original_method
        return super().dispatch(request, *args, **kwargs)

    def get_queryset(self):
        return Window.objects.filter(house=self.kwargs["house_id"])

This way, if we attempt to access /houses/5/windows/1/, any errors that would arise from accessing GET /houses/5/ are handled without writing any extra permissions code.

[timstallmann]

In case it's helpful to anyone else running into this situation, I'm dealing with the simplest case of a nested view where the permissions for the nested endpoints should be identical to the parent object permissions, so building on @anx-ckreuzberger's response, am doing the following on the nested viewset (manually subbing out <parent_model> appropriately):

    permission_classes = (
        permissions.IsAuthenticated,
        permissions.DjangoObjectPermissions
    )

    queryset = <parent_model>.objects.all()

    def initial(self, request, *args, **kwargs):
        self.parent_object = get_object_or_404(self.queryset, uuid=kwargs['<parent_model>_uuid'])
        return super().initial(request, args, kwargs)

    def check_permissions(self, request):
        super().check_permissions(request)
        super().check_object_permissions(request, self.parent_object)

[okapies]

Another (generalized) solution inspired by @timstallmann.

class CheckParentPermissionMixin:
    parent_queryset: QuerySet
    parent_lookup_field: str
    parent_lookup_url_kwarg: str

    def __init__(self, **kwargs):
        self.parent_obj: Any = None
        super().__init__(**kwargs)

    def check_permissions(self, request):
        super().check_permissions(request)

        # check permissions for the parent object
        parent_lookup_url_kwarg = self.parent_lookup_url_kwarg or self.parent_lookup_field
        filter_kwargs = {
            self.parent_lookup_field: kwargs[parent_lookup_url_kwarg]
        }
        self.parent_obj = get_object_or_404(self.parent_queryset, **filter_kwargs)
        self.parent_obj._is_parent_obj = True
        super().check_object_permissions(request, self.parent_obj)

class ChildViewSet(CheckParentPermissionMixin, ...):
    permission_classes = [...]

    parent_queryset = Parent.objects.all()
    parent_lookup_field = 'uuid'
    parent_lookup_url_kwarg = 'parent_uuid'

    lookup_field = 'uuid'
    ...

[ruslan-korneev]

Hey, @okapies 👋🏻 i just noticed that you're using super().check_object_permissions(request, self.parent_obj). This will run every found object permission, which could be not only for parent object, i guess thats why you saved _is_parent_obj = True, right?

I did this code, based on yours:

Check Parent Permissions, needs in parent object. Next mixin would be for parent object

class CheckParentPermissionMixin:
    parent_object: Any

    def check_permissions(self, request):
        super().check_permissions(request)
        self.parent_object = self.get_parent_object()

    def check_parent_object_permissions(self, request, obj):
        """
        Check if the request should be permitted for a given object.
        Raises an appropriate exception if the request is not permitted.
        """
        for permission in self.get_permissions():
            if not hasattr(permission, "has_parent_object_permission"):
                continue

            if not permission.has_parent_object_permission(request, self, obj):
                self.permission_denied(
                    request,
                    message=getattr(permission, "message", None),
                    code=getattr(permission, "code", None),
                )

Parent Object Mixin

class ParentObjectMixin:
    parent_queryset: QuerySet
    parent_lookup_prefix: str
    parent_lookup_field: str
    parent_lookup_url_kwarg: str | None = None

    def get_parent_queryset(self):
        assert self.parent_queryset is not None, (
            "'%s' should either include a `parent_queryset` attribute, "
            "or override the `get_parent_queryset()` method." % self.__class__.__name__
        )

        queryset = self.parent_queryset
        if isinstance(queryset, QuerySet):
            # Ensure queryset is re-evaluated on each request.
            queryset = queryset.all()
        return queryset

    def get_parent_object(self):
        parent_lookup_url_kwarg = (
            self.parent_lookup_url_kwarg
            or f"{self.parent_lookup_prefix}_{self.parent_lookup_field}"
        )
        filter_kwargs = {self.parent_lookup_field: self.kwargs[parent_lookup_url_kwarg]}
        instance = get_object_or_404(self.get_parent_queryset(), **filter_kwargs)
        self.check_parent_object_permissions(self.request, instance)
        return instance

    def get_serializer_context(self):
        """
        Extra context provided to the serializer class.
        Adding parent object to context.
        """
        context = super().get_serializer_context()
        context["parent_object"] = getattr(
            self, "parent_object", self.get_parent_object()
        )
        return context

By the way, this context in serializer, which i've got from get_serializer_context would be useful

# serializers
class ChildSerializer(serializers.ModelSerializer):
    parent = serializers.HiddenField(default=CurrentParentObjectDefault())

# fields
from rest_framework.fields import CurrentUserDefault

class CurrentParentObjectDefault(CurrentUserDefault):
    def __call__(self, serializer_field):
        return serializer_field.context["parent_object"]

The last part is connect each other and override the Nested Viewset Mixin

class CustomNestedViewSetMixin(
    ParentObjectMixin, CheckParentPermissionMixin, NestedViewSetMixin
):
    parent_queryset: QuerySet = None

    def _get_parent_lookup_kwargs(self) -> dict:
        """
        Locates and returns the `parent_lookup_kwargs` dict informing
        how the kwargs in the URL maps to the parents of the model instance

        For now, fetches from `parent_lookup_kwargs`
        on the ViewSet or Serializer attached. This may change on the future.
        """
        if not hasattr(self, "parent_lookup_prefix") or not hasattr(
            self, "parent_lookup_field"
        ):
            raise ImproperlyConfigured(
                "CustomNestedViewSetMixin needs `parent_lookup_prefix` and `parent_lookup_field` "
                "to find the parent from the URL"
            )

        parent_lookup_url_kwarg = (
            self.parent_lookup_url_kwarg
            or f"{self.parent_lookup_prefix}_{self.parent_lookup_field}"
        )

        parent_lookup_kwargs = getattr(
            self,
            "parent_lookup_kwargs",
            {
                parent_lookup_url_kwarg: f"{self.parent_lookup_prefix}__{self.parent_lookup_field}"
            },
        )

        return parent_lookup_kwargs

[ruslan-korneev]

An example of using the code of the previous comment https://github.com/alanjds/drf-nested-routers/issues/73#issuecomment-1400474595

# views
class ModelViewSet(CustomNestedViewSetMixin, ModelViewSet):
    parent_queryset = ParentModel.objects.all()
    queryset = ChildModel.objects.all()
    parent_lookup_prefix = "parent"
    parent_lookup_field = "pk"
    permission_classes = [IsParentOwner]
    serializer_class = ChildModelSerializer

# permissions
class IsParentOwner(BasePermission):
    def has_object_permission(self, request, view, obj): 
        """
        THIS WILL NOT CHECK THE PARENT OBJ ON POST REQUEST, 
        bcs there wouldn't be executed `view.get_object()` which check object permission
        """
        return request.user == obj.parent.user

    def has_parent_object_permission(self, request, view, obj):
        """ This will be checked on any request """
        return request.user == obj.user

[zmfink]

    parent_queryset = Parent.objects.all()
    parent_lookup_field = 'uuid'
    parent_lookup_url_kwarg = 'parent_uuid'

    lookup_field = 'uuid'

@okapies Can you elaborate where the 'kwargs' comes from in the filter_kwargs line?