-
-
Notifications
You must be signed in to change notification settings - Fork 6.9k
Document how to handle permissions and filtering for related fields. #1985
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
Comments
I tried digging through the code, but I couldn't find an easy way to do this. Ideally, you should be able to call the permissions' "has_object_permission" for every related object. Right now, the serializer doesn't have access to the permission object. |
Except that this isn't quite as simple as that. Which permission object? These are relationships to other objects, so the permission and filter classes on the current view won't necessarily be the same rules you'd want to apply to the object relationships. For hyperlinked relationships you could in theory determine the view that they pointed at (+) and determine the filtering/permissions based on that, but it'd certainly end up as a horrible tightly-coupled design. For non-hyperlinked relationships you can't even do that. There's no guarantee that each each model is exposed once on a single canonical view, so you can't try to automagically determine the permissions you'd want to use for non-hyperlinked relationships. (+) Actually probably not actually possible to do that in any sensible way, but let's pretend for the moment. |
Maybe have a "has_<related_obj_name>_permission"? Each permission object would then be able to tell which related objects are viewable or not. How do people use the filtering? Are they using it only to hide objects the user doesn't have permissions? Because if that's the use case, then maybe filters aren't needed. |
One of the referenced issues #1646 deals with limiting the choices shown on the browsable API pages for related fields. I love the browsable API and think it a great tool not just for me as the backend developer but also for the front end developers / users of the REST API. I would love to ship the product with the browsable API turned ON (ie.. it runs even when the site is no loner in DEBUG mode). For me to be able to do that, I can't have information leakage happen through the browsable API pages. (This of course in addition to the requirement that those pages are generally production ready and secure). What that means is that no more information about the existence of related fields should be learnable through the HTML pages than would be learnable through POSTing. |
I ended up creating a mixin class for my serializers that uses the related field's View to provide the filtering. class RelatedFieldPermissionsSerializerMixin(object):
"""
Limit related fields based on the permissions in the related object's view.
To use, mixin the class, and add a dictionary to the Serializer's Meta class
named "related_queryset_filters" mapping the field name to the string name
of the appropriate view class. Example:
class MySerializer(serializers.ModelSerializer):
class Meta:
related_queryset_filters = {
'user': 'UserViewSet',
}
"""
def __init__(self, *args, **kwargs):
super(RelatedFieldPermissionsSerializerMixin, self).__init__(*args, **kwargs)
self._filter_related_fields_for_html()
def _filter_related_fields_for_html(self):
"""
Ensure thatk related fields are ownership filtered for
the browseable HTML views.
"""
import views
try:
# related_queryset_filters is a map of the fieldname and the viewset name (str)
related_queryset_filters = self.Meta.related_queryset_filters
except AttributeError:
related_queryset_filters = {}
for field, viewset in related_queryset_filters.items():
try:
self.fields[field].queryset = self._filter_related_qs(self.context['request'], getattr(views, viewset))
except KeyError:
pass
def _filter_related_qs(self, request, ViewSet):
"""
Helper function to filter related fields using
existing filtering logic in ViewSets.
"""
view = ViewSet()
view.request = request
view.action = 'retrieve'
queryset = view.get_queryset()
try:
return view.queryset_ownership_filter(queryset)
except AttributeError:
return queryset |
I solved this using a View mixin: #1935 rather than mixing Serializers and Views. Rather than needing a dictionary, I just used a list of |
I've used this simple Serializer mixin to filter querysets in related fields: class FilterRelatedMixin(object):
def __init__(self, *args, **kwargs):
super(FilterRelatedMixin, self).__init__(*args, **kwargs)
for name, field in self.fields.iteritems():
if isinstance(field, serializers.RelatedField):
method_name = 'filter_%s' % name
try:
func = getattr(self, method_name)
except AttributeError:
pass
else:
field.queryset = func(field.queryset) Usage is simple too: class SocialPageSerializer(FilterRelatedMixin, serializers.ModelSerializer):
account = serializers.PrimaryKeyRelatedField()
class Meta:
model = models.SocialPage
def filter_account(self, queryset):
request = self.context['request']
return queryset.filter(user=request.user) How is it? Do you see any problems with it? |
For me, I like to keep logic about users and requests out of the Serializer and leave that in the View. |
The problem is with related fields. A user can have access to the view but On Wed, Nov 5, 2014 at 6:16 PM, Alex Rothberg notifications@github.com
|
Please, read this, seems it is related topic. How can we separate filtering related fields objects of Meta (ModelSerializer) for OPTIONS method and a POST or PUT method? https://groups.google.com/forum/#!topic/django-rest-framework/jMePw1vS66A If we set model = serializers.PrimaryKeyRelatedField(queryset=Model.objects.none()), then we can't save current object with related model instance, because sealizer PrimaryKeyRelatedField "queryset used for model instance lookups when validating the field input". If model = serializers.PrimaryKeyRelatedField(queryset=Model.objects.all()) (as default for ModelSerializer we can comment this), then all "related" objects (they are not related really, because OPTIONS display actions (POST, PUT) properties for main Model class, not instance with related objects) displayed in choices for the "model" field (OPTIONS method). update. @cancan101 +1 . But not only "user". I think, this is bad idea mix logic and serializers, as i see queryset in serializers: "serializers.PrimaryKeyRelatedField(queryset=". of course, it is good: class ModelSerializer: because Serializer must to know how and which fields automaticlly creating from Model. Nevertheless, I could be wrong. |
This seems to work: class BlogSerializer(serializers.ModelSerializer):
entries = serializers.SerializerMethodField()
class Meta:
model = Blog
def get_entries(self, obj):
queryset = obj.entries.all()
if 'request' in self.context:
queryset = queryset.filter(author=self.context['request'].user)
serializer = EntrySerializer(queryset, many=True, context=self.context)
return serializer.data |
@dustinfarris That makes it a read-only field... but it does work. |
Ran into an issue that seems related to this thread. When a filtering backend (Django Filter in my case) is enabled the browsable API adds a Example: class Item(models.Model):
project = models.ForeignKey(Project)
class ItemSerializer(serializers.ModelSerializer):
def __init__(self, *args, **kwargs):
request = kwargs.get('context', {}).get('request')
self.fields['project'].queryset = request.user.project_set.all()
super(ItemSerializer, self).__init__(*args, **kwargs) The example above limits the Item add/edit form's project dropdown to the correct projects but everything still shows in the |
nailgun's approach has worked pretty well for me, but only for One-to-Many relations. Now, I have one model where my relationship is a ManyToManyField. In this case, the Mixin approach doesn't work. Any idea how to solve it for these? |
@fibbs change nailgun's approach by adding following: if isinstance(field, serializers.ManyRelatedField):
method_name = 'filter_%s' % name
try:
func = getattr(self, method_name)
except AttributeError:
pass
else:
field.child_relation.queryset = func(field.child_relation.queryset) |
Apparently somebody contributed a clean solution to this and its now possible without hacking init methods: https://medium.com/django-rest-framework/limit-related-data-choices-with-django-rest-framework-c54e96f5815e I wonder why this thread has not been updated / closed? |
that someone being me ;) |
It is great that the |
Probably. Want to go ahead with that ? :) |
Wow... amazing, if this is documented, can you please point me in the right direction? Took me a long time to figure this one out! Here's a summary of my issue for edification: For this example, my model names are "deployedEnvs" and "Host". Unfortunately I couldn't limit the returned results in the drop down bar to just choices for host objects which are owned by the current user. Here's my fix code adapted to use slugRelatedField
I'm about 5 books into Django ( I can list them all if you'd like), and none of the reference texts show how to work with this particular area / functionality of the Framework. At first I thought I was doing something wrong, am I? Is there a better way to do what I am trying to do? Feel free to contact me OOB so I don't end up fudging up the comments for this issue. Thanks to all for taking the time to read my comment (as a django newbie, this was really hard to figure out). |
Each
Going off the rough outline of the models you've provided, below is an outline of how RelatedFields work, with SlugRelatedField being a specialised version: class UserHostsRelatedField(serializers.RelatedField):
def get_queryset(self):
# do any permission checks and filtering here
return Host.objects.filter(user=self.context['request'].user)
def to_representation(self, obj):
# this is the data that "goes out"
# convert a Python ORM object into a string value, that will in turn be shown in the JSON
return str(obj.fqdn)
def to_internal_value(self, data):
# turn an INCOMING JSON value into a Python value, in this case a Django ORM object
# lets say the value 'ADSF-1234' comes into the serializer, you want to grab it from the ORM
return self.get_queryset().get(fqdn=data) In reality, you normally want to put a bunch of checks in either the A more complete example might look like this from rest_framework.exceptions import (
ValidationError,
PermissionError,
)
class UserHostsRelatedField(serializers.RelatedField):
def get_queryset(self):
return Host.objects.filter(user=self.context['request'].user)
def to_representation(self, obj):
return str(obj.fqdn)
def to_internal_value(self, data):
if not isinstance(data, str):
raise ValidationError({'error': 'Host fields must be strings, you passed in type %s' % type(data)})
try:
return self.get_queryset().get(fqdn=data)
except Host.DoesNotExist:
raise PermissionError({'error': 'You do not have access to this resource'}) |
In regards to what @cancan101 wrote some time ago:
This is still true as far as I can see. This can be remedied via a custom In any case it would need additional documentation. I am documenting below how to solve this via a custom filter set: Sample Workentry Model:
Base model view set:
Custom FilterSet (overrides
queryset callable as documented here: http://django-filter.readthedocs.io/en/latest/ref/filters.html#modelchoicefilter
|
take a look at this code: Does this not fix the data leakage issue you are referring to? My search fields are present in the browsable api, but yet the results are still limited to the queryset filtered by owner.
|
@Lcstyle I am not trying to filter the hosts, I am trying to filter instances of a related field (for example the owners of a host) |
I am looking at this particular problem I'd like to solve in my REST ... usually the examples are based on Let's say I have a
I'd like the REST interface to limit This is what I've come up with so far,
...is this method something that could be abstracted? It's based a little on @nailgun's #1985 (comment) I'm also thinking that I could also |
Looking at #3605 I see this can also be done with a custom serializer for the field -- let's use CEO instead of Employee of the Month:
This is designed specifically to return no objects for selection unless we're looking at a specific company. In other words, a new company couldn't have a CEO until it had employees, which you can't have until the Company is created. My only regret with this approach is it seems like this could be made DRYer/generic. |
I looked for a clean queryset filtering solution without having to add a field class or modifying the field in class MyModelSerializer(serializers.HyperlinkedModelSerializer):
class Meta:
model = MyModel
fields = ["my_field"]
def get_my_field_queryset(self):
user = self.context["request"].user
return MyModel.objects.filter(owner=user)
def get_extra_kwargs(self):
kwargs = super().get_extra_kwargs()
kwargs["my_field"] = {"queryset": self.get_my_field_queryset()}
return kwargs I thought that it would be really nice to eliminate the boilerplate code in |
This is such a great solution @robwa <3 |
Currently relationships do not automatically apply the same set of permissions and filtering that are applied to views. If you need permissions or filter on relationships you need to deal with it explicitly.
Personally I don't see any good ways of dealing with this automatically, but it's obviously something that we could at least do with documenting better.
Right now my opinion is that we should try to come up with a simple example case and document how you'd deal with that explicitly. Any automatic code for dealing with this should be left for third party package authors to handle. This allows other contributors to explore the problem and see if they can come up with any good solutions that could potentially be included in core.
In the future this issue might get promoted from 'Documentation' to 'Enhancement', but unless there's any concrete proposals that are backed up by a third part package then it'll stay in this state.
The text was updated successfully, but these errors were encountered: