Merge branch 'VALIDATE_FILTERS_doesnt_work_for_subdocument_fields_#1125'

Closes #1123.

AUTHORS

@@ -89,6 +89,7 @@ Patches and Contributions
 - Kurt Bonne
 - Kurt Doherty
 - Luca Di Gaspero
+- Luca Moretto
 - Luis Fernando Gomes
 - Magdas Adrian
 - Mandar Vaze

CHANGES

@@ -8,6 +8,8 @@ Development
 
 Version 0.8
 ~~~~~~~~~~~
+- Fix: ``VALIDATE_FILTERS`` and ``ALLOWED_FILTERS`` do not work with
+  sub-document fields. Closes #1123 (Luca Moretto).
 - Fix documentation typos (Olof Johansson)
 - Fix a changelog typo (kreynen).
 - Update: bump Flask requirement to <=0.13. Closes #1111.
@@ -20,12 +22,12 @@ Version 0.8
   (Marcin Puhacz).
 - Change: ``JSON`` and ``XML`` settings are deprecated and will be removed in
   a future update. Use ``RENDERERS`` instead (Marcin Puhacz).
-- New: Refactor index creation. We now have a new 
+- New: Refactor index creation. We now have a new
   ``eve.io.mongo.ensure_mongo_indexes()`` function which ensures that eventual
-  ``mongo_indexes`` defined for a resource are created on the active
-  database. The function can be imported and invoked, for example in
-  multi-db workflows where a db is activated based on the
-  authenticated user performing the request (via custom auth classes).
+  ``mongo_indexes`` defined for a resource are created on the active database.
+  The function can be imported and invoked, for example in multi-db workflows
+  where a db is activated based on the authenticated user performing the
+  request (via custom auth classes).
 - Fix: add sphinxcontrib-embedly to dev-requirements.txt.
 - New: when the media endpoint is enabled, the default authentication class
   will be used to secure  it. Closes #1083.

docs/config.rst

@@ -114,6 +114,14 @@ uppercase.
                                     ``/v1/<endpoint>``). Defaults to ``''``.
 
 ``ALLOWED_FILTERS``                 List of fields on which filtering is allowed.
+                                    Entries in this list work in a hierarchical
+                                    way. This means that, for instance, filtering
+                                    on ``'dict.sub_dict.foo'`` is allowed if
+                                    ``ALLOWED_FILTERS`` contains any of
+                                    ``'dict.sub_dict.foo``, ``'dict.sub_dict'``
+                                    or ``'dict'``. Instead filtering on
+                                    ``'dict'`` is allowed if ``ALLOWED_FILTERS``
+                                    contains ``'dict'``.
                                     Can be set to ``[]`` (no filters allowed)
                                     or ``['*']`` (filters allowed on every
                                     field). Unless your API is comprised of
@@ -798,6 +806,14 @@ always lowercase.
                                 :ref:`subresources`.
 
 ``allowed_filters``             List of fields on which filtering is allowed.
+                                Entries in this list work in a hierarchical
+                                way. This means that, for instance, filtering
+                                on ``'dict.sub_dict.foo'`` is allowed if
+                                ``allowed_filters`` contains any of
+                                ``'dict.sub_dict.foo``, ``'dict.sub_dict'``
+                                or ``'dict'``. Instead filtering on
+                                ``'dict'`` is allowed if ``allowed_filters``
+                                contains ``'dict'``.
                                 Can be set to ``[]`` (no filters allowed), or
                                 ``['*']`` (fields allowed on every field).
                                 Defaults to ``['*']``.

eve/tests/methods/get.py

@@ -535,6 +535,31 @@ def test_get_where_allowed_filters(self):
                                            '?where=%s' % where))
         self.assert200(r.status_code)
 
+        # `allowed_filters` contains "rows" --> filter key "rows.price"
+        # must be allowed
+        self.app.config['DOMAIN'][self.known_resource]['allowed_filters'] = \
+            ['rows']
+        where = '{"rows.price": 10}'
+        r = self.test_client.get('%s%s' % (self.known_resource_url,
+                                           '?where=%s' % where))
+        self.assert200(r.status_code)
+
+        # `allowed_filters` contains "rows.price" --> filter key "rows.price"
+        # must be allowed
+        self.app.config['DOMAIN'][self.known_resource]['allowed_filters'] = \
+            ['rows.price']
+        r = self.test_client.get('%s%s' % (self.known_resource_url,
+                                           '?where=%s' % where))
+        self.assert200(r.status_code)
+
+        # `allowed_filters` contains "rows.price" --> filter key "rows"
+        # must NOT be allowed
+        where = '{"rows": {"sku": "value", "price": 10}}'
+        r = self.test_client.get('%s%s' % (self.known_resource_url,
+                                           '?where=%s' % where))
+        self.assert400(r.status_code)
+        self.assertTrue(b"'rows' not allowed" in r.get_data())
+
     def test_get_with_post_override(self):
         # POST request with GET override turns into a GET
         headers = [('X-HTTP-Method-Override', 'GET')]
