From a1fca312bc12eff307e7e122568eb1617022696d Mon Sep 17 00:00:00 2001 From: Pierre-Yves Chibon Date: Jun 07 2017 10:38:15 +0000 Subject: Introduce the external committer feature This feature allows to grant commit ACL to user via groups that are specified by the authentication system upon login. This allows by-passing the group mechanism of pagure itself, thus allowing to grant commit access without creating the corresponding group and without notifying every member of that group for changes to project they have access to. This feature allows to grant access to all the projects of the instance, only some (using `restrict`) or to all but some (using `exclude`). Signed-off-by: Pierre-Yves Chibon --- diff --git a/doc/configuration.rst b/doc/configuration.rst index 9690307..de1dc3f 100644 --- a/doc/configuration.rst +++ b/doc/configuration.rst @@ -651,6 +651,49 @@ Defaults to: ``['pretty please pagure-ci rebuild']`` case only! +EXTERNAL_COMMITTER +~~~~~~~~~~~~~~~~~~ + +The external committer feature is a way to allow members of groups defined +outside pagure (and provided to pagure upon login by the authentication +system) to be consider committers on pagure. + +This feature can give access to all the projects on the instance, all but +some or just some. + +Defaults to: ``{}`` + +To give access to all the projects to a group named ``fedora-altarch`` use +a such a structure:: + + EXTERNAL_COMMITTER = { + 'fedora-altarch': {} + } + +To give access to all the projects but one (named ``rpms/test``) to a group +named ``provenpackager`` use a such a structure:: + + EXTERNAL_COMMITTER = { + 'fedora-altarch': {}, + 'provenpackager': { + 'exclude': ['rpms/test'] + } + } + +To give access to just some projects (named ``rpms/test`` and +``modules/test``) to a group named ``testers`` use a such a structure:: + + EXTERNAL_COMMITTER = { + 'fedora-altarch': {}, + 'provenpackager': { + 'exclude': ['rpms/test'] + }, + 'testers': { + 'restrict': ['rpms/test', 'modules/test'] + } + } + + Deprecated configuration keys ----------------------------- diff --git a/pagure/__init__.py b/pagure/__init__.py index 124ccd0..335a12a 100644 --- a/pagure/__init__.py +++ b/pagure/__init__.py @@ -325,6 +325,21 @@ def is_repo_committer(repo_obj): if is_admin(): return True + grps = flask.g.fas_user.groups + ext_committer = APP.config.get('EXTERNAL_COMMITTER', None) + if ext_committer: + overlap = set(ext_committer).intersection(grps) + if overlap: + for grp in overlap: + restrict = ext_committer[grp].get('restrict', []) + exclude = ext_committer[grp].get('exclude', []) + if restrict and repo_obj.fullname not in restrict: + return False + elif repo_obj.fullname in exclude: + return False + else: + return True + usergrps = [ usr.user for grp in repo_obj.committer_groups @@ -573,6 +588,8 @@ def auth_login(): # pragma: no cover SESSION, group_type='user') ] groups = set(groups).union(admins) + ext_committer = set(APP.config.get('EXTERNAL_COMMITTER', {})) + groups = set(groups).union(ext_committer) return FAS.login(return_url=return_point, groups=groups) elif APP.config.get('PAGURE_AUTH', None) == 'local': form = pagure.login_forms.LoginForm() diff --git a/pagure/default_config.py b/pagure/default_config.py index 21b0318..fa75262 100644 --- a/pagure/default_config.py +++ b/pagure/default_config.py @@ -330,3 +330,7 @@ LOGGING = { }, } } + +# Gives commit access to all, all but some or just some project based on +# groups provided by the auth system. +EXTERNAL_COMMITTER = {} diff --git a/tests/test_pagure_flask.py b/tests/test_pagure_flask.py index 4d789de..9157677 100644 --- a/tests/test_pagure_flask.py +++ b/tests/test_pagure_flask.py @@ -17,6 +17,7 @@ import sys import os import mock +import munch import pygit2 import werkzeug @@ -27,6 +28,7 @@ import pagure.lib import pagure.lib.model import tests + class PagureGetRemoteRepoPath(tests.Modeltests): """ Tests for pagure """ @@ -49,6 +51,139 @@ class PagureGetRemoteRepoPath(tests.Modeltests): self.assertTrue(output.endswith('repos_test2.git_master')) + def test_is_repo_committer_logged_out(self): + """ Test is_repo_committer in pagure when there is no logged in user. + """ + repo = pagure.lib._get_project(self.session, 'test') + output = pagure.is_repo_committer(repo) + self.assertFalse(output) + + def test_is_repo_committer_logged_in(self): + """ Test is_repo_committer in pagure with the appropriate user logged + in. """ + repo = pagure.lib._get_project(self.session, 'test') + + g = munch.Munch() + g.fas_user = tests.FakeUser(username='pingou') + with mock.patch('pagure.flask.g', g): + output = pagure.is_repo_committer(repo) + self.assertTrue(output) + + def test_is_repo_committer_logged_in_wrong_user(self): + """ Test is_repo_committer in pagure with the wrong user logged in. + """ + repo = pagure.lib._get_project(self.session, 'test') + + g = munch.Munch() + g.fas_user = tests.FakeUser() + with mock.patch('pagure.flask.g', g): + output = pagure.is_repo_committer(repo) + self.assertFalse(output) + + # Mocked config + config = { + 'provenpackager': {} + } + + @mock.patch.dict('pagure.APP.config', {'EXTERNAL_COMMITTER': config}) + def test_is_repo_committer_external_committer_generic_no_member(self): + """ Test is_repo_committer in pagure with EXTERNAL_COMMITTER + configured to give access to all the provenpackager, but the user + is not one. + """ + repo = pagure.lib._get_project(self.session, 'test') + + user = tests.FakeUser() + g = munch.Munch() + g.fas_user = user + with mock.patch('pagure.flask.g', g): + output = pagure.is_repo_committer(repo) + self.assertFalse(output) + + @mock.patch.dict('pagure.APP.config', {'EXTERNAL_COMMITTER': config}) + def test_is_repo_committer_external_committer_generic_member(self): + """ Test is_repo_committer in pagure with EXTERNAL_COMMITTER + configured to give access to all the provenpackager, and the user + is one + """ + repo = pagure.lib._get_project(self.session, 'test') + + g = munch.Munch() + g.fas_user = tests.FakeUser() + g.fas_user.groups.append('provenpackager') + with mock.patch('pagure.flask.g', g): + output = pagure.is_repo_committer(repo) + self.assertTrue(output) + + config = { + 'provenpackager': { + 'exclude': ['test'] + } + } + + @mock.patch.dict('pagure.APP.config', {'EXTERNAL_COMMITTER': config}) + def test_is_repo_committer_external_committer_excluding_one(self): + """ Test is_repo_committer in pagure with EXTERNAL_COMMITTER + configured to give access to all the provenpackager but for this + one repo + """ + repo = pagure.lib._get_project(self.session, 'test') + + g = munch.Munch() + g.fas_user = tests.FakeUser() + g.fas_user.groups.append('provenpackager') + with mock.patch('pagure.flask.g', g): + output = pagure.is_repo_committer(repo) + self.assertFalse(output) + + config = { + 'provenpackager': { + 'restrict': ['test'] + } + } + + @mock.patch.dict('pagure.APP.config', {'EXTERNAL_COMMITTER': config}) + def test_is_repo_committer_external_committer_restricted_not_member(self): + """ Test is_repo_committer in pagure with EXTERNAL_COMMITTER + configured to give access the provenpackager just for one repo + """ + repo = pagure.lib._get_project(self.session, 'test') + + g = munch.Munch() + g.fas_user = tests.FakeUser() + with mock.patch('pagure.flask.g', g): + output = pagure.is_repo_committer(repo) + self.assertFalse(output) + + @mock.patch.dict('pagure.APP.config', {'EXTERNAL_COMMITTER': config}) + def test_is_repo_committer_external_committer_restricting_to_one(self): + """ Test is_repo_committer in pagure with EXTERNAL_COMMITTER + configured to give access the provenpackager just for one repo + """ + repo = pagure.lib._get_project(self.session, 'test') + + g = munch.Munch() + g.fas_user = tests.FakeUser() + g.fas_user.groups.append('provenpackager') + with mock.patch('pagure.flask.g', g): + output = pagure.is_repo_committer(repo) + self.assertTrue(output) + + @mock.patch.dict('pagure.APP.config', {'EXTERNAL_COMMITTER': config}) + def test_is_repo_committer_external_committer_restricting_another_one(self): + """ Test is_repo_committer in pagure with EXTERNAL_COMMITTER + configured to give access the provenpackager just for one repo not + this one + """ + repo = pagure.lib._get_project(self.session, 'test2') + + g = munch.Munch() + g.fas_user = tests.FakeUser() + g.fas_user.groups.append('provenpackager') + with mock.patch('pagure.flask.g', g): + output = pagure.is_repo_committer(repo) + self.assertFalse(output) + if __name__ == '__main__': unittest.main(verbosity=2)