fix: Validate the order by names of the entity queries

CHANGELOG.md

@@ -96,3 +96,5 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 * BER unpacker now properly handles unknown type numbers by falling back to sequence (constructed) or octet string (primitive) unpacking
 * Certificate DER parsing now correctly extracts RSA public key from SubjectPublicKeyInfo structure
 * Certificate parser now handles optional version field correctly, supporting both v1 certificates (no version) and v2/v3 certificates
+* Sorting a list by an unknown field, such as one sent by a translated page, now keeps the default order instead of failing - [#47](https://github.com/hivesolutions/colony-plugins/issues/47)
+* Filtering a list by a field of a related entity defined in another module no longer discards the filtered value

data/src/entity_manager/system.py

@@ -8183,6 +8183,11 @@ def _resolve_name(self, entity_class, name):
         # relation information into the name buffer for latter
         # processing
         for name_partial in name_partials:
+            # validates that the partial name exists in the context
+            # of the current entity class (this is a security validation
+            # to avoid possible injection)
+            entity_class._validate_name(name_partial)
+
             # write the relation name and prefix into the name
             # buffer for relation indication
             name_buffer.write("__")
@@ -8192,6 +8197,24 @@ def _resolve_name(self, entity_class, name):
             # of the current relation (partial name)
             entity_class = entity_class.get_target(name_partial)
 
+            # checks if the target class is a "data reference" and
+            # in case it is, tries to resolve it into the appropriate
+            # concrete (real) class in case the resolution fails it's
+            # impossible to order by the relation (relation not joined)
+            target_is_reference = entity_class.is_reference()
+            if target_is_reference:
+                entity_class = self.get_entity(entity_class.__name__)
+            if not entity_class:
+                raise exceptions.ValidationError(
+                    "invalid name '%s', referenced class not found for '%s'"
+                    % (name, name_partial)
+                )
+
+        # validates that the final name exists in the context of the
+        # (target) entity class (this is a security validation to
+        # avoid possible injection)
+        entity_class._validate_name(final_name)
+
         # retrieves the name of the identifier attribute
         # for the current entity class
         id = entity_class.get_id()

data/src/entity_manager/test.py

@@ -1400,6 +1400,175 @@ def test_order_by(self):
         self.assertEqual(persons[1].object_id, person_c.object_id)
         self.assertEqual(persons[2].object_id, person_a.object_id)
 
+    def test_order_by_identifier(self):
+        # creates the required entity classes in the data source
+        self.entity_manager.create(mocks.Person)
+
+        # creates the the various person entities and saves them
+        # in an order that is not the one of their identifiers
+        person_a = mocks.Person()
+        person_a.object_id = 1
+        person_a.name = "name_person_a"
+        person_b = mocks.Person()
+        person_b.object_id = 2
+        person_b.name = "name_person_b"
+        person_c = mocks.Person()
+        person_c.object_id = 3
+        person_c.name = "name_person_c"
+        self.entity_manager.save(person_b)
+        self.entity_manager.save(person_c)
+        self.entity_manager.save(person_a)
+
+        # retrieves the persons from the data source ordered by
+        # the default value, which refers the identifier attribute
+        persons = self.entity_manager.find(
+            mocks.Person, dict(order_by=(("__default__", "descending"),))
+        )
+
+        # verifies that the retrieved list is not empty and that
+        # the various persons are ordered by their identifiers
+        self.assertNotEqual(persons, [])
+        self.assertEqual(persons[0].object_id, person_c.object_id)
+        self.assertEqual(persons[1].object_id, person_b.object_id)
+        self.assertEqual(persons[2].object_id, person_a.object_id)
+
+        # retrieves the persons from the data source ordered by
+        # the identifier value in ascending order
+        persons = self.entity_manager.find(
+            mocks.Person, dict(order_by=(("__identifier__", "ascending"),))
+        )
+
+        # verifies that the retrieved list is not empty and that
+        # the various persons are ordered by their identifiers
+        self.assertNotEqual(persons, [])
+        self.assertEqual(persons[0].object_id, person_a.object_id)
+        self.assertEqual(persons[1].object_id, person_b.object_id)
+        self.assertEqual(persons[2].object_id, person_c.object_id)
+
+    def test_order_by_invalid(self):
+        # creates the required entity classes in the data source
+        self.entity_manager.create(mocks.Person)
+        self.entity_manager.create(mocks.Address)
+
+        # verifies that ordering by an attribute that does not
+        # exist in the entity class raises a validation error
+        self.assert_raises(
+            exceptions.ValidationError,
+            self.entity_manager.find,
+            mocks.Person,
+            dict(order_by=(("unknown", "descending"),)),
+        )
+
+        # verifies that ordering by a value that is not a name
+        # (eg: markup added by the translation of a page) raises
+        # a validation error instead of reaching the data source
+        self.assert_raises(
+            exceptions.ValidationError,
+            self.entity_manager.find,
+            mocks.Person,
+            dict(order_by=(('<font dir="auto" style="vertical-align', None),)),
+        )
+
+        # verifies that ordering by a value that contains an extra
+        # order by expression raises a validation error
+        self.assert_raises(
+            exceptions.ValidationError,
+            self.entity_manager.find,
+            mocks.Person,
+            dict(order_by=(("name desc, age", "ascending"),)),
+        )
+
+        # verifies that ordering by an attribute that does not exist
+        # in the target class of the relation raises a validation error
+        # even if it exists in the (top level) entity class
+        self.assert_raises(
+            exceptions.ValidationError,
+            self.entity_manager.find,
+            mocks.Person,
+            dict(eager=("address",), order_by=(("address.name", "descending"),)),
+        )
+
+        # verifies that ordering through a relation that does not exist
+        # in the entity class raises a validation error
+        self.assert_raises(
+            exceptions.ValidationError,
+            self.entity_manager._resolve_name,
+            mocks.Person,
+            "unknown.street",
+        )
+
+    def test_order_by_reserved(self):
+        # creates the required entity classes in the data source
+        self.entity_manager.create(mocks.Person)
+
+        # creates a person entity and saves it so that there's
+        # a value to be retrieved by the ordered retrieval
+        person = mocks.Person()
+        person.object_id = 1
+        person.name = "name_person"
+        self.entity_manager.save(person)
+
+        # retrieves the persons from the data source ordered by the
+        # modification time, which is a reserved name (not an attribute)
+        # and so it's valid for the ordering of the entities
+        persons = self.entity_manager.find(
+            mocks.Person, dict(order_by=(("_mtime", "descending"),))
+        )
+
+        # verifies that the person is retrieved from the data source
+        # even though the name is not an attribute of the entity class
+        self.assertEqual(len(persons), 1)
+        self.assertEqual(persons[0].object_id, person.object_id)
+
+    def test_order_by_reference(self):
+        # creates a data reference for the address entity class without
+        # any of its attributes and a data reference for an entity class
+        # that is not registered in the entity manager (never resolved)
+        class Address(mocks.RootEntity):
+            data_reference = True
+
+        class Kennel(mocks.RootEntity):
+            data_reference = True
+            street = dict(type="text")
+
+        # creates an entity class with relations to both data references
+        # so that they must be resolved for the ordering
+        class Tenant(mocks.RootEntity):
+            address = dict(type="relation")
+            kennel = dict(type="relation")
+
+            @staticmethod
+            def _relation_address():
+                return dict(type="to-one", target=Address, is_mapper=True)
+
+            @staticmethod
+            def _relation_kennel():
+                return dict(type="to-one", target=Kennel, is_mapper=True)
+
+        # verifies that the name is resolved against the concrete address
+        # entity class, that contains the attribute (unlike the reference)
+        name = self.entity_manager._resolve_name(Tenant, "address.street")
+        self.assertEqual(name, "__address.street")
+
+        # verifies that ordering by an attribute that does not exist in
+        # the concrete class of the reference raises a validation error
+        self.assert_raises(
+            exceptions.ValidationError,
+            self.entity_manager._resolve_name,
+            Tenant,
+            "address.unknown",
+        )
+
+        # verifies that ordering through a data reference that is not
+        # resolved raises a validation error, even if the attribute is
+        # defined in the data reference (as the relation is not joined)
+        self.assert_raises(
+            exceptions.ValidationError,
+            self.entity_manager._resolve_name,
+            Tenant,
+            "kennel.street",
+        )
+
     def test_range(self):
         # creates the required entity classes in the data source
         self.entity_manager.create(mocks.Person)

mvc/src/mvc_utils/entity_model.py

@@ -79,6 +79,10 @@
 """ The list of tokens that may be used to represent the
 sort order in the entity manager """
 
+RESERVED_NAMES = ("_class", "_mtime")
+""" The tuple containing the names that are considered to be
+reserved (special cases) for the queries """
+
 DATA_TYPE_CAST_TYPES_MAP = dict(
     text=colony.legacy.UNICODE,
     string=colony.legacy.UNICODE,
@@ -807,15 +811,6 @@ def _class_create_filter(cls, data, defaults={}, entity_manager=None):
     if sort_order == "__default__":
         sort_order = None
 
-    # in case there's a valid sort value present (which includes the
-    # "__default__" and the "__identifier__" values) then builds the
-    # order by tuple with both the sort value and the sort order
-    # (which can be unset as None)
-    # if no sort value exists the default order by (that is set above
-    # in the function) is used
-    if sort_value:
-        order_by = ((sort_value, sort_order),)
-
     # tries to retrieve the proper value for the paged element
     # taking into account a possible boolean approach
     if paged_s:
@@ -868,10 +863,19 @@ def resolve(cls, eager, path):
             return None, None
 
         # retrieves the base (name) value from the eager
-        # map and then in case there are no more names
-        # remaining returns this map (end of recursion)
+        # map and the target class of the relation
         map = eager[base]
         target = cls.get_target(base)
+
+        # checks if the target class is a "data reference" and
+        # in case it is, tries to resolve it into the appropriate
+        # concrete (real) class, as done by the entity manager for
+        # the relation, keeping the reference in case it fails
+        if target.is_reference():
+            target = entity_manager.get_entity(target.__name__) or target
+
+        # in case there are no more names remaining
+        # returns this map (end of recursion)
         if not remaining:
             return map, target
 
@@ -915,11 +919,53 @@ def resolve_s(attribute):
         # the target (class) and the top level name of the attribute
         return _filters, target, name
 
+    def sortable(attribute):
+        # in case the attribute is one of the special values that
+        # refer the identifier of the entity it's considered to be
+        # valid for sorting (no resolution is required)
+        if attribute in ("__default__", "__identifier__"):
+            return True
+
+        # splits the attribute (complete) name using the dot based
+        # separator and then retrieves the base (path) value and
+        # the trailing name value
+        path = attribute.rsplit(".")
+        base, name = path[:-1], path[-1]
+
+        # in case the base value is defined a resolution operation
+        # must occur to retrieve the target class, in case the returned
+        # relation is invalid (not eager loaded) the attribute is not
+        # considered to be valid for sorting
+        target = cls
+        if base:
+            relation, target = resolve(cls, eager, base)
+            if relation == None:
+                return False
+
+        # in case the trailing name is a reserved one, it's considered
+        # to be valid according to the entity manager structure
+        if name in RESERVED_NAMES:
+            return True
+
+        # returns if the trailing name exists as an attribute in the
+        # context of the target class (security validation)
+        return target.has_name(name)
+
     # runs the resolution process for the eager sequence, meaning
     # that allowed relations will be set of the eager structure
     if eager_s:
         eager_r(eager_s)
 
+    # in case there's a valid sort value present (which includes the
+    # "__default__" and the "__identifier__" values) then builds the
+    # order by tuple with both the sort value and the sort order
+    # (which can be unset as None), note that the sort value must
+    # refer an attribute of the entity (or of an eager loaded relation)
+    # if no valid sort value exists the default order by (that is set
+    # above in the function) is used
+    if sort_value and sortable(sort_value):
+        order_by = ((sort_value, sort_order),)
+
     # in case the name is defined the "special" wildcard filter
     # is added to the list of filters to be used in the query
     # this is the base value for the search, note that this extra

mvc/src/mvc_utils/mocks.py

@@ -199,3 +199,64 @@ def get_literal_value(self, value):
 
     def _validate_accept_node(self, node, accept_node):
         return accept_node
+
+
+class MockEntityManager(object):
+    def __init__(self, entities=None):
+        self.entities = entities or dict()
+
+    def get_entity(self, entity_name):
+        return self.entities.get(entity_name, None)
+
+    def normalize_options(self, options):
+        eager = options.get("eager", None)
+        if type(eager) in (list, tuple):
+            options["eager"] = dict([(name, dict()) for name in eager])
+        return options
+
+
+class MockEntity(object):
+    data_reference = False
+    _entity_manager = MockEntityManager()
+    _names = ()
+    _targets = dict()
+
+    @classmethod
+    def is_reference(cls):
+        return cls.data_reference
+
+    @classmethod
+    def has_name(cls, name):
+        return name in cls._names
+
+    @classmethod
+    def get_target(cls, relation_name):
+        return cls._targets.get(relation_name, cls)
+
+    @classmethod
+    def _cast_value(cls, name, value):
+        if not name in cls._names:
+            return None
+        return value
+
+
+class MockAddress(MockEntity):
+    _names = ("object_id", "street")
+
+
+class MockAddressReference(MockEntity):
+    data_reference = True
+
+
+class MockPerson(MockEntity):
+    _names = ("object_id", "name", "age", "address")
+    _targets = dict(address=MockAddress)
+
+
+class MockPersonReference(MockEntity):
+    data_reference = True
+
+
+class MockCompany(MockEntity):
+    _names = ("object_id", "name", "address", "owner")
+    _targets = dict(address=MockAddressReference, owner=MockPersonReference)