Commit 1d85c569 authored by Sean McGivern's avatar Sean McGivern

Handle protected branches when squashing

If the temporary branch we want to push to is protected by a wildcard,
we need to add a temporary protected branch entry to allow the current
user to push to it, even if they can't administer the project.
parent a42cd8be
......@@ -24,13 +24,15 @@ module MergeRequests
# Squashing would ideally be possible by applying a patch to a bare repo
# and creating a commit object, in which case wouldn't need this dance.
#
temp_branch = SecureRandom.uuid
temp_branch = "temporary-gitlab-squash-branch-#{SecureRandom.uuid}"
if merge_request.squash_in_progress?
log_error('Squash task canceled: Another squash is already in progress')
return false
end
protected_branch = create_protected_branch_exception(temp_branch)
run_git_command(
%W(clone -b #{merge_request.target_branch} -- #{repository.path_to_repo} #{tree_path}),
nil,
......@@ -73,6 +75,8 @@ module MergeRequests
log_error(e.message)
false
ensure
protected_branch.destroy if protected_branch
clean_dir
end
......@@ -83,5 +87,26 @@ module MergeRequests
def merge_request_to_patch
@merge_request_to_patch ||= rugged.diff(merge_request.diff_base_sha, merge_request.diff_head_sha).patch
end
def create_protected_branch_exception(temp_branch)
user_access = Gitlab::UserAccess.new(current_user, project: target_project)
return if user_access.can_push_to_branch?(temp_branch)
protected_branch_params = {
name: temp_branch,
push_access_levels_attributes: [{ user_id: current_user.id }],
merge_access_levels_attributes: [{ user_id: current_user.id }]
}
create_service = ProtectedBranches::CreateService.new(target_project, current_user, protected_branch_params)
protected_branch = create_service.execute(skip_authorization: true)
unless protected_branch.persisted?
raise "Failed to create protected branch override #{ref}"
end
protected_branch
end
end
end
......@@ -2,8 +2,10 @@ module ProtectedBranches
class CreateService < BaseService
attr_reader :protected_branch
def execute
raise Gitlab::Access::AccessDeniedError unless can?(current_user, :admin_project, project)
def execute(skip_authorization: false)
unless skip_authorization || can?(current_user, :admin_project, project)
raise Gitlab::Access::AccessDeniedError
end
project.protected_branches.create(params)
end
......
......@@ -2,7 +2,7 @@ require 'spec_helper'
describe MergeRequests::SquashService do
let(:service) { described_class.new(project, user, {}) }
let(:user) { create(:user) }
let(:user) { project.owner }
let(:project) { create(:project) }
let(:merge_request) do
......@@ -17,8 +17,11 @@ describe MergeRequests::SquashService do
target_branch: 'master', target_project: project)
end
before do
project.team << [user, :master]
def stub_git_command(command, &block)
git_command = a_collection_starting_with([Gitlab.config.git.bin_path, command])
allow(service).to receive(:popen).and_call_original
allow(service).to receive(:popen).with(git_command, anything, anything, &block)
end
describe '#execute' do
......@@ -36,6 +39,51 @@ describe MergeRequests::SquashService do
end
end
context 'when the chosen branch name is protected with a wildcard' do
let!(:protected_branch) { create(:protected_branch, :no_one_can_push, name: '*', project: project) }
before do
# We don't run hooks in tests, so fake this case. This does involve
# duplicating logic from the service itself, but that is worth it to
# test this case.
user_access = Gitlab::UserAccess.new(user, project: project)
stub_git_command('push') do |cmd|
ref = cmd.last.split(':').last
if user_access.can_push_to_branch?(ref)
['', 0]
else
['You are not allowed to push code to protected branches on this project', 1]
end
end
end
it 'allows the user to push to that protected branch' do
branch_params = a_hash_including(name: a_string_starting_with('temporary-gitlab-squash-branch'))
expect(ProtectedBranches::CreateService).to receive(:new).with(project, user, branch_params)
service.execute(merge_request)
end
it 'returns the squashed commit SHA' do
result = service.execute(merge_request)
expect(result).to match(status: :success, squash_sha: a_string_matching(/\h{40}/))
end
it 'cleans up the temporary directory and the protected branch' do
expect(service).to receive(:clean_dir).and_call_original
expect_any_instance_of(ProtectedBranch).to receive(:destroy).and_call_original
expect { service.execute(merge_request) }
.not_to change { project.protected_branches.count }
expect(protected_branch).to be_persisted
end
end
context 'when the squash succeeds' do
it 'returns the squashed commit SHA' do
result = service.execute(merge_request)
......@@ -63,7 +111,7 @@ describe MergeRequests::SquashService do
end
it 'sets the current user as the committer' do
expect(squash_commit.committer_name).to eq(user.name.delete('.'))
expect(squash_commit.committer_name).to eq(user.name.gsub('.', ''))
expect(squash_commit.committer_email).to eq(user.email)
end
......@@ -90,13 +138,7 @@ describe MergeRequests::SquashService do
let(:error) { 'A test error' }
before do
git_command = a_collection_starting_with([Gitlab.config.git.bin_path, command])
allow(service).to receive(:popen).and_return(['', 0])
allow(service).to receive(:popen).with(git_command, anything, anything) do
[error, 1]
end
stub_git_command(command) { [error, 1] }
end
it 'logs the stage and output' do
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment