Mercurial > p > roundup > code
changeset 8126:f7bd22bdef9d permission-performance
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 | Ralf Schlatterbeck <rsc@runtux.com> |
|---|---|
| date | Mon, 21 Oct 2024 18:12:03 +0200 |
| parents | b358da7c89e5 |
| children | 618dccf7199d |
| files | roundup/cgi/templating.py roundup/hyperdb.py roundup/rest.py roundup/xmlrpc.py |
| diffstat | 4 files changed, 62 insertions(+), 46 deletions(-) [+] |
line wrap: on
line diff
--- a/roundup/cgi/templating.py Mon Oct 21 16:11:13 2024 +0200 +++ b/roundup/cgi/templating.py Mon Oct 21 18:12:03 2024 +0200 @@ -3421,14 +3421,13 @@ 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 @@ 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,
--- a/roundup/hyperdb.py Mon Oct 21 16:11:13 2024 +0200 +++ b/roundup/hyperdb.py Mon Oct 21 18:12:03 2024 +0200 @@ -1796,6 +1796,56 @@ # 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.
--- a/roundup/rest.py Mon Oct 21 16:11:13 2024 +0200 +++ b/roundup/rest.py Mon Oct 21 18:12:03 2024 +0200 @@ -953,7 +953,7 @@ 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 @@ 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),
--- a/roundup/xmlrpc.py Mon Oct 21 16:11:13 2024 +0200 +++ b/roundup/xmlrpc.py Mon Oct 21 18:12:03 2024 +0200 @@ -94,15 +94,9 @@ 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)