# HG changeset patch # User Marcin Kuzminski # Date 2018-08-28 18:24:40 # Node ID 1e14730dc0d36d88737dcae1e97f413be9975bf7 # Parent 5cb2321ac1559cb1ec37d31a7de99d2f5504d216 pull-requests: add merge validation to prevent merges to protected branches. diff --git a/rhodecode/api/tests/test_create_pull_request.py b/rhodecode/api/tests/test_create_pull_request.py --- a/rhodecode/api/tests/test_create_pull_request.py +++ b/rhodecode/api/tests/test_create_pull_request.py @@ -252,8 +252,9 @@ class TestCreatePullRequestApi(object): id_, params = build_data( self.apikey_regular, 'create_pull_request', **data) response = api_call(self.app, params) - expected_message = 'The specified branch `{}` does not exist'.format( - branch_name) + expected_message = 'The specified value:{type}:`{name}` ' \ + 'does not exist, or is not allowed.'.format(type='branch', + name=branch_name) assert_error(id_, expected_message, given=response.body) @pytest.mark.backends("git", "hg") diff --git a/rhodecode/api/tests/test_utils.py b/rhodecode/api/tests/test_utils.py --- a/rhodecode/api/tests/test_utils.py +++ b/rhodecode/api/tests/test_utils.py @@ -87,7 +87,8 @@ class TestResolveRefOrError(object): ref = 'ancestor:ref' with pytest.raises(JSONRPCError) as excinfo: utils.resolve_ref_or_error(ref, repo) - expected_message = 'The specified ancestor `ref` does not exist' + expected_message = ( + 'The specified value:ancestor:`ref` does not exist, or is not allowed.') assert excinfo.value.message == expected_message def test_branch_is_not_found(self): @@ -99,7 +100,7 @@ class TestResolveRefOrError(object): with pytest.raises(JSONRPCError) as excinfo: utils.resolve_ref_or_error(ref, repo) expected_message = ( - 'The specified branch `non-existing-one` does not exist') + 'The specified value:branch:`non-existing-one` does not exist, or is not allowed.') assert excinfo.value.message == expected_message def test_bookmark_is_not_found(self): @@ -111,7 +112,7 @@ class TestResolveRefOrError(object): with pytest.raises(JSONRPCError) as excinfo: utils.resolve_ref_or_error(ref, repo) expected_message = ( - 'The specified bookmark `non-existing-one` does not exist') + 'The specified value:bookmark:`non-existing-one` does not exist, or is not allowed.') assert excinfo.value.message == expected_message @pytest.mark.parametrize("ref", ['ref', '12345', 'a:b:c:d']) diff --git a/rhodecode/api/utils.py b/rhodecode/api/utils.py --- a/rhodecode/api/utils.py +++ b/rhodecode/api/utils.py @@ -403,7 +403,7 @@ def resolve_ref_or_error(ref, repo): ref_hash = ref_hash or _get_ref_hash(repo, ref_type, ref_name) except (KeyError, ValueError): raise JSONRPCError( - 'The specified {type} `{name}` does not exist'.format( + 'The specified value:{type}:`{name}` does not exist, or is not allowed.'.format( type=ref_type, name=ref_name)) return ':'.join([ref_type, ref_name, ref_hash]) @@ -431,7 +431,6 @@ def _get_commit_dict( } -# TODO: mikhail: Think about moving this function to some library def _get_ref_hash(repo, type_, name): vcs_repo = repo.scm_instance() if type_ == 'branch' and vcs_repo.alias in ('hg', 'git'): diff --git a/rhodecode/api/views/pull_request_api.py b/rhodecode/api/views/pull_request_api.py --- a/rhodecode/api/views/pull_request_api.py +++ b/rhodecode/api/views/pull_request_api.py @@ -284,7 +284,7 @@ def merge_pull_request( raise JSONRPCError('userid is not the same as your user') check = MergeCheck.validate( - pull_request, user=apiuser, translator=request.translate) + pull_request, auth_user=apiuser, translator=request.translate) merge_possible = not check.failed if not merge_possible: diff --git a/rhodecode/apps/repository/views/repo_pull_requests.py b/rhodecode/apps/repository/views/repo_pull_requests.py --- a/rhodecode/apps/repository/views/repo_pull_requests.py +++ b/rhodecode/apps/repository/views/repo_pull_requests.py @@ -341,7 +341,7 @@ class RepoPullRequestsView(RepoAppView, # check merge capabilities _merge_check = MergeCheck.validate( - pull_request_latest, user=self._rhodecode_user, + pull_request_latest, auth_user=self._rhodecode_user, translator=self.request.translate, force_shadow_repo_refresh=force_refresh) c.pr_merge_errors = _merge_check.error_details @@ -1056,8 +1056,9 @@ class RepoPullRequestsView(RepoAppView, self.request.matchdict['pull_request_id']) self.load_default_context() - check = MergeCheck.validate(pull_request, self._rhodecode_db_user, - translator=self.request.translate) + check = MergeCheck.validate( + pull_request, auth_user=self._rhodecode_user, + translator=self.request.translate) merge_possible = not check.failed for err_type, error_msg in check.errors: diff --git a/rhodecode/model/pull_request.py b/rhodecode/model/pull_request.py --- a/rhodecode/model/pull_request.py +++ b/rhodecode/model/pull_request.py @@ -449,7 +449,7 @@ class PullRequestModel(BaseModel): translator = translator or get_current_request().translate created_by_user = self._get_user(created_by) - auth_user = auth_user or created_by_user + auth_user = auth_user or created_by_user.AuthUser() source_repo = self._get_repo(source_repo) target_repo = self._get_repo(target_repo) @@ -534,7 +534,7 @@ class PullRequestModel(BaseModel): # prepare workspace, and run initial merge simulation MergeCheck.validate( - pull_request, user=created_by_user, translator=translator) + pull_request, auth_user=auth_user, translator=translator) self.notify_reviewers(pull_request, reviewer_ids) self._trigger_pull_request_hook( @@ -1599,19 +1599,38 @@ class MergeCheck(object): ) @classmethod - def validate(cls, pull_request, user, translator, fail_early=False, + def validate(cls, pull_request, auth_user, translator, fail_early=False, force_shadow_repo_refresh=False): _ = translator merge_check = cls() # permissions to merge user_allowed_to_merge = PullRequestModel().check_user_merge( - pull_request, user) + pull_request, auth_user) if not user_allowed_to_merge: log.debug("MergeCheck: cannot merge, approval is pending.") - msg = _('User `{}` not allowed to perform merge.').format(user.username) - merge_check.push_error('error', msg, cls.PERM_CHECK, user.username) + msg = _('User `{}` not allowed to perform merge.').format(auth_user.username) + merge_check.push_error('error', msg, cls.PERM_CHECK, auth_user.username) + if fail_early: + return merge_check + + # permission to merge into the target branch + target_commit_id = pull_request.target_ref_parts.commit_id + if pull_request.target_ref_parts.type == 'branch': + branch_name = pull_request.target_ref_parts.name + else: + # for mercurial we can always figure out the branch from the commit + # in case of bookmark + target_commit = pull_request.target_repo.get_commit(target_commit_id) + branch_name = target_commit.branch + + rule, branch_perm = auth_user.get_rule_and_branch_permission( + pull_request.target_repo.repo_name, branch_name) + if branch_perm and branch_perm == 'branch.none': + msg = _('Target branch `{}` changes rejected by rule {}.').format( + branch_name, rule) + merge_check.push_error('error', msg, cls.PERM_CHECK, auth_user.username) if fail_early: return merge_check