Move permission check code to hyperdb

Now the hyperdb has a method filter_with_permissions that performs the
permission checks before (for filtering on sort/group/filterspec
arguments) and after a call to hyperdb.filter.

This also fixes possible problems on the unfiltered
sort/group/filterspec arguments in roundup/rest.py and
roundup/cgi/templating.py

Author: schlatterbeck

roundup/cgi/templating.py

@@ -3421,14 +3421,13 @@ def base_javascript(self):
     def batch(self, permission='View'):
         """ Return a batch object for results from the "current search"
         """
-        sec = self._client.db.security
-        check = sec.hasPermission
+        check = self._client.db.security.hasPermission
         userid = self._client.userid
         if not check('Web Access', userid):
             return Batch(self.client, [], self.pagesize, self.startwith,
                          classname=self.classname)
 
-        filterspec = self.filterspec
+        fspec = self.filterspec
         sort = self.sort
         group = self.group
 
@@ -3454,35 +3453,9 @@ def batch(self, permission='View'):
             matches = None
 
         # filter for visibility
-        item_ids = klass.filter(matches, filterspec, sort, group)
-        cn = self.classname
-        if check(permission, userid, cn, only_no_check = True):
-            allowed = item_ids
-        else:
-            # Note that is_filterable returns True if no permissions are
-            # found. This makes it fail early (with an empty allowed list)
-            # instead of running through all ids with an empty
-            # permission list.
-            if sec.is_filterable(permission, userid, cn):
-                new_ids = set(item_ids)
-                confirmed = set()
-                for perm in sec.filter_iter(permission, userid, cn):
-                    fargs = perm.filter(self._client.db, userid, klass)
-                    for farg in fargs:
-                        farg.update(sort=sort, group=group, retired=False)
-                        result = klass.filter(list(new_ids), **farg)
-                        new_ids.difference_update(result)
-                        confirmed.update(result)
-                        # all allowed?
-                        if not new_ids:
-                            break
-                    # all allowed?
-                    if not new_ids:
-                        break
-                allowed = list(confirmed)
-            else:
-                allowed = [id for id in item_ids
-                           if check(permission, userid, cn, itemid=id)]
+        allowed = klass.filter_with_permissions(
+            matches, fspec, sort, group, permission=permission, userid=userid
+        )
 
         # return the batch object, using IDs only
         return Batch(self.client, allowed, self.pagesize, self.startwith,

roundup/hyperdb.py

@@ -1796,6 +1796,56 @@ def filter(self, search_matches, filterspec, sort=[], group=[],
     # anyway).
     filter_iter = filter
 
+    def filter_with_permissions(self, search_matches, filterspec, sort=[],
+                                group=[], retired=False, exact_match_spec={},
+                                limit=None, offset=None,
+                                permission='View', userid=None):
+        """ Do the same as filter but return only the items the user is
+            entitled to see, running the results through security checks.
+            The userid defaults to the current database user.
+        """
+        if userid is None:
+            userid = self.db.getuid()
+        cn = self.classname
+        sec = self.db.security
+        filterspec = sec.filterFilterspec(userid, cn, filterspec)
+        sort = sec.filterSortspec(userid, cn, sort)
+        group = sec.filterSortspec(userid, cn, group)
+        item_ids = self.filter(search_matches, filterspec, sort, group,
+                               retired, exact_match_spec, limit, offset)
+        check = sec.hasPermission
+        if check(permission, userid, cn, only_no_check = True):
+            allowed = item_ids
+        else:
+            # Note that is_filterable returns True if no permissions are
+            # found. This makes it fail early (with an empty allowed list)
+            # instead of running through all ids with an empty
+            # permission list.
+            if sec.is_filterable(permission, userid, cn):
+                new_ids = set(item_ids)
+                confirmed = set()
+                for perm in sec.filter_iter(permission, userid, cn):
+                    fargs = perm.filter(self._client.db, userid, klass)
+                    for farg in fargs:
+                        farg.update(sort=[], group=[], retired=None)
+                        result = klass.filter(list(new_ids), **farg)
+                        new_ids.difference_update(result)
+                        confirmed.update(result)
+                        # all allowed?
+                        if not new_ids:
+                            break
+                    # all allowed?
+                    if not new_ids:
+                        break
+                # Need to sort again in database
+                allowed = self.filter(confirmed, {}, sort=sort, group=group,
+                                      retired=None)
+            else: # Last resort: filter in python
+                allowed = [id for id in item_ids
+                           if check(permission, userid, cn, itemid=id)]
+        return allowed
+
+
     def count(self):
         """Get the number of nodes in this class.
 

roundup/rest.py

@@ -953,7 +953,7 @@ def get_collection(self, class_name, input):
             kw['limit'] = self.max_response_row_size
             if page['index'] is not None and page['index'] > 1:
                 kw['offset'] = (page['index'] - 1) * page['size']
-        obj_list = class_obj.filter(None, *l, **kw)
+        obj_list = class_obj.filter_with_permissions(None, *l, **kw)
 
         # Have we hit the max number of returned rows?
         # If so there may be more data that the client
@@ -973,10 +973,9 @@ def get_collection(self, class_name, input):
         result['collection'] = []
         for item_id in obj_list:
             r = {}
-            if self.db.security.hasPermission(
-                'View', uid, class_name, itemid=item_id, property='id',
-            ):
-                r = {'id': item_id, 'link': class_path + item_id}
+            # No need to check permission on id here, as we have only
+            # security-checked results
+            r = {'id': item_id, 'link': class_path + item_id}
             if display_props:
                 # format_item does the permission checks
                 r.update(self.format_item(class_obj.getnode(item_id),

roundup/xmlrpc.py

@@ -94,15 +94,9 @@ def list(self, classname, propname=None):
     def filter(self, classname, search_matches, filterspec,
                sort=[], group=[]):
         cl = self.db.getclass(classname)
-        uid = self.db.getuid()
-        security = self.db.security
-        filterspec = security.filterFilterspec(uid, classname, filterspec)
-        sort = security.filterSortspec(uid, classname, sort)
-        group = security.filterSortspec(uid, classname, group)
-        result = cl.filter(search_matches, filterspec, sort=sort, group=group)
-        check = security.hasPermission
-        x = [id for id in result if check('View', uid, classname, itemid=id)]
-        return x
+        return cl.filter_with_permissions(
+            search_matches, filterspec, sort=sort, group=group
+        )
 
     def lookup(self, classname, key):
         cl = self.db.getclass(classname)