Fix actions

Permission for retire in roundup/actions.py was with 'Edit' permission,
not 'Retire' permission. Add a 'restore' action to roundup/actions.py.
Both are now correctly used in rest.py and xmlrpc.py (the latter had
some errors when printint error messages).

Also reworked the rest implementation: Despite the warnings in the
roundup API in hyperdb.py the DELETE http method would *destroy* and not
*retire* an item. This has been fixed. We also do not allow retire of a
complete class (although this was implemented) because this seems to
dangerous and we see no use-case.

Author: schlatterbeck

roundup/actions.py

@@ -2,6 +2,7 @@
 # Copyright (C) 2009 Stefan Seefeld
 # All rights reserved.
 # For license terms see the file COPYING.txt.
+# Actions used in REST and XMLRPC APIs
 #
 
 from roundup.exceptions import Unauthorised
@@ -40,7 +41,19 @@ def gettext(self, msgid):
     _ = gettext
 
 
-class Retire(Action):
+class PermCheck(Action):
+    def permission(self, designator):
+
+        classname, itemid = hyperdb.splitDesignator(designator)
+        perm = self.db.security.hasPermission
+
+        if not perm('Retire', self.db.getuid(), classname=classname
+                   , itemid=itemid):
+            raise Unauthorised(self._('You do not have permission to retire '
+                                      'or restore the %(classname)s class.')
+                                      %locals())
+
+class Retire(PermCheck):
 
     def handle(self, designator):
 
@@ -57,12 +70,13 @@ def handle(self, designator):
         self.db.commit()
 
 
-    def permission(self, designator):
+class Restore(PermCheck):
+
+    def handle(self, designator):
 
         classname, itemid = hyperdb.splitDesignator(designator)
 
-        if not self.db.security.hasPermission('Edit', self.db.getuid(),
-                                              classname=classname, itemid=itemid):
-            raise Unauthorised(self._('You do not have permission to '
-                                      'retire the %(classname)s class.')%classname)
-            
+        # do the restore
+        self.db.getclass(classname).restore(itemid)
+        self.db.commit()
+

roundup/rest.py

@@ -243,8 +243,11 @@ def __init__(self, client, db):
         self.client = client
         self.db = db
         self.translator = client.translator
-        self.actions = client.instance.actions.copy()
-        self.actions.update({'retire': actions.Retire})
+        # This used to be initialized from client.instance.actions which
+        # would include too many actions that do not make sense in the
+        # REST-API context, so for now we only permit the retire and
+        # restore actions.
+        self.actions = dict (retire = actions.Retire, restore = actions.Restore)
 
         protocol = 'http'
         host = self.client.env['HTTP_HOST']
@@ -716,7 +719,9 @@ def put_attribute(self, class_name, item_id, attr_name, input):
     @Routing.route("/data/<:class_name>", 'DELETE')
     @_data_decorator
     def delete_collection(self, class_name, input):
-        """DELETE all objects in a class
+        """DELETE (retire) all objects in a class
+           There is currently no use-case, so this is disabled and
+           always returns Unauthorised.
 
         Args:
             class_name (string): class name of the resource (Ex: issue, msg)
@@ -728,25 +733,26 @@ def delete_collection(self, class_name, input):
                 status (string): 'ok'
                 count (int): number of deleted objects
         """
+        raise Unauthorised('Deletion of a whole class disabled')
         if class_name not in self.db.classes:
             raise NotFound('Class %s not found' % class_name)
         if not self.db.security.hasPermission(
-            'Delete', self.db.getuid(), class_name
+            'Retire', self.db.getuid(), class_name
         ):
             raise Unauthorised('Permission to delete %s denied' % class_name)
 
         class_obj = self.db.getclass(class_name)
         for item_id in class_obj.list():
             if not self.db.security.hasPermission(
-                'Delete', self.db.getuid(), class_name, itemid=item_id
+                'Retire', self.db.getuid(), class_name, itemid=item_id
             ):
                 raise Unauthorised(
-                    'Permission to delete %s %s denied' % (class_name, item_id)
+                    'Permission to retire %s %s denied' % (class_name, item_id)
                 )
 
         count = len(class_obj.list())
         for item_id in class_obj.list():
-            self.db.destroynode(class_name, item_id)
+            class_obj.retire (item_id)
 
         self.db.commit()
         result = {
@@ -759,7 +765,7 @@ def delete_collection(self, class_name, input):
     @Routing.route("/data/<:class_name>/<:item_id>", 'DELETE')
     @_data_decorator
     def delete_element(self, class_name, item_id, input):
-        """DELETE an object in a class
+        """DELETE (retire) an object in a class
 
         Args:
             class_name (string): class name of the resource (Ex: issue, msg)
@@ -773,14 +779,15 @@ def delete_element(self, class_name, item_id, input):
         """
         if class_name not in self.db.classes:
             raise NotFound('Class %s not found' % class_name)
+        class_obj = self.db.classes [class_name]
         if not self.db.security.hasPermission(
-            'Delete', self.db.getuid(), class_name, itemid=item_id
+            'Retire', self.db.getuid(), class_name, itemid=item_id
         ):
             raise Unauthorised(
-                'Permission to delete %s %s denied' % (class_name, item_id)
+                'Permission to retire %s %s denied' % (class_name, item_id)
             )
 
-        self.db.destroynode(class_name, item_id)
+        class_obj.retire (item_id)
         self.db.commit()
         result = {
             'status': 'ok'

roundup/xmlrpc.py

@@ -179,7 +179,7 @@ def set(self, designator, *args):
         return result
 
 
-    builtin_actions = {'retire': actions.Retire}
+    builtin_actions = dict (retire = actions.Retire, restore = actions.Restore)
 
     def action(self, name, *args):
         """Execute a named action."""
@@ -189,7 +189,8 @@ def action(self, name, *args):
         elif name in self.builtin_actions:
             action_type = self.builtin_actions[name]
         else:
-            raise Exception('action "%s" is not supported %s' % (name, ','.join(self.actions.keys())))
+            raise Exception('action "%s" is not supported %s'
+                           % (name, ','.join(self.actions.keys())))
         action = action_type(self.db, self.translator)
         return action.execute(*args)
 

test/rest_common.py

@@ -39,6 +39,9 @@ def setUp(self):
         self.db.commit()
         self.db.close()
         self.db = self.instance.open('joe')
+        # Allow joe to retire
+        p = self.db.security.addPermission(name='Retire', klass='issue')
+        self.db.security.addPermissionToRole('User', p)
 
         self.db.tx_Source = 'web'