Mercurial > p > roundup > code
changeset 5604:ed02a1e0aa5d REST-rebased
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 | Ralf Schlatterbeck <rsc@runtux.com> |
|---|---|
| date | Wed, 30 Jan 2019 14:12:27 +0100 |
| parents | 79da1ca2f94b |
| children | 33f8bb777659 |
| files | roundup/actions.py roundup/rest.py roundup/xmlrpc.py test/rest_common.py |
| diffstat | 4 files changed, 45 insertions(+), 20 deletions(-) [+] |
line wrap: on
line diff
--- a/roundup/actions.py Wed Jan 30 13:58:18 2019 +0100 +++ b/roundup/actions.py Wed Jan 30 14:12:27 2019 +0100 @@ -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 @@ _ = 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 @@ 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() +
--- a/roundup/rest.py Wed Jan 30 13:58:18 2019 +0100 +++ b/roundup/rest.py Wed Jan 30 14:12:27 2019 +0100 @@ -243,8 +243,11 @@ 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 @@ @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 @@ 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 @@ @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 @@ """ 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'
--- a/roundup/xmlrpc.py Wed Jan 30 13:58:18 2019 +0100 +++ b/roundup/xmlrpc.py Wed Jan 30 14:12:27 2019 +0100 @@ -179,7 +179,7 @@ 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 @@ 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)
--- a/test/rest_common.py Wed Jan 30 13:58:18 2019 +0100 +++ b/test/rest_common.py Wed Jan 30 14:12:27 2019 +0100 @@ -39,6 +39,9 @@ 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'