From 6fb4f7be9a2f5ebc7c72bb44e050265bc4da43d3 Mon Sep 17 00:00:00 2001
From: Pierre-Yves Chibon <pingou@pingoured.fr>
Date: May 06 2016 08:49:51 +0000
Subject: [PATCH 1/5] Enforce that the userid is an integer


---

diff --git a/pagure/ui/repo.py b/pagure/ui/repo.py
index cd2b1ef..63aefcd 100644
--- a/pagure/ui/repo.py
+++ b/pagure/ui/repo.py
@@ -1174,8 +1174,9 @@ def new_repo_hook_token(repo, username=None):
         flask.url_for('view_settings', repo=repo.name, username=username))
 
 
-@APP.route('/<repo:repo>/dropuser/<userid>', methods=['POST'])
-@APP.route('/fork/<username>/<repo:repo>/dropuser/<userid>', methods=['POST'])
+@APP.route('/<repo:repo>/dropuser/<int:userid>', methods=['POST'])
+@APP.route('/fork/<username>/<repo:repo>/dropuser/<int:userid>',
+           methods=['POST'])
 @login_required
 def remove_user(repo, userid, username=None):
     """ Remove the specified user from the project.

From 761d06a56e3a394f32e6702b9aabda60687946f1 Mon Sep 17 00:00:00 2001
From: Pierre-Yves Chibon <pingou@pingoured.fr>
Date: May 06 2016 08:49:51 +0000
Subject: [PATCH 2/5] Add endpoint to remove a group from a project


---

diff --git a/pagure/ui/repo.py b/pagure/ui/repo.py
index 63aefcd..5dfe78e 100644
--- a/pagure/ui/repo.py
+++ b/pagure/ui/repo.py
@@ -1287,6 +1287,59 @@ def add_user(repo, username=None):
     )
 
 
+@APP.route('/<repo:repo>/dropgroup/<int:groupid>', methods=['POST'])
+@APP.route('/fork/<username>/<repo:repo>/dropgroup/<int:groupid>', methods=['POST'])
+@login_required
+def remove_group_project(repo, groupid, username=None):
+    """ Remove the specified group from the project.
+    """
+    if admin_session_timedout():
+        flask.flash('Action canceled, try it again', 'error')
+        url = flask.url_for(
+            'view_settings', username=username, repo=repo)
+        return flask.redirect(
+            flask.url_for('auth_login', next=url))
+
+    repo = pagure.lib.get_project(SESSION, repo, user=username)
+
+    if not repo:
+        flask.abort(404, 'Project not found')
+
+    if not is_repo_admin(repo):
+        flask.abort(
+            403,
+            'You are not allowed to change the users for this project')
+
+    form = pagure.forms.ConfirmationForm()
+    if form.validate_on_submit():
+        grpids = [grp.id for grp in repo.groups]
+
+        if groupid not in grpids:
+            flask.flash(
+                'Group does not seem to be part of this group', 'error')
+            return flask.redirect(
+                flask.url_for(
+                    '.view_settings', repo=repo.name, username=username)
+            )
+
+        for grp in repo.groups:
+            if grp.id == groupid:
+                repo.groups.remove(grp)
+                break
+        try:
+            SESSION.commit()
+            pagure.lib.git.generate_gitolite_acls()
+            flask.flash('Group removed')
+        except SQLAlchemyError as err:  # pragma: no cover
+            SESSION.rollback()
+            APP.logger.exception(err)
+            flask.flash('Group could not be removed', 'error')
+
+    return flask.redirect(
+        flask.url_for('.view_settings', repo=repo.name, username=username)
+    )
+
+
 @APP.route('/<repo:repo>/addgroup/', methods=('GET', 'POST'))
 @APP.route('/<repo:repo>/addgroup', methods=('GET', 'POST'))
 @APP.route('/fork/<username>/<repo:repo>/addgroup/', methods=('GET', 'POST'))

From accfc9bfd6dcf6494706d8286b2ee2b7b22ebe75 Mon Sep 17 00:00:00 2001
From: Pierre-Yves Chibon <pingou@pingoured.fr>
Date: May 06 2016 08:49:52 +0000
Subject: [PATCH 3/5] Adjust the settings template to make the delete group button work


---

diff --git a/pagure/templates/settings.html b/pagure/templates/settings.html
index c73f220..94bc1e2 100644
--- a/pagure/templates/settings.html
+++ b/pagure/templates/settings.html
@@ -151,10 +151,12 @@
                 {{ form.csrf_token }}
               </form>
             {% endif %}
-              <button type="button" class="btn btn-link" data-toggle="modal" data-target="#acls{{ token.id }}">
+              <button type="button" class="btn btn-link"
+                data-toggle="modal" data-target="#acls{{ token.id }}">
                 ACLs
               </button>
-              <div class="modal fade" id="acls{{ token.id }}" tabindex="-1" role="dialog" aria-labelledby="ACLs" aria-hidden="true">
+              <div class="modal fade" id="acls{{ token.id }}" tabindex="-1"
+                    role="dialog" aria-labelledby="ACLs" aria-hidden="true">
                 <div class="modal-dialog" role="document">
                   <div class="modal-content">
                     <div class="modal-header">
@@ -283,12 +285,18 @@
         </div>
           <ul class="list-group list-group-flush">
             <li class="list-group-item">
-              <a href="{{ url_for('.view_user', username=repo.user.user) }}"><span class="oi" data-glyph="person"></span>&nbsp; {{ repo.user.user }}</a>
+              <a href="{{ url_for('.view_user', username=repo.user.user) }}">
+                <span class="oi" data-glyph="person"></span>
+                &nbsp; {{ repo.user.user }}
+              </a>
               (owner)
             </li>
           {% for user in repo.users %}
             <li class="list-group-item">
-              <a href="{{ url_for('.view_user', username=user.user) }}"><span class="oi" data-glyph="person"></span>&nbsp; {{ user.user }}</a>
+              <a href="{{ url_for('.view_user', username=user.user) }}">
+                <span class="oi" data-glyph="person"></span>
+                &nbsp; {{ user.user }}
+              </a>
               <form class="pull-xs-right" method="POST"
                 action="{{ url_for('.remove_user', username=username, repo=repo.name, userid=user.id) }}">
                 <button
@@ -302,9 +310,12 @@
           {% endfor %}
           {% for group in repo.groups %}
             <li class="list-group-item">
-              <a href="{{ url_for('.view_group', group=group.group_name) }}"><span class="oi" data-glyph="people"></span>&nbsp; {{ group.group_name }}</a>
+              <a href="{{ url_for('.view_group', group=group.group_name) }}">
+                <span class="oi" data-glyph="people"></span>
+                &nbsp; {{ group.group_name }}
+              </a>
               <form class="pull-xs-right" method="POST"
-                action="">
+                action="{{ url_for('.remove_group_project', username=username, repo=repo.name, groupid=group.id) }}">
                 <button
                   onclick="return confirm('You sure you want to remove this group from this project?');"
                   title="Remove group" class="btn btn-danger btn-sm pull-xs-right">
@@ -333,12 +344,16 @@
             <div class="panel panel-default" >
               <div class="panel-heading" role="tab" id="pluginheading{{idcount}}">
                 <h4 class="panel-title">
-                  <a data-toggle="collapse" data-parent="#accordions" href="#plugincollapse{{idcount}}" aria-expanded="true" aria-controls="plugincollapse{{idcount}}">
+                  <a data-toggle="collapse" data-parent="#accordions"
+                      href="#plugincollapse{{idcount}}" aria-expanded="true"
+                      aria-controls="plugincollapse{{idcount}}">
                     <span id="dropdowncaret" class="oi" data-glyph="caret-right"></span>&nbsp;{{ plugin }}
                   </a>
                 </h4>
               </div>
-              <div id="plugincollapse{{ idcount }}" data-plugin="{{ plugin }}" class="panel-collapse collapse" role="tabpanel" aria-labelledby="pluginheading{{ idcount }}">
+              <div id="plugincollapse{{ idcount }}" data-plugin="{{ plugin }}"
+                  class="panel-collapse collapse" role="tabpanel"
+                  aria-labelledby="pluginheading{{ idcount }}">
                 <span class="oi oi-spin pull-left" data-glyph="loop-circular"></span>
               </div>
             </div>

From a95090b49b95de15cddc03f25b804910207040ea Mon Sep 17 00:00:00 2001
From: Pierre-Yves Chibon <pingou@pingoured.fr>
Date: May 06 2016 08:49:52 +0000
Subject: [PATCH 4/5] Adjust the error message returned to the user


---

diff --git a/pagure/ui/repo.py b/pagure/ui/repo.py
index 5dfe78e..e1e5882 100644
--- a/pagure/ui/repo.py
+++ b/pagure/ui/repo.py
@@ -1316,7 +1316,7 @@ def remove_group_project(repo, groupid, username=None):
 
         if groupid not in grpids:
             flask.flash(
-                'Group does not seem to be part of this group', 'error')
+                'Group does not seem to be part of this project', 'error')
             return flask.redirect(
                 flask.url_for(
                     '.view_settings', repo=repo.name, username=username)

From 114c3d340d139939381c2f7b342346e17fd8759d Mon Sep 17 00:00:00 2001
From: Pierre-Yves Chibon <pingou@pingoured.fr>
Date: May 06 2016 08:49:52 +0000
Subject: [PATCH 5/5] Add unit-tests for the procedure to remove a group from a project


---

diff --git a/tests/test_pagure_flask_ui_repo.py b/tests/test_pagure_flask_ui_repo.py
index ffc6e0c..683137e 100644
--- a/tests/test_pagure_flask_ui_repo.py
+++ b/tests/test_pagure_flask_ui_repo.py
@@ -287,6 +287,94 @@ class PagureFlaskRepotests(tests.Modeltests):
                 '</button>\n                      User removed', output.data)
 
     @patch('pagure.ui.repo.admin_session_timedout')
+    def test_remove_group_project(self, ast):
+        """ Test the remove_group_project endpoint. """
+        ast.return_value = False
+
+        output = self.app.post('/foo/dropgroup/1')
+        self.assertEqual(output.status_code, 302)
+
+        user = tests.FakeUser()
+        with tests.user_set(pagure.APP, user):
+            output = self.app.post('/foo/dropgroup/1')
+            self.assertEqual(output.status_code, 404)
+
+            tests.create_projects(self.session)
+
+            output = self.app.post('/test/dropgroup/1')
+            self.assertEqual(output.status_code, 403)
+
+            ast.return_value = True
+            output = self.app.post('/test/dropgroup/1')
+            self.assertEqual(output.status_code, 302)
+            ast.return_value = False
+
+        user.username = 'pingou'
+        with tests.user_set(pagure.APP, user):
+            tests.create_projects_git(tests.HERE)
+            output = self.app.post('/test/settings')
+
+            csrf_token = output.data.split(
+                'name="csrf_token" type="hidden" value="')[1].split('">')[0]
+
+            data = {'csrf_token': csrf_token}
+
+            output = self.app.post(
+                '/test/dropgroup/2', data=data, follow_redirects=True)
+            self.assertEqual(output.status_code, 200)
+            self.assertIn(
+                '<title>Settings - test - Pagure</title>', output.data)
+            self.assertIn('<h3>Settings for test</h3>', output.data)
+            self.assertIn(
+                '</button>\n                      '
+                'Group does not seem to be part of this project',
+                output.data)
+
+        # Create the new group
+        msg = pagure.lib.add_group(
+            session=self.session,
+            group_name='testgrp',
+            group_type='user',
+            user='pingou',
+            is_admin=False,
+            blacklist=[],
+        )
+        self.assertEqual(msg, 'User `pingou` added to the group `testgrp`.')
+        self.session.commit()
+
+        repo = pagure.lib.get_project(self.session, 'test')
+        # Add the group to a project
+        msg = pagure.lib.add_group_to_project(
+            session=self.session,
+            project=repo,
+            new_group='testgrp',
+            user='pingou',
+        )
+        self.session.commit()
+        self.assertEqual(msg, 'Group added')
+
+        with tests.user_set(pagure.APP, user):
+            output = self.app.post('/test/dropgroup/1', follow_redirects=True)
+            self.assertEqual(output.status_code, 200)
+            self.assertIn(
+                '<title>Settings - test - Pagure</title>', output.data)
+            self.assertIn('<h3>Settings for test</h3>', output.data)
+            self.assertNotIn(
+                '</button>\n                      Group removed',
+                output.data)
+
+            data = {'csrf_token': csrf_token}
+            output = self.app.post(
+                '/test/dropgroup/1', data=data, follow_redirects=True)
+            self.assertEqual(output.status_code, 200)
+            self.assertIn(
+                '<title>Settings - test - Pagure</title>', output.data)
+            self.assertIn('<h3>Settings for test</h3>', output.data)
+            self.assertIn(
+                '</button>\n                      Group removed',
+                output.data)
+
+    @patch('pagure.ui.repo.admin_session_timedout')
     def test_update_project(self, ast):
         """ Test the update_project endpoint. """
         ast.return_value = True