From 13a34523a26faa23c5c10feb4be773e7f761232c Mon Sep 17 00:00:00 2001 From: Pierre-Yves Chibon Date: Jul 17 2018 12:52:29 +0000 Subject: Ensure we check the required group membership when giving a project away Signed-off-by: Pierre-Yves Chibon --- diff --git a/pagure/lib/__init__.py b/pagure/lib/__init__.py index 0a48a86..0dfd6dc 100644 --- a/pagure/lib/__init__.py +++ b/pagure/lib/__init__.py @@ -4767,14 +4767,30 @@ def search_token( return query.all() -def set_project_owner(session, project, user): +def set_project_owner(session, project, user, required_groups=None): ''' Set the ownership of a project :arg session: the session to use to connect to the database. :arg project: a Project object representing the project's ownership to change. :arg user: a User object representing the new owner of the project. + :arg required_groups: a dict of {pattern: [list of groups]} the new user + should be in to become owner if one of the pattern matches the + project fullname. :return: None ''' + + if required_groups: + for key in required_groups: + if fnmatch.fnmatch(project.fullname, key): + user_grps = set(user.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) + ) + for contributor in project.users: if user.id == contributor.id: project.users.remove(contributor) diff --git a/pagure/ui/repo.py b/pagure/ui/repo.py index 21d3ee9..082da09 100644 --- a/pagure/ui/repo.py +++ b/pagure/ui/repo.py @@ -2575,7 +2575,10 @@ def give_project(repo, username=None, namespace=None): 'No such user %s found' % new_username) try: old_main_admin = repo.user.user - pagure.lib.set_project_owner(flask.g.session, repo, new_owner) + pagure.lib.set_project_owner( + flask.g.session, repo, new_owner, + required_groups=pagure_config.get('REQUIRED_GROUPS') + ) # If the person doing the action is the former main admin, keep # them as admins if flask.g.fas_user.username == old_main_admin: @@ -2586,6 +2589,10 @@ def give_project(repo, username=None, namespace=None): pagure.lib.git.generate_gitolite_acls(project=repo) flask.flash( 'The project has been transferred to %s' % new_username) + except pagure.exceptions.PagureException as msg: + flask.g.session.rollback() + _log.debug(msg) + flask.flash(str(msg), 'error') except SQLAlchemyError: # pragma: no cover flask.g.session.rollback() flask.flash( diff --git a/tests/test_pagure_flask_ui_app_give_project.py b/tests/test_pagure_flask_ui_app_give_project.py index 3e71b22..46e8efc 100644 --- a/tests/test_pagure_flask_ui_app_give_project.py +++ b/tests/test_pagure_flask_ui_app_give_project.py @@ -41,6 +41,7 @@ class PagureFlaskGiveRepotests(tests.SimplePagureTest): tests.create_projects(self.session) tests.create_projects_git(os.path.join(self.path, 'repos'), bare=True) + self._check_user(user='pingou') def _check_user(self, user='pingou'): self.session.commit() @@ -320,6 +321,95 @@ class PagureFlaskGiveRepotests(tests.SimplePagureTest): self.assertEqual(len(project.users), 1) self.assertEqual(project.users[0].user, 'pingou') + @patch.dict('pagure.config.config', {'REQUIRED_GROUPS': {'*': ['packager']}}) + @patch.dict('pagure.config.config', {'PAGURE_ADMIN_USERS': 'foo'}) + @patch('pagure.lib.git.generate_gitolite_acls', MagicMock()) + def test_give_project_not_in_required_group(self): + """ Test the give_project endpoint. """ + + user = tests.FakeUser() + user.username = 'pingou' + with tests.user_set(self.app.application, user): + csrf_token = self.get_csrf() + + self._check_user() + + # User not a packager + data = { + 'user': 'foo', + 'csrf_token': csrf_token, + } + + output = self.app.post( + '/test/give', data=data, follow_redirects=True) + self.assertEqual(output.status_code, 200) + self.assertIn( + ' This user must be in one of the following groups to ' + 'be allowed to be added to this project: packager', + output.get_data(as_text=True)) + + self._check_user(user='pingou') + + @patch.dict('pagure.config.config', {'REQUIRED_GROUPS': {'*': ['packager']}}) + @patch.dict('pagure.config.config', {'PAGURE_ADMIN_USERS': 'foo'}) + @patch('pagure.lib.git.generate_gitolite_acls', MagicMock()) + def test_give_project_in_required_group(self): + """ Test the give_project endpoint. """ + + # Create the packager group + msg = pagure.lib.add_group( + self.session, + group_name='packager', + display_name='packager group', + description=None, + group_type='user', + user='pingou', + is_admin=False, + blacklist=[], + ) + self.session.commit() + self.assertEqual(msg, 'User `pingou` added to the group `packager`.') + + # Add foo to the packager group + group = pagure.lib.search_groups(self.session, group_name='packager') + msg = pagure.lib.add_user_to_group( + self.session, + username='foo', + group=group, + user='pingou', + is_admin=False, + ) + self.session.commit() + self.assertEqual(msg, 'User `foo` added to the group `packager`.') + + # pingou transferts test to foo + user = tests.FakeUser() + user.username = 'pingou' + with tests.user_set(self.app.application, user): + csrf_token = self.get_csrf() + + self._check_user() + + # User not a packager + data = { + 'user': 'foo', + 'csrf_token': csrf_token, + } + + output = self.app.post( + '/test/give', data=data, follow_redirects=True) + self.assertEqual(output.status_code, 200) + self.assertIn( + ' The project has been transferred to foo', + output.get_data(as_text=True)) + + self._check_user('foo') + # Make sure that the user giving the project is still an admin + project = pagure.lib.get_authorized_project( + self.session, project_name='test') + self.assertEqual(len(project.users), 1) + self.assertEqual(project.users[0].user, 'pingou') + if __name__ == '__main__': unittest.main(verbosity=2)