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.
 
 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:
 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::
 
 
 .. 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
 ==============================
 
 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
 
         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:
         # 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):
                                       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
 
                 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 = 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.db = self.db
         cl.userid = userid
         cl.language = ('en',)

+        cl.error_message = []
+        cl.template = 'item'

         return cl
 
     def testClassPermission(self):
         return cl
 
     def testClassPermission(self):


@@ -636,7 +639,8 @@ class FormTestCase(unittest.TestCase):
 
     def testCheckAndPropertyPermission(self):
         self.db.security.permissions = {}
 
     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)
         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(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)
 
         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')
     def testRoles(self):
         cl = self._make_client({})
         self.db.user.set('1', roles='aDmin,    uSer')