engineering/unrhodecode
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

pull-requests: add merge validation to prevent merges to protected branches.

Browse source
This commit is contained in:
Marcin Kuzminski 2018-08-28 20:24:40 +02:00
parent 16bc6cc415
commit 6458ef9105
6 changed files with 38 additions and 17 deletions
Download patch file Download diff file Expand all files Collapse all files

5
rhodecode/api/tests/test_create_pull_request.py
View file

@ -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")

7
rhodecode/api/tests/test_utils.py
View file

@ -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'])

3
rhodecode/api/utils.py
View file

@ -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'):

2
rhodecode/api/views/pull_request_api.py
View file

@ -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:

7
rhodecode/apps/repository/views/repo_pull_requests.py
View file

@ -341,7 +341,7 @@ class RepoPullRequestsView(RepoAppView, DataGridAppView):
# 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, DataGridAppView):
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:

31
rhodecode/model/pull_request.py
View file

@ -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