Skip to content

Ability to permission related fields #1935

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
cancan101 opened this issue Oct 9, 2014 · 5 comments
Closed

Ability to permission related fields #1935

cancan101 opened this issue Oct 9, 2014 · 5 comments

Comments

@cancan101
Copy link
Contributor

Right now I am using this mixin for my Viewsets in which I want to limit the the values a related view can take on to those that the user is permissioned to. In other words this extends the filter_backends and permission_classes concept to the related fields:

class SecuredFieldMixin(object):
    """
    Filters the queryset for a related field to those items the user has permissions for
    """

    def get_serializer(self, *args, **kwargs):
        parent = super(SecuredFieldMixin, self).get_serializer(*args, **kwargs)
        for secured_field in self.secured_fields:
            if secured_field in parent.fields:
                parent.fields[secured_field].queryset = self.filter_queryset(parent.fields[secured_field].queryset)

        return parent

Right now the mixin applies this filtering to the fields listed in secured_fields.

It seems like having this permisioning/ filtering is valuable. Is this the best way to implement the logic? Does it make sense to add this to the core DRF?

@maryokhin
Copy link
Contributor

We're doing something similar, but validating on the Serializer level. And have something like this almost in each serializer.

class DraftListSerializer(DraftInstanceSerializer):
    """
    Serializer for listing and creating drafts.
    """
    user = NestedUserSerializer(read_only=True)
    user_id = serializers.PrimaryKeyRelatedField(source='user', write_only=True)
    channel = NestedChannelSerializer(read_only=True)
    channel_id = serializers.PrimaryKeyRelatedField(source='channel', write_only=True)

    validate_user_id = validate_is_allowed_to_set_user_id
    validate_channel_id = validate_is_allowed_to_set_channel_id

And in a separate file have the shortcuts.

def validate_is_allowed_to_set_user_id(self, attrs, source):
    """
    Shortcut for validating that the current user is allowed to set the input user ID field.
    """
    request = self.context['request']
    user_id = attrs[source].pk

    if request.user.pk == user_id or request.user.is_superuser:
        return attrs

    raise ValidationError("Current user cannot set user ID {}.".format(user_id))

Basically when a user_id field is used somewhere on any serializer, we want to make sure that this user_id can actually be specified. Also interested if there is a better way.

@Ismael
Copy link

Ismael commented Oct 13, 2014

I can't make a django query filter for my needs. So I'm forced to do:

class ThingSerializer(serializers.ModelSerializer):
    def to_native(self, obj):
        if not obj.has_perm(self.context["request"].user):
            return None
        return super(serializers.ModelSerializer, self).to_native(obj)

This has the problem that each object that can't be seen leaves a "null" there, which I have to filter afterwards.

class ThingViewSet(viewsets.ReadOnlyModelViewSet):
    serializer_class = ThingSerializer
    queryset = Something.objects.all()
    permission_classes = (permissions.IsAuthenticated, )

    def list(self, request, *args, **kwargs):
#snip: Same code from the mixin list method
        resp = serializer.data
        if type(resp) is list:
           #take out the Nones
            while None in resp:
                resp.remove(None)

        return Response(resp)

It'd be great if has_object_permissions where called for each related field, so that it'd only serialize the correct objects.

@tomchristie
Copy link
Member

Now using #1985 as the place to track this and associated requests.

@gabn88
Copy link
Contributor

gabn88 commented Sep 10, 2015

By the way, although the elegance of the method described here. It is not working anymore, parent.fields does not exist anymore and became parent.child.fields. Also it does not play nice with user defined Filtersets.

@gabn88
Copy link
Contributor

gabn88 commented Oct 24, 2015

I got it working in the end, but it does not limit POST requests. So no go for me.

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

No branches or pull requests

5 participants