Skip to content
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

Use more secure default for RelatedFilter.queryset #146

Merged
merged 5 commits into from
Nov 18, 2016
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
53 changes: 44 additions & 9 deletions README.rst
Original file line number Diff line number Diff line change
Expand Up @@ -113,15 +113,15 @@ You can easily traverse multiple relationships when filtering by using ``Related


class DepartmentFilter(filters.FilterSet):
manager = filters.RelatedFilter(ManagerFilter, name='manager')
manager = filters.RelatedFilter(ManagerFilter, name='manager', queryset=Manager.objects.all())

class Meta:
model = Department
fields = {'name': ['exact', 'in', 'startswith']}


class CompanyFilter(filters.FilterSet):
department = filters.RelatedFilter(DepartmentFilter, name='department')
department = filters.RelatedFilter(DepartmentFilter, name='department', queryset=Department.objects.all())

class Meta:
model = Company
Expand All @@ -140,13 +140,32 @@ Example filter calls:
/api/companies?department__name=Accounting
/api/companies?department__manager__name__startswith=Bob

``queryset`` callables
""""""""""""""""""""""

Since ``RelatedFilter`` is a subclass of ``ModelChoiceFilter``, the ``queryset`` argument supports callable behavior.
In the following example, the set of departments is restricted to those in the user's company.

.. code-block:: python

def departments(request):
company = request.user.company
return company.department_set.all()

class EmployeeFilter(filters.FilterSet):
department = filters.RelatedFilter(filterset=DepartmentFilter, queryset=departments)
...

Recursive relationships
"""""""""""""""""""""""

Recursive relations are also supported. It may be necessary to specify the full module path.

.. code-block:: python

class PersonFilter(filters.FilterSet):
name = filters.AllLookupsFilter(name='name')
best_friend = filters.RelatedFilter('people.views.PersonFilter', name='best_friend')
best_friend = filters.RelatedFilter('people.views.PersonFilter', name='best_friend', queryset=Person.objects.all())

class Meta:
model = Person
Expand Down Expand Up @@ -185,7 +204,7 @@ to all filter classes. It incorporates some of the implementation details of the
return qs.filter(**{lookup_expr: isnull})

class AuthorFilter(filters.FilterSet):
posts = filters.RelatedFilter('PostFilter')
posts = filters.RelatedFilter('PostFilter', queryset=Post.objects.all())

class Meta:
model = Author
Expand Down Expand Up @@ -285,15 +304,15 @@ You cannot combine ``AllLookupsFilter`` with ``RelatedFilter`` as the filter nam
.. code-block:: python

class ProductFilter(filters.FilterSet):
manufacturer = filters.RelatedFilter('ManufacturerFilter')
manufacturer = filters.RelatedFilter('ManufacturerFilter', queryset=Manufacturer.objects.all())
manufacturer = filters.AllLookupsFilter()

To work around this, you have the following options:

.. code-block:: python

class ProductFilter(filters.FilterSet):
manufacturer = filters.RelatedFilter('ManufacturerFilter')
manufacturer = filters.RelatedFilter('ManufacturerFilter', queryset=Manufacturer.objects.all())

class Meta:
model = Product
Expand All @@ -304,7 +323,7 @@ To work around this, you have the following options:
# or

class ProductFilter(filters.FilterSet):
manufacturer = filters.RelatedFilter('ManufacturerFilter', lookups='__all__') # `lookups` also accepts a list
manufacturer = filters.RelatedFilter('ManufacturerFilter', queryset=Manufacturer.objects.all(), lookups='__all__') # `lookups` also accepts a list

class Meta:
model = Product
Expand All @@ -326,15 +345,15 @@ and ``FilterSet``s from either package are compatible with the other's DRF backe
...

class DRFFilter(rest_framework_filters.FilterSet):
vanilla = rest_framework_filters.RelatedFilter(filterset=VanillaFilter)
vanilla = rest_framework_filters.RelatedFilter(filterset=VanillaFilter, queryset=...)


# invalid
class DRFFilter(rest_framework_filters.FilterSet):
...

class VanillaFilter(django_filters.FilterSet):
drf = rest_framework_filters.RelatedFilter(filterset=DRFFilter)
drf = rest_framework_filters.RelatedFilter(filterset=DRFFilter, queryset=...)


Caveats & Limitations
Expand Down Expand Up @@ -380,6 +399,22 @@ The recommended solutions are to either:
?publish_date__range=2016-01-01,2016-02-01


Migrating to 1.0
----------------

``RelatedFilter.queryset`` now required
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

The related filterset's model is no longer used to provide the default value for ``RelatedFilter.queryset``. This
change reduces the chance of unintentionally exposing data in the rendered filter forms. You must now explicitly
provide the ``queryset`` argument, or override the ``get_queryset()`` method (see `queryset callables`_).


``get_filters()`` renamed to ``expand_filters()``
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

django-filter has add a ``get_filters()`` classmethod to it's API, so this method has been renamed.

License
-------
Copyright (c) 2013-2015 Philip Neustrom <[email protected]>,
Expand Down
6 changes: 3 additions & 3 deletions rest_framework_filters/filters.py
Original file line number Diff line number Diff line change
Expand Up @@ -38,9 +38,9 @@ def fset(self, value):

def get_queryset(self, request):
queryset = super(RelatedFilter, self).get_queryset(request)
if queryset is not None:
return queryset
return self.filterset._meta.model._default_manager.all()
assert queryset is not None, \
"Expected `.get_queryset()` to return a `QuerySet`, but got `None`."
return queryset


class AllLookupsFilter(AutoFilter):
Expand Down
23 changes: 23 additions & 0 deletions tests/test_filtering.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
)

