Check in enhanced form for check command used by addPermission.

New form can include a **context dictionary that allows access to the
name of the property, class, and permission being checked.  This
should make designing more complex permission requirements easier.

Author: rouilj

CHANGES.txt

@@ -167,6 +167,15 @@ Features:
   usually be the same template used to edit the form. See the section
   on "Implementing Modal Editing Using @template" in
   ``customizing.txt``. (John Rouillard)
+- New form of check function is permitted in permission definitions.
+  If the check function is defined as:
+      check(db, userid, itemid, **ctx)
+  the ctx variable will have:
+     ctx['property'] the name of the property being checked or None
+     ctx['classname'] the class that is being checked or None
+     ctx['permission'] the name of the permission (e.g. View, Edit)
+  At some future date the older 3 argument style check command will
+  be deprecated. See ``upgrading.txt`` for details.
 
 Fixed:
 

doc/customizing.txt

@@ -1183,7 +1183,7 @@ Put together, these settings appear in the tracker's ``schema.py`` file::
     # Users should be able to edit their own details -- this permission
     # is limited to only the situation where the Viewed or Edited item
     # is their own.
-    def own_record(db, userid, itemid):
+    def own_record(db, userid, itemid, **ctx):
         '''Determine whether the userid matches the item being accessed.'''
         return userid == itemid
     p = db.security.addPermission(name='View', klass='user', check=own_record,
@@ -1297,11 +1297,35 @@ The ``addPermission`` method takes a couple of optional parameters:
   new Permission to (eg. ``... klass='user', properties=('name',
   'email') ...``)
 **check**
-  A function to be execute which returns boolean determining whether the
-  Permission is allowed. The function has the signature ``check(db, userid,
-  itemid)`` where ``db`` is a handle on the open database, ``userid`` is
+  A function to be executed which returns boolean determining whether
+  the Permission is allowed. If it returns True, the permission is
+  allowed, if it returns False the permission is denied.  The function
+  can have one of two signatures::
+
+     check(db, userid, itemid)
+
+  or::
+
+     check(db, userid, itemid, **ctx)
+
+  where ``db`` is a handle on the open database, ``userid`` is
   the user attempting access and ``itemid`` is the specific item being
-  accessed.
+  accessed. If the second form is used the ``ctx`` dictionary is
+  defined with the following values::
+
+     ctx['property'] the name of the property being checked or None if
+            it's a class check.
+
+     ctx['classname'] the name of the class that is being checked
+            (issue, query ....).
+
+     ctx['permission'] the name of the permission (e.g. View, Edit...).
+
+The second form is preferred as it makes it easier to implement more
+complex permission schemes. An example of the use of ``ctx`` can be
+found in the ``upgrading.txt`` or `upgrading.html`_ document.
+
+.. _`upgrading.html`: upgrading.html
 
 Example Scenarios
 ~~~~~~~~~~~~~~~~~

doc/upgrading.txt

@@ -345,6 +345,57 @@ permission for the user role. After the change it should look like::
 
 where the last three lines are the ones you need to add.
 
+Enhancement to check command for Permissions
+--------------------------------------------
+
+A new form of check function is permitted in permission definitions.
+The three argument form is still supported and will work the same
+as it always has (although it may be depricated in the future).
+
+If the check function is defined as::
+
+      check(db, userid, itemid, **ctx)
+
+the ctx variable will have the context to use when determining access
+rights::
+
+     ctx['property'] the name of the property being checked or None if
+            it's a class check.
+
+     ctx['classname'] the name of the class that is being checked
+            (issue, query ....).
+
+     ctx['permission'] the name of the permission (e.g. View, Edit...).
+
+This should make defining complex permissions much easier. Consider::
+
+    def issue_private_access(db, userid, itemid, **ctx):
+        if not db.issue.get(itemid, 'private'):
+           # allow access to everything if not private	
+	   return True
+
+	# It is a private issue hide nosy list
+	# Note that the nosy property *must* be listed
+	# in permissions argument to the addPermission
+	# definition otherwise this check command
+	# is not run.
+	if ctx['property'] == 'nosy':
+	   return False # deny access to this property
+
+	# allow access for editing, viewing etc. of the class
+	return True
+
+
+    e = db.security.addPermission(name='Edit', klass='issue',
+                                  check=issue_private_access,
+				  properties=['nosy'],
+                                  description="Edit issue checks")
+
+It is suggested that you change your checks to use the ``**ctx``
+parameter. This is expected to be the preferred form in the future.
+You do not need to use the ``ctx`` parameter in the function if you do
+not need it.
+
 Migrating from 1.5.0 to 1.5.1
 =============================
 

roundup/security.py

@@ -27,13 +27,29 @@ class Permission:
     '''
     def __init__(self, name='', description='', klass=None,
             properties=None, check=None):
+        import inspect
         self.name = name
         self.description = description
         self.klass = klass
         self.properties = properties
         self._properties_dict = support.TruthDict(properties)
         self.check = check
 
+        if check is None:
+            self.check_version = 0
+        else:
+            args=inspect.getargspec(check)
+            # FIXME change args[2] to args.keywords since python
+            # 2.6 made getargspec a named tuple once roundup 1.6 released.
+            # If there is a **parameter defined in the function spec, the
+            #  value of the 3rd argument in the tuple is not None.
+            if args[2] is None:
+                # function definition is function(db, userid, itemid)
+                self.check_version = 1
+            else:
+                # function definition is function(db, userid, itemid, **other)
+                self.check_version = 2
+
     def test(self, db, permission, classname, property, userid, itemid):
         if permission != self.name:
             return 0
@@ -48,8 +64,12 @@ def test(self, db, permission, classname, property, userid, itemid):
 
         # check code
         if itemid is not None and self.check is not None:
-            if not self.check(db, userid, itemid):
-                return 0
+            if self.check_version == 1:
+                if not self.check(db, userid, itemid):
+                    return 0
+            elif self.check_version == 2:
+                if not self.check(db, userid, itemid, property=property, permission=permission, classname=classname):
+                    return 0
 
         # we have a winner
         return 1

test/test_security.py

@@ -155,8 +155,13 @@ def testAccessControls(self):
         self.assertEquals(has('Test', none, 'test', property='c'), 0)
         self.assertEquals(has('Test', none, 'test'), 0)
 
-        # check function
-        check = lambda db, userid, itemid: itemid == '1'
+        # check function new style. Make sure that other args are passed.
+        def check(db,userid,itemid, **other):
+            prop = other['property']
+            prop = other['classname']
+            prop = other['permission']
+            return (itemid == '1')
+
         addRole(name='Role3')
         addToRole('Role3', add(name="Test", klass="test", check=check))
         user3 = self.db.user.create(username='user3', roles='Role3')