encode/django-rest-framework
1
1
Fork 0
mirror of https://github.com/encode/django-rest-framework.git synced 2024-11-26 11:33:59 +03:00
Code Issues Packages Projects Releases Wiki Activity

Security update to OrderingFilter

Browse Source
This commit is contained in:
Tom Christie 2014-01-15 14:27:41 +00:00
parent e9fda70b4a
commit 71c03b9db9
5 changed files with 160 additions and 8 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

26
docs/api-guide/filtering.md
View File

@ -282,13 +282,37 @@ Multiple orderings may also be specified:
http://example.com/api/users?ordering=account,username
### Specifying which fields may be ordered against
It's recommended that you explicitly specify which fields the API should allowing in the ordering filter. You can do this by setting an `ordering_fields` attribute on the view, like so:
class UserListView(generics.ListAPIView):
queryset = User.objects.all()
serializer_class = UserSerializer
filter_backends = (filters.OrderingFilter,)
ordering_fields = ('username', 'email')
This helps prevent unexpected data leakage, such as allowing users to order against a password hash field or other sensitive data.
If you *don't* specify an `ordering_fields` attribute on the view, the filter class will default to allowing the user to filter on any readable fields on the serializer specified by the `serializer_class` attribute.
If you are confident that the queryset being used by the view doesn't contain any sensitive data, you can also explicitly specify that a view should allow ordering on *any* model field or queryset aggregate, by using the special value `'__all__'`.
class BookingsListView(generics.ListAPIView):
queryset = Booking.objects.all()
serializer_class = BookingSerializer
filter_backends = (filters.OrderingFilter,)
ordering_fields = '__all__'
### Specifying a default ordering
If an `ordering` attribute is set on the view, this will be used as the default ordering.
Typically you'd instead control this by setting `order_by` on the initial queryset, but using the `ordering` parameter on the view allows you to specify the ordering in a way that it can then be passed automatically as context to a rendered template. This makes it possible to automatically render column headers differently if they are being used to order the results.
class UserListView(generics.ListAPIView):
queryset = User.objects.all()
serializer = UserSerializer
serializer_class = UserSerializer
filter_backends = (filters.OrderingFilter,)
ordering = ('username',)

7
docs/topics/release-notes.md
View File

@ -40,6 +40,13 @@ You can determine your currently installed version using `pip freeze`:
## 2.3.x series
### 2.3.12
**Date**: 15th January 2014
* **Security fix**: `OrderingField` now only allows ordering on readable serializer fields, or on fields explicitly specified using `ordering_fields`. This prevents users being able to order by fields that are not visible in the API, and exploiting the ordering of sensitive data such as password hashes.
* Bugfix: `write_only = True` fields now display in the browsable API.
### 2.3.11
**Date**: 14th January 2014

2
rest_framework/__init__.py
View File

