Repository : http://git.fedorahosted.org/cgit/copr.git
On branch : master
commit 8fe7f0ac8991df619754a60f253a690f7b129e1a Author: Bohuslav Kabrda bkabrda@redhat.com Date: Thu Mar 28 12:50:09 2013 +0100
Move authorization checking for updating permissions into logic
- refactor the raise_if_cant_update_copr method into newly introduced UsersLogic class
coprs_frontend/coprs/logic/coprs_logic.py | 16 ++++--------- coprs_frontend/coprs/logic/users_logic.py | 11 +++++++++ .../coprs/views/coprs_ns/coprs_general.py | 24 ++++++++++---------- 3 files changed, 28 insertions(+), 23 deletions(-)
diff --git a/coprs_frontend/coprs/logic/coprs_logic.py b/coprs_frontend/coprs/logic/coprs_logic.py index 2b4b2d9..6922789 100644 --- a/coprs_frontend/coprs/logic/coprs_logic.py +++ b/coprs_frontend/coprs/logic/coprs_logic.py @@ -6,6 +6,7 @@ from coprs import helpers from coprs import models from coprs import signals from coprs import whoosheers +from coprs.logic import users_logic
class CoprsLogic(object): """Used for manipulating Coprs. All methods accept user object as a first argument, as this may be needed in future.""" @@ -94,7 +95,8 @@ class CoprsLogic(object): def update(cls, user, copr, check_for_duplicates = True): cls.raise_if_unfinished_action(user, copr, 'Can't change this Copr name, another operation is in progress: {action}') - cls.raise_if_cant_update(user, copr, 'Only owners and admins may update their Coprs.') + users_logic.UsersLogic.raise_if_cant_update_copr(user, copr, + 'Only owners and admins may update their Coprs.')
existing = cls.exists_for_user(copr.owner, copr.name).first() if existing: @@ -160,16 +162,6 @@ class CoprsLogic(object): if unfinished_actions: raise exceptions.ActionInProgressException(message, unfinished_actions[0])
- @classmethod - def raise_if_cant_update(cls, user, copr, message): - """This method raises InsufficientRightsException if given user cant update - given copr. Returns None otherwise. - """ - # TODO: this is a bit inconsistent - shouldn't the user method be called can_update? - if not user.can_edit(copr): - raise exceptions.InsufficientRightsException(message) - - class CoprPermissionsLogic(object): @classmethod def get(cls, user, copr, searched_user): @@ -190,6 +182,8 @@ class CoprPermissionsLogic(object):
@classmethod def update_permissions(cls, user, copr, copr_permission, new_builder, new_admin): + users_logic.UsersLogic.raise_if_cant_update_copr(user, copr, + 'Only owners and admins may update their Coprs permissions.') models.CoprPermission.query.filter(models.CoprPermission.copr_id == copr.id).\ filter(models.CoprPermission.user_id == copr_permission.user_id).\ update({'copr_builder': new_builder, diff --git a/coprs_frontend/coprs/logic/users_logic.py b/coprs_frontend/coprs/logic/users_logic.py new file mode 100644 index 0000000..8ff0eb2 --- /dev/null +++ b/coprs_frontend/coprs/logic/users_logic.py @@ -0,0 +1,11 @@ +import exceptions + +class UsersLogic(object): + @classmethod + def raise_if_cant_update_copr(cls, user, copr, message): + """This method raises InsufficientRightsException if given user cant update + given copr. Returns None otherwise. + """ + # TODO: this is a bit inconsistent - shouldn't the user method be called can_update? + if not user.can_edit(copr): + raise exceptions.InsufficientRightsException(message) diff --git a/coprs_frontend/coprs/views/coprs_ns/coprs_general.py b/coprs_frontend/coprs/views/coprs_ns/coprs_general.py index adba1d0..51bc7d1 100644 --- a/coprs_frontend/coprs/views/coprs_ns/coprs_general.py +++ b/coprs_frontend/coprs/views/coprs_ns/coprs_general.py @@ -216,20 +216,20 @@ def copr_update_permissions(username, coprname): permissions = copr.copr_permissions permissions_form = forms.PermissionsFormFactory.create_form_cls(permissions)()
- # only owner can update copr permissions - if not flask.g.user.can_edit(copr): - flask.flash('Only owners and admins may update their Coprs permissions.') - return flask.redirect(flask.url_for('coprs_ns.copr_detail', username = copr.owner.name, coprname = copr.name)) - if permissions_form.validate_on_submit(): # we don't change owner (yet) - for perm in permissions: - new_builder = permissions_form['copr_builder_{0}'.format(perm.user_id)].data - new_admin = permissions_form['copr_admin_{0}'.format(perm.user_id)].data - coprs_logic.CoprPermissionsLogic.update_permissions(flask.g.user, copr, perm, new_builder, new_admin) - - db.session.commit() - flask.flash('Copr permissions were updated successfully.') + try: + for perm in permissions: + new_builder = permissions_form['copr_builder_{0}'.format(perm.user_id)].data + new_admin = permissions_form['copr_admin_{0}'.format(perm.user_id)].data + coprs_logic.CoprPermissionsLogic.update_permissions(flask.g.user, copr, perm, new_builder, new_admin) + # for now, we don't check for actions here, as permissions operation don't collide with any actions + except exceptions.InsufficientRightsException as e: + db.session.rollback() + flask.flash(e) + else: + db.session.commit() + flask.flash('Copr permissions were updated successfully.')
return flask.redirect(flask.url_for('coprs_ns.copr_detail', username = copr.owner.name, coprname = copr.name))
copr-devel@lists.fedorahosted.org