From 27463995133e86299a5fc8d15d13c1c4229d258f Mon Sep 17 00:00:00 2001
From: Pierre-Yves Chibon <pingou@pingoured.fr>
Date: Jun 07 2017 12:39:33 +0000
Subject: [PATCH 1/2] Small code style change


Signed-off-by: Pierre-Yves Chibon <pingou@pingoured.fr>

---

diff --git a/pagure/ui/issues.py b/pagure/ui/issues.py
index e5337c9..07dcdbd 100644
--- a/pagure/ui/issues.py
+++ b/pagure/ui/issues.py
@@ -1272,9 +1272,9 @@ def upload_issue(repo, issueid, username=None, namespace=None):
                     repo=repo.name,
                     username=username,
                     namespace=repo.namespace,
-                    filename='files/%s' % new_filename,
+                    filename='files/%s' % nfilename,
                 )
-                for new_filename in filenames
+                for nfilename in filenames
             ]
         })
     else:

From e1d70f284335d4a93d2bce97b20b395b00dea23d Mon Sep 17 00:00:00 2001
From: Pierre-Yves Chibon <pingou@pingoured.fr>
Date: Jun 07 2017 12:51:07 +0000
Subject: [PATCH 2/2] Add the required groups feature


This feature allows to specify a list of groups of which a person must
be included in at least one to be allowed to be added to a project with
admin or commit access.

Signed-off-by: Pierre-Yves Chibon <pingou@pingoured.fr>

---

diff --git a/doc/configuration.rst b/doc/configuration.rst
index de1dc3f..ce15216 100644
--- a/doc/configuration.rst
+++ b/doc/configuration.rst
@@ -693,6 +693,37 @@ To give access to just some projects (named ``rpms/test`` and
         }
     }
 
++REQUIRED_GROUPS
+~~~~~~~~~~~~~~~~
+
+The required groups allows to specify in which group an user must be to be
+added to a project with commit or admin access.
+
+Defaults to: ``{}``
+
+Example configuration::
+
+    REQUIRED_GROUPS = {
+        'rpms/kernel': ['packager', 'kernel-team'],
+        'modules/*': ['module-packager', 'packager'],
+        'rpms/*': ['packager'],
+        '*': ['contributor'],
+    }
+
+With this configuration (evaluated in the provided order):
+
+* only users that are in the groups ``packager`` and ``kernel-team`` will be
+  allowed to be added the ``rpms/kernel`` project (where ``rpms`` is the
+  namespace and ``kernel`` the project name).
+
+* only users that are in the groups ``module-packager`` and ``packager``
+  will be allowed to be added to projects in the ``modules`` namespace.
+
+* only users that are in the group ``packager`` will be allowed to be added
+  to projects in the ``rpms`` namespace.
+
+* only users in the ``contributor`` group will be allowed to be added to
+  any project on this pagure instance.
 
 
 Deprecated configuration keys
