From cb6d804aac7cf46239363eb13fb3ff3b56afe1a6 Mon Sep 17 00:00:00 2001 From: schlatterbeck Date: Mon, 21 Dec 2009 12:00:57 +0000 Subject: [PATCH] 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 --- CHANGES.txt | 10 ++++++ doc/upgrading.txt | 9 +++++ roundup/cgi/actions.py | 9 ++--- test/test_cgi.py | 80 ++++++++++++++++++++++++++++++++++++++++-- 4 files changed, 99 insertions(+), 9 deletions(-) diff --git a/CHANGES.txt b/CHANGES.txt index 80d3dc9..0da596d 100644 --- 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 cd86481..84a703a 100644 --- 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 c198494..36dfee3 100755 --- 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 99ba771..d5ec2ff 100644 --- 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') -- 2.30.2