Allow transitive properties in @fields in REST API

These transitive properties may not cross Multilinks, e.g., when
querying 'issue' the property 'messages.author' is not allowed (because
'messages' is a multilink). A multilink at the end (e.g. messages in the
example) is fine.

Author: schlatterbeck

CHANGES.txt

@@ -53,6 +53,11 @@ Features:
   request handler from the request displatcher. Also allow
   customisation of tracker instance creation via an overridable
   "get_tracker" context manager.
+- Allow transitive properties in @fields in REST API. These transitive
+  properties may not cross Multilinks, e.g., when querying 'issue' the
+  property 'messages.author' is not allowed (because 'messages' is a
+  multilink). A multilink at the end (e.g. messages in the example) is
+  fine.
 
 Fixed:
 

doc/rest.txt

@@ -508,21 +508,25 @@ In addition collections support the ``@fields`` parameter which is a
 colon or comma separated list of fields to embed in the response. For
 example ``https://.../rest/data/issue?@verbose=2`` is the same as:
 ``https://.../rest/data/issue?@fields=title`` since the label property
-for an issue is its title. You can use both ``@verbose`` and
+for an issue is its title.
+The @fields option supports transitive properties, e.g.
+``status.name``. The transitive property may not include multilinks in
+the path except for the last component. So ``messages.author`` is not
+allowed because ``messages`` is a multilink while ``messages`` alone
+would be allowed.  You can use both ``@verbose`` and
 ``@fields`` to get additional info. For example
 ``https://.../rest/data/issue?@verbose=2&@fields=status`` returns::
 
