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