eamigo86 / graphene-django-extras

Extras functionalities for Graphene-Django
MIT License
414 stars 102 forks source link

Ordering should only accept fields specified in only_fields #95

Open kamilkijak opened 5 years ago

kamilkijak commented 5 years ago

Currently the default pagination (LimitOffsetGraphqlPagination) allows to order by any of the model's fields. It could be a security issue (f.e. ordering by some private fields like address, budget, etc.).

It'd be useful to add a check if order value belongs to only_fields from specific DjangoObjectType. It should be added somewhere here probably.

Optionally there could be an extra setting (f.e. order_fields) added to a Meta class of DjangoObjectType (similarly to filter_fields), but I believe that checking it against only_fields is enough.

@eamigo86 What do you think?

sulabhjain commented 5 years ago

Just ran into this issue while searching if there is an existing issue with default ordering not working. I would suggest that it checks only_fields with the exception if ordering is set on the server side along with pagination.

class TodoListType(DjangoListObjectType):
    class Meta(object):
        model = Todo
        description = " Type definition for todo list"
        filter_fields = {
            'uuid': ['exact'],
            'name': ['icontains', 'iexact'],
        }
        only_fields = ('uuid', 'name',)
        pagination = LimitOffsetGraphqlPagination(default_limit=20, ordering="order")
eamigo86 commented 5 years ago

Hi @kamilkijak and @sulabhjain, thanks for writing, in these last days I have been a bit busy in other personal projects (a baby), for which I have not been able to dedicate the time I would like to update this module. But very soon I will release a new version with all these issues reported already fixed.