diff options
5 files changed, 61 insertions, 31 deletions
diff --git a/app/views/projects/protected_branches/shared/_index.html.haml b/app/views/projects/protected_branches/shared/_index.html.haml index 4997770321e..539b184e5c2 100644 --- a/app/views/projects/protected_branches/shared/_index.html.haml +++ b/app/views/projects/protected_branches/shared/_index.html.haml @@ -12,7 +12,7 @@ %p By default, protected branches are designed to: %ul - %li prevent their creation, if not already created, from everybody except users who are allowed to merge + %li prevent their creation, if not already created, from everybody except Maintainers %li prevent pushes from everybody except Maintainers %li prevent <strong>anyone</strong> from force pushing to the branch %li prevent <strong>anyone</strong> from deleting the branch diff --git a/changelogs/unreleased/59289-fix-push-to-create-protected-branches.yml b/changelogs/unreleased/59289-fix-push-to-create-protected-branches.yml new file mode 100644 index 00000000000..76dd63fef7a --- /dev/null +++ b/changelogs/unreleased/59289-fix-push-to-create-protected-branches.yml @@ -0,0 +1,6 @@ +--- +title: Allow users who can push to protected branches to create protected branches + via CLI +merge_request: 26413 +author: +type: fixed diff --git a/doc/user/project/protected_branches.md b/doc/user/project/protected_branches.md index 480cc921d76..2060b5dd4a2 100644 --- a/doc/user/project/protected_branches.md +++ b/doc/user/project/protected_branches.md @@ -10,7 +10,7 @@ created protected branches. By default, a protected branch does four simple things: - it prevents its creation, if not already created, from everybody except users - who are allowed to merge + with Maintainer permission - it prevents pushes from everybody except users with Maintainer permission - it prevents **anyone** from force pushing to the branch - it prevents **anyone** from deleting the branch diff --git a/lib/gitlab/checks/branch_check.rb b/lib/gitlab/checks/branch_check.rb index ad926739752..1dbd564fb6f 100644 --- a/lib/gitlab/checks/branch_check.rb +++ b/lib/gitlab/checks/branch_check.rb @@ -59,6 +59,8 @@ module Gitlab def protected_branch_creation_checks logger.log_timed(LOG_MESSAGES[:protected_branch_creation_checks]) do + break if user_access.can_push_to_branch?(branch_name) + unless user_access.can_merge_to_branch?(branch_name) raise GitAccess::UnauthorizedError, ERROR_MESSAGES[:create_protected_branch] end diff --git a/spec/lib/gitlab/checks/branch_check_spec.rb b/spec/lib/gitlab/checks/branch_check_spec.rb index 12beeecd470..8d5ab27a17c 100644 --- a/spec/lib/gitlab/checks/branch_check_spec.rb +++ b/spec/lib/gitlab/checks/branch_check_spec.rb @@ -108,64 +108,86 @@ describe Gitlab::Checks::BranchCheck do end context 'protected branch creation feature is enabled' do - context 'user is not allowed to create protected branches' do + context 'user can push to branch' do before do allow(user_access) - .to receive(:can_merge_to_branch?) + .to receive(:can_push_to_branch?) .with('feature') - .and_return(false) + .and_return(true) end - it 'raises an error' do - expect { subject.validate! }.to raise_error(Gitlab::GitAccess::UnauthorizedError, 'You are not allowed to create protected branches on this project.') + it 'does not raise an error' do + expect { subject.validate! }.not_to raise_error end end - context 'user is allowed to create protected branches' do + context 'user cannot push to branch' do before do allow(user_access) - .to receive(:can_merge_to_branch?) + .to receive(:can_push_to_branch?) .with('feature') - .and_return(true) - - allow(project.repository) - .to receive(:branch_names_contains_sha) - .with(newrev) - .and_return(['branch']) + .and_return(false) end - context "newrev isn't in any protected branches" do + context 'user cannot merge to branch' do before do - allow(ProtectedBranch) - .to receive(:any_protected?) - .with(project, ['branch']) + allow(user_access) + .to receive(:can_merge_to_branch?) + .with('feature') .and_return(false) end it 'raises an error' do - expect { subject.validate! }.to raise_error(Gitlab::GitAccess::UnauthorizedError, 'You can only use an existing protected branch ref as the basis of a new protected branch.') + expect { subject.validate! }.to raise_error(Gitlab::GitAccess::UnauthorizedError, 'You are not allowed to create protected branches on this project.') end end - context 'newrev is included in a protected branch' do + context 'user can merge to branch' do before do - allow(ProtectedBranch) - .to receive(:any_protected?) - .with(project, ['branch']) + allow(user_access) + .to receive(:can_merge_to_branch?) + .with('feature') .and_return(true) + + allow(project.repository) + .to receive(:branch_names_contains_sha) + .with(newrev) + .and_return(['branch']) end - context 'via web interface' do - let(:protocol) { 'web' } + context "newrev isn't in any protected branches" do + before do + allow(ProtectedBranch) + .to receive(:any_protected?) + .with(project, ['branch']) + .and_return(false) + end - it 'allows branch creation' do - expect { subject.validate! }.not_to raise_error + it 'raises an error' do + expect { subject.validate! }.to raise_error(Gitlab::GitAccess::UnauthorizedError, 'You can only use an existing protected branch ref as the basis of a new protected branch.') end end - context 'via SSH' do - it 'raises an error' do - expect { subject.validate! }.to raise_error(Gitlab::GitAccess::UnauthorizedError, 'You can only create protected branches using the web interface and API.') + context 'newrev is included in a protected branch' do + before do + allow(ProtectedBranch) + .to receive(:any_protected?) + .with(project, ['branch']) + .and_return(true) + end + + context 'via web interface' do + let(:protocol) { 'web' } + + it 'allows branch creation' do + expect { subject.validate! }.not_to raise_error + end + end + + context 'via SSH' do + it 'raises an error' do + expect { subject.validate! }.to raise_error(Gitlab::GitAccess::UnauthorizedError, 'You can only create protected branches using the web interface and API.') + end end end end |