@@ -1099,6 +1124,36 @@ def test_get_invalid_where_fields(self):
         response, status = self.get(self.known_resource, where)
         self.assert200(status)
 
+        # test for nested resource field validating correctly
+        # (location is dict)
+        where = '?where={"location.address": "str 1"}'
+        response, status = self.get(self.known_resource, where)
+        self.assert200(status)
+
+        # test for nested resource field validating correctly
+        # (rows is list of dicts)
+        where = '?where={"rows.price": 10}'
+        response, status = self.get(self.known_resource, where)
+        self.assert200(status)
+
+        # test for nested resource field validating correctly
+        # (dict_list_fixed_len is a fixed-size list of dicts)
+        where = '?where={"dict_list_fixed_len.key2": 1}'
+        response, status = self.get(self.known_resource, where)
+        self.assert200(status)
+
+        # test for nested resource field not validating correctly
+        # (bad_base_key doesn't exist in the base resource schema)
+        where = '?where={"bad_base_key.sub": 1}'
+        response, status = self.get(self.known_resource, where)
+        self.assert400(status)
+
+        # test for nested resource field not validating correctly
+        # (bad_sub_key doesn't exist in the dict_list_fixed_len schema)
+        where = '?where={"dict_list_fixed_len.bad_sub_key": 1}'
+        response, status = self.get(self.known_resource, where)
+        self.assert400(status)
+
     def test_get_lookup_field_as_string(self):
         # Test that a resource where 'item_lookup_field' is set to a field
         # of string type and which value is castable to a ObjectId is still

eve/tests/test_settings.py

@@ -100,6 +100,19 @@
             'type': 'list',
             'items': [{'type': 'objectid'}]
         },
+        'dict_list_fixed_len': {
+            'type': 'list',
+            'items': [
+                {
+                    'type': 'dict',
+                    'schema': {'key1': {'type': 'string'}}
+                },
+                {
+                    'type': 'dict',
+                    'schema': {'key2': {'type': 'integer'}}
+                }
+            ]
+        },
         'dependency_field1': {
             'type': 'string',
             'default': 'default'

eve/utils.py

@@ -386,8 +386,17 @@ def validate_filters(where, resource):
 
     def validate_filter(filter):
         for key, value in filter.items():
-            if '*' not in allowed and key not in allowed:
-                return "filter on '%s' not allowed" % key
+            if '*' not in allowed:
+                def recursive_check_allowed(filter_key, allowed_filters):
+                    if filter_key not in allowed_filters:
+                        base_composed_key, _, _ = filter_key.rpartition('.')
+                        return base_composed_key and recursive_check_allowed(
+                            base_composed_key, allowed_filters)
+
+                    return True
+
+                if not recursive_check_allowed(key, allowed):
+                    return "filter on '%s' not allowed" % key
 
             if key in ('$or', '$and', '$nor'):
                 if not isinstance(value, list):
@@ -401,16 +410,58 @@ def validate_filter(filter):
                         return r
             else:
                 if config.VALIDATE_FILTERS:
+                    def get_sub_schemas(base_schema):
+                        def dict_sub_schema(base):
+                            if base.get('type') == 'dict':
+                                return base.get('schema')
+
+                            return None
+
+                        if base_schema.get('type') == 'list':
+                            if 'schema' in base_schema:
+                                # Try to get dict sub-schema for arbitrary
+                                # sized list
+                                sub = dict_sub_schema(base_schema['schema'])
+                                return [sub] if sub is not None else []
+                            elif 'items' in base_schema:
+                                # Try to get dict sub-schema(s) for
+                                # fixed-size list
+                                items = base_schema['items']
+                                sub_schemas = []
+                                for item in items:
+                                    sub = dict_sub_schema(item)
+                                    if sub is not None:
+                                        sub_schemas.append(sub)
+
+                                return sub_schemas
+                        else:
+                            sub = dict_sub_schema(base_schema)
+                            return [sub] if sub is not None else []
+
+                    def recursive_validate_filter(key, value, schema):
+                        if key not in schema:
+                            base_key, _, sub_keys = key.partition('.')
+                            if sub_keys and base_key in schema:
+                                # key is the composition of base field and
+                                # sub-fields
+                                sub_schemas = get_sub_schemas(schema[base_key])
+                                for sub_schema in sub_schemas:
+                                    if recursive_validate_filter(sub_keys,
+                                                                 value,
+                                                                 sub_schema):
+                                        return True
+
+                            return False
+                        else:
+                            field_schema = schema.get(key)
+                            v = Validator({key: field_schema})
+                            return v.validate({key: value})
+
                     res_schema = config.DOMAIN[resource]['schema']
-                    if key not in res_schema:
+                    if not recursive_validate_filter(key, value, res_schema):
                         return "filter on '%s' is invalid"
-                    else:
-                        field_schema = res_schema.get(key)
-                        v = Validator({key: field_schema})
-                        if not v.validate({key: value}):
-                            return "filter on '%s' is invalid"
-                        else:
-                            return None
+
+                    return None
 
     if '*' in allowed and not config.VALIDATE_FILTERS:
         return None