Skip to content

How do we check permissions for related entities? #450

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
Anton-Shutik opened this issue Aug 1, 2018 · 2 comments
Closed

How do we check permissions for related entities? #450

Anton-Shutik opened this issue Aug 1, 2018 · 2 comments

Comments

@Anton-Shutik
Copy link
Contributor

Let's say we have config like this:

views.py

class CustomerProfileViewSet(viewsets.ModelViewSet):
    permission_classes = (IsCustomerOrCreation,)


class ProductViewSet(viewsets.ModelViewSet):
    permission_classes = []

    def get_object(self,):
        customer_pk = self.kwargs.get('customer_pk', None)
            if customer_pk is not None:
                customer = get_object_or_None(Customer, pk=customer_pk)
                if customer is not None:
                    return customer.favourite_product
        return super(ProductViewSet, self).get_object()

urls.py

url(r'^api/customer/(?P<pk>\d+)/$', CustomerProfileViewSet.as_view({'get': 'retrieve'})),
url(r'^api/customer/(?P<customer_pk>\d+)/favorite-product$', ProductViewSet.as_view({'get': 'retrieve'})),

GET api/customer/1/ Will give 403 error for any user whose id differs from 1. And that is fine.
Then if anon user goes to api/customer/1/favourite-product/ they will get 200 response with product payload, because permissions for product checked in ProductViewSet, which have no permissions for any role. But I think it is wrong and should return 403, since we have to check permission for customer first. We actually just told everybody what is customer's 1 favorite product!

I understand that permissions it is out of json api spec, but may be there is a nice way I can handle permissions with this lib correctly?

For now I do it like below:
A RelatedMixin class will get a related entity and will find corresponding a serializer for that. And all that stuff will happen after we check permissions for parent ("customer" in the case) entity

class RelatedMixin(object):
    serializer_mapping = {}
    field_name_mapping = {}

    def get_related(self, request, *args, **kwargs):
        serializer_kwargs = {}
        instance = self.get_related_instance()

        if callable(instance):
            instance = instance()

        if hasattr(instance, 'all'):
            instance = instance.all()

        if instance is None:
            return Response(data=None)

        if isinstance(instance, Iterable):
            serializer_kwargs['many'] = True

        serializer = self.get_serializer(instance, **serializer_kwargs)
        return Response(serializer.data)

    def get_serializer_class(self):
        field_name = self.get_related_field_name()
        return self.serializer_mapping[field_name]

    def get_related_field_name(self):
        field_name = self.kwargs['related_field']
        if field_name in self.field_name_mapping:
            return self.field_name_mapping[field_name]
        return field_name

    def get_related_instance(self):
        try:
            return getattr(self.get_object(), self.get_related_field_name())
        except AttributeError:
            from rest_framework.exceptions import NotFound
            raise NotFound

and then just make a view that will handle all Customer's related entities:

class CustomerProfileRelatedViewSet(RelatedMixin, CustomerProfileViewSet):
    serializer_mapping = {
        'favourite_product': ProductSerializer,
        #'order_list': OrderSerializer
}

urls.py # Only one url route required for all related entities

url(r'^api/customer/(?P<customer_pk>\d+)/(?P<related_field>\w+)/$', CustomerProfileRelatedViewSet.as_view({'get': 'get_related'})),

What do you think about it ? If you like the idea I'm ready to come up with PR

@sliverc
Copy link
Member

sliverc commented Aug 2, 2018

Permissions is not part of JSON API spec but certainly something addressed by DRF. Hence I like the idea to solve this issue for DJA specific case of related links.

Your approach outlined here also seems to make it easier to implement related links. Beside some details (can be later on discussed in the PR) this potentially needs two queries as when implementing ProductViewSet as its own view it could be reduced to one sql query. But I guess a small difference which can be neglected if it makes it easier to implement related links.

Feel free to open a PR and we can then discuss details there.

@Anton-Shutik
Copy link
Contributor Author

@sliverc I've added initial implementation here

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants