From 80cd6f610ee95dbd3f49638ec2ebf3f16e3587c0 Mon Sep 17 00:00:00 2001 From: schlatterbeck Date: Thu, 21 Oct 2010 08:59:43 +0000 Subject: [PATCH] more fixes to search permissions: - require that for links and multilinks the searching user has access to at least the orderprop, labelprop, and ID of the linked class - allow combinations of roles: we previosly required that for transitive properties all elements where searchable by the same role. We now allow that the roles can be different for each property. This allows assigning different roles to different sub-systems and allowing users having all required roles to search across subsystems. - regression test updated - fix doc/upgrading example for new signature of roleHasSearchPermission git-svn-id: http://svn.roundup-tracker.org/svnroot/roundup/roundup/trunk@4553 57a73879-2fb5-44c3-a270-3262357dd7e2 --- doc/upgrading.txt | 2 +- roundup/security.py | 57 +++++++++++++++++++++++++++++-------------- test/test_security.py | 38 +++++++++++++++++++++++++---- 3 files changed, 73 insertions(+), 24 deletions(-) diff --git a/doc/upgrading.txt b/doc/upgrading.txt index acdef46..43051a1 100644 --- a/doc/upgrading.txt +++ b/doc/upgrading.txt @@ -47,7 +47,7 @@ search for this property:: print " Property:", p roles = [] for role in sorted(db.security.role.iterkeys()): - if db.security.roleHasSearchPermission(role,cl,p): + if db.security.roleHasSearchPermission(cl,p,role): roles.append(role) print " roles may search:", ', '.join(roles) diff --git a/roundup/security.py b/roundup/security.py index e46e291..e3841ba 100644 --- a/roundup/security.py +++ b/roundup/security.py @@ -197,29 +197,54 @@ class Security: return 1 return 0 - def roleHasSearchPermission(self, rolename, classname, property): - """ For each of the user's Roles, check the permissions. + def roleHasSearchPermission(self, classname, property, *rolenames): + """ For each of the given roles, check the permissions. Property can be a transitive property. """ - cn = classname - last = None + 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 + prev = None + prop = None + Link = hyperdb.Link + Multilink = hyperdb.Multilink for propname in property.split('.'): - if last: + if prev: try: - cls = self.db.getclass(cn) - lprop = cls.getprops()[last] - except KeyError: + cn = prop.classname + except AttributeError: break - cn = lprop.classname - last = propname - for perm in self.role[rolename].permissions: + prev = propname + try: + cls = self.db.getclass(cn) + prop = cls.getprops()[propname] + except KeyError: + break + for perm in perms: if perm.searchable(cn, propname): break else: break else: + # for Link and Multilink require search permission on label- + # and order-properties and on ID + if isinstance(prop, Multilink) or isinstance(prop, Link): + try: + cls = self.db.getclass(prop.classname) + except KeyError: + return 0 + props = dict.fromkeys(('id', cls.labelprop(), cls.orderprop())) + for p in props.iterkeys(): + for perm in perms: + if perm.searchable(prop.classname, p): + break + else: + return 0 return 1 return 0 @@ -243,13 +268,9 @@ class Security: either no properties listed or the property must appear in the list. ''' - for rolename in self.db.user.get_roles(userid): - if not rolename or not self.role.has_key(rolename): - continue - # for each of the user's Roles, check the permissions - if self.roleHasSearchPermission (rolename, classname, property): - return 1 - return 0 + roles = [r for r in self.db.user.get_roles(userid) + if r and self.role.has_key(r)] + return self.roleHasSearchPermission (classname, property, *roles) def addPermission(self, **propspec): ''' Create a new Permission with the properties defined in diff --git a/test/test_security.py b/test/test_security.py index 595dad3..c7d5128 100644 --- a/test/test_security.py +++ b/test/test_security.py @@ -183,21 +183,31 @@ class PermissionTest(MyTestCase): has = self.db.security.hasSearchPermission addRole = self.db.security.addRole addToRole = self.db.security.addPermissionToRole - user = self.db.user.create(username='user1', roles='User') - anon = self.db.user.create(username='anonymous', roles='Anonymous') addRole(name='User') addRole(name='Anonymous') + addRole(name='Issue') + addRole(name='Msg') + addRole(name='UV') + user = self.db.user.create(username='user1', roles='User') + anon = self.db.user.create(username='anonymous', roles='Anonymous') + ui = self.db.user.create(username='user2', roles='Issue') + uim = self.db.user.create(username='user3', roles='Issue,Msg') + uimu = self.db.user.create(username='user4', roles='Issue,Msg,UV') iv = add(name="View", klass="issue") addToRole('User', iv) addToRole('Anonymous', iv) + addToRole('Issue', iv) ms = add(name="Search", klass="msg") addToRole('User', ms) addToRole('Anonymous', ms) - addToRole('User', add(name="View", klass="user")) + addToRole('Msg', ms) + uv = add(name="View", klass="user") + addToRole('User', uv) + addToRole('UV', uv) self.assertEquals(has(anon, 'issue', 'messages'), 1) - self.assertEquals(has(anon, 'issue', 'messages.author'), 1) + self.assertEquals(has(anon, 'issue', 'messages.author'), 0) self.assertEquals(has(anon, 'issue', 'messages.author.username'), 0) - self.assertEquals(has(anon, 'issue', 'messages.recipients'), 1) + self.assertEquals(has(anon, 'issue', 'messages.recipients'), 0) self.assertEquals(has(anon, 'issue', 'messages.recipients.username'), 0) self.assertEquals(has(user, 'issue', 'messages'), 1) self.assertEquals(has(user, 'issue', 'messages.author'), 1) @@ -205,6 +215,24 @@ class PermissionTest(MyTestCase): self.assertEquals(has(user, 'issue', 'messages.recipients'), 1) self.assertEquals(has(user, 'issue', 'messages.recipients.username'), 1) + self.assertEquals(has(ui, 'issue', 'messages'), 0) + self.assertEquals(has(ui, 'issue', 'messages.author'), 0) + self.assertEquals(has(ui, 'issue', 'messages.author.username'), 0) + self.assertEquals(has(ui, 'issue', 'messages.recipients'), 0) + self.assertEquals(has(ui, 'issue', 'messages.recipients.username'), 0) + + self.assertEquals(has(uim, 'issue', 'messages'), 1) + self.assertEquals(has(uim, 'issue', 'messages.author'), 0) + self.assertEquals(has(uim, 'issue', 'messages.author.username'), 0) + self.assertEquals(has(uim, 'issue', 'messages.recipients'), 0) + self.assertEquals(has(uim, 'issue', 'messages.recipients.username'), 0) + + self.assertEquals(has(uimu, 'issue', 'messages'), 1) + self.assertEquals(has(uimu, 'issue', 'messages.author'), 1) + self.assertEquals(has(uimu, 'issue', 'messages.author.username'), 1) + self.assertEquals(has(uimu, 'issue', 'messages.recipients'), 1) + self.assertEquals(has(uimu, 'issue', 'messages.recipients.username'), 1) + def test_suite(): suite = unittest.TestSuite() suite.addTest(unittest.makeSuite(PermissionTest)) -- 2.30.2