Merge branch...

Merge branch '211944-provide-instance-level-setting-to-enable-or-disable-default-branch-protection-at-the-group-add-policies' into 'master'

Resolve "Add policies for managing 'default_branch_protection' setting in groups"

See merge request gitlab-org/gitlab!29879

app/helpers/groups_helper.rb

@@ -49,6 +49,10 @@ module GroupsHelper
     can?(current_user, :change_visibility_level, group)
   end
 
+  def can_update_default_branch_protection?(group)
+    can?(current_user, :update_default_branch_protection, group)
+  end
+
   def can_change_share_with_group_lock?(group)
     can?(current_user, :change_share_with_group_lock, group)
   end

app/policies/global_policy.rb

@@ -74,6 +74,10 @@ class GlobalPolicy < BasePolicy
     enable :create_group
   end
 
+  rule { can?(:create_group) }.policy do
+    enable :create_group_with_default_branch_protection
+  end
+
   rule { can_create_fork }.policy do
     enable :create_fork
   end

app/policies/group_policy.rb

@@ -123,6 +123,7 @@ class GroupPolicy < BasePolicy
 
     enable :set_note_created_at
     enable :set_emails_disabled
+    enable :update_default_branch_protection
   end
 
   rule { can?(:read_nested_project_resources) }.policy do

app/services/groups/create_service.rb

@@ -38,6 +38,10 @@ module Groups
       # overridden in EE
     end
 
+    def remove_unallowed_params
+      params.delete(:default_branch_protection) unless can?(current_user, :create_group_with_default_branch_protection)
+    end
+
     def create_chat_team?
       Gitlab.config.mattermost.enabled && @chat_team && group.chat_team.nil?
     end

app/services/groups/update_service.rb

@@ -66,6 +66,7 @@ module Groups
     # overridden in EE
     def remove_unallowed_params
       params.delete(:emails_disabled) unless can?(current_user, :set_emails_disabled, group)
+      params.delete(:default_branch_protection) unless can?(current_user, :update_default_branch_protection, group)
     end
 
     def valid_share_with_group_lock_change?

app/views/groups/settings/_default_branch_protection.html.haml

+- return unless can_update_default_branch_protection?(group)
+
+= render 'shared/default_branch_protection', f: f, selected_level: group.default_branch_protection

app/views/groups/settings/_permissions.html.haml

@@ -33,7 +33,7 @@
     = render_if_exists 'groups/settings/ip_restriction', f: f, group: @group
     = render_if_exists 'groups/settings/allowed_email_domain', f: f, group: @group
     = render 'groups/settings/lfs', f: f
-    = render 'shared/default_branch_protection', f: f, selected_level: @group.default_branch_protection
+    = render 'groups/settings/default_branch_protection', f: f, group: @group
     = render 'groups/settings/project_creation_level', f: f, group: @group
     = render 'groups/settings/subgroup_creation_level', f: f, group: @group
     = render 'groups/settings/two_factor_auth', f: f

changelogs/unreleased/211944-provide-instance-level-setting-to-enable-or-disable-default-branch-add-policies.yml

+---
+title: Add policies for managing 'default_branch_protection' setting in groups
+merge_request: 29879
+author:
+type: added

ee/app/services/ee/groups/create_service.rb

@@ -25,6 +25,8 @@ module EE
           params.delete(:shared_runners_minutes_limit)
           params.delete(:extra_shared_runners_minutes_limit)
         end
+
+        super
       end
 
       def log_audit_event

spec/controllers/groups_controller_spec.rb

@@ -270,6 +270,37 @@ describe GroupsController do
 
       it { expect(subject).to render_template(:new) }
     end
+
+    context 'when creating a group with `default_branch_protection` attribute' do
+      before do
+        sign_in(user)
+      end
+
+      subject do
+        post :create, params: { group: { name: 'new_group', path: 'new_group', default_branch_protection: Gitlab::Access::PROTECTION_NONE } }
+      end
+
+      context 'for users who have the ability to create a group with `default_branch_protection`' do
+        it 'creates group with the specified branch protection level' do
+          subject
+
+          expect(response).to have_gitlab_http_status(:found)
+          expect(Group.last.default_branch_protection).to eq(Gitlab::Access::PROTECTION_NONE)
+        end
+      end
+
+      context 'for users who do not have the ability to create a group with `default_branch_protection`' do
+        it 'does not create the group with the specified branch protection level' do
+          allow(Ability).to receive(:allowed?).and_call_original
+          allow(Ability).to receive(:allowed?).with(user, :create_group_with_default_branch_protection) { false }
+
+          subject
+
+          expect(response).to have_gitlab_http_status(:found)
+          expect(Group.last.default_branch_protection).not_to eq(Gitlab::Access::PROTECTION_NONE)
+        end
+      end
+    end
   end
 
   describe 'GET #index' do
@@ -423,12 +454,32 @@ describe GroupsController do
       expect(group.reload.project_creation_level).to eq(::Gitlab::Access::MAINTAINER_PROJECT_ACCESS)
     end
 
-    it 'updates the default_branch_protection successfully' do
-      post :update, params: { id: group.to_param, group: { default_branch_protection: ::Gitlab::Access::PROTECTION_DEV_CAN_MERGE } }
+    context 'updating default_branch_protection' do
+      subject do
+        put :update, params: { id: group.to_param, group: { default_branch_protection: ::Gitlab::Access::PROTECTION_DEV_CAN_MERGE } }
+      end
+
+      context 'for users who have the ability to update default_branch_protection' do
+        it 'updates the attribute' do
+          subject
 
           expect(response).to have_gitlab_http_status(:found)
           expect(group.reload.default_branch_protection).to eq(::Gitlab::Access::PROTECTION_DEV_CAN_MERGE)
         end
+      end
+
+      context 'for users who do not have the ability to update default_branch_protection' do
+        it 'does not update the attribute' do
+          allow(Ability).to receive(:allowed?).and_call_original
+          allow(Ability).to receive(:allowed?).with(user, :update_default_branch_protection, group) { false }
+
+          subject
+
+          expect(response).to have_gitlab_http_status(:found)
+          expect(group.reload.default_branch_protection).not_to eq(::Gitlab::Access::PROTECTION_DEV_CAN_MERGE)
+        end
+      end
+    end
 
     context 'when a project inside the group has container repositories' do
       before do

spec/helpers/groups_helper_spec.rb

@@ -340,4 +340,31 @@ describe GroupsHelper do
       end
     end
   end
+
+  describe '#can_update_default_branch_protection?' do
+    let(:current_user) { create(:user) }
+    let(:group) { create(:group) }
+
+    subject { helper.can_update_default_branch_protection?(group) }
+
+    before do
+      allow(helper).to receive(:current_user) { current_user }
+    end
+
+    context 'for users who can update default branch protection of the group' do
+      before do
+        allow(helper).to receive(:can?).with(current_user, :update_default_branch_protection, group) { true }
+      end
+
+      it { is_expected.to be_truthy }
+    end
+
+    context 'for users who cannot update default branch protection of the group' do
+      before do
+        allow(helper).to receive(:can?).with(current_user, :update_default_branch_protection, group) { false }
+      end
+
+      it { is_expected.to be_falsey }
+    end
+  end
 end

spec/policies/global_policy_spec.rb

@@ -80,6 +80,34 @@ describe GlobalPolicy do
     end
   end
 
+  describe 'create group' do
+    context 'when user has the ability to create group' do
+      let(:current_user) { create(:user, can_create_group: true) }
+
+      it { is_expected.to be_allowed(:create_group) }
+    end
+
+    context 'when user does not have the ability to create group' do
+      let(:current_user) { create(:user, can_create_group: false) }
+
+      it { is_expected.not_to be_allowed(:create_group) }
+    end
+  end
+
+  describe 'create group with default branch protection' do
+    context 'when user has the ability to create group' do
+      let(:current_user) { create(:user, can_create_group: true) }
+
+      it { is_expected.to be_allowed(:create_group_with_default_branch_protection) }
+    end
+
+    context 'when user does not have the ability to create group' do
+      let(:current_user) { create(:user, can_create_group: false) }
+
+      it { is_expected.not_to be_allowed(:create_group_with_default_branch_protection) }
+    end
+  end
+
   describe 'custom attributes' do
     context 'regular user' do
       it { is_expected.not_to be_allowed(:read_custom_attribute) }

spec/requests/api/groups_spec.rb

@@ -642,6 +642,33 @@ describe API::Groups do
         expect(json_response['default_branch_protection']).to eq(::Gitlab::Access::MAINTAINER_PROJECT_ACCESS)
       end
 
