From e4e6bd39dd9b34aad8ee06bade09376ea1683846 Mon Sep 17 00:00:00 2001 From: schlatterbeck Date: Tue, 15 Dec 2009 15:11:27 +0000 Subject: [PATCH] Clean up all the places where role processing occurs. This is now in a central place in hyperdb.Class and is used consistently throughout. This also means now a template can override the way role processing occurs (e.g. for elaborate permission schemes). Thanks to intevation for funding the change. Note: On first glance the hyperdb.Class may not be the ideal place for role processing. On second thought: Roles may appear in other classes, too (e.g., a user_group or similar) which then don't need to reinvent the wheel. And I didn't want to introduce a separate UserClass (as is the case for the HTML classes) due to compatibility issues with existing schema.py out there. git-svn-id: http://svn.roundup-tracker.org/svnroot/roundup/roundup/trunk@4410 57a73879-2fb5-44c3-a270-3262357dd7e2 --- CHANGES.txt | 5 +++++ roundup/cgi/templating.py | 9 +++------ roundup/hyperdb.py | 39 +++++++++++++++++++++++++++++++++++++++ roundup/mailgw.py | 23 +++-------------------- roundup/security.py | 5 +---- test/test_cgi.py | 18 ++++++++++++++++++ 6 files changed, 69 insertions(+), 30 deletions(-) diff --git a/CHANGES.txt b/CHANGES.txt index 7c26ad4..57afd30 100644 --- a/CHANGES.txt +++ b/CHANGES.txt @@ -41,6 +41,11 @@ Fixes: would trigger a traceback about an unbound variable. Add new regression test for this case. May be related to (now closed) issue1177477. Thanks to Intevation for funding the fix. +- Clean up all the places where role processing occurs. This is now in a + central place in hyperdb.Class and is used consistently throughout. + This also means now a template can override the way role processing + occurs (e.g. for elaborate permission schemes). Thanks to intevation + for funding the change. 2009-10-09 1.4.10 (r4374) diff --git a/roundup/cgi/templating.py b/roundup/cgi/templating.py index 6f0584f..fad7952 100644 --- a/roundup/cgi/templating.py +++ b/roundup/cgi/templating.py @@ -1205,12 +1205,9 @@ class _HTMLUser(_HTMLItem): return self._db.security.hasPermission(permission, self._nodeid, classname, property, itemid) - def hasRole(self, rolename): - """Determine whether the user has the Role.""" - roles = self._db.user.get(self._nodeid, 'roles').split(',') - for role in roles: - if role.strip() == rolename: return True - return False + def hasRole(self, *rolenames): + """Determine whether the user has any role in rolenames.""" + return self._db.user.has_role(self._nodeid, *rolenames) def HTMLItem(client, classname, nodeid, anonymous=0): if classname == 'user': diff --git a/roundup/hyperdb.py b/roundup/hyperdb.py index 91a9fef..8825eb7 100644 --- a/roundup/hyperdb.py +++ b/roundup/hyperdb.py @@ -760,6 +760,16 @@ All methods except __repr__ must be implemented by a concrete backend Database. """ +def iter_roles(roles): + ''' handle the text processing of turning the roles list + into something python can use more easily + ''' + if not roles or not roles.strip(): + raise StopIteration, "Empty roles given" + for role in [x.lower().strip() for x in roles.split(',')]: + yield role + + # # The base Class class # @@ -1227,6 +1237,35 @@ class Class: propnames = self.getprops().keys() propnames.sort() return propnames + # + # convenience methods + # + def get_roles(self, nodeid): + """Return iterator for all roles for this nodeid. + + Yields string-processed roles. + This method can be overridden to provide a hook where we can + insert other permission models (e.g. get roles from database) + In standard schemas only a user has a roles property but + this may be different in customized schemas. + Note that this is the *central place* where role + processing happens! + """ + node = self.db.getnode(self.classname, nodeid) + return iter_roles(node['roles']) + + def has_role(self, nodeid, *roles): + '''See if this node has any roles that appear in roles. + + For convenience reasons we take a list. + In standard schemas only a user has a roles property but + this may be different in customized schemas. + ''' + roles = dict.fromkeys ([r.strip().lower() for r in roles]) + for role in self.get_roles(nodeid): + if role in roles: + return True + return False class HyperdbValueError(ValueError): diff --git a/roundup/mailgw.py b/roundup/mailgw.py index fb7b6a2..97848a7 100644 --- a/roundup/mailgw.py +++ b/roundup/mailgw.py @@ -86,6 +86,7 @@ from email.Header import decode_header from roundup import configuration, hyperdb, date, password, rfc2822, exceptions from roundup.mailer import Mailer, MessageSendError from roundup.i18n import _ +from roundup.hyperdb import iter_roles try: import pyme, pyme.core, pyme.gpgme @@ -163,24 +164,6 @@ def gpgh_sigs(sig): yield sig sig = sig.next - -def iter_roles(roles): - ''' handle the text processing of turning the roles list - into something python can use more easily - ''' - for role in [x.lower().strip() for x in roles.split(',')]: - yield role - -def user_has_role(db, userid, role_list): - ''' see if the given user has any roles that appear - in the role_list - ''' - for role in iter_roles(db.user.get(userid, 'roles')): - if role in iter_roles(role_list): - return True - return False - - def check_pgp_sigs(sig, gpgctx, author): ''' Theoretically a PGP message can have several signatures. GPGME returns status on all signatures in a linked list. Walk that @@ -1256,8 +1239,8 @@ Subject was: "%(subject)s" # or we will skip PGP processing def pgp_role(): if self.instance.config.PGP_ROLES: - return user_has_role(self.db, author, - self.instance.config.PGP_ROLES) + return self.db.user.has_role(author, + iter_roles(self.instance.config.PGP_ROLES)) else: return True diff --git a/roundup/security.py b/roundup/security.py index 6138e27..3dfa8bd 100644 --- a/roundup/security.py +++ b/roundup/security.py @@ -162,12 +162,9 @@ class Security: Note that this functionality is actually implemented by the Permission.test() method. ''' - roles = self.db.user.get(userid, 'roles') - if roles is None: - return 0 if itemid and classname is None: raise ValueError, 'classname must accompany itemid' - for rolename in [x.lower().strip() for x in roles.split(',')]: + 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 diff --git a/test/test_cgi.py b/test/test_cgi.py index cfccae8..99ba771 100644 --- a/test/test_cgi.py +++ b/test/test_cgi.py @@ -648,6 +648,24 @@ class FormTestCase(unittest.TestCase): self.failUnlessRaises(exceptions.Unauthorised, actions.EditItemAction(cl).handle) + def testRoles(self): + cl = self._make_client({}) + self.db.user.set('1', roles='aDmin, uSer') + item = HTMLItem(cl, 'user', '1') + self.assert_(item.hasRole('Admin')) + self.assert_(item.hasRole('User')) + self.assert_(item.hasRole('AdmiN')) + self.assert_(item.hasRole('UseR')) + self.assert_(item.hasRole('UseR','Admin')) + self.assert_(item.hasRole('UseR','somethingelse')) + self.assert_(item.hasRole('somethingelse','Admin')) + self.assert_(not item.hasRole('userr')) + self.assert_(not item.hasRole('adminn')) + self.assert_(not item.hasRole('')) + self.assert_(not item.hasRole(' ')) + self.db.user.set('1', roles='') + self.assert_(not item.hasRole('')) + def testCSVExport(self): cl = self._make_client({'@columns': 'id,name'}, nodeid=None, userid='1') -- 2.30.2