diff options
| -rw-r--r-- | app/models/project.rb | 19 | ||||
| -rw-r--r-- | app/services/projects/protect_default_branch_service.rb | 67 | ||||
| -rw-r--r-- | lib/gitlab/access/branch_protection.rb | 42 | ||||
| -rw-r--r-- | spec/lib/gitlab/access/branch_protection_spec.rb | 54 | ||||
| -rw-r--r-- | spec/services/projects/protect_default_branch_service_spec.rb | 242 |
5 files changed, 406 insertions, 18 deletions
diff --git a/app/models/project.rb b/app/models/project.rb index 7ab2fc30c24..e31ee0f5228 100644 --- a/app/models/project.rb +++ b/app/models/project.rb @@ -1602,24 +1602,7 @@ class Project < ActiveRecord::Base # rubocop: disable CodeReuse/ServiceClass 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 Gitlab::CurrentSettings.default_branch_protection != Gitlab::Access::PROTECTION_NONE && !ProtectedBranch.protected?(self, default_branch) - params = { - name: default_branch, - push_access_levels_attributes: [{ - access_level: Gitlab::CurrentSettings.default_branch_protection == Gitlab::Access::PROTECTION_DEV_CAN_PUSH ? Gitlab::Access::DEVELOPER : Gitlab::Access::MAINTAINER - }], - merge_access_levels_attributes: [{ - access_level: Gitlab::CurrentSettings.default_branch_protection == Gitlab::Access::PROTECTION_DEV_CAN_MERGE ? Gitlab::Access::DEVELOPER : Gitlab::Access::MAINTAINER - }] - } - - ProtectedBranches::CreateService.new(self, creator, params).execute(skip_authorization: true) - end + Projects::ProtectDefaultBranchService.new(self).execute end # rubocop: enable CodeReuse/ServiceClass diff --git a/app/services/projects/protect_default_branch_service.rb b/app/services/projects/protect_default_branch_service.rb new file mode 100644 index 00000000000..245490791bf --- /dev/null +++ b/app/services/projects/protect_default_branch_service.rb @@ -0,0 +1,67 @@ +# frozen_string_literal: true + +module Projects + # Service class that can be used to execute actions necessary after creating a + # default branch. + class ProtectDefaultBranchService + attr_reader :project, :default_branch_protection + + # @param [Project] project + def initialize(project) + @project = project + + @default_branch_protection = Gitlab::Access::BranchProtection + .new(Gitlab::CurrentSettings.default_branch_protection) + end + + def execute + protect_default_branch if default_branch + end + + def protect_default_branch + # Ensure HEAD points to the default branch in case it is not master + project.change_head(default_branch) + + create_protected_branch if protect_branch? + end + + def create_protected_branch + params = { + name: default_branch, + push_access_levels_attributes: [{ access_level: push_access_level }], + merge_access_levels_attributes: [{ access_level: merge_access_level }] + } + + # The creator of the project is always allowed to create protected + # branches, so we skip the authorization check in this service class. + ProtectedBranches::CreateService + .new(project, project.creator, params) + .execute(skip_authorization: true) + end + + def protect_branch? + default_branch_protection.any? && + !ProtectedBranch.protected?(project, default_branch) + end + + def default_branch + project.default_branch + end + + def push_access_level + if default_branch_protection.developer_can_push? + Gitlab::Access::DEVELOPER + else + Gitlab::Access::MAINTAINER + end + end + + def merge_access_level + if default_branch_protection.developer_can_merge? + Gitlab::Access::DEVELOPER + else + Gitlab::Access::MAINTAINER + end + end + end +end diff --git a/lib/gitlab/access/branch_protection.rb b/lib/gitlab/access/branch_protection.rb new file mode 100644 index 00000000000..f039e5c011f --- /dev/null +++ b/lib/gitlab/access/branch_protection.rb @@ -0,0 +1,42 @@ +# frozen_string_literal: true + +module Gitlab + module Access + # A wrapper around Integer based branch protection levels. + # + # This wrapper can be used to work with branch protection levels without + # having to directly refer to the constants. For example, instead of this: + # + # if access_level == Gitlab::Access::PROTECTION_DEV_CAN_PUSH + # ... + # end + # + # You can write this instead: + # + # protection = BranchProtection.new(access_level) + # + # if protection.developer_can_push? + # ... + # end + class BranchProtection + attr_reader :level + + # @param [Integer] level The branch protection level as an Integer. + def initialize(level) + @level = level + end + + def any? + level != PROTECTION_NONE + end + + def developer_can_push? + level == PROTECTION_DEV_CAN_PUSH + end + + def developer_can_merge? + level == PROTECTION_DEV_CAN_MERGE + end + end + end +end diff --git a/spec/lib/gitlab/access/branch_protection_spec.rb b/spec/lib/gitlab/access/branch_protection_spec.rb new file mode 100644 index 00000000000..7f2979e8e28 --- /dev/null +++ b/spec/lib/gitlab/access/branch_protection_spec.rb @@ -0,0 +1,54 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe Gitlab::Access::BranchProtection do + describe '#any?' do + using RSpec::Parameterized::TableSyntax + + where(:level, :result) do + Gitlab::Access::PROTECTION_NONE | false + Gitlab::Access::PROTECTION_DEV_CAN_PUSH | true + Gitlab::Access::PROTECTION_DEV_CAN_MERGE | true + Gitlab::Access::PROTECTION_FULL | true + end + + with_them do + it { expect(described_class.new(level).any?).to eq(result) } + end + end + + describe '#developer_can_push?' do + using RSpec::Parameterized::TableSyntax + + where(:level, :result) do + Gitlab::Access::PROTECTION_NONE | false + Gitlab::Access::PROTECTION_DEV_CAN_PUSH | true + Gitlab::Access::PROTECTION_DEV_CAN_MERGE | false + Gitlab::Access::PROTECTION_FULL | false + end + + with_them do + it do + expect(described_class.new(level).developer_can_push?).to eq(result) + end + end + end + + describe '#developer_can_merge?' do + using RSpec::Parameterized::TableSyntax + + where(:level, :result) do + Gitlab::Access::PROTECTION_NONE | false + Gitlab::Access::PROTECTION_DEV_CAN_PUSH | false + Gitlab::Access::PROTECTION_DEV_CAN_MERGE | true + Gitlab::Access::PROTECTION_FULL | false + end + + with_them do + it do + expect(described_class.new(level).developer_can_merge?).to eq(result) + end + end + end +end diff --git a/spec/services/projects/protect_default_branch_service_spec.rb b/spec/services/projects/protect_default_branch_service_spec.rb new file mode 100644 index 00000000000..c145b2c06c6 --- /dev/null +++ b/spec/services/projects/protect_default_branch_service_spec.rb @@ -0,0 +1,242 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe Projects::ProtectDefaultBranchService do + let(:service) { described_class.new(project) } + let(:project) { instance_spy(Project) } + + describe '#execute' do + before do + allow(service) + .to receive(:protect_default_branch) + end + + context 'without a default branch' do + it 'does nothing' do + allow(service) + .to receive(:default_branch) + .and_return(nil) + + service.execute + + expect(service) + .not_to have_received(:protect_default_branch) + end + end + + context 'with a default branch' do + it 'protects the default branch' do + allow(service) + .to receive(:default_branch) + .and_return('master') + + service.execute + + expect(service) + .to have_received(:protect_default_branch) + end + end + end + + describe '#protect_default_branch' do + before do + allow(service) + .to receive(:default_branch) + .and_return('master') + + allow(project) + .to receive(:change_head) + .with('master') + + allow(service) + .to receive(:create_protected_branch) + end + + context 'when branch protection is needed' do + before do + allow(service) + .to receive(:protect_branch?) + .and_return(true) + + allow(service) + .to receive(:create_protected_branch) + end + + it 'changes the HEAD of the project' do + service.protect_default_branch + + expect(project) + .to have_received(:change_head) + end + + it 'protects the default branch' do + service.protect_default_branch + + expect(service) + .to have_received(:create_protected_branch) + end + end + + context 'when branch protection is not needed' do + before do + allow(service) + .to receive(:protect_branch?) + .and_return(false) + end + + it 'changes the HEAD of the project' do + service.protect_default_branch + + expect(project) + .to have_received(:change_head) + end + + it 'does not protect the default branch' do + service.protect_default_branch + + expect(service) + .not_to have_received(:create_protected_branch) + end + end + end + + describe '#create_protected_branch' do + it 'creates the protected branch' do + creator = instance_spy(User) + create_service = instance_spy(ProtectedBranches::CreateService) + access_level = Gitlab::Access::DEVELOPER + params = { + name: 'master', + push_access_levels_attributes: [{ access_level: access_level }], + merge_access_levels_attributes: [{ access_level: access_level }] + } + + allow(project) + .to receive(:creator) + .and_return(creator) + + allow(ProtectedBranches::CreateService) + .to receive(:new) + .with(project, creator, params) + .and_return(create_service) + + allow(service) + .to receive(:push_access_level) + .and_return(access_level) + + allow(service) + .to receive(:merge_access_level) + .and_return(access_level) + + allow(service) + .to receive(:default_branch) + .and_return('master') + + allow(create_service) + .to receive(:execute) + .with(skip_authorization: true) + + service.create_protected_branch + + expect(create_service) + .to have_received(:execute) + end + end + + describe '#protect_branch?' do + context 'when default branch protection is disabled' do + it 'returns false' do + allow(Gitlab::CurrentSettings) + .to receive(:default_branch_protection) + .and_return(Gitlab::Access::PROTECTION_NONE) + + expect(service.protect_branch?).to eq(false) + end + end + + context 'when default branch protection is enabled' do + before do + allow(Gitlab::CurrentSettings) + .to receive(:default_branch_protection) + .and_return(Gitlab::Access::PROTECTION_DEV_CAN_MERGE) + + allow(service) + .to receive(:default_branch) + .and_return('master') + end + + it 'returns false if the branch is already protected' do + allow(ProtectedBranch) + .to receive(:protected?) + .with(project, 'master') + .and_return(true) + + expect(service.protect_branch?).to eq(false) + end + + it 'returns true if the branch is not yet protected' do + allow(ProtectedBranch) + .to receive(:protected?) + .with(project, 'master') + .and_return(false) + + expect(service.protect_branch?).to eq(true) + end + end + end + + describe '#default_branch' do + it 'returns the default branch of the project' do + allow(project) + .to receive(:default_branch) + .and_return('master') + + expect(service.default_branch).to eq('master') + end + end + + describe '#push_access_level' do + context 'when developers can push' do + it 'returns the DEVELOPER access level' do + allow(Gitlab::CurrentSettings) + .to receive(:default_branch_protection) + .and_return(Gitlab::Access::PROTECTION_DEV_CAN_PUSH) + + expect(service.push_access_level).to eq(Gitlab::Access::DEVELOPER) + end + end + + context 'when developers can not push' do + it 'returns the MAINTAINER access level' do + allow(Gitlab::CurrentSettings) + .to receive(:default_branch_protection) + .and_return(Gitlab::Access::PROTECTION_DEV_CAN_MERGE) + + expect(service.push_access_level).to eq(Gitlab::Access::MAINTAINER) + end + end + end + + describe '#merge_access_level' do + context 'when developers can merge' do + it 'returns the DEVELOPER access level' do + allow(Gitlab::CurrentSettings) + .to receive(:default_branch_protection) + .and_return(Gitlab::Access::PROTECTION_DEV_CAN_MERGE) + + expect(service.merge_access_level).to eq(Gitlab::Access::DEVELOPER) + end + end + + context 'when developers can not merge' do + it 'returns the MAINTAINER access level' do + allow(Gitlab::CurrentSettings) + .to receive(:default_branch_protection) + .and_return(Gitlab::Access::PROTECTION_DEV_CAN_PUSH) + + expect(service.merge_access_level).to eq(Gitlab::Access::MAINTAINER) + end + end + end +end |