+      context 'updating the `default_branch_protection` attribute' do
+        subject do
+          put api("/groups/#{group1.id}", user1), params: { default_branch_protection: ::Gitlab::Access::PROTECTION_NONE }
+        end
+
+        context 'for users who have the ability to update default_branch_protection' do
+          it 'updates the attribute' do
+            subject
+
+            expect(response).to have_gitlab_http_status(:ok)
+            expect(json_response['default_branch_protection']).to eq(Gitlab::Access::PROTECTION_NONE)
+          end
+        end
+
+        context 'for users who does not have the ability to update default_branch_protection`' do
+          it 'does not update the attribute' do
+            allow(Ability).to receive(:allowed?).and_call_original
+            allow(Ability).to receive(:allowed?).with(user1, :update_default_branch_protection, group1) { false }
+
+            subject
+
+            expect(response).to have_gitlab_http_status(:ok)
+            expect(json_response['default_branch_protection']).not_to eq(Gitlab::Access::PROTECTION_NONE)
+          end
+        end
+      end
+
       context 'malicious group name' do
         subject { put api("/groups/#{group1.id}", user1), params: { name: "<SCRIPT>alert('DOUBLE-ATTACK!')</SCRIPT>" } }
 
@@ -1111,6 +1138,33 @@ describe API::Groups do
         it { expect { subject }.not_to change { Group.count } }
       end
 
+      context 'when creating a group with `default_branch_protection` attribute' do
+        let(:params) { attributes_for_group_api default_branch_protection: Gitlab::Access::PROTECTION_NONE }
+
+        subject { post api("/groups", user3), params: params }
+
+        context 'for users who have the ability to create a group with `default_branch_protection`' do
+          it 'creates group with the specified branch protection level' do
+            subject
+
+            expect(response).to have_gitlab_http_status(:created)
+            expect(json_response['default_branch_protection']).to eq(Gitlab::Access::PROTECTION_NONE)
+          end
+        end
+
+        context 'for users who do not have the ability to create a group with `default_branch_protection`' do
+          it 'does not create the group with the specified branch protection level' do
+            allow(Ability).to receive(:allowed?).and_call_original
+            allow(Ability).to receive(:allowed?).with(user3, :create_group_with_default_branch_protection) { false }
+
+            subject
+
+            expect(response).to have_gitlab_http_status(:created)
+            expect(json_response['default_branch_protection']).not_to eq(Gitlab::Access::PROTECTION_NONE)
+          end
+        end
+      end
+
       it "does not create group, duplicate" do
         post api("/groups", user3), params: { name: 'Duplicate Test', path: group2.path }
 

spec/services/groups/create_service_spec.rb

@@ -24,6 +24,27 @@ describe Groups::CreateService, '#execute' do
     end
   end
 
+  context 'creating a group with `default_branch_protection` attribute' do
+    let(:params) { group_params.merge(default_branch_protection: Gitlab::Access::PROTECTION_NONE) }
+    let(:service) { described_class.new(user, params) }
+    let(:created_group) { service.execute }
+
+    context 'for users who have the ability to create a group with `default_branch_protection`' do
+      it 'creates group with the specified branch protection level' do
+        expect(created_group.default_branch_protection).to eq(Gitlab::Access::PROTECTION_NONE)
+      end
+    end
+
+    context 'for users who do not have the ability to create a group with `default_branch_protection`' do
+      it 'does not create the group with the specified branch protection level' do
+        allow(Ability).to receive(:allowed?).and_call_original
+        allow(Ability).to receive(:allowed?).with(user, :create_group_with_default_branch_protection) { false }
+
+        expect(created_group.default_branch_protection).not_to eq(Gitlab::Access::PROTECTION_NONE)
+      end
+    end
+  end
+
   describe 'creating a top level group' do
     let(:service) { described_class.new(user, group_params) }
 

spec/services/groups/update_service_spec.rb

@@ -148,6 +148,26 @@ describe Groups::UpdateService do
     end
   end
 
+  context 'updating default_branch_protection' do
+    let(:service) do
+      described_class.new(internal_group, user, default_branch_protection: Gitlab::Access::PROTECTION_NONE)
+    end
+
+    context 'for users who have the ability to update default_branch_protection' do
+      it 'updates the attribute' do
+        internal_group.add_owner(user)
+
+        expect { service.execute }.to change { internal_group.default_branch_protection }.to(Gitlab::Access::PROTECTION_NONE)
+      end
+    end
+
+    context 'for users who do not have the ability to update default_branch_protection' do
+      it 'does not update the attribute' do
+        expect { service.execute }.not_to change { internal_group.default_branch_protection }
+      end
+    end
+  end
+
   context 'rename group' do
     let!(:service) { described_class.new(internal_group, user, path: SecureRandom.hex) }
 

spec/support/shared_contexts/policies/group_policy_shared_context.rb

@@ -35,7 +35,8 @@ RSpec.shared_context 'GroupPolicy context' do
       :change_visibility_level,
       :set_note_created_at,
       :create_subgroup,
-      :read_statistics
+      :read_statistics,
+      :update_default_branch_protection
     ].compact
   end