Mercurial > p > roundup > code
changeset 8119:c12377fb4144 permission-performance
Change permission representation
Now permissions are checked in different order. Permissions without a
check method (which are cheap to check) are checked first. Only if no
permission is found do we check permissions with check methods.
| author | Ralf Schlatterbeck <rsc@runtux.com> |
|---|---|
| date | Fri, 18 Oct 2024 16:52:42 +0200 |
| parents | c88ad93f0e77 |
| children | d4fa7a9c3a21 |
| files | roundup/admin.py roundup/security.py test/db_test_base.py test/test_admin.py test/test_mailgw.py |
| diffstat | 5 files changed, 160 insertions(+), 90 deletions(-) [+] |
line wrap: on
line diff
--- a/roundup/admin.py Thu Oct 17 19:13:26 2024 -0400 +++ b/roundup/admin.py Fri Oct 18 16:52:42 2024 +0200 @@ -1920,7 +1920,7 @@ roles.sort() for _rolename, role in roles: sys.stdout.write(_('Role "%(name)s":\n') % role.__dict__) - for permission in role.permissions: + for permission in role.permission_list(): d = permission.__dict__ if permission.klass: if permission.properties:
--- a/roundup/security.py Thu Oct 17 19:13:26 2024 -0400 +++ b/roundup/security.py Fri Oct 18 16:52:42 2024 +0200 @@ -184,12 +184,73 @@ def __init__(self, name='', description='', permissions=None): self.name = name.lower() self.description = description + # This is a dict of permission names each containing a dict of + # *class names*, with a special entry for non-class permissions + # where the key is None. In each class dict we have a dictionary + # with the values True and False for permission with and without + # a check method. These dicts each contain a list of permissions. if permissions is None: - permissions = [] - self.permissions = permissions + self._permissions = {} + elif isinstance(permissions, list): + self._permissions = {} + for p in permissions: + self.addPermission(p) + else: + raise ValueError("Invalid permissions for Role: %s" % permissions) def __repr__(self): - return '<Role 0x%x %r,%r>' % (id(self), self.name, self.permissions) + pl = self.permission_list() + return '<Role 0x%x %r,%r>' % (id(self), self.name, pl) + + def addPermission (self, *permissions): + for p in permissions: + pn = p.name + self._permissions.setdefault(pn, {}) + cn = p.klass + if p.klass not in self._permissions[pn]: + self._permissions[pn][cn] = dict (((False, []), (True, []))) + self._permissions[pn][cn][bool(p.check)].append(p) + + def hasPermission (self, db, perm, uid, classname, property, itemid, chk): + # if itemid is given a classname must, too, checked in caller + assert not itemid or classname + perms = self._permissions + if perm not in perms: + return False + # If we have a classname we also need to check permission with + # an empty classname (e.g. 'admin' has access on everything) + if classname is not None and None in perms[perm]: + for p in perms[perm][None][chk]: + # permission match? + if p.test(db, perm, classname, property, uid, itemid): + return True + if classname not in perms[perm]: + return False + for p in perms[perm][classname][chk]: + # permission match? + if p.test(db, perm, classname, property, uid, itemid): + return True + + def permission_list (self): + """ Used for reporting in admin tool """ + l = [] + for p in self._permissions: + for c in self._permissions[p]: + for cond in (False, True): + l.extend (self._permissions[p][c][cond]) + l.sort (key = lambda x: (x.klass or '', x.name)) + return l + + def searchable (self, classname, propname): + for perm in 'View', 'Search': + # Only permissions without a check method + if perm not in self._permissions: + continue + if classname not in self._permissions[perm]: + continue + for p in self._permissions[perm][classname][False]: + if p.searchable(classname, propname): + return True class Security: @@ -199,7 +260,7 @@ ''' self.db = weakref.proxy(db) # use a weak ref to avoid circularity - # permssions are mapped by name to a list of Permissions by class + # Permissions are mapped by name to a list of Permissions by class self.permission = {} # roles are mapped by name to the Role @@ -278,28 +339,30 @@ Permission.test() method. ''' + if classname == 'status': + import pdb; pdb.set_trace() if itemid and classname is None: raise ValueError('classname must accompany itemid') - for rolename in self.db.user.get_roles(userid): - if not rolename or (rolename not in self.role): - continue - # for each of the user's Roles, check the permissions - for perm in self.role[rolename].permissions: - # permission match? - if perm.test(self.db, permission, classname, property, - userid, itemid): - return 1 - return 0 + # for each of the user's Roles, check the permissions + # Note that checks with a check method are typically a lot more + # expensive than the ones without. So we check the ones without + # a check method first + for has_check in False, True: + for rolename in self.db.user.get_roles(userid): + if not rolename or (rolename not in self.role): + continue + r = self.role[rolename] + v = r.hasPermission(self.db, permission, userid, classname, + property, itemid, has_check) + if v: + return v + return False def roleHasSearchPermission(self, classname, property, *rolenames): """ For each of the given roles, check the permissions. Property can be a transitive property. """ perms = [] - # pre-compute permissions - for rn in rolenames: - for perm in self.role[rn].permissions: - perms.append(perm) # Note: break from inner loop means "found" # break from outer loop means "not found" cn = classname @@ -319,8 +382,8 @@ prop = cls.getprops()[propname] except KeyError: break - for perm in perms: - if perm.searchable(cn, propname): + for rn in rolenames: + if self.role[rn].searchable(cn, propname): break else: break @@ -334,8 +397,8 @@ return 0 props = dict.fromkeys(('id', cls.labelprop(), cls.orderprop())) for p in props.keys(): - for perm in perms: - if perm.searchable(prop.classname, p): + for rn in rolenames: + if self.role[rn].searchable(prop.classname, p): break else: return 0 @@ -409,7 +472,7 @@ permission = self.getPermission(permission, classname, properties, check, props_only) role = self.role[rolename.lower()] - role.permissions.append(permission) + role.addPermission(permission) # Convenience methods for removing non-allowed properties from a # filterspec or sort/group list
--- a/test/db_test_base.py Thu Oct 17 19:13:26 2024 -0400 +++ b/test/db_test_base.py Fri Oct 18 16:52:42 2024 +0200 @@ -3324,14 +3324,14 @@ 'Role "admin":\n', ' User may create everything (Create)\n', ' User may edit everything (Edit)\n', + ' User may use the email interface (Email Access)\n', + ' User may access the rest interface (Rest Access)\n', ' User may restore everything (Restore)\n', ' User may retire everything (Retire)\n', ' User may view everything (View)\n', ' User may access the web interface (Web Access)\n', - ' User may access the rest interface (Rest Access)\n', + ' User may manipulate user Roles through the web (Web Roles)\n', ' User may access the xmlrpc interface (Xmlrpc Access)\n', - ' User may manipulate user Roles through the web (Web Roles)\n', - ' User may use the email interface (Email Access)\n', 'Role "anonymous":\n', 'Role "user":\n', ' User is allowed to access msg (View for "msg" only)\n', ' Prevent users from seeing roles (View for "user": [\'username\', \'supervisor\', \'assignable\'] only)\n']
--- a/test/test_admin.py Thu Oct 17 19:13:26 2024 -0400 +++ b/test/test_admin.py Fri Oct 18 16:52:42 2024 +0200 @@ -1420,33 +1420,33 @@ ret = self.admin.main() result = """Role "user": - User may access the web interface (Web Access) User may use the email interface (Email Access) User may access the rest interface (Rest Access) + User may access the web interface (Web Access) User may access the xmlrpc interface (Xmlrpc Access) - User is allowed to access issue (View for "issue" only) - User is allowed to edit issue (Edit for "issue" only) + User is allowed to create file (Create for "file" only) + User is allowed to edit file (Edit for "file" only) + User is allowed to access file (View for "file" only) User is allowed to create issue (Create for "issue" only) - User is allowed to access file (View for "file" only) - User is allowed to edit file (Edit for "file" only) - User is allowed to create file (Create for "file" only) + User is allowed to edit issue (Edit for "issue" only) + User is allowed to access issue (View for "issue" only) + User is allowed to create keyword (Create for "keyword" only) + User is allowed to edit keyword (Edit for "keyword" only) + User is allowed to access keyword (View for "keyword" only) + User is allowed to create msg (Create for "msg" only) + User is allowed to edit msg (Edit for "msg" only) User is allowed to access msg (View for "msg" only) - User is allowed to edit msg (Edit for "msg" only) - User is allowed to create msg (Create for "msg" only) - User is allowed to access keyword (View for "keyword" only) - User is allowed to edit keyword (Edit for "keyword" only) - User is allowed to create keyword (Create for "keyword" only) User is allowed to access priority (View for "priority" only) + User is allowed to create queries (Create for "query" only) + User is allowed to edit their queries (Edit for "query" only) + User is allowed to restore their queries (Restore for "query" only) + User is allowed to retire their queries (Retire for "query" only) + (Search for "query" only) + User is allowed to view their own and public queries (View for "query" only) User is allowed to access status (View for "status" only) + User is allowed to edit their own user details (Edit for "user": ('username', 'password', 'address', 'realname', 'phone', 'organisation', 'alternate_addresses', 'queries', 'timezone') only) (View for "user": ('id', 'organisation', 'phone', 'realname', 'timezone', 'username') only) User is allowed to view their own user details (View for "user" only) - User is allowed to edit their own user details (Edit for "user": ('username', 'password', 'address', 'realname', 'phone', 'organisation', 'alternate_addresses', 'queries', 'timezone') only) - User is allowed to view their own and public queries (View for "query" only) - (Search for "query" only) - User is allowed to edit their queries (Edit for "query" only) - User is allowed to retire their queries (Retire for "query" only) - User is allowed to restore their queries (Restore for "query" only) - User is allowed to create queries (Create for "query" only) """ print(out.getvalue()) @@ -1496,52 +1496,52 @@ Role "admin": User may create everything (Create) User may edit everything (Edit) + User may use the email interface (Email Access) + User may access the rest interface (Rest Access) User may restore everything (Restore) User may retire everything (Retire) User may view everything (View) User may access the web interface (Web Access) - User may access the rest interface (Rest Access) + User may manipulate user Roles through the web (Web Roles) User may access the xmlrpc interface (Xmlrpc Access) - User may manipulate user Roles through the web (Web Roles) - User may use the email interface (Email Access) Role "anonymous": User may access the web interface (Web Access) - User is allowed to register new user (Register for "user" only) + User is allowed to access file (View for "file" only) User is allowed to access issue (View for "issue" only) - User is allowed to access file (View for "file" only) + User is allowed to access keyword (View for "keyword" only) User is allowed to access msg (View for "msg" only) - User is allowed to access keyword (View for "keyword" only) User is allowed to access priority (View for "priority" only) User is allowed to access status (View for "status" only) + User is allowed to register new user (Register for "user" only) (Search for "user" only) Role "user": - User may access the web interface (Web Access) User may use the email interface (Email Access) User may access the rest interface (Rest Access) + User may access the web interface (Web Access) User may access the xmlrpc interface (Xmlrpc Access) - User is allowed to access issue (View for "issue" only) - User is allowed to edit issue (Edit for "issue" only) + User is allowed to create file (Create for "file" only) + User is allowed to edit file (Edit for "file" only) + User is allowed to access file (View for "file" only) User is allowed to create issue (Create for "issue" only) - User is allowed to access file (View for "file" only) - User is allowed to edit file (Edit for "file" only) - User is allowed to create file (Create for "file" only) + User is allowed to edit issue (Edit for "issue" only) + User is allowed to access issue (View for "issue" only) + User is allowed to create keyword (Create for "keyword" only) + User is allowed to edit keyword (Edit for "keyword" only) + User is allowed to access keyword (View for "keyword" only) + User is allowed to create msg (Create for "msg" only) + User is allowed to edit msg (Edit for "msg" only) User is allowed to access msg (View for "msg" only) - User is allowed to edit msg (Edit for "msg" only) - User is allowed to create msg (Create for "msg" only) - User is allowed to access keyword (View for "keyword" only) - User is allowed to edit keyword (Edit for "keyword" only) - User is allowed to create keyword (Create for "keyword" only) User is allowed to access priority (View for "priority" only) + User is allowed to create queries (Create for "query" only) + User is allowed to edit their queries (Edit for "query" only) + User is allowed to restore their queries (Restore for "query" only) + User is allowed to retire their queries (Retire for "query" only) + (Search for "query" only) + User is allowed to view their own and public queries (View for "query" only) User is allowed to access status (View for "status" only) + User is allowed to edit their own user details (Edit for "user": ('username', 'password', 'address', 'realname', 'phone', 'organisation', 'alternate_addresses', 'queries', 'timezone') only) (View for "user": ('id', 'organisation', 'phone', 'realname', 'timezone', 'username') only) User is allowed to view their own user details (View for "user" only) - User is allowed to edit their own user details (Edit for "user": ('username', 'password', 'address', 'realname', 'phone', 'organisation', 'alternate_addresses', 'queries', 'timezone') only) - User is allowed to view their own and public queries (View for "query" only) - (Search for "query" only) - User is allowed to edit their queries (Edit for "query" only) - User is allowed to retire their queries (Retire for "query" only) - User is allowed to restore their queries (Restore for "query" only) - User is allowed to create queries (Create for "query" only) """ print(out.getvalue()) @@ -1579,43 +1579,50 @@ Role "admin": User may create everything (Create) User may edit everything (Edit) + User may use the email interface (Email Access) + User may access the rest interface (Rest Access) User may restore everything (Restore) User may retire everything (Retire) User may view everything (View) User may access the web interface (Web Access) - User may access the rest interface (Rest Access) + User may manipulate user Roles through the web (Web Roles) User may access the xmlrpc interface (Xmlrpc Access) - User may manipulate user Roles through the web (Web Roles) - User may use the email interface (Email Access) Role "anonymous": User may access the web interface (Web Access) - User is allowed to register new user (Register for "user" only) + User is allowed to access file (View for "file" only) User is allowed to access issue (View for "issue" only) - User is allowed to access file (View for "file" only) + User is allowed to access keyword (View for "keyword" only) User is allowed to access msg (View for "msg" only) - User is allowed to access keyword (View for "keyword" only) User is allowed to access priority (View for "priority" only) User is allowed to access status (View for "status" only) + User is allowed to register new user (Register for "user" only) (Search for "user" only) Role "user": - User may access the web interface (Web Access) User may use the email interface (Email Access) User may access the rest interface (Rest Access) + User may access the web interface (Web Access) User may access the xmlrpc interface (Xmlrpc Access) - User is allowed to access issue (View for "issue" only) - User is allowed to edit issue (Edit for "issue" only) + User is allowed to create file (Create for "file" only) + User is allowed to edit file (Edit for "file" only) + User is allowed to access file (View for "file" only) User is allowed to create issue (Create for "issue" only) - User is allowed to access file (View for "file" only) - User is allowed to edit file (Edit for "file" only) - User is allowed to create file (Create for "file" only) + User is allowed to edit issue (Edit for "issue" only) + User is allowed to access issue (View for "issue" only) + User is allowed to create keyword (Create for "keyword" only) + User is allowed to edit keyword (Edit for "keyword" only) + User is allowed to access keyword (View for "keyword" only) + User is allowed to create msg (Create for "msg" only) + User is allowed to edit msg (Edit for "msg" only) User is allowed to access msg (View for "msg" only) - User is allowed to edit msg (Edit for "msg" only) - User is allowed to create msg (Create for "msg" only) - User is allowed to access keyword (View for "keyword" only) - User is allowed to edit keyword (Edit for "keyword" only) - User is allowed to create keyword (Create for "keyword" only) User is allowed to access priority (View for "priority" only) + User is allowed to create queries (Create for "query" only) + User is allowed to edit their queries (Edit for "query" only) + User is allowed to restore their queries (Restore for "query" only) + User is allowed to retire their queries (Retire for "query" only) + (Search for "query" only) + User is allowed to view their own and public queries (View for "query" only) User is allowed to access status (View for "status" only) + User is allowed to edit their own user details (Edit for "user": ('username', 'password', 'address', 'realname', 'phone', 'organisation', 'alternate_addresses', 'queries', 'timezone') only) (View for "user": ('id', 'organization', 'phone', 'realname', 'timezone', 'username') only) **Invalid properties for user: ['organization']
--- a/test/test_mailgw.py Thu Oct 17 19:13:26 2024 -0400 +++ b/test/test_mailgw.py Fri Oct 18 16:52:42 2024 +0200 @@ -241,7 +241,7 @@ self.db.security.getPermission('Create', 'issue'), self.db.security.getPermission('Create', 'msg'), ] - self.db.security.role['anonymous'].permissions = p + self.db.security.role['anonymous'].addPermission(*p) def _create_mailgw(self, message, args=()): class MailGW(self.instance.MailGW): @@ -2867,7 +2867,7 @@ This is a test submission of a new issue. ''' - self.db.security.role['anonymous'].permissions=[] + self.db.security.role['anonymous']._permissions={} anonid = self.db.user.lookup('anonymous') self.db.user.set(anonid, roles='Anonymous') try: @@ -2888,7 +2888,7 @@ self.db.security.getPermission('Register', 'user'), self.db.security.getPermission('Web Access', None), ] - self.db.security.role['anonymous'].permissions=p + self.db.security.role['anonymous'].addPermission(*p) try: self._handle_mail(message) except Unauthorized as value: @@ -2915,7 +2915,7 @@ self.db.security.getPermission('Register', 'user'), self.db.security.getPermission('Email Access', None), ] - self.db.security.role['anonymous'].permissions=p + self.db.security.role['anonymous'].addPermission(*p) self._handle_mail(message) m = self.db.user.list() m.sort()