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.

git-svn-id: http://svn.roundup-tracker.org/svnroot/roundup/roundup/trunk@4414 57a73879-2fb5-44c3-a270-3262357dd7e2

diff --git a/CHANGES.txt b/CHANGES.txt
index 80d3dc985c346f5be697edbc8ad9dfc3b383b567..0da596de9446ec2ecd5e9f83e1cc072f4c1706c6 100644 (file)
--- a/CHANGES.txt
+++ b/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:

diff --git a/doc/upgrading.txt b/doc/upgrading.txt
index cd864817f312e1dbcff2e4b9facb51736a2d7bba..84a703a0aa39be4a42cb0e5713a1b827d47f7ea6 100644 (file)
--- a/doc/upgrading.txt
+++ b/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
 ==============================
 

diff --git a/roundup/cgi/actions.py b/roundup/cgi/actions.py
index c19849413aff73dc169ead6b0e6ba347f8996267..36dfee3707f96a02c0c12c0cd49c70f66c8f718c 100755 (executable)
--- a/roundup/cgi/actions.py
+++ b/roundup/cgi/actions.py
@@ -551,16 +551,11 @@ class EditCommon(Action):
         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
 

diff --git a/test/test_cgi.py b/test/test_cgi.py
index 99ba7711ac0f1cb83c217376d999f15dd99caffd..d5ec2ff35dac891e7c3a29ae57cda9b439bccf2b 100644 (file)
--- a/test/test_cgi.py
+++ b/test/test_cgi.py
@@ -620,10 +620,13 @@ class FormTestCase(unittest.TestCase):
         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,7 +639,8 @@ class FormTestCase(unittest.TestCase):
 
     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)
@@ -644,10 +648,82 @@ class FormTestCase(unittest.TestCase):
         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':'new_user@bork.bork',
+            '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':'new_user@bork.bork',
+            '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':'new_user@bork.bork'},
+            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')