Add query parameter validation filter (#481)

Author: n2ygk

README.rst

@@ -173,6 +173,7 @@ override ``settings.REST_FRAMEWORK``
         ),
         'DEFAULT_METADATA_CLASS': 'rest_framework_json_api.metadata.JSONAPIMetadata',
         'DEFAULT_FILTER_BACKENDS': (
+            'rest_framework_json_api.filters.QueryParameterValidationFilter',
             'rest_framework_json_api.filters.OrderingFilter',
             'rest_framework_json_api.django_filters.DjangoFilterBackend',
             'rest_framework.filters.SearchFilter',

docs/usage.md

@@ -33,6 +33,7 @@ REST_FRAMEWORK = {
     ),
     'DEFAULT_METADATA_CLASS': 'rest_framework_json_api.metadata.JSONAPIMetadata',
     'DEFAULT_FILTER_BACKENDS': (
+        'rest_framework_json_api.filters.QueryParameterValidationFilter',
         'rest_framework_json_api.filters.OrderingFilter',
         'rest_framework_json_api.django_filters.DjangoFilterBackend',
         'rest_framework.filters.SearchFilter',
@@ -104,9 +105,32 @@ class MyLimitPagination(JsonApiLimitOffsetPagination):
 
 ### Filter Backends
 
-Following are descriptions for two JSON:API-specific filter backends and documentation on suggested usage
+Following are descriptions of JSON:API-specific filter backends and documentation on suggested usage
 for a standard DRF keyword-search filter backend that makes it consistent with JSON:API.
 
+#### `QueryParameterValidationFilter`
+`QueryParameterValidationFilter` validates query parameters to be one of the defined JSON:API query parameters
+(sort, include, filter, fields, page) and returns a `400 Bad Request` if a non-matching query parameter
+is used. This can help the client identify misspelled query parameters, for example.
+
+If you want to change the list of valid query parameters, override the `.query_regex` attribute:
+```python
+# compiled regex that matches the allowed http://jsonapi.org/format/#query-parameters
+# `sort` and `include` stand alone; `filter`, `fields`, and `page` have []'s
+query_regex = re.compile(r'^(sort|include)$|^(filter|fields|page)(\[[\w\.\-]+\])?$')
+```
+For example:
+```python
+import re
+from rest_framework_json_api.filters import QueryValidationFilter
+
+class MyQPValidator(QueryValidationFilter):
+    query_regex = re.compile(r'^(sort|include|page|page_size)$|^(filter|fields|page)(\[[\w\.\-]+\])?$')
+```
+
+If you don't care if non-JSON:API query parameters are allowed (and potentially silently ignored),
+simply don't use this filter backend.
+
 #### `OrderingFilter`
 `OrderingFilter` implements the [JSON:API `sort`](http://jsonapi.org/format/#fetching-sorting) and uses
 DRF's [ordering filter](http://django-rest-framework.readthedocs.io/en/latest/api-guide/filtering/#orderingfilter).
@@ -176,6 +200,7 @@ for `GET http://127.0.0.1:8000/nopage-entries?filter[bad]=1`:
     ]
 }
 ```
+
 #### `SearchFilter`
 
 To comply with JSON:API query parameter naming standards, DRF's
@@ -186,6 +211,7 @@ adding the `.search_param` attribute to a custom class derived from `SearchFilte
 use [`DjangoFilterBackend`](#djangofilterbackend), make sure you set the same values for both classes.
 
 
+
 #### Configuring Filter Backends
 
 You can configure the filter backends either by setting the `REST_FRAMEWORK['DEFAULT_FILTER_BACKENDS']` as shown
@@ -200,13 +226,15 @@ from models import MyModel
 class MyViewset(ModelViewSet):
     queryset = MyModel.objects.all()
     serializer_class = MyModelSerializer
-    filter_backends = (filters.OrderingFilter, django_filters.DjangoFilterBackend,)
+    filter_backends = (filters.QueryParameterValidationFilter, filters.OrderingFilter,
+	                   django_filters.DjangoFilterBackend, SearchFilter)
     filterset_fields = {
         'id': ('exact', 'lt', 'gt', 'gte', 'lte', 'in'),
         'descriptuon': ('icontains', 'iexact', 'contains'),
         'tagline': ('icontains', 'iexact', 'contains'),
     }
     search_fields = ('id', 'description', 'tagline',)
+
 ```
 
 

example/tests/test_filters.py

@@ -251,13 +251,13 @@ def test_filter_invalid_association_name(self):
     def test_filter_empty_association_name(self):
         """
         test for filter with missing association name
+        error texts are different depending on whether QueryParameterValidationFilter is in use.
         """
         response = self.client.get(self.url, data={'filter[]': 'foobar'})
         self.assertEqual(response.status_code, 400,
                          msg=response.content.decode("utf-8"))
         dja_response = response.json()
-        self.assertEqual(dja_response['errors'][0]['detail'],
-                         "invalid filter: filter[]")
+        self.assertEqual(dja_response['errors'][0]['detail'], "invalid query parameter: filter[]")
 
     def test_filter_no_brackets(self):
         """
@@ -268,7 +268,17 @@ def test_filter_no_brackets(self):
                          msg=response.content.decode("utf-8"))
         dja_response = response.json()
         self.assertEqual(dja_response['errors'][0]['detail'],
-                         "invalid filter: filter")
+                         "invalid query parameter: filter")
+
+    def test_filter_missing_right_bracket(self):
+        """
+        test for filter missing right bracket
+        """
+        response = self.client.get(self.url, data={'filter[headline': 'foobar'})
+        self.assertEqual(response.status_code, 400, msg=response.content.decode("utf-8"))
+        dja_response = response.json()
+        self.assertEqual(dja_response['errors'][0]['detail'],
+                         "invalid query parameter: filter[headline")
 
     def test_filter_no_brackets_rvalue(self):
         """
@@ -279,7 +289,7 @@ def test_filter_no_brackets_rvalue(self):
                          msg=response.content.decode("utf-8"))
         dja_response = response.json()
         self.assertEqual(dja_response['errors'][0]['detail'],
-                         "invalid filter: filter")
+                         "invalid query parameter: filter")
 
     def test_filter_no_brackets_equal(self):
         """
@@ -290,7 +300,7 @@ def test_filter_no_brackets_equal(self):
                          msg=response.content.decode("utf-8"))
         dja_response = response.json()
         self.assertEqual(dja_response['errors'][0]['detail'],
-                         "invalid filter: filter")
+                         "invalid query parameter: filter")
 
     def test_filter_malformed_left_bracket(self):
         """
@@ -300,19 +310,7 @@ def test_filter_malformed_left_bracket(self):
         self.assertEqual(response.status_code, 400,
                          msg=response.content.decode("utf-8"))
         dja_response = response.json()
-        self.assertEqual(dja_response['errors'][0]['detail'],
-                         "invalid filter: filter[")
-
-    def test_filter_missing_right_bracket(self):
-        """
-        test for filter missing right bracket
-        """
-        response = self.client.get(self.url, data={'filter[headline': 'foobar'})
-        self.assertEqual(response.status_code, 400,
-                         msg=response.content.decode("utf-8"))
-        dja_response = response.json()
-        self.assertEqual(dja_response['errors'][0]['detail'],
-                         "invalid filter: filter[headline")
+        self.assertEqual(dja_response['errors'][0]['detail'], "invalid query parameter: filter[")
 
     def test_filter_missing_rvalue(self):
         """
@@ -331,7 +329,7 @@ def test_filter_missing_rvalue_equal(self):
         """
         test for filter with missing value to test against
         this should probably be an error rather than ignoring the filter:
-            """
+        """
         response = self.client.get(self.url + '?filter[headline]')
         self.assertEqual(response.status_code, 400,
                          msg=response.content.decode("utf-8"))
@@ -478,3 +476,30 @@ def test_search_multiple_keywords(self):
             self.assertEqual(len(dja_response['data']), expected_len)
             returned_ids = set([k['id'] for k in dja_response['data']])
             self.assertEqual(returned_ids, expected_ids)
+
+    def test_param_invalid(self):
+        """
+        Test a "wrong" query parameter
+        """
+        response = self.client.get(self.url, data={'garbage': 'foo'})
+        self.assertEqual(response.status_code, 400,
+                         msg=response.content.decode("utf-8"))
+        dja_response = response.json()
+        self.assertEqual(dja_response['errors'][0]['detail'],
+                         "invalid query parameter: garbage")
+
+    def test_param_duplicate(self):
+        """
+        Test a duplicated query parameter:
+        `?sort=headline&page[size]=3&sort=bodyText` is not allowed.
+        This is not so obvious when using a data dict....
+        """
+        response = self.client.get(self.url,
+                                   data={'sort': ['headline', 'bodyText'],
+                                         'page[size]': 3}
+                                   )
+        self.assertEqual(response.status_code, 400,
+                         msg=response.content.decode("utf-8"))
+        dja_response = response.json()
+        self.assertEqual(dja_response['errors'][0]['detail'],
+                         "repeated query parameter not allowed: sort")

example/views.py

@@ -1,11 +1,14 @@
 import rest_framework.exceptions as exceptions
 import rest_framework.parsers
 import rest_framework.renderers
+from rest_framework.filters import SearchFilter
 
 import rest_framework_json_api.metadata
 import rest_framework_json_api.parsers
 import rest_framework_json_api.renderers
 from django_filters import rest_framework as filters
+from rest_framework_json_api.django_filters import DjangoFilterBackend
+from rest_framework_json_api.filters import OrderingFilter, QueryParameterValidationFilter
 from rest_framework_json_api.pagination import PageNumberPagination
 from rest_framework_json_api.utils import format_drf_errors
 from rest_framework_json_api.views import ModelViewSet, RelationshipView
@@ -91,6 +94,10 @@ class NoPagination(PageNumberPagination):
 
 class NonPaginatedEntryViewSet(EntryViewSet):
     pagination_class = NoPagination
+    # override the default filter backends in order to test QueryParameterValidationFilter without
+    # breaking older usage of non-standard query params like `page_size`.
+    filter_backends = (QueryParameterValidationFilter, OrderingFilter,
+                       DjangoFilterBackend, SearchFilter)
     ordering_fields = ('headline', 'body_text', 'blog__name', 'blog__id')
     rels = ('exact', 'iexact',
             'contains', 'icontains',

rest_framework_json_api/django_filters/backends.py

@@ -52,6 +52,13 @@ class DjangoFilterBackend(DjangoFilterBackend):
     filter_regex = re.compile(r'^filter(?P<ldelim>\[?)(?P<assoc>[\w\.\-]*)(?P<rdelim>\]?$)')
 
     def _validate_filter(self, keys, filterset_class):
+        """
+        Check that all the filter[key] are valid.
+
+        :param keys: list of FilterSet keys
+        :param filterset_class: :py:class:`django_filters.rest_framework.FilterSet`
+        :raises ValidationError: if key not in FilterSet keys or no FilterSet.
+        """
         for k in keys:
             if ((not filterset_class) or (k not in filterset_class.base_filters)):
                 raise ValidationError("invalid filter[{}]".format(k))
@@ -75,6 +82,8 @@ def get_filterset_kwargs(self, request, queryset, view):
         """
         Turns filter[<field>]=<value> into <field>=<value> which is what
         DjangoFilterBackend expects
+
+        :raises ValidationError: for bad filter syntax
         """
         filter_keys = []
         # rewrite filter[field] query params to make DjangoFilterBackend work.
@@ -83,7 +92,7 @@ def get_filterset_kwargs(self, request, queryset, view):
             m = self.filter_regex.match(qp)
             if m and (not m.groupdict()['assoc'] or
                       m.groupdict()['ldelim'] != '[' or m.groupdict()['rdelim'] != ']'):
-                raise ValidationError("invalid filter: {}".format(qp))
+                raise ValidationError("invalid query parameter: {}".format(qp))
             if m and qp != self.search_param:
                 if not val:
                     raise ValidationError("missing {} test value".format(qp))

rest_framework_json_api/filters/__init__.py

@@ -1 +1,2 @@
 from .sort import OrderingFilter  # noqa: F401
+from .queryvalidation import QueryParameterValidationFilter  # noqa: F401

rest_framework_json_api/filters/queryvalidation.py

@@ -0,0 +1,37 @@
+import re
+
+from rest_framework.exceptions import ValidationError
+from rest_framework.filters import BaseFilterBackend
+
+
+class QueryParameterValidationFilter(BaseFilterBackend):
+    """
+    A backend filter that performs strict validation of query parameters for
+    jsonapi spec conformance and raises a 400 error if non-conforming usage is
+    found.
+
+    If you want to add some additional non-standard query parameters,
+    override :py:attr:`query_regex` adding the new parameters. Make sure to comply with
+    the rules at http://jsonapi.org/format/#query-parameters.
+    """
+    #: compiled regex that matches the allowed http://jsonapi.org/format/#query-parameters
+    #: `sort` and `include` stand alone; `filter`, `fields`, and `page` have []'s
+    query_regex = re.compile(r'^(sort|include)$|^(filter|fields|page)(\[[\w\.\-]+\])?$')
+
+    def validate_query_params(self, request):
+        """
+        Validate that query params are in the list of valid query keywords
+        Raises ValidationError if not.
+        """
+        # TODO: For jsonapi error object conformance, must set jsonapi errors "parameter" for
+        # the ValidationError. This requires extending DRF/DJA Exceptions.
+        for qp in request.query_params.keys():
+            if not self.query_regex.match(qp):
+                raise ValidationError('invalid query parameter: {}'.format(qp))
+            if len(request.query_params.getlist(qp)) > 1:
+                raise ValidationError(
+                    'repeated query parameter not allowed: {}'.format(qp))
+
+    def filter_queryset(self, request, queryset, view):
+        self.validate_query_params(request)
+        return queryset