Proper handling of 'Create' permissions in both mail gateway...

...(earlier commit 2009-12-07 00:13:[email protected]
by Richard) and web interface, this used to check 'Edit' permission
previously. See

http://thread.gmane.org/gmane.comp.bug-tracking.roundup.devel/5133
Add regression tests for proper handling of 'Create' and 'Edit'
permissions.

Author: Ralf Schlatterbeck

CHANGES.txt

@@ -1,6 +1,16 @@
 This file contains the changes to the Roundup system over time. The entries
 are given with the most recent entry first.
 
+2010-XX-XX 1.4.12 (rXXXX)
+
+Fixes:
+- Proper handling of 'Create' permissions in both mail gateway (earlier
+  commit r4405 by Richard) and web interface, this used to check 'Edit'
+  permission previously. See
+  http://thread.gmane.org/gmane.comp.bug-tracking.roundup.devel/5133
+  Add regression tests for proper handling of 'Create' and 'Edit'
+  permissions.
+
 2009-12-21 1.4.11 (r4411)
 
 Features:

doc/upgrading.txt

@@ -13,6 +13,15 @@ steps.
 
 .. contents::
 
+Migrating from 1.4.x to 1.4.12
+==============================
+
+Item creation now checks the "Create" permission instead of the "Edit"
+permission for individual properties. If you have modified your tracker
+permissions from the default distribution, you should check that
+"Create" permissions exist for all properties you want users to be able
+to create.
+
 Migrating from 1.4.x to 1.4.11
 ==============================
 

roundup/cgi/actions.py

@@ -551,16 +551,11 @@ def newItemPermission(self, props, classname=None):
         if not self.hasPermission('Create', classname=classname):
             return 0
 
-        # Check Edit permission for each property, to avoid being able
+        # Check Create permission for each property, to avoid being able
         # to set restricted ones on new item creation
         for key in props:
-            if not self.hasPermission('Edit', classname=classname,
+            if not self.hasPermission('Create', classname=classname,
                                       property=key):
-                # We restrict by default and special-case allowed properties
-                if key == 'date' or key == 'content':
-                    continue
-                elif key == 'author' and props[key] == self.userid:
-                    continue
                 return 0
         return 1
 

test/test_cgi.py

@@ -620,10 +620,13 @@ def _make_client(self, form, classname='user', nodeid='1', userid='2'):
         cl = client.Client(self.instance, None, {'PATH_INFO':'/',
             'REQUEST_METHOD':'POST'}, makeForm(form))
         cl.classname = 'user'
-        cl.nodeid = nodeid
+        if nodeid is not None:
+            cl.nodeid = nodeid
         cl.db = self.db
         cl.userid = userid
         cl.language = ('en',)
+        cl.error_message = []
+        cl.template = 'item'
         return cl
 
     def testClassPermission(self):
@@ -636,18 +639,91 @@ def testClassPermission(self):
 
     def testCheckAndPropertyPermission(self):
         self.db.security.permissions = {}
-        def own_record(db, userid, itemid): return userid == itemid
+        def own_record(db, userid, itemid):
+            return userid == itemid
         p = self.db.security.addPermission(name='Edit', klass='user',
             check=own_record, properties=("password", ))
         self.db.security.addPermissionToRole('User', p)
 
         cl = self._make_client(dict(username='bob'))
         self.assertRaises(exceptions.Unauthorised,
             actions.EditItemAction(cl).handle)
+        cl = self._make_client(dict(roles='User,Admin'), userid='4', nodeid='4')
+        self.assertRaises(exceptions.Unauthorised,
+            actions.EditItemAction(cl).handle)
+        cl = self._make_client(dict(roles='User,Admin'), userid='4')
+        self.assertRaises(exceptions.Unauthorised,
+            actions.EditItemAction(cl).handle)
+        cl = self._make_client(dict(roles='User,Admin'))
+        self.assertRaises(exceptions.Unauthorised,
+            actions.EditItemAction(cl).handle)
+        # working example, mary may change her pw
+        cl = self._make_client({'password':'ob', '@confirm@password':'ob'},
+            nodeid='4', userid='4')
+        self.assertRaises(exceptions.Redirect,
+            actions.EditItemAction(cl).handle)
         cl = self._make_client({'password':'bob', '@confirm@password':'bob'})
         self.failUnlessRaises(exceptions.Unauthorised,
             actions.EditItemAction(cl).handle)
 
+    def testCreatePermission(self):
+        # this checks if we properly differentiate between create and
+        # edit permissions
+        self.db.security.permissions = {}
+        self.db.security.addRole(name='UserAdd')
+        # Don't allow roles
+        p = self.db.security.addPermission(name='Create', klass='user',
+            properties=("username", "password", "address",
+            "alternate_address", "realname", "phone", "organisation",
+            "timezone"))
+        self.db.security.addPermissionToRole('UserAdd', p)
+        # Don't allow roles *and* don't allow username
+        p = self.db.security.addPermission(name='Edit', klass='user',
+            properties=("password", "address", "alternate_address",
+            "realname", "phone", "organisation", "timezone"))
+        self.db.security.addPermissionToRole('UserAdd', p)
+        self.db.user.set('4', roles='UserAdd')
+
+        # anonymous may not
+        cl = self._make_client({'username':'new_user', 'password':'secret',
+            '@confirm@password':'secret', 'address':'[email protected]',
+            'roles':'Admin'}, nodeid=None, userid='2')
+        self.assertRaises(exceptions.Unauthorised,
+            actions.NewItemAction(cl).handle)
+        # Don't allow creating new user with roles
+        cl = self._make_client({'username':'new_user', 'password':'secret',
+            '@confirm@password':'secret', 'address':'[email protected]',
+            'roles':'Admin'}, nodeid=None, userid='4')
+        self.assertRaises(exceptions.Unauthorised,
+            actions.NewItemAction(cl).handle)
+        self.assertEqual(cl.error_message,[])
+        # this should work
+        cl = self._make_client({'username':'new_user', 'password':'secret',
+            '@confirm@password':'secret', 'address':'[email protected]'},
+            nodeid=None, userid='4')
+        self.assertRaises(exceptions.Redirect,
+            actions.NewItemAction(cl).handle)
+        self.assertEqual(cl.error_message,[])
+        # don't allow changing (my own) username (in this example)
+        cl = self._make_client(dict(username='new_user42'), userid='4')
+        self.assertRaises(exceptions.Unauthorised,
+            actions.EditItemAction(cl).handle)
+        cl = self._make_client(dict(username='new_user42'), userid='4',
+            nodeid='4')
+        self.assertRaises(exceptions.Unauthorised,
+            actions.EditItemAction(cl).handle)
+        # don't allow changing (my own) roles
+        cl = self._make_client(dict(roles='User,Admin'), userid='4',
+            nodeid='4')
+        self.assertRaises(exceptions.Unauthorised,
+            actions.EditItemAction(cl).handle)
+        cl = self._make_client(dict(roles='User,Admin'), userid='4')
+        self.assertRaises(exceptions.Unauthorised,
+            actions.EditItemAction(cl).handle)
+        cl = self._make_client(dict(roles='User,Admin'))
+        self.assertRaises(exceptions.Unauthorised,
+            actions.EditItemAction(cl).handle)
+
     def testRoles(self):
         cl = self._make_client({})
         self.db.user.set('1', roles='aDmin,    uSer')