Commit 15f7f52b authored by Douwe Maan's avatar Douwe Maan

Merge branch '3968-protected-branch-is-not-set-for-default-branch-on-import' into 'master'

Protected branch is now created for default branch on import

Closes #3968

See merge request gitlab-org/gitlab-ce!16198
parents 2c66b942 819fc98f
Pipeline #15895440 passed with stages
in 38 minutes and 21 seconds
......@@ -1450,6 +1450,7 @@ class Project < ActiveRecord::Base
import_finish
remove_import_jid
update_project_counter_caches
after_create_default_branch
end
def update_project_counter_caches
......@@ -1463,6 +1464,27 @@ class Project < ActiveRecord::Base
end
end
def after_create_default_branch
return unless default_branch
# Ensure HEAD points to the default branch in case it is not master
change_head(default_branch)
if current_application_settings.default_branch_protection != Gitlab::Access::PROTECTION_NONE && !ProtectedBranch.protected?(self, default_branch)
params = {
name: default_branch,
push_access_levels_attributes: [{
access_level: current_application_settings.default_branch_protection == Gitlab::Access::PROTECTION_DEV_CAN_PUSH ? Gitlab::Access::DEVELOPER : Gitlab::Access::MASTER
}],
merge_access_levels_attributes: [{
access_level: current_application_settings.default_branch_protection == Gitlab::Access::PROTECTION_DEV_CAN_MERGE ? Gitlab::Access::DEVELOPER : Gitlab::Access::MASTER
}]
}
ProtectedBranches::CreateService.new(self, creator, params).execute(skip_authorization: true)
end
end
def remove_import_jid
return unless import_jid
......
......@@ -154,24 +154,7 @@ class GitPushService < BaseService
offset = [@push_commits_count - PROCESS_COMMIT_LIMIT, 0].max
@push_commits = project.repository.commits(params[:newrev], offset: offset, limit: PROCESS_COMMIT_LIMIT)
# Ensure HEAD points to the default branch in case it is not master
project.change_head(branch_name)
# Set protection on the default branch if configured
if current_application_settings.default_branch_protection != PROTECTION_NONE && !ProtectedBranch.protected?(@project, @project.default_branch)
params = {
name: @project.default_branch,
push_access_levels_attributes: [{
access_level: current_application_settings.default_branch_protection == PROTECTION_DEV_CAN_PUSH ? Gitlab::Access::DEVELOPER : Gitlab::Access::MASTER
}],
merge_access_levels_attributes: [{
access_level: current_application_settings.default_branch_protection == PROTECTION_DEV_CAN_MERGE ? Gitlab::Access::DEVELOPER : Gitlab::Access::MASTER
}]
}
ProtectedBranches::CreateService.new(@project, current_user, params).execute
end
@project.after_create_default_branch
end
def build_push_data
......
......@@ -2,8 +2,8 @@ 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)
raise Gitlab::Access::AccessDeniedError unless skip_authorization || can?(current_user, :admin_project, project)
project.protected_branches.create(params)
end
......
---
title: Protected branch is now created for default branch on import
merge_request: 16198
author:
type: fixed
......@@ -3079,9 +3079,51 @@ describe Project do
expect(project).to receive(:import_finish)
expect(project).to receive(:update_project_counter_caches)
expect(project).to receive(:remove_import_jid)
expect(project).to receive(:after_create_default_branch)
project.after_import
end
context 'branch protection' do
let(:project) { create(:project, :repository) }
it 'does not protect when branch protection is disabled' do
stub_application_setting(default_branch_protection: Gitlab::Access::PROTECTION_NONE)
project.after_import
expect(project.protected_branches).to be_empty
end
it "gives developer access to push when branch protection is set to 'developers can push'" do
stub_application_setting(default_branch_protection: Gitlab::Access::PROTECTION_DEV_CAN_PUSH)
project.after_import
expect(project.protected_branches).not_to be_empty
expect(project.default_branch).to eq(project.protected_branches.first.name)
expect(project.protected_branches.first.push_access_levels.map(&:access_level)).to eq([Gitlab::Access::DEVELOPER])
end
it "gives developer access to merge when branch protection is set to 'developers can merge'" do
stub_application_setting(default_branch_protection: Gitlab::Access::PROTECTION_DEV_CAN_MERGE)
project.after_import
expect(project.protected_branches).not_to be_empty
expect(project.default_branch).to eq(project.protected_branches.first.name)
expect(project.protected_branches.first.merge_access_levels.map(&:access_level)).to eq([Gitlab::Access::DEVELOPER])
end
it 'protects default branch' do
project.after_import
expect(project.protected_branches).not_to be_empty
expect(project.default_branch).to eq(project.protected_branches.first.name)
expect(project.protected_branches.first.push_access_levels.map(&:access_level)).to eq([Gitlab::Access::MASTER])
expect(project.protected_branches.first.merge_access_levels.map(&:access_level)).to eq([Gitlab::Access::MASTER])
end
end
end
describe '#update_project_counter_caches' do
......
......@@ -19,5 +19,21 @@ describe ProtectedBranches::CreateService do
expect(project.protected_branches.last.push_access_levels.map(&:access_level)).to eq([Gitlab::Access::MASTER])
expect(project.protected_branches.last.merge_access_levels.map(&:access_level)).to eq([Gitlab::Access::MASTER])
end
context 'when user does not have permission' do
let(:user) { create(:user) }
before do
project.add_developer(user)
end
it 'creates a new protected branch if we skip authorization step' do
expect { service.execute(skip_authorization: true) }.to change(ProtectedBranch, :count).by(1)
end
it 'raises Gitlab::Access:AccessDeniedError' do
expect { service.execute }.to raise_error(Gitlab::Access::AccessDeniedError)
end
end
end
end
......@@ -32,6 +32,7 @@ describe RepositoryImportWorker do
expect_any_instance_of(Projects::ImportService).to receive(:execute)
.and_return({ status: :ok })
expect_any_instance_of(Project).to receive(:after_import).and_call_original
expect_any_instance_of(Repository).to receive(:expire_emptiness_caches)
expect_any_instance_of(Project).to receive(:import_finish)
......
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