from .testapp.filters import (
UserFilter,
PersonFilter,
PostFilter,
BlogPostFilter,
Expand Down Expand Up @@ -326,6 +327,28 @@ def test_related_filters_caching(self):

self.assertEqual(len(filters), 1)

def test_relatedfilter_queryset_required(self):
# Use a secure default queryset. Previous behavior was to use the default model
# manager's `all()`, however this has the side effect of exposing related data.
# The default behavior should not expose information, which requires users to
# explicitly set the `queryset` argument.
class NoteFilter(FilterSet):
title = filters.CharFilter(name='title')
author = filters.RelatedFilter(UserFilter, name='author')

class Meta:
model = Note
fields = []

GET = {'author': User.objects.get(username='user2').pk}
f = NoteFilter(GET, queryset=Note.objects.all())

with self.assertRaises(AssertionError) as excinfo:
f.qs

msg = str(excinfo.exception)
self.assertEqual("Expected `.get_queryset()` to return a `QuerySet`, but got `None`.", msg)


class MiscTests(TestCase):
def test_multiwidget_incompatibility(self):
Expand Down
8 changes: 5 additions & 3 deletions tests/test_performance.py
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
from __future__ import print_function

import argparse
from timeit import timeit
from timeit import repeat

from django.test import TestCase, Client, override_settings

Expand Down Expand Up @@ -34,14 +34,16 @@ def test_sanity(self):
def test_response_time(self):
data = {'author__username': 'bob'}

df_time = timeit(lambda: self.client.get('/df-notes/', data), number=1000)
drf_time = timeit(lambda: self.client.get('/drf-notes/', data), number=1000)
df_time = min(repeat(lambda: self.client.get('/df-notes/', data), number=200, repeat=5))
drf_time = min(repeat(lambda: self.client.get('/drf-notes/', data), number=200, repeat=5))
diff = (drf_time - df_time) / df_time * 100.0

if args.verbosity >= 2:
print('\n' + '-' * 32)
print('Response time performance')
print('django-filter time:\t%.4fs' % df_time)
print('drf-filters time:\t%.4fs' % drf_time)
print('performance diff:\t%+.2f%% ' % diff)
print('-' * 32)

self.assertTrue(drf_time < (df_time * self.threshold))
18 changes: 9 additions & 9 deletions tests/testapp/filters.py
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ class Meta:

class NoteFilterWithRelated(FilterSet):
title = filters.CharFilter(name='title')
author = RelatedFilter(UserFilter, name='author')
author = RelatedFilter(UserFilter, name='author', queryset=User.objects.all())

class Meta:
model = Note
Expand All @@ -57,7 +57,7 @@ class Meta:

class NoteFilterWithRelatedAll(FilterSet):
title = filters.CharFilter(name='title')
author = RelatedFilter(UserFilterWithAll, name='author')
author = RelatedFilter(UserFilterWithAll, name='author', queryset=User.objects.all())

class Meta:
model = Note
Expand All @@ -66,7 +66,7 @@ class Meta:

class NoteFilterWithRelatedAllDifferentFilterName(FilterSet):
title = filters.CharFilter(name='title')
writer = RelatedFilter(UserFilterWithAll, name='author')
writer = RelatedFilter(UserFilterWithAll, name='author', queryset=User.objects.all())

class Meta:
model = Note
Expand All @@ -75,7 +75,7 @@ class Meta:

class PostFilter(FilterSet):
# Used for Related filter and Filter.method regression tests
note = RelatedFilter(NoteFilterWithRelatedAll, name='note')
note = RelatedFilter(NoteFilterWithRelatedAll, name='note', queryset=Note.objects.all())
date_published = filters.AllLookupsFilter()
is_published = filters.BooleanFilter(name='date_published', method='filter_is_published')

Expand All @@ -96,7 +96,7 @@ def filter_is_published(self, qs, name, value):

class CoverFilterWithRelatedMethodFilter(FilterSet):
comment = filters.CharFilter(name='comment')
post = RelatedFilter(PostFilter, name='post')
post = RelatedFilter(PostFilter, name='post', queryset=Post.objects.all())

class Meta:
model = Cover
Expand All @@ -105,7 +105,7 @@ class Meta:

class CoverFilterWithRelated(FilterSet):
comment = filters.CharFilter(name='comment')
post = RelatedFilter(PostFilter, name='post')
post = RelatedFilter(PostFilter, name='post', queryset=Post.objects.all())

class Meta:
model = Cover
Expand All @@ -114,7 +114,7 @@ class Meta:

class PageFilterWithRelated(FilterSet):
title = filters.CharFilter(name='title')
previous_page = RelatedFilter(PostFilter, name='previous_page')
previous_page = RelatedFilter(PostFilter, name='previous_page', queryset=Post.objects.all())

class Meta:
model = Page
Expand All @@ -131,7 +131,7 @@ class Meta:

class BlogPostFilter(FilterSet):
title = filters.CharFilter(name='title')
tags = RelatedFilter(TagFilter, name='tags')
tags = RelatedFilter(TagFilter, name='tags', queryset=Tag.objects.all())

class Meta:
model = BlogPost
Expand Down Expand Up @@ -186,7 +186,7 @@ class Meta:

class PersonFilter(FilterSet):
name = AllLookupsFilter(name='name')
best_friend = RelatedFilter('tests.testapp.filters.PersonFilter', name='best_friend')
best_friend = RelatedFilter('tests.testapp.filters.PersonFilter', name='best_friend', queryset=Person.objects.all())

class Meta:
model = Person
Expand Down