+
   {
       "data": {
 	  "collection": [
 	      {
-		  "link":
-      "https://.../rest/data/issue/1",
+		  "link": "https://.../rest/data/issue/1",
 		  "title": "Welcome to the tracker START HERE",
 		  "id": "1",
 		  "status": {
-		      "link":
-      "https://.../rest/data/status/1",
+		      "link": "https://.../rest/data/status/1",
 		      "id": "1",
 		      "name": "new"
 		  }
@@ -841,7 +845,8 @@ By default all (visible to the current user) attributes/properties are
 returned. You can limit this by using the ``@fields`` query parameter
 similar to how it is used in collections. This way you can only return
 the fields you are interested in reducing network load as well as
-memory and parsing time on the client side. By default protected
+memory and parsing time on the client side. Or you can add additional
+transitive properties.  By default protected
 properties (read only in the database) are not listed. This
 makes it easier to submit the attributes from a
 ``@verbose=0`` query using PUT. To include protected properties

roundup/rest.py

@@ -470,6 +470,45 @@ def prop_from_arg(self, cl, key, value, itemid=None):
 
         return prop
 
+    def transitive_props (self, class_name, props):
+        """Construct a list of transitive properties from the given
+        argument, and return it after permission check. Raises
+        Unauthorised if no permission. Permission is checked by checking
+        search-permission on the path (delimited by '.') excluding the
+        last component and then checking View permission on the last
+        component. We do not allow to traverse multilinks -- the last
+        item of an expansion *may* be a multilink but in the middle of a
+        transitive prop.
+        """
+        checked_props = []
+        uid = self.db.getuid()
+        for p in props:
+            pn = p
+            cn = class_name
+            if '.' in p:
+                path, lc = p.rsplit('.', 1)
+                if not self.db.security.hasSearchPermission(uid, class_name, p):
+                    raise (Unauthorised
+                        ('User does not have permission on "%s.%s"'
+                        % (class_name, p)))
+                prev = prop = None
+                # This shouldn't raise any errors, otherwise the search
+                # permission check above would have failed
+                for pn in path.split('.'):
+                    cls = self.db.getclass(cn)
+                    prop = cls.getprops(protected=True)[pn]
+                    if isinstance(prop, hyperdb.Multilink):
+                        raise UsageError(
+                            'Multilink Traversal not allowed: %s' % p)
+                    cn = prop.classname
+                cls = self.db.getclass(cn)
+            # Now we have the classname in cn and the prop name in pn.
+            if not self.db.security.hasPermission('View', uid, cn, pn):
+                raise(Unauthorised
+                    ('User does not have permission on "%s.%s"' % (cn, pn)))
+            checked_props.append (p)
+        return checked_props
+
     def error_obj(self, status, msg, source=None):
         """Return an error object"""
         self.client.response_code = status
@@ -567,12 +606,24 @@ def format_item(self, node, item_id, props=None, verbose=1):
         try:
             # pn = propname
             for pn in sorted(props):
-                prop = props[pn]
-                if not self.db.security.hasPermission(
-                        'View', uid, class_name, pn, item_id
-                ):
+                ok = False
+                id = item_id
+                nd = node
+                cn = class_name
+                for p in pn.split('.'):
+                    if not self.db.security.hasPermission(
+                            'View', uid, cn, p, id
+                    ):
+                        break
+                    cl = self.db.getclass(cn)
+                    nd = cl.getnode(id)
+                    id = v = getattr(nd, p)
+                    prop = cl.getprops(protected=True)[p]
+                    cn = getattr(prop, 'classname', None)
+                else:
+                    ok = True
+                if not ok:
                     continue
-                v = getattr(node, pn)
                 if isinstance(prop, (hyperdb.Link, hyperdb.Multilink)):
                     linkcls = self.db.getclass(prop.classname)
                     cp = '%s/%s/' % (self.data_path, prop.classname)
@@ -654,7 +705,7 @@ def get_collection(self, class_name, input):
             'index': 1   # setting just size starts at page 1
         }
         verbose = 1
-        display_props = {}
+        display_props = set()
         sort = []
         for form_field in input.value:
             key = form_field.name
@@ -670,12 +721,7 @@ def get_collection(self, class_name, input):
                 if len(f) == 1:
                     f = value.split(":")
                 allprops = class_obj.getprops(protected=True)
-                for i in f:
-                    try:
-                        display_props[i] = allprops[i]
-                    except KeyError:
-                        raise UsageError("Failed to find property '%s' "
-                                         "for class %s." % (i, class_name))
+                display_props.update(self.transitive_props(class_name, f))
             elif key == "@sort":
                 f = value.split(",")
                 allprops = class_obj.getprops(protected=True)
@@ -780,8 +826,7 @@ def get_collection(self, class_name, input):
         # add verbose elements. 2 and above get identifying label.
         if verbose > 1:
             lp = class_obj.labelprop()
-            # Label prop may be a protected property like activity
-            display_props[lp] = class_obj.getprops(protected=True)[lp]
+            display_props.add(lp)
 
         # extract result from data
         result = {}
@@ -891,17 +936,13 @@ def get_element(self, class_name, item_id, input):
             value = form_field.value
             if key == "@fields" or key == "@attrs":
                 if props is None:
-                    props = {}
+                    props = set()
                 # support , or : separated elements
                 f = value.split(",")
                 if len(f) == 1:
                     f = value.split(":")
                 allprops = class_obj.getprops(protected=True)
-                for i in f:
-                    try:
-                        props[i] = allprops[i]
-                    except KeyError:
-                        raise UsageError("Failed to find property '%s' for class %s." % (i, class_name))
+                props.update(self.transitive_props(class_name, f))
             elif key == "@protected":
                 # allow client to request read only
                 # properties like creator, activity etc.
@@ -912,11 +953,11 @@ def get_element(self, class_name, item_id, input):
 
         result = {}
         if props is None:
-            props = class_obj.getprops(protected=protected)
+            props = set(class_obj.getprops(protected=protected))
         else:
             if verbose > 1:
                 lp = class_obj.labelprop()
-                props[lp] = class_obj.properties[lp]
+                props.add(lp)
 
         result = {
             'id': itemid,

test/rest_common.py

@@ -1,3 +1,4 @@
+import pytest
 import unittest
 import os
 import shutil
@@ -666,6 +667,28 @@ def testSorting(self):
         results = self.server.get_collection('issue', form)
         self.assertDictEqual(expected, results)
 
+    def testTransitiveField(self):
+        """ Test a transitive property in @fields """
+        base_path = self.db.config['TRACKER_WEB'] + 'rest/data/'
+        # create sample data
+        self.create_stati()
+        self.db.issue.create(
+            title='foo4',
+            status=self.db.status.lookup('closed'),
+            priority=self.db.priority.lookup('critical')
+        )
+        # Retrieve all issue @fields=status.name
+        form = cgi.FieldStorage()
+        form.list = [
+            cgi.MiniFieldStorage('@fields', 'status.name')
+        ]
+        results = self.server.get_collection('issue', form)
+        self.assertEqual(self.dummy_client.response_code, 200)
+
+        exp = [
+            {'link': base_path + 'issue/1', 'id': '1', 'status.name': 'closed'}]
+        self.assertEqual(results['data']['collection'], exp)
+
     def testFilter(self):
         """
         Retrieve all three users