@ -8,7 +8,7 @@ ______ _____ _____ _____ __ _
"""
__title__ = 'Django REST framework'
__version__ = '2.3.11'
__version__ = '2.3.12'
__author__ = 'Tom Christie'
__license__ = 'BSD 2-Clause'
__copyright__ = 'Copyright 2011-2013 Tom Christie'

29
rest_framework/filters.py
View File

@ -3,6 +3,7 @@ Provides generic filtering backends that can be used to filter the results
returned by list views.
"""
from __future__ import unicode_literals
from django.core.exceptions import ImproperlyConfigured
from django.db import models
from rest_framework.compat import django_filters, six, guardian, get_model_name
from functools import reduce
@ -107,6 +108,7 @@ class SearchFilter(BaseFilterBackend):
class OrderingFilter(BaseFilterBackend):
ordering_param = 'ordering' # The URL query parameter used for the ordering.
ordering_fields = None
def get_ordering(self, request):
"""
@ -122,17 +124,34 @@ class OrderingFilter(BaseFilterBackend):
return (ordering,)
return ordering
def remove_invalid_fields(self, queryset, ordering):
field_names = [field.name for field in queryset.model._meta.fields]
field_names += queryset.query.aggregates.keys()
return [term for term in ordering if term.lstrip('-') in field_names]
def remove_invalid_fields(self, queryset, ordering, view):
valid_fields = getattr(view, 'ordering_fields', self.ordering_fields)
if valid_fields is None:
# Default to allowing filtering on serializer fields
serializer_class = getattr(view, 'serializer_class')
if serializer_class is None:
msg = ("Cannot use %s on a view which does not have either a "
"'serializer_class' or 'ordering_fields' attribute.")
raise ImproperlyConfigured(msg % self.__class__.__name__)
valid_fields = [
field.source or field_name
for field_name, field in serializer_class().fields.items()
if not getattr(field, 'write_only', False)
]
elif valid_fields == '__all__':
# View explictly allows filtering on any model field
valid_fields = [field.name for field in queryset.model._meta.fields]
valid_fields += queryset.query.aggregates.keys()
return [term for term in ordering if term.lstrip('-') in valid_fields]
def filter_queryset(self, request, queryset, view):
ordering = self.get_ordering(request)
if ordering:
# Skip any incorrect parameters
ordering = self.remove_invalid_fields(queryset, ordering)
ordering = self.remove_invalid_fields(queryset, ordering, view)
if not ordering:
# Use 'ordering' attribute by default

104
rest_framework/tests/test_filters.py
View File

@ -368,7 +368,6 @@ class OrderingFilterRelatedModel(models.Model):
related_name="relateds")
class OrderingFilterTests(TestCase):
def setUp(self):
# Sequence of title/text is:
@ -394,6 +393,7 @@ class OrderingFilterTests(TestCase):
model = OrdringFilterModel
filter_backends = (filters.OrderingFilter,)
ordering = ('title',)
ordering_fields = ('text',)
view = OrderingListView.as_view()
request = factory.get('?ordering=text')
@ -412,6 +412,7 @@ class OrderingFilterTests(TestCase):
model = OrdringFilterModel
filter_backends = (filters.OrderingFilter,)
ordering = ('title',)
ordering_fields = ('text',)
view = OrderingListView.as_view()
request = factory.get('?ordering=-text')
@ -430,6 +431,7 @@ class OrderingFilterTests(TestCase):
model = OrdringFilterModel
filter_backends = (filters.OrderingFilter,)
ordering = ('title',)
ordering_fields = ('text',)
view = OrderingListView.as_view()
request = factory.get('?ordering=foobar')
@ -448,6 +450,7 @@ class OrderingFilterTests(TestCase):
model = OrdringFilterModel
filter_backends = (filters.OrderingFilter,)
ordering = ('title',)
oredering_fields = ('text',)
view = OrderingListView.as_view()
request = factory.get('')
@ -466,6 +469,7 @@ class OrderingFilterTests(TestCase):
model = OrdringFilterModel
filter_backends = (filters.OrderingFilter,)
ordering = 'title'
ordering_fields = ('text',)
view = OrderingListView.as_view()
request = factory.get('')
@ -494,6 +498,7 @@ class OrderingFilterTests(TestCase):
model = OrdringFilterModel
filter_backends = (filters.OrderingFilter,)
ordering = 'title'
ordering_fields = '__all__'
queryset = OrdringFilterModel.objects.all().annotate(
models.Count("relateds"))
@ -510,4 +515,101 @@ class OrderingFilterTests(TestCase):
)
class SensitiveOrderingFilterModel(models.Model):
username = models.CharField(max_length=20)
password = models.CharField(max_length=100)
# Three different styles of serializer.
# All should allow ordering by username, but not by password.
class SensitiveDataSerializer1(serializers.ModelSerializer):
username = serializers.CharField()
class Meta:
model = SensitiveOrderingFilterModel
fields = ('id', 'username')
class SensitiveDataSerializer2(serializers.ModelSerializer):
username = serializers.CharField()
password = serializers.CharField(write_only=True)
class Meta:
model = SensitiveOrderingFilterModel
fields = ('id', 'username', 'password')
class SensitiveDataSerializer3(serializers.ModelSerializer):
user = serializers.CharField(source='username')
class Meta:
model = SensitiveOrderingFilterModel
fields = ('id', 'user')
class SensitiveOrderingFilterTests(TestCase):
def setUp(self):
for idx in range(3):
username = {0: 'userA', 1: 'userB', 2: 'userC'}[idx]
password = {0: 'passA', 1: 'passC', 2: 'passB'}[idx]
SensitiveOrderingFilterModel(username=username, password=password).save()
def test_order_by_serializer_fields(self):
for serializer_cls in [
SensitiveDataSerializer1,
SensitiveDataSerializer2,
SensitiveDataSerializer3
]:
class OrderingListView(generics.ListAPIView):
queryset = SensitiveOrderingFilterModel.objects.all().order_by('username')
filter_backends = (filters.OrderingFilter,)
serializer_class = serializer_cls
view = OrderingListView.as_view()
request = factory.get('?ordering=-username')
response = view(request)
if serializer_cls == SensitiveDataSerializer3:
username_field = 'user'
else:
username_field = 'username'
# Note: Inverse username ordering correctly applied.
self.assertEqual(
response.data,
[
{'id': 3, username_field: 'userC'},
{'id': 2, username_field: 'userB'},
{'id': 1, username_field: 'userA'},
]
)
def test_cannot_order_by_non_serializer_fields(self):
for serializer_cls in [
SensitiveDataSerializer1,
SensitiveDataSerializer2,
SensitiveDataSerializer3
]:
class OrderingListView(generics.ListAPIView):
queryset = SensitiveOrderingFilterModel.objects.all().order_by('username')
filter_backends = (filters.OrderingFilter,)
serializer_class = serializer_cls
view = OrderingListView.as_view()
request = factory.get('?ordering=password')
response = view(request)
if serializer_cls == SensitiveDataSerializer3:
username_field = 'user'
else:
username_field = 'username'
# Note: The passwords are not in order. Default ordering is used.
self.assertEqual(
response.data,
[
{'id': 1, username_field: 'userA'}, # PassB
{'id': 2, username_field: 'userB'}, # PassC
{'id': 3, username_field: 'userC'}, # PassA
]
)