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.

Author: Ralf Schlatterbeck

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)
 

roundup/cgi/templating.py

@@ -1205,12 +1205,9 @@ def hasPermission(self, permission, classname=_marker,
         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':

roundup/hyperdb.py

@@ -760,6 +760,16 @@ def close(self):
 
         """
 
+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 @@ def export_propnames(self):
         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):

roundup/mailgw.py

@@ -86,6 +86,7 @@ class node. Any parts of other types are each stored in separate files
 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 @@ def _handle_message(self, message):
         # 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
 

roundup/security.py

@@ -162,12 +162,9 @@ def hasPermission(self, permission, userid, classname=None,
            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

test/test_cgi.py

@@ -648,6 +648,24 @@ def own_record(db, userid, itemid): return userid == itemid
         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')