##// END OF EJS Templates
pull-requests: add merge validation to prevent merges to protected branches.
marcink -
r2981:1e14730d default
parent child Browse files
Show More
@@ -252,8 +252,9 b' class TestCreatePullRequestApi(object):'
252 252 id_, params = build_data(
253 253 self.apikey_regular, 'create_pull_request', **data)
254 254 response = api_call(self.app, params)
255 expected_message = 'The specified branch `{}` does not exist'.format(
256 branch_name)
255 expected_message = 'The specified value:{type}:`{name}` ' \
256 'does not exist, or is not allowed.'.format(type='branch',
257 name=branch_name)
257 258 assert_error(id_, expected_message, given=response.body)
258 259
259 260 @pytest.mark.backends("git", "hg")
@@ -87,7 +87,8 b' class TestResolveRefOrError(object):'
87 87 ref = 'ancestor:ref'
88 88 with pytest.raises(JSONRPCError) as excinfo:
89 89 utils.resolve_ref_or_error(ref, repo)
90 expected_message = 'The specified ancestor `ref` does not exist'
90 expected_message = (
91 'The specified value:ancestor:`ref` does not exist, or is not allowed.')
91 92 assert excinfo.value.message == expected_message
92 93
93 94 def test_branch_is_not_found(self):
@@ -99,7 +100,7 b' class TestResolveRefOrError(object):'
99 100 with pytest.raises(JSONRPCError) as excinfo:
100 101 utils.resolve_ref_or_error(ref, repo)
101 102 expected_message = (
102 'The specified branch `non-existing-one` does not exist')
103 'The specified value:branch:`non-existing-one` does not exist, or is not allowed.')
103 104 assert excinfo.value.message == expected_message
104 105
105 106 def test_bookmark_is_not_found(self):
@@ -111,7 +112,7 b' class TestResolveRefOrError(object):'
111 112 with pytest.raises(JSONRPCError) as excinfo:
112 113 utils.resolve_ref_or_error(ref, repo)
113 114 expected_message = (
114 'The specified bookmark `non-existing-one` does not exist')
115 'The specified value:bookmark:`non-existing-one` does not exist, or is not allowed.')
115 116 assert excinfo.value.message == expected_message
116 117
117 118 @pytest.mark.parametrize("ref", ['ref', '12345', 'a:b:c:d'])
@@ -403,7 +403,7 b' def resolve_ref_or_error(ref, repo):'
403 403 ref_hash = ref_hash or _get_ref_hash(repo, ref_type, ref_name)
404 404 except (KeyError, ValueError):
405 405 raise JSONRPCError(
406 'The specified {type} `{name}` does not exist'.format(
406 'The specified value:{type}:`{name}` does not exist, or is not allowed.'.format(
407 407 type=ref_type, name=ref_name))
408 408
409 409 return ':'.join([ref_type, ref_name, ref_hash])
@@ -431,7 +431,6 b' def _get_commit_dict('
431 431 }
432 432
433 433
434 # TODO: mikhail: Think about moving this function to some library
435 434 def _get_ref_hash(repo, type_, name):
436 435 vcs_repo = repo.scm_instance()
437 436 if type_ == 'branch' and vcs_repo.alias in ('hg', 'git'):
@@ -284,7 +284,7 b' def merge_pull_request('
284 284 raise JSONRPCError('userid is not the same as your user')
285 285
286 286 check = MergeCheck.validate(
287 pull_request, user=apiuser, translator=request.translate)
287 pull_request, auth_user=apiuser, translator=request.translate)
288 288 merge_possible = not check.failed
289 289
290 290 if not merge_possible:
@@ -341,7 +341,7 b' class RepoPullRequestsView(RepoAppView, '
341 341
342 342 # check merge capabilities
343 343 _merge_check = MergeCheck.validate(
344 pull_request_latest, user=self._rhodecode_user,
344 pull_request_latest, auth_user=self._rhodecode_user,
345 345 translator=self.request.translate,
346 346 force_shadow_repo_refresh=force_refresh)
347 347 c.pr_merge_errors = _merge_check.error_details
@@ -1056,8 +1056,9 b' class RepoPullRequestsView(RepoAppView, '
1056 1056 self.request.matchdict['pull_request_id'])
1057 1057
1058 1058 self.load_default_context()
1059 check = MergeCheck.validate(pull_request, self._rhodecode_db_user,
1060 translator=self.request.translate)
1059 check = MergeCheck.validate(
1060 pull_request, auth_user=self._rhodecode_user,
1061 translator=self.request.translate)
1061 1062 merge_possible = not check.failed
1062 1063
1063 1064 for err_type, error_msg in check.errors:
@@ -449,7 +449,7 b' class PullRequestModel(BaseModel):'
449 449 translator = translator or get_current_request().translate
450 450
451 451 created_by_user = self._get_user(created_by)
452 auth_user = auth_user or created_by_user
452 auth_user = auth_user or created_by_user.AuthUser()
453 453 source_repo = self._get_repo(source_repo)
454 454 target_repo = self._get_repo(target_repo)
455 455
@@ -534,7 +534,7 b' class PullRequestModel(BaseModel):'
534 534
535 535 # prepare workspace, and run initial merge simulation
536 536 MergeCheck.validate(
537 pull_request, user=created_by_user, translator=translator)
537 pull_request, auth_user=auth_user, translator=translator)
538 538
539 539 self.notify_reviewers(pull_request, reviewer_ids)
540 540 self._trigger_pull_request_hook(
@@ -1599,19 +1599,38 b' class MergeCheck(object):'
1599 1599 )
1600 1600
1601 1601 @classmethod
1602 def validate(cls, pull_request, user, translator, fail_early=False,
1602 def validate(cls, pull_request, auth_user, translator, fail_early=False,
1603 1603 force_shadow_repo_refresh=False):
1604 1604 _ = translator
1605 1605 merge_check = cls()
1606 1606
1607 1607 # permissions to merge
1608 1608 user_allowed_to_merge = PullRequestModel().check_user_merge(
1609 pull_request, user)
1609 pull_request, auth_user)
1610 1610 if not user_allowed_to_merge:
1611 1611 log.debug("MergeCheck: cannot merge, approval is pending.")
1612 1612
1613 msg = _('User `{}` not allowed to perform merge.').format(user.username)
1614 merge_check.push_error('error', msg, cls.PERM_CHECK, user.username)
1613 msg = _('User `{}` not allowed to perform merge.').format(auth_user.username)
1614 merge_check.push_error('error', msg, cls.PERM_CHECK, auth_user.username)
1615 if fail_early:
1616 return merge_check
1617
1618 # permission to merge into the target branch
1619 target_commit_id = pull_request.target_ref_parts.commit_id
1620 if pull_request.target_ref_parts.type == 'branch':
1621 branch_name = pull_request.target_ref_parts.name
1622 else:
1623 # for mercurial we can always figure out the branch from the commit
1624 # in case of bookmark
1625 target_commit = pull_request.target_repo.get_commit(target_commit_id)
1626 branch_name = target_commit.branch
1627
1628 rule, branch_perm = auth_user.get_rule_and_branch_permission(
1629 pull_request.target_repo.repo_name, branch_name)
1630 if branch_perm and branch_perm == 'branch.none':
1631 msg = _('Target branch `{}` changes rejected by rule {}.').format(
1632 branch_name, rule)
1633 merge_check.push_error('error', msg, cls.PERM_CHECK, auth_user.username)
1615 1634 if fail_early:
1616 1635 return merge_check
1617 1636
General Comments 0
You need to be logged in to leave comments. Login now