diff --git a/pagure/exceptions.py b/pagure/exceptions.py index 5ff4655..dcd569e 100644 --- a/pagure/exceptions.py +++ b/pagure/exceptions.py @@ -111,3 +111,9 @@ class PagureEncodingException(PagureException, ValueError): """ pass + + +class PagurePushDenied(PagureException): + """ Exception raised if a remote hook rejected a push """ + + pass diff --git a/pagure/hooks/__init__.py b/pagure/hooks/__init__.py index 7f567ef..d5467fc 100644 --- a/pagure/hooks/__init__.py +++ b/pagure/hooks/__init__.py @@ -21,6 +21,7 @@ from pagure.config import config as pagure_config from pagure.exceptions import FileNotFoundException import pagure.lib import pagure.lib.git +from pagure.lib.git_auth import get_git_auth_helper from pagure.lib.plugins import get_enabled_plugins @@ -243,7 +244,15 @@ class BaseHook(object): def run_project_hooks( - session, username, project, hooktype, repotype, repodir, changes + session, + username, + project, + hooktype, + repotype, + repodir, + changes, + is_internal, + pull_request, ): """ Function to run the hooks on a project @@ -264,9 +273,63 @@ def run_project_hooks( or post-receive changes (dict): A dict with keys being the ref to update, values being a tuple of (from, to). + is_internal (bool): Whether this push originated from Pagure internally + pull_request (model.PullRequest or None): The pull request whose merge + is initiating this hook run. """ debug = pagure_config.get("HOOK_DEBUG", False) + # First we run dynamic ACLs + authbackend = get_git_auth_helper() + + if ( + is_internal + and username == "pagure" + and repotype in ("tickets", "requests") + ): + if debug: + print("This is an internal push, dynamic ACL is pre-approved") + elif not authbackend.is_dynamic: + if debug: + print("Auth backend %s is static-only" % authbackend) + else: + if debug: + print( + "Checking push request against auth backend %s" % authbackend + ) + todeny = [] + for refname in changes: + change = changes[refname] + authresult = authbackend.check_acl( + session, + project, + username, + refname, + is_update=hooktype == "update", + revfrom=change[0], + revto=change[1], + is_internal=is_internal, + pull_request=pull_request, + repotype=repotype, + ) + if debug: + print( + "Auth result for ref %s: %s" + % (refname, "Accepted" if authresult else "Denied") + ) + if not authresult: + print( + "Denied push for ref '%s' for user '%s'" + % (refname, username) + ) + todeny.append(refname) + for toremove in todeny: + del changes[toremove] + if not changes: + print("All changes have been rejected") + sys.exit(1) + + # Now we run the hooks for plugins for plugin, _ in get_enabled_plugins(project): if not plugin.runner: if debug: @@ -368,7 +431,17 @@ def run_hook_file(hooktype): else: raise ValueError("Hook type %s not valid" % hooktype) + session = pagure.lib.create_session(pagure_config["DB_URL"]) + if not session: + raise Exception("Unable to initialize db session") + pushuser = os.environ.get("GL_USER") + is_internal = os.environ.get("internal", False) == "yes" + pull_request = None + if "pull_request_uid" in os.environ: + pull_request = pagure.lib.get_request_by_uid( + session, os.environ["pull_request_uid"] + ) if pagure_config.get("HOOK_DEBUG", False): print("Changes: %s" % changes) @@ -381,14 +454,18 @@ def run_hook_file(hooktype): repo, ) = pagure.lib.git.get_repo_info_from_path(gitdir) - session = pagure.lib.create_session(pagure_config["DB_URL"]) - if not session: - raise Exception("Unable to initialize db session") - project = pagure.lib._get_project( session, repo, user=username, namespace=namespace ) run_project_hooks( - session, pushuser, project, hooktype, repotype, gitdir, changes + session, + pushuser, + project, + hooktype, + repotype, + gitdir, + changes, + is_internal, + pull_request, ) diff --git a/pagure/lib/git.py b/pagure/lib/git.py index 8e304aa..50b8c1d 100644 --- a/pagure/lib/git.py +++ b/pagure/lib/git.py @@ -974,20 +974,37 @@ class TemporaryClone(object): ) try: + _log.debug( + "Running a git push of %s to %s" + % (pushref, self._project.fullname) + ) env = os.environ.copy() env["GL_USER"] = username env.update(extra) - subprocess.check_output( + out = subprocess.check_output( ["git"] + opts + ["push", "origin", pushref], cwd=self.repopath, stderr=subprocess.STDOUT, env=env, ) + _log.debug("Output: %s" % out) except subprocess.CalledProcessError as ex: # This should never really happen, since we control the repos, but # this way, we can be sure to get the output logged - _log.exception("Error pushing. Output: %s", ex.output) - raise + remotes = [] + for line in ex.output.decode("utf-8").split("\n"): + if line.startswith("remote: "): + _log.debug("Remote: %s" % line) + remotes.append(line[len("remote: ") :].strip()) + if remotes: + _log.info("Remote rejected with: %s" % remotes) + raise pagure.exceptions.PagurePushDenied( + "Remote hook declined the push: %s" % "\n".join(remotes) + ) + else: + # Something else happened, pass the original + _log.exception("Error pushing. Output: %s", ex.output) + raise def _update_file_in_git( diff --git a/pagure/lib/git_auth.py b/pagure/lib/git_auth.py index c7a2f58..eed98e3 100644 --- a/pagure/lib/git_auth.py +++ b/pagure/lib/git_auth.py @@ -10,6 +10,7 @@ from __future__ import print_function, unicode_literals import abc +import json import logging import os import pkg_resources @@ -64,6 +65,8 @@ class GitAuthHelper(with_metaclass(abc.ABCMeta, object)): helper. """ + is_dynamic = False + @classmethod @abc.abstractmethod def generate_acls(self, project, group=None): @@ -106,6 +109,45 @@ class GitAuthHelper(with_metaclass(abc.ABCMeta, object)): """ pass + @classmethod + # This method can't be marked as abstract, since it's new and that would + # break backwards compatibility + def check_acl(cls, session, project, username, refname, **info): + """ This method is used in Dynamic Git Auth helpers to check acls. + + It is acceptable for implementations to print things, which will be + returned to the user. + + Please make sure to add a **kwarg in any implementation, even if + specific keyword arguments are added for the known fields, to make + sure your implementation remains working if new items are added. + + Args: + session (sqlalchemy.Session): Database session + project (model.Project): Project instance push is for + username (string): The name of the user trying to push + refname (string): The name of the ref being pushed to + Kwargs: + Extra arguments to help in deciding whether to approve or deny a + push. This may get additional possible values later on, but will + have at least: + - is_update (bool): Whether this is being run at the "update" hook + moment. See the return type notes to see the differences. + - revfrom (string): The commit hash the update is happening from. + - revto (string): The commit hash the update is happening to. + - pull_request (model.PullRequest or None): The PR that is trying + to be merged. + Returns (bool): Whether to allow this push. + If is_update is False and the ACL returns False, the entire push + is aborted. If is_update is True and the ACL returns True, only + a single ref update is blocked. So if you want to block just a + single ref from being updated, only return False if is_update + is True. + """ + raise NotImplementedError( + "check_acl on static Git Auth Backend called" + ) + def _read_file(filename): """ Reads the specified file and return its content. @@ -778,6 +820,8 @@ class Gitolite3Auth(Gitolite2Auth): class GitAuthTestHelper(GitAuthHelper): """ Simple test auth module to check the auth customization system. """ + is_dynamic = True + @classmethod def generate_acls(cls, project, group=None): """ Print a statement when called, useful for debugging, only. @@ -816,3 +860,28 @@ class GitAuthTestHelper(GitAuthHelper): ) print(out) return out + + @classmethod + def check_acl( + cls, session, project, username, refname, pull_request, **info + ): + testfile = pagure_config.get("TEST_AUTH_STATUS", None) + if not testfile or not os.path.exists(testfile): + # If we are not configured, we will assume allowed + return True + + with open(testfile, "r") as statusfile: + status = json.loads(statusfile.read()) + + if status is True or status is False: + return status + + # Other option would be a dict with ref->allow + # (with allow True, pronly), missing means False) + if refname not in status: + print("ref '%s' not in status" % refname) + return False + elif status[refname] is True: + return True + elif status[refname] == "pronly": + return pull_request is not None diff --git a/tests/__init__.py b/tests/__init__.py index d24842c..aeb0d2e 100644 --- a/tests/__init__.py +++ b/tests/__init__.py @@ -73,6 +73,9 @@ PAGLOG = logging.getLogger('pagure') PAGLOG.setLevel(logging.CRITICAL) PAGLOG.handlers = [] +if 'PYTHONPATH' not in os.environ: + os.environ['PYTHONPATH'] = os.path.normpath(os.path.join(HERE, '../')) + CONFIG_TEMPLATE = """ GIT_FOLDER = '%(path)s/repos' ENABLE_DOCS = %(enable_docs)s @@ -94,6 +97,8 @@ CELERY_CONFIG = { "task_always_eager": True, #"task_eager_propagates": True, } +GIT_AUTH_BACKEND = '%(authbackend)s' +TEST_AUTH_STATUS = '%(path)s/testauth_status.json' REPOSPANNER_NEW_REPO = %(repospanner_new_repo)s REPOSPANNER_NEW_REPO_ADMIN_OVERRIDE = %(repospanner_admin_override)s REPOSPANNER_NEW_FORK = %(repospanner_new_fork)s @@ -377,6 +382,7 @@ class SimplePagureTest(unittest.TestCase): 'enable_tickets': True, 'tickets_folder': '%s/repos/tickets' % self.path, 'global_path': tests_state["path"], + 'authbackend': 'gitolite3', 'repospanner_gitport': '8443', 'repospanner_new_repo': 'None', @@ -451,6 +457,11 @@ class SimplePagureTest(unittest.TestCase): self.session.execute("SET FOREIGN_KEY_CHECKS = 1") self.session.commit() + def set_auth_status(self, value): + """ Set the return value for the test auth """ + with open(os.path.join(self.path, 'testauth_status.json'), 'w') as statusfile: + statusfile.write(six.u(json.dumps(value))) + def get_csrf(self, url='/new', output=None): """Retrieve a CSRF token from given URL.""" if output is None: @@ -498,6 +509,29 @@ class Modeltests(SimplePagureTest): tests_state["broker_client"].flushall() super(Modeltests, self).tearDown() + def create_project_full(self, projectname): + """ Create a project via the API. + + This makes sure that the repo is fully setup the way a normal new + project would be, with hooks and all setup. + """ + + headers = {'Authorization': 'token aaabbbcccddd'} + data = { + 'name': projectname, + 'description': 'A test repo', + } + + # Valid request + output = self.app.post( + '/api/0/new/', data=data, headers=headers) + self.assertEqual(output.status_code, 200) + data = json.loads(output.get_data(as_text=True)) + self.assertDictEqual( + data, + {'message': 'Project "%s" created' % projectname} + ) + class FakeGroup(object): # pylint: disable=too-few-public-methods """ Fake object used to make the FakeUser object closer to the diff --git a/tests/test_pagure_lib_git_auth.py b/tests/test_pagure_lib_git_auth.py new file mode 100644 index 0000000..d6e0b6a --- /dev/null +++ b/tests/test_pagure_lib_git_auth.py @@ -0,0 +1,227 @@ +# -*- coding: utf-8 -*- + +""" + (c) 2015-2018 - Copyright Red Hat Inc + + Authors: + Pierre-Yves Chibon + Patrick Uiterwijk + +""" + +from __future__ import unicode_literals + +__requires__ = ['SQLAlchemy >= 0.8'] + +import pkg_resources + +import datetime +import os +import shutil +import sys +import tempfile +import time +import unittest + +import pygit2 +import six +from mock import patch, MagicMock + +sys.path.insert(0, os.path.join(os.path.dirname( + os.path.abspath(__file__)), '..')) + +import pagure +import pagure.lib.git +import tests + +from pagure.lib.repo import PagureRepo + + +class PagureLibGitAuthtests(tests.Modeltests): + """ Tests for pagure.lib.git_auth """ + config_values = {'authbackend': 'test_auth'} + + def setUp(self): + super(PagureLibGitAuthtests, self).setUp() + + tests.create_projects(self.session) + tests.create_tokens(self.session) + tests.create_tokens_acl(self.session) + self.create_project_full('hooktest') + + def test_edit_with_all_allowed(self): + """Tests that editing a file is possible if ACLs say allowed.""" + user = tests.FakeUser() + user.username = 'pingou' + with tests.user_set(self.app.application, user): + # Add some content to the git repo + tests.add_content_git_repo( + os.path.join(self.path, 'repos', 'hooktest.git')) + + data = { + 'content': 'foo\n bar\n baz', + 'commit_title': 'test commit', + 'commit_message': 'Online commits from the gure.lib.get', + 'email': 'bar@pingou.com', + 'branch': 'master', + 'csrf_token': self.get_csrf(), + } + + output = self.app.post( + '/hooktest/edit/master/f/sources', data=data, + follow_redirects=True) + self.assertEqual(output.status_code, 200) + output_text = output.get_data(as_text=True) + self.assertIn( + 'Commits - hooktest - Pagure', output_text) + self.assertIn('test commit', output_text) + + # Check file after the commit + output = self.app.get('/hooktest/raw/master/f/sources') + self.assertEqual(output.status_code, 200) + output_text = output.get_data(as_text=True) + self.assertEqual(output_text, 'foo\n bar\n baz') + + def test_edit_with_all_denied(self): + """Tests that editing a file is not possible if ACLs say denied.""" + self.set_auth_status(False) + + user = tests.FakeUser() + user.username = 'pingou' + with tests.user_set(self.app.application, user): + # Add some content to the git repo + tests.add_content_git_repo( + os.path.join(self.path, 'repos', 'hooktest.git')) + + output = self.app.get('/hooktest/edit/master/f/sources') + self.assertEqual(output.status_code, 200) + output_text = output.get_data(as_text=True) + csrf_token = output_text.split( + 'name="csrf_token" type="hidden" value="')[1].split('">')[0] + + data = { + 'content': 'foo\n bar\n baz', + 'commit_title': 'test commit', + 'commit_message': 'Online commits from the gure.lib.get', + 'email': 'bar@pingou.com', + 'branch': 'master', + 'csrf_token': csrf_token, + } + + output = self.app.post( + '/hooktest/edit/master/f/sources', data=data, + follow_redirects=True) + self.assertEqual(output.status_code, 200) + output_text = output.get_data(as_text=True) + self.assertIn( + "Remote hook declined the push: " + "Denied push for ref 'refs/heads/master' for user 'pingou'\n" + "All changes have been rejected", + output_text + ) + + # Check file after the commit: + output = self.app.get('/hooktest/raw/master/f/sources') + self.assertEqual(output.status_code, 200) + output_text = output.get_data(as_text=True) + self.assertEqual(output_text, 'foo\n bar') + + def test_edit_pr(self): + """Tests the ACLs if they only accept PRs.""" + self.set_auth_status({'refs/heads/master': 'pronly', + 'refs/heads/source': True}) + + user = tests.FakeUser() + user.username = 'pingou' + with tests.user_set(self.app.application, user): + # Add some content to the git repo + tests.add_content_git_repo( + os.path.join(self.path, 'repos', 'hooktest.git')) + + output = self.app.get('/hooktest/edit/master/f/sources') + self.assertEqual(output.status_code, 200) + output_text = output.get_data(as_text=True) + csrf_token = output_text.split( + 'name="csrf_token" type="hidden" value="')[1].split('">')[0] + + # Try editing master branch, should fail (only PRs allowed) + data = { + 'content': 'foo\n bar\n baz', + 'commit_title': 'test commit', + 'commit_message': 'Online commits from the gure.lib.get', + 'email': 'bar@pingou.com', + 'branch': 'master', + 'csrf_token': csrf_token, + } + output = self.app.post( + '/hooktest/edit/master/f/sources', data=data, + follow_redirects=True) + self.assertEqual(output.status_code, 200) + output_text = output.get_data(as_text=True) + self.assertIn( + "Remote hook declined the push: " + "Denied push for ref 'refs/heads/master' for user 'pingou'\n" + "All changes have been rejected", + output_text + ) + + # Change something in the "source" branch + data = { + 'content': 'foo\n bar\n baz', + 'commit_title': 'test commit', + 'commit_message': 'Online commits from the gure.lib.get', + 'email': 'bar@pingou.com', + 'branch': 'source', + 'csrf_token': csrf_token, + } + + output = self.app.post( + '/hooktest/edit/master/f/sources', data=data, + follow_redirects=True) + self.assertEqual(output.status_code, 200) + output_text = output.get_data(as_text=True) + self.assertIn( + 'Commits - hooktest - Pagure', output_text) + self.assertIn('test commit', output_text) + + # Check file after the commit: + output = self.app.get('/hooktest/raw/source/f/sources') + self.assertEqual(output.status_code, 200) + output_text = output.get_data(as_text=True) + self.assertEqual(output_text, 'foo\n bar\n baz') + + # Create the PRs + project = pagure.lib.get_authorized_project(self.session, 'hooktest') + req = pagure.lib.new_pull_request( + session=self.session, + repo_from=project, + branch_from="source", + repo_to=project, + branch_to='master', + title='PR to master', + user='pingou', + ) + self.session.add(req) + self.session.commit() + + # Check file before the merge + output = self.app.get('/hooktest/raw/master/f/sources') + self.assertEqual(output.status_code, 200) + output_text = output.get_data(as_text=True) + self.assertEqual(output_text, 'foo\n bar') + + # Try to merge (should work) + output = self.app.post( + '/hooktest/pull-request/1/merge', data=data, follow_redirects=True) + self.assertEqual(output.status_code, 200) + output_text = output.get_data(as_text=True) + self.assertIn( + 'Overview - hooktest - Pagure', + output_text + ) + + # Check file after the merge + output = self.app.get('/hooktest/raw/master/f/sources') + self.assertEqual(output.status_code, 200) + output_text = output.get_data(as_text=True) + self.assertEqual(output_text, 'foo\n bar\n baz')