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

diff --git a/CHANGES.txt b/CHANGES.txt
index 7c26ad46768cbe14a6b3560dfe742d01a8447290..57afd30a7771bca8d43cbe0a5a1e4acb5f7e7344 100644 (file)
--- 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 6f0584f10d76a69698cb49225f3c108e4f4e3dc6..fad7952fbef9b83d0dc78239f9cd8d10d7d3f2bb 100644 (file)
--- 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 91a9fef974b905b5b1163da219a4b61f0fd7fd4a..8825eb7fe5e08b3b58da806e3158c8df9046c12b 100644 (file)
--- 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 fb7b6a2e3ee796065580c80a38e2e8cf8ea7f080..97848a72ca541d599de85678d85fd32df6c225be 100644 (file)
--- 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 6138e271a1ab430ddb9f01d7bf084ada4c3d75b4..3dfa8bd442c13ec068ca90589a10e55fc9f5b9fa 100644 (file)
--- 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 cfccae85c30a886df93dcf4a0ab5070e31a231f8..99ba7711ac0f1cb83c217376d999f15dd99caffd 100644 (file)
--- 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')