From a71ef51c443aa44a7a97bfd8df3ef222b3ec147a Mon Sep 17 00:00:00 2001 From: Pierre-Yves Chibon Date: Jun 07 2017 10:19:30 +0000 Subject: [PATCH 1/3] Fix the FakeUser object to the groups are always re-created Signed-off-by: Pierre-Yves Chibon --- diff --git a/tests/__init__.py b/tests/__init__.py index 6297aa4..521316b 100644 --- a/tests/__init__.py +++ b/tests/__init__.py @@ -337,7 +337,7 @@ class FakeGroup(object): # pylint: disable=too-few-public-methods class FakeUser(object): # pylint: disable=too-few-public-methods """ Fake user used to test the fedocallib library. """ - def __init__(self, groups=[], username='username', cla_done=True, id=1): + def __init__(self, groups=None, username='username', cla_done=True, id=1): """ Constructor. :arg groups: list of the groups in which this fake user is supposed to be. @@ -345,7 +345,7 @@ class FakeUser(object): # pylint: disable=too-few-public-methods if isinstance(groups, basestring): groups = [groups] self.id = id - self.groups = groups + self.groups = groups or [] self.user = username self.username = username self.name = username From a1fca312bc12eff307e7e122568eb1617022696d Mon Sep 17 00:00:00 2001 From: Pierre-Yves Chibon Date: Jun 07 2017 10:38:15 +0000 Subject: [PATCH 2/3] 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) From aec95f154df5ff60e92ba1bb93d30b21e200b649 Mon Sep 17 00:00:00 2001 From: Pierre-Yves Chibon Date: Jun 07 2017 10:38:21 +0000 Subject: [PATCH 3/3] Fix warning when generating the sphinx documentation Signed-off-by: Pierre-Yves Chibon --- diff --git a/doc/usage/forks.rst b/doc/usage/forks.rst index 5b4ac4d..362280d 100644 --- a/doc/usage/forks.rst +++ b/doc/usage/forks.rst @@ -64,7 +64,7 @@ Afterwards, you are ready to push your changes to your remote fork:: $ git push -u origin my-feature-or-bugfix # upload changes In case you cloned the repo via HTTP, for example using a command like `git -clone https://...`_, the push will fail. Pagure.io does not support pushing +clone https://...`, the push will fail. Pagure.io does not support pushing over HTTP. An easy workaround is to use:: $ git push -u origin my-feature-or-bugfix ssh://git@pagure.io/forks/jcline/pagure.git