[copr] master: Move authorization checking for updating permissions into logic (8fe7f0a)

bkabrda at fedoraproject.org bkabrda at fedoraproject.org
Thu Mar 28 12:11:24 UTC 2013


Repository : http://git.fedorahosted.org/cgit/copr.git

On branch  : master

>---------------------------------------------------------------

commit 8fe7f0ac8991df619754a60f253a690f7b129e1a
Author: Bohuslav Kabrda <bkabrda at 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))
 



More information about the copr-devel mailing list