diff --git a/pagure/default_config.py b/pagure/default_config.py
index fa75262..52f0f6f 100644
--- a/pagure/default_config.py
+++ b/pagure/default_config.py
@@ -334,3 +334,7 @@ LOGGING = {
 # Gives commit access to all, all but some or just some project based on
 # groups provided by the auth system.
 EXTERNAL_COMMITTER = {}
+
+# Allows to require that the users are members of a certain group to be added
+# to a project (not a fork).
+REQUIRED_GROUPS = {}
diff --git a/pagure/lib/__init__.py b/pagure/lib/__init__.py
index 778f463..8501478 100644
--- a/pagure/lib/__init__.py
+++ b/pagure/lib/__init__.py
@@ -22,6 +22,7 @@ except ImportError:  # pragma: no cover
     import json
 
 import datetime
+import fnmatch
 import hashlib
 import logging
 import markdown
@@ -929,10 +930,26 @@ def add_deploykey_to_project(session, project, ssh_key, pushaccess, user):
     return 'Deploy key added'
 
 
-def add_user_to_project(session, project, new_user, user, access='admin'):
-    ''' Add a specified user to a specified project with a specified access'''
+def add_user_to_project(
+        session, project, new_user, user, access='admin',
+        required_groups=None):
+    ''' Add a specified user to a specified project with a specified access
+    '''
 
     new_user_obj = get_user(session, new_user)
+
+    if required_groups and access != 'ticket':
+        for key in required_groups:
+            if fnmatch.fnmatch(project.fullname, key):
+                user_grps = set(new_user_obj.groups)
+                req_grps = set(required_groups[key])
+                if not user_grps.intersection(req_grps):
+                    raise pagure.exceptions.PagureException(
+                        'This user must be in one of the following groups '
+                        'to be allowed to be added to this project: %s' %
+                        ', '.join(req_grps)
+                    )
+
     user_obj = get_user(session, user)
 
     users = set([
diff --git a/pagure/ui/repo.py b/pagure/ui/repo.py
index d749bc5..b2e3b26 100644
--- a/pagure/ui/repo.py
+++ b/pagure/ui/repo.py
@@ -1727,6 +1727,7 @@ def add_user(repo, username=None, namespace=None):
                 new_user=form.user.data,
                 user=flask.g.fas_user.username,
                 access=form.access.data,
+                required_groups=APP.config.get('REQUIRED_GROUPS')
             )
             SESSION.commit()
             pagure.lib.git.generate_gitolite_acls()
diff --git a/tests/test_fnmatch.py b/tests/test_fnmatch.py
new file mode 100644
index 0000000..f634ca2
--- /dev/null
+++ b/tests/test_fnmatch.py
@@ -0,0 +1,50 @@
+#!/usr/bin/env python
+# -*- coding: utf-8 -*-
+
+"""
+ (c) 2017 - Copyright Red Hat Inc
+
+ Authors:
+   Pierre-Yves Chibon <pingou@pingoured.fr>
+
+Tests the fnmatch method of the stdlib to ensure it works as expected
+elsewhere in the code.
+
+"""
+
+import os
+import sys
+import unittest
+
+import fnmatch
+
+
+class FnmatchTests(unittest.TestCase):
+    """Tests for the streaming server."""
+
+    def test_fnmatch(self):
+        """ Test the matching done by fnmatch. """
+        matrix = [
+            ['pagure', '*', True],
+            ['ns/pagure', '*', True],
+            ['forks/user/ns/pagure', '*', True],
+            ['forks/user/pagure', '*', True],
+            ['pagure', 'rpms/*', False],
+            ['rpms/pagure', 'rpms/*', True],
+            ['forks/user/pagure', 'rpms/*', False],
+            ['forks/user/pagure', 'rpms/*', False],
+            ['pagure', 'pagure', True],
+            ['rpms/pagure', 'pagure', False],
+            ['forks/user/pagure', 'pagure', False],
+            ['forks/user/pagure', 'pagure', False],
+            ['pagure', 'pag*', True],
+            ['rpms/pagure', 'pag*', False],
+            ['forks/user/pagure', 'pag*', False],
+            ['forks/user/pagure', 'pag*', False],
+        ]
+        for row in matrix:
+            self.assertEqual(fnmatch.fnmatch(row[0], row[1]), row[2])
+
+
+if __name__ == '__main__':
+    unittest.main(verbosity=2)
diff --git a/tests/test_pagure_lib_add_user_to_project.py b/tests/test_pagure_lib_add_user_to_project.py
new file mode 100644
index 0000000..dedae96
--- /dev/null
+++ b/tests/test_pagure_lib_add_user_to_project.py
@@ -0,0 +1,333 @@
+# -*- coding: utf-8 -*-
+
+"""
+ (c) 2017 - Copyright Red Hat Inc
+
+ Authors:
+   Pierre-Yves Chibon <pingou@pingoured.fr>
+
+"""
+
+import unittest
+import sys
+import os
+
+from mock import patch, MagicMock
+
+sys.path.insert(0, os.path.join(os.path.dirname(
+    os.path.abspath(__file__)), '..'))
+
+import pagure.lib
+import tests
+
+
+class PagureLibAddUserToProjecttests(tests.Modeltests):
+    """ Tests for pagure.lib.add_user_to_project """
+
+    def setUp(self):
+        """ Set up the environnment, ran before every tests. """
+        super(PagureLibAddUserToProjecttests, self).setUp()
+
+        tests.create_projects(self.session)
+
+        item = pagure.lib.model.User(
+            user='bar',
+            fullname='bar baz',
+            password='foo',
+            default_email='bar@bar.com',
+        )
+        self.session.add(item)
+        item = pagure.lib.model.UserEmail(
+            user_id=3,
+            email='bar@bar.com')
+        self.session.add(item)
+
+        self.session.commit()
+
+        # Before
+        repo = pagure.lib._get_project(self.session, 'test')
+        self.assertEqual(len(repo.users), 0)
+
+        msg = pagure.lib.add_user_to_project(
+            session=self.session,
+            project=repo,
+            new_user='foo',
+            user='pingou',
+        )
+        self.session.commit()
+        self.assertEqual(msg, 'User added')
+
+        # After
+        repo = pagure.lib._get_project(self.session, 'test')
+        self.assertEqual(len(repo.users), 1)
+        self.assertEqual(repo.users[0].user, 'foo')
+        self.assertEqual(repo.admins[0].user, 'foo')
+
+    @patch('pagure.lib.notify.send_email', MagicMock(return_value=True))
+    def test_re_add_user_to_project_default(self):
+        """ Update an existing user but to the same access level. """
+        repo = pagure.lib._get_project(self.session, 'test')
+
+        # Try adding the same user with the same access
+        self.assertRaises(
+            pagure.exceptions.PagureException,
+            pagure.lib.add_user_to_project,
+            session=self.session,
+            project=repo,
+            new_user='foo',
+            user='pingou',
+            access='admin'
+        )
+
+    @patch('pagure.lib.notify.send_email', MagicMock(return_value=True))
+    def test_update_user_to_project_default(self):
+        """ Update an existing user without any required group membership.
+        """
+        repo = pagure.lib._get_project(self.session, 'test')
+
+        # Update the access of the user
+        msg = pagure.lib.add_user_to_project(
+            session=self.session,
+            project=repo,
+            new_user='foo',
+            user='pingou',
+            access='commit'
+        )
+        self.session.commit()
+        self.assertEqual(msg, 'User access updated')
+        self.assertEqual(len(repo.users), 1)
+        self.assertEqual(repo.users[0].user, 'foo')
+        self.assertEqual(repo.committers[0].user, 'foo')
+
+    @patch('pagure.lib.notify.send_email', MagicMock(return_value=True))
+    def test_update_user_to_project_require_packager_on_all(self):
+        """
+        Update an existing user but required group membership on all
+        projects.
+        """
+        repo = pagure.lib._get_project(self.session, 'test')
+        config = {
+            '*': ['packager']
+        }
+
+        # Update the access of the user
+        self.assertRaises(
+            pagure.exceptions.PagureException,
+            pagure.lib.add_user_to_project,
+            session=self.session,
+            project=repo,
+            new_user='foo',
+            user='pingou',
+            access='admin',
+            required_groups=config
+        )
+        self.session.commit()
+        self.assertEqual(len(repo.users), 1)
+        self.assertEqual(repo.users[0].user, 'foo')
+        self.assertEqual(repo.committers[0].user, 'foo')
+
+    @patch('pagure.lib.notify.send_email', MagicMock(return_value=True))
+    def test_update_user_to_project_require_packager_on_st(self):
+        """
+        Update an existing user but required group membership on all
+        projects match *st.
+        """
+        repo = pagure.lib._get_project(self.session, 'test')
+        config = {
+            '*st': ['packager']
+        }
+
+        # Update the access of the user
+        self.assertRaises(
+            pagure.exceptions.PagureException,
+            pagure.lib.add_user_to_project,
+            session=self.session,
+            project=repo,
+            new_user='foo',
+            user='pingou',
+            access='admin',
+            required_groups=config
+        )
+        self.session.commit()
+        self.assertEqual(len(repo.users), 1)
+        self.assertEqual(repo.users[0].user, 'foo')
+        self.assertEqual(repo.committers[0].user, 'foo')
+
+    @patch('pagure.lib.notify.send_email', MagicMock(return_value=True))
+    def test_update_user_to_project_require_packager_on_te(self):
+        """
+        Update an existing user but required group membership on all
+        projects match te*.
+        """
+        repo = pagure.lib._get_project(self.session, 'test')
+        config = {
+            'te*': ['packager']
+        }
+
+        # Update the access of the user
+        self.assertRaises(
+            pagure.exceptions.PagureException,
+            pagure.lib.add_user_to_project,
+            session=self.session,
+            project=repo,
+            new_user='foo',
+            user='pingou',
+            access='admin',
+            required_groups=config
+        )
+        self.session.commit()
+        self.assertEqual(len(repo.users), 1)
+        self.assertEqual(repo.users[0].user, 'foo')
+        self.assertEqual(repo.committers[0].user, 'foo')
+
+    @patch('pagure.lib.notify.send_email', MagicMock(return_value=True))
+    def test_update_user_to_project_require_packager_on_test(self):
+        """
+        Update an existing user but required group membership on a specific
+        project: test.
+        """
+        repo = pagure.lib._get_project(self.session, 'test')
+        config = {
+            'test': ['packager']
+        }
+
+        # Update the access of the user
+        self.assertRaises(
+            pagure.exceptions.PagureException,
+            pagure.lib.add_user_to_project,
+            session=self.session,
+            project=repo,
+            new_user='foo',
+            user='pingou',
+            access='admin',
+            required_groups=config
+        )
+        self.session.commit()
+        self.assertEqual(len(repo.users), 1)
+        self.assertEqual(repo.users[0].user, 'foo')
+        self.assertEqual(repo.committers[0].user, 'foo')
+
+    @patch('pagure.lib.notify.send_email', MagicMock(return_value=True))
+    def test_add_user_to_test2_require_packager_on_test(self):
+        """
+        Add user to project test2 while the configuration requires group
+        membership on the project test.
+        """
+        repo = pagure.lib._get_project(self.session, 'test2')
+        self.assertEqual(len(repo.users), 0)
+
+        config = {
+            'test': ['packager']
+        }
+
+        # Add the user
+        pagure.lib.add_user_to_project(
+            session=self.session,
+            project=repo,
+            new_user='foo',
+            user='pingou',
+            access='admin',
+            required_groups=config
+        )
+        self.session.commit()
+        self.assertEqual(len(repo.users), 1)
+        self.assertEqual(repo.users[0].user, 'foo')
+        self.assertEqual(repo.committers[0].user, 'foo')
+
+class PagureLibAddUserToProjectWithGrouptests(
+        PagureLibAddUserToProjecttests):
+    """ Tests for pagure.lib.add_user_to_project """
+
+    def setUp(self):
+        """ Set up the environnment, ran before every tests. """
+        super(PagureLibAddUserToProjectWithGrouptests, self).setUp()
+
+        # Create group
+        msg = pagure.lib.add_group(
+            self.session,
+            group_name='packager',
+            display_name='packager',
+            description='The Fedora packager groups',
+            group_type='user',
+            user='pingou',
+            is_admin=False,
+            blacklist=[])
+        self.session.commit()
+        self.assertEqual(msg, 'User `pingou` added to the group `packager`.')
+
+        # Add user to group
+        group = pagure.lib.search_groups(self.session, group_name='packager')
+        msg = pagure.lib.add_user_to_group(
+            self.session,
+            username='bar',
+            group=group,
+            user='pingou',
+            is_admin=True)
+        self.session.commit()
+        self.assertEqual(msg, 'User `bar` added to the group `packager`.')
+
+    @patch('pagure.lib.notify.send_email', MagicMock(return_value=True))
+    def test_add_user_to_test_require_packager_on_test(self):
+        """
+        Add user to project test while the configuration requires group
+        membership on the project test.
+        """
+        repo = pagure.lib._get_project(self.session, 'test')
+        self.assertEqual(len(repo.users), 1)
+
+        config = {
+            'test': ['packager']
+        }
+
+        # Add the user to the project
+        pagure.lib.add_user_to_project(
+            session=self.session,
+            project=repo,
+            new_user='bar',
+            user='pingou',
+            access='commit',
+            required_groups=config
+        )
+        self.session.commit()
+
+        repo = pagure.lib._get_project(self.session, 'test')
+        self.assertEqual(len(repo.users), 2)
+        self.assertEqual(repo.users[0].user, 'foo')
+        self.assertEqual(repo.committers[0].user, 'foo')
+        self.assertEqual(repo.users[1].user, 'bar')
+        self.assertEqual(repo.committers[1].user, 'bar')
+
+    @patch('pagure.lib.notify.send_email', MagicMock(return_value=True))
+    def test_add_user_to_test_require_packager(self):
+        """
+        Add user to project test while the configuration requires group
+        membership on all the projects.
+        """
+        repo = pagure.lib._get_project(self.session, 'test')
+        self.assertEqual(len(repo.users), 1)
+
+        config = {
+            '*': ['packager']
+        }
+
+        # Add the user to the project
+        pagure.lib.add_user_to_project(
+            session=self.session,
+            project=repo,
+            new_user='bar',
+            user='pingou',
+            access='commit',
+            required_groups=config
+        )
+        self.session.commit()
+
+        repo = pagure.lib._get_project(self.session, 'test')
+        self.assertEqual(len(repo.users), 2)
+        self.assertEqual(repo.users[0].user, 'foo')
+        self.assertEqual(repo.committers[0].user, 'foo')
+        self.assertEqual(repo.users[1].user, 'bar')
+        self.assertEqual(repo.committers[1].user, 'bar')
+
+
+if __name__ == '__main__':
+    unittest.main(verbosity=2)

