Merge branch '34370-configure-group-push-rules-services-and-frontend-v2' into 'master'

Add push rules configuration for groups

Closes #34370

See merge request gitlab-org/gitlab!31085

app/views/layouts/nav/sidebar/_group.html.haml

@@ -102,6 +102,8 @@
 
       = render_if_exists "layouts/nav/ee/security_link" # EE-specific
 
+      = render_if_exists "layouts/nav/ee/push_rules_link" # EE-specific
+
       - if group_sidebar_link?(:kubernetes)
         = nav_link(controller: [:clusters]) do
           = link_to group_clusters_path(@group) do

ee/app/controllers/groups/push_rules_controller.rb

+# frozen_string_literal: true
+class Groups::PushRulesController < Groups::ApplicationController
+  include Gitlab::Utils::StrongMemoize
+  include PushRulesHelper
+
+  layout 'group'
+
+  before_action :check_push_rules_available!
+  before_action :push_rule
+
+  respond_to :html
+
+  def edit
+  end
+
+  def update
+    if @push_rule.update(push_rule_params)
+      flash[:notice] = _('Push Rule updated successfully.')
+    else
+      flash[:alert] = @push_rule.errors.full_messages.join(', ').html_safe
+    end
+
+    redirect_to edit_group_push_rules_path(group)
+  end
+
+  private
+
+  def push_rule_params
+    allowed_fields = %i[deny_delete_tag delete_branch_regex commit_message_regex commit_message_negative_regex
+                        branch_name_regex force_push_regex author_email_regex
+                        member_check file_name_regex max_file_size prevent_secrets]
+
+    if can?(current_user, :change_reject_unsigned_commits, group)
+      allowed_fields << :reject_unsigned_commits
+    end
+
+    if can?(current_user, :change_commit_committer_check, group)
+      allowed_fields << :commit_committer_check
+    end
+
+    params.require(:push_rule).permit(allowed_fields)
+  end
+
+  def push_rule
+    strong_memoize(:push_rule) do
+      group.update(push_rule: PushRule.create!) unless group.push_rule
+      group.push_rule
+    end
+  end
+
+  def check_push_rules_available!
+    render_404 unless can_modify_group_push_rules?(current_user, group)
+  end
+end

ee/app/helpers/ee/projects_helper.rb

@@ -137,10 +137,10 @@ module EE
       ::Gitlab.config.alternative_gitlab_kerberos_url?
     end
 
-    def can_change_push_rule?(push_rule, rule)
+    def can_change_push_rule?(push_rule, rule, context)
       return true if push_rule.global?
 
-      can?(current_user, :"change_#{rule}", @project)
+      can?(current_user, :"change_#{rule}", context)
     end
 
     def ci_cd_projects_available?

ee/app/helpers/push_rules_helper.rb

 # frozen_string_literal: true
 
 module PushRulesHelper
+  def can_modify_group_push_rules?(current_user, group)
+    can?(current_user, :change_push_rules, group)
+  end
+
   def reject_unsigned_commits_description(push_rule)
     message = s_("ProjectSettings|Only signed commits can be pushed to this repository.")
 

ee/app/models/ee/group.rb

@@ -351,9 +351,9 @@ module EE
 
     def predefined_push_rule
       strong_memoize(:predefined_push_rule) do
-        if push_rule.present?
-          push_rule
-        elsif has_parent?
+        next push_rule if push_rule
+
+        if has_parent?
           parent.predefined_push_rule
         else
           PushRule.global

ee/app/models/push_rule.rb

@@ -21,7 +21,6 @@ class PushRule < ApplicationRecord
 
   belongs_to :project
 
-  validates :project, presence: true, unless: :is_sample?
   validates :max_file_size, numericality: { greater_than_or_equal_to: 0, only_integer: true }
   validates(*REGEX_COLUMNS, untrusted_regexp: true)
 
@@ -91,11 +90,12 @@ class PushRule < ApplicationRecord
     is_sample?
   end
 
-  def available?(feature_sym)
+  def available?(feature_sym, object: nil)
     if global?
       License.feature_available?(feature_sym)
     else
-      project&.feature_available?(feature_sym)
+      object ||= project
+      object&.feature_available?(feature_sym)
     end
   end
 

ee/app/policies/ee/group_policy.rb

@@ -70,6 +70,28 @@ module EE
         License.feature_available?(:cluster_health)
       end
 
+      with_scope :global
+      condition(:commit_committer_check_disabled_globally) do
+        !PushRule.global&.commit_committer_check
+      end
+
+      with_scope :global
+      condition(:reject_unsigned_commits_disabled_globally) do
+        !PushRule.global&.reject_unsigned_commits
+      end
+
+      condition(:commit_committer_check_available) do
+        @subject.feature_available?(:commit_committer_check)
+      end
+
+      condition(:reject_unsigned_commits_available) do
+        @subject.feature_available?(:reject_unsigned_commits)
+      end
+
+      condition(:push_rules_available) do
+        ::Feature.enabled?(:group_push_rules, @subject.root_ancestor) && @subject.feature_available?(:push_rules)
+      end
+
       rule { public_group | logged_in_viewable }.policy do
         enable :read_wiki
         enable :download_wiki_code
@@ -212,6 +234,26 @@ module EE
         prevent(*create_read_update_admin_destroy(:wiki))
         prevent(:download_wiki_code)
       end
+
+      rule { admin | (commit_committer_check_disabled_globally & can?(:maintainer_access)) }.policy do
+        enable :change_commit_committer_check
+      end
+
+      rule { commit_committer_check_available }.policy do
+        enable :read_commit_committer_check
+      end
+
+      rule { ~commit_committer_check_available }.policy do
+        prevent :change_commit_committer_check
+      end
+
+      rule { admin | (reject_unsigned_commits_disabled_globally & can?(:maintainer_access)) }.enable :change_reject_unsigned_commits
+
+      rule { reject_unsigned_commits_available }.enable :read_reject_unsigned_commits
+
+      rule { ~reject_unsigned_commits_available }.prevent :change_reject_unsigned_commits
+
+      rule { can?(:maintainer_access) & push_rules_available }.enable :change_push_rules
     end
 
     override :lookup_access_level!

ee/app/policies/ee/project_policy.rb

@@ -80,6 +80,34 @@ module EE
         License.feature_available?(:cluster_health)
       end
 
+      with_scope :subject
+      condition(:group_push_rules_enabled) do
+        @subject.group && ::Feature.enabled?(:group_push_rules, @subject.group.root_ancestor)
+      end
+
+      with_scope :subject
+      condition(:group_push_rule_present) do
+        group_push_rules_enabled? && subject.group.push_rule
+      end
+
+      with_scope :subject
+      condition(:reject_unsigned_commits_disabled_by_group) do
+        if group_push_rule_present?
+          !subject.group.push_rule.reject_unsigned_commits
+        else
+          true
+        end
+      end
+
+      with_scope :subject
+      condition(:commit_committer_check_disabled_by_group) do
+        if group_push_rule_present?
+          !subject.group.push_rule.commit_committer_check
+        else
+          true
+        end
+      end
+
       with_scope :subject
       condition(:commit_committer_check_available) do
         @subject.feature_available?(:commit_committer_check)
@@ -273,13 +301,13 @@ module EE
 
       rule { ~can?(:push_code) }.prevent :push_code_to_protected_branches
 
-      rule { admin | (reject_unsigned_commits_disabled_globally & can?(:maintainer_access)) }.enable :change_reject_unsigned_commits
+      rule { admin | (reject_unsigned_commits_disabled_globally & reject_unsigned_commits_disabled_by_group & can?(:maintainer_access)) }.enable :change_reject_unsigned_commits
 
       rule { reject_unsigned_commits_available }.enable :read_reject_unsigned_commits
 
       rule { ~reject_unsigned_commits_available }.prevent :change_reject_unsigned_commits
 
-      rule { admin | (commit_committer_check_disabled_globally & can?(:maintainer_access)) }.policy do
+      rule { admin | (commit_committer_check_disabled_globally & commit_committer_check_disabled_by_group & can?(:maintainer_access)) }.policy do
         enable :change_commit_committer_check
       end
 

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

@@ -7,7 +7,12 @@ module EE
 
       override :execute
       def execute
-        super.tap { |group| log_audit_event if group&.persisted? }
+        super.tap do |group|
+          next unless group&.persisted?
+
+          log_audit_event
+          create_push_rule_for_group
+        end
       end
 
       private
@@ -36,6 +41,18 @@ module EE
           action: :create
         ).for_group.security_event
       end
+
+      def create_push_rule_for_group
+        return unless ::Feature.enabled?(:group_push_rules, group.root_ancestor) && group.feature_available?(:push_rules)
+
+        push_rule = group.predefined_push_rule
+        return unless push_rule
+
+        attributes = push_rule.attributes.symbolize_keys.except(:is_sample, :id)
+        PushRule.create(attributes.compact).tap do |push_rule|
+          group.update(push_rule: push_rule)
+        end
+      end
     end
   end
 end

ee/app/services/ee/projects/create_service.rb

@@ -56,13 +56,29 @@ module EE
 
         predefined_push_rule = PushRule.find_by(is_sample: true)
 
-        if predefined_push_rule
+        if group_push_rule_available?
+          create_push_rule_from_group
+        elsif predefined_push_rule
+          log_info(predefined_push_rule)
           push_rule = predefined_push_rule.dup.tap { |gh| gh.is_sample = false }
           project.push_rule = push_rule
           project.project_setting.update(push_rule: push_rule)
         end
       end
 
+      def group_push_rule_available?
+        return false unless project.group
+        return false unless ::Feature.enabled?(:group_push_rules, project.group.root_ancestor)
+
+        !!project.group.predefined_push_rule
+      end
+
+      def create_push_rule_from_group
+        push_rule_attributes = project.group.predefined_push_rule.attributes.except("id")
+        project.create_push_rule(push_rule_attributes.merge(is_sample: false))
+        project.project_setting.update(push_rule: project.push_rule)
+      end
+
       # When using a project template from a Group, the new project can only be created
       # under the template owner's group or subgroups
       def validate_namespace_used_with_template(project, group_with_project_templates_id)

ee/app/views/admin/push_rules/_push_rules.html.haml

@@ -3,4 +3,4 @@
     .alert.alert-danger
       - @push_rule.errors.full_messages.each do |msg|
         %p= msg
-  = render "shared/push_rules/form", f: f
+  = render "shared/push_rules/form", f: f, context: nil

ee/app/views/groups/push_rules/edit.html.haml

+- page_title "Push Rules"
+%h3
+  = _("Pre-defined push rules.")
+%p.light
+  = _("Rules that define what git pushes are accepted for a project in this group. All newly created projects in this group will use these settings.")
+
+%hr.clearfix
+
+= form_for @push_rule, url: group_push_rules_path(@group), as: :push_rule, method: :put do |f|
+  - if @push_rule.errors.any?
+    .alert.alert-danger
+      - @push_rule.errors.full_messages.each do |msg|
+        %p= msg
+  = render "shared/push_rules/form", f: f, context: @group

ee/app/views/layouts/nav/ee/_push_rules_link.html.haml

+- return unless can_modify_group_push_rules?(current_user, @group)
+
+= nav_link(controller: :push_rules) do
+  = link_to edit_group_push_rules_path(@group), title: _('Push Rules') do
+    .nav-icon-container
+      = sprite_icon('push-rules')
+    %span.nav-item-name
+      = _('Push Rules')
+  %ul.sidebar-sub-level-items.is-fly-out-only
+    = nav_link(controller: :push_rules, html_options: { class: "fly-out-top-item" } ) do
+      = link_to edit_group_push_rules_path(@group), title: _('Push Rules') do
+        %strong.fly-out-top-item-name
+          = _('Push Rules')

ee/app/views/projects/push_rules/_index.html.haml

@@ -15,4 +15,4 @@
     = form_for [@project.namespace.becomes(Namespace), @project, @push_rule] do |f|
       = form_errors(@push_rule)
 
-      = render "shared/push_rules/form", f: f
+      = render "shared/push_rules/form", f: f, context: @project

ee/app/views/shared/push_rules/_commit_committer_check_setting.html.haml

-- return unless push_rule.available?(:commit_committer_check)
+- context = local_assigns.fetch(:context)
+
+- return unless push_rule.available?(:commit_committer_check, object: context)
 
 - form = local_assigns.fetch(:form)
 - push_rule = local_assigns.fetch(:push_rule)
 
 .form-check
-  = form.check_box :commit_committer_check, class: "form-check-input", disabled: !can_change_push_rule?(form.object, :commit_committer_check), data: { qa_selector: 'committer_restriction_checkbox' }
+  = form.check_box :commit_committer_check, class: "form-check-input", disabled: !can_change_push_rule?(form.object, :commit_committer_check, context), data: { qa_selector: 'committer_restriction_checkbox' }
   = form.label :commit_committer_check, class: "label-bold form-check-label" do
     = s_("PushRule|Committer restriction")
   %p.text-muted

ee/app/views/shared/push_rules/_form.html.haml

-= render 'shared/push_rules/commit_committer_check_setting', form: f, push_rule: f.object
+= render 'shared/push_rules/commit_committer_check_setting', form: f, push_rule: f.object, context: context
 
-= render 'shared/push_rules/reject_unsigned_commits_setting', form: f, push_rule: f.object
+= render 'shared/push_rules/reject_unsigned_commits_setting', form: f, push_rule: f.object, context: context
 
 .form-check
   = f.check_box :deny_delete_tag, class: "form-check-input", data: { qa_selector: 'deny_delete_tag_checkbox' }

ee/app/views/shared/push_rules/_reject_unsigned_commits_setting.html.haml

-- return unless push_rule.available?(:reject_unsigned_commits)
+- context = local_assigns.fetch(:context)
+
+- return unless push_rule.available?(:reject_unsigned_commits, object: context)
 
 - form = local_assigns.fetch(:form)
 - push_rule = local_assigns.fetch(:push_rule)
 
 .form-check
-  = form.check_box :reject_unsigned_commits, class: "form-check-input", disabled: !can_change_push_rule?(form.object, :reject_unsigned_commits), data: { qa_selector: 'reject_unsigned_commits_checkbox' }
+  = form.check_box :reject_unsigned_commits, class: "form-check-input", disabled: !can_change_push_rule?(form.object, :reject_unsigned_commits, context), data: { qa_selector: 'reject_unsigned_commits_checkbox' }
   = form.label :reject_unsigned_commits, class: "label-bold form-check-label" do
     Reject unsigned commits
   %p.text-muted

ee/changelogs/unreleased/34370-configure-group-push-rules-services-and-frontend.yml

+---
+title: Add push rules configuration for groups - frontend
+merge_request: 31085
+author:
+type: added

ee/config/routes/group.rb

@@ -146,6 +146,8 @@ constraints(::Constraints::GroupUrlConstrainer.new) do
       end
     end
 
+    resource :push_rules, only: [:edit, :update]
+
     resource :saml_providers, path: 'saml', only: [:show, :create, :update] do
       callback_methods = Rails.env.test? ? [:get, :post] : [:post]
       match :callback, to: 'omniauth_callbacks#group_saml', via: callback_methods

ee/spec/controllers/groups/push_rules_controller_spec.rb

+# frozen_string_literal: true
+require 'spec_helper'
+
+describe Groups::PushRulesController do
+  let_it_be(:group) { create(:group, :private) }
+  let_it_be(:user) { create(:user) }
+
+  describe '#show' do
+    context 'when user is at least a maintainer' do
+      before do
+        sign_in(user)
+        group.add_maintainer(user)
+      end
+
+      context 'when push rules feature is disabled' do
+        before do
+          stub_licensed_features(push_rules: false)
+        end
+
+        it 'returns 404 status' do
+          get :edit, params: { group_id: group }
+
+          expect(response).to have_gitlab_http_status(:not_found)
+        end
+      end
+
+      context 'when push rules feature is enabled' do
+        before do
+          stub_licensed_features(push_rules: true)
+        end
+
+        it 'returns 200 status' do
+          get :edit, params: { group_id: group }
+
+          expect(response).to have_gitlab_http_status(:ok)
+        end
+      end
+    end
+
+    context 'when user role is lower than maintainer' do
+      before do
+        sign_in(user)
+        group.add_developer(user)
+      end
+
+      context 'when push rules feature is disabled' do
+        before do
+          stub_licensed_features(push_rules: false)
+        end
+
+        it 'returns 404 status' do
+          get :edit, params: { group_id: group }
+
+          expect(response).to have_gitlab_http_status(:not_found)
+        end
+      end
+
+      context 'when push rules feature is enabled' do
+        before do
+          stub_licensed_features(push_rules: true)
+        end
+
+        it 'returns 404 status' do
+          get :edit, params: { group_id: group }
+
+          expect(response).to have_gitlab_http_status(:not_found)
+        end
+      end
+    end
+  end
+
+  describe '#update' do
+    def do_update
+      patch :update, params: { group_id: group, push_rule: { prevent_secrets: true } }
+    end
+
+    context 'when user is at least a maintainer' do
+      before do
+        sign_in(user)
+        group.add_maintainer(user)
+      end
+
+      context 'push rules unlicensed' do
+        before do
+          stub_licensed_features(push_rules: false)
+        end
+
+        it 'returns 404 status' do
+          do_update
+
+          expect(response).to have_gitlab_http_status(:not_found)
+        end
+      end
+
+      context 'push rules licensed', :enable_admin_mode do
+        before do
+          stub_licensed_features(push_rules: true)
+        end
+
+        it 'updates the push rule' do
+          do_update
+
+          expect(response).to have_gitlab_http_status(:found)
+          expect(group.reload.push_rule.prevent_secrets).to be_truthy
+        end
+
+        shared_examples 'updateable setting' do |rule_attr, new_value|
+          it 'updates the setting' do
+            patch :update, params: { group_id: group, push_rule: { rule_attr => new_value } }
+
+            expect(group.reload.push_rule.public_send(rule_attr)).to eq(new_value)
+          end
+        end
+
+        shared_examples 'not updateable setting' do |rule_attr, new_value|
+          it 'does not update the setting' do
+            expect do
+              patch :update, params: { group_id: group, push_rule: { rule_attr => new_value } }
+            end.not_to change { group.reload.push_rule.public_send(rule_attr) }
+          end
+        end
+
+        shared_examples 'an updatable setting with global default' do |rule_attr|
+          context "when #{rule_attr} not specified on global level" do
+            before do
+              stub_licensed_features(rule_attr => true)
+            end
+
+            it_behaves_like 'updateable setting', rule_attr, true
+          end
+
+          context "when global setting #{rule_attr} is enabled" do
+            before do
+              stub_licensed_features(rule_attr => true)
+              create(:push_rule_sample, rule_attr => true)
+            end
+
+            it_behaves_like 'updateable setting', rule_attr, true
+          end
+        end
+
+        shared_examples 'a not updatable setting with global default' do |rule_attr|
+          context "when #{rule_attr} is disabled" do
+            before do
+              stub_licensed_features(rule_attr => false)
+            end
+
+            it_behaves_like 'not updateable setting', rule_attr, true
+          end
+
+          context "when global setting #{rule_attr} is enabled" do
+            before do
+              stub_licensed_features(rule_attr => true)
+              create(:push_rule_sample, rule_attr => true)
+            end
+
+            it_behaves_like 'not updateable setting', rule_attr, true
+          end
+        end
+
+        PushRule::SETTINGS_WITH_GLOBAL_DEFAULT.each do |rule_attr|
+          context "Updating #{rule_attr} rule" do
+            let(:push_rule_for_group) { create(:push_rule, rule_attr => false) }
+
+            before do
+              group.update!(push_rule_id: push_rule_for_group.id)
+            end
+
+            context 'as an admin' do
+              let(:user) { create(:admin) }
+
+              it_behaves_like 'an updatable setting with global default', rule_attr, updates: true
+            end
+
+            context 'as a maintainer user' do
+              before do
+                group.add_maintainer(user)
+              end
+
+              context "when global setting #{rule_attr} is disabled" do
+                before do
+                  stub_licensed_features(rule_attr => false)
+                  create(:push_rule_sample, rule_attr => true)
+                end
+
+                it_behaves_like 'updateable setting', rule_attr, true
+              end
+
+              context "when global setting #{rule_attr} is enabled" do
+                before do
+                  stub_licensed_features(rule_attr => true)
+                  create(:push_rule_sample, rule_attr => true)
+                end
+
+                it_behaves_like 'not updateable setting', rule_attr, true
+              end
+            end
+
+            context 'as a developer user' do
+              before do
+                group.add_developer(user)
+              end
+
+              it_behaves_like 'a not updatable setting with global default', rule_attr
+            end
+          end
+        end
+      end
+    end
+
+    context 'when user role is lower than maintainer' do
+      before do
+        sign_in(user)
+        group.add_developer(user)
+      end
+
+      context 'push rules unlicensed' do
+        before do
+          stub_licensed_features(push_rules: false)
+        end
+
+        it 'returns 404 status' do
+          do_update
+
+          expect(response).to have_gitlab_http_status(:not_found)
+        end
+      end
+
+      context 'push rules licensed' do
+        before do
+          stub_licensed_features(push_rules: true)
+        end
+
+        it 'returns 404 status' do
+          do_update
+
+          expect(response).to have_gitlab_http_status(:not_found)
+        end
+      end
+    end
+  end
+end

ee/spec/factories/push_rules.rb

@@ -20,5 +20,9 @@ FactoryBot.define do
     factory :push_rule_sample do
       is_sample { true }
     end
+
+    factory :push_rule_without_project do
+      project { nil }
+    end
   end
 end

ee/spec/features/groups/navbar_spec.rb

@@ -13,6 +13,7 @@ describe 'Group navbar' do
 
   before do
     group.add_maintainer(user)
+    stub_feature_flags(group_push_rules: false)
     sign_in(user)
   end
 
@@ -154,7 +155,6 @@ describe 'Group navbar' do
           nav_sub_items: [_('Package Registry')]
         }
       )
-
       visit group_path(group)
     end
 
@@ -176,4 +176,27 @@ describe 'Group navbar' do
       it_behaves_like 'verified navigation bar'
     end
   end
+
+  context 'when push_rules for groups are available' do
+    before do
+      group.add_owner(user)
+
+      stub_feature_flags(group_push_rules: true)
+
+      insert_after_nav_item(
+        _('Merge Requests'),
+        new_nav_item: {
+          nav_item: _('Push Rules'),
+          nav_sub_items: []
+        }
+      )
+
+      insert_after_nav_item(_('Members'), new_nav_item: settings_nav_item)
+      insert_after_nav_item(_('Settings'), new_nav_item: administration_nav_item)
+
+      visit group_path(group)
+    end
+
+    it_behaves_like 'verified navigation bar'
+  end
 end

ee/spec/features/groups/push_rules_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+describe 'Groups > Push Rules', :js do
+  let_it_be(:user) { create(:user) }
+  let_it_be(:push_rule) { create(:push_rule_without_project) }
+  let_it_be(:group) { create(:group, push_rule: push_rule) }
+
+  before do
+    group.add_maintainer(user)
+    sign_in(user)
+  end
+
+  push_rules_with_titles = {
+    reject_unsigned_commits: 'Reject unsigned commits',
+    commit_committer_check: 'Committer restriction'
+  }
+
+  push_rules_with_titles.each do |rule_attr, title|
+    describe "#{rule_attr} rule" do
+      context 'unlicensed' do
+        before do
+          stub_licensed_features(rule_attr => false)
+        end
+
+        it 'does not render the setting checkbox' do
+          visit edit_group_push_rules_path(group)
+
+          expect(page).not_to have_content(title)
+        end
+      end
+
+      context 'licensed' do
+        before do
+          stub_licensed_features(rule_attr => true)
+        end
+
+        it 'renders the setting checkbox' do
+          visit edit_group_push_rules_path(group)
+
+          expect(page).to have_content(title)
+        end
+
+        describe 'with GL.com plans' do
+          before do
+            stub_application_setting(check_namespace_plan: true)
+          end
+
+          context 'when disabled' do
+            it 'does not render the setting checkbox' do
+              create(:gitlab_subscription, :bronze, namespace: group)
+
+              visit edit_group_push_rules_path(group)
+
+              expect(page).not_to have_content(title)
+            end
+          end
+
+          context 'when enabled' do
+            it 'renders the setting checkbox' do
+              create(:gitlab_subscription, :gold, namespace: group)
+
+              visit edit_group_push_rules_path(group)
+
+              expect(page).to have_content(title)
+            end
+          end
+        end
+      end
+    end
+  end
+end

ee/spec/models/push_rule_spec.rb

@@ -15,7 +15,6 @@ describe PushRule do
   end
 
   describe "Validation" do
-    it { is_expected.to validate_presence_of(:project) }
     it { is_expected.to validate_numericality_of(:max_file_size).is_greater_than_or_equal_to(0).only_integer }
 
     it 'validates RE2 regex syntax' do

ee/spec/policies/group_policy_spec.rb

@@ -728,6 +728,115 @@ describe GroupPolicy do
     end
   end
 
+  context 'commit_committer_check is not enabled by the current license' do
+    before do
+      stub_licensed_features(commit_committer_check: false)
+    end
+
+    let(:current_user) { maintainer }
+
+    it { is_expected.not_to be_allowed(:change_commit_committer_check) }
+    it { is_expected.not_to be_allowed(:read_commit_committer_check) }
+  end
+
+  context 'commit_committer_check is enabled by the current license' do
+    before do
+      stub_licensed_features(commit_committer_check: true)
+    end
+
+    context 'the user is a maintainer' do
+      let(:current_user) { maintainer }
+
+      it { is_expected.to be_allowed(:change_commit_committer_check) }
+      it { is_expected.to be_allowed(:read_commit_committer_check) }
+    end
+
+    context 'the user is a developer' do
+      let(:current_user) { developer }
+
+      it { is_expected.not_to be_allowed(:change_commit_committer_check) }
+      it { is_expected.to be_allowed(:read_commit_committer_check) }
+    end
+
+    context 'it is enabled on global level' do
+      before do
+        create(:push_rule_sample, commit_committer_check: true)
+      end
+
+      context 'the user is a maintainer' do
+        let(:current_user) { maintainer }
+
+        it { is_expected.not_to be_allowed(:change_commit_committer_check) }
+        it { is_expected.to be_allowed(:read_commit_committer_check) }
+      end
+
+      context 'the user is a developer' do
+        let(:current_user) { developer }
+
+        it { is_expected.not_to be_allowed(:change_commit_committer_check) }
+        it { is_expected.to be_allowed(:read_commit_committer_check) }
+      end
+    end
+  end
+
+  context 'reject_unsigned_commits is not enabled by the current license' do
+    before do
+      stub_licensed_features(reject_unsigned_commits: false)
+    end
+
+    let(:current_user) { maintainer }
+
+    it { is_expected.not_to be_allowed(:change_reject_unsigned_commits) }
+    it { is_expected.not_to be_allowed(:read_reject_unsigned_commits) }
+  end
+
+  context 'reject_unsigned_commits is enabled by the current license' do
+    before do
+      stub_licensed_features(reject_unsigned_commits: true)
+    end
+
+    context 'the user is a maintainer' do
+      let(:current_user) { maintainer }
+
+      it { is_expected.to be_allowed(:change_reject_unsigned_commits) }
+      it { is_expected.to be_allowed(:read_reject_unsigned_commits) }
+    end
+
+    context 'the user is a developer' do
+      let(:current_user) { developer }
+
+      it { is_expected.not_to be_allowed(:change_reject_unsigned_commits) }
+      it { is_expected.to be_allowed(:read_reject_unsigned_commits) }
+    end
+
+    context 'it is enabled on global level' do
+      before do
+        create(:push_rule_sample, reject_unsigned_commits: true)
+      end
+
+      context 'the user is a maintainer' do
+        let(:current_user) { maintainer }
+
+        it { is_expected.not_to be_allowed(:change_reject_unsigned_commits) }
+        it { is_expected.to be_allowed(:read_reject_unsigned_commits) }
+      end
+
+      context 'the user is a developer' do
+        let(:current_user) { developer }
+
+        it { is_expected.not_to be_allowed(:change_reject_unsigned_commits) }
+        it { is_expected.to be_allowed(:read_reject_unsigned_commits) }
+      end
+
+      context 'the user is an admin', :enable_admin_mode do
+        let(:current_user) { admin }
+
+        it { is_expected.to be_allowed(:change_reject_unsigned_commits) }
+        it { is_expected.to be_allowed(:read_reject_unsigned_commits) }
+      end
+    end
+  end
+
   shared_examples 'analytics policy' do |action|
     shared_examples 'policy by role' do |role|
       context role do

ee/spec/policies/project_policy_spec.rb

@@ -1126,6 +1126,46 @@ describe ProjectPolicy do
       it { is_expected.not_to be_allowed(:change_commit_committer_check) }
       it { is_expected.to be_allowed(:read_commit_committer_check) }
     end
+
+    context 'it is enabled on global level' do
+      before do
+        create(:push_rule_sample, commit_committer_check: true)
+      end
+
+      context 'when the user is a maintainer' do
+        let(:current_user) { maintainer }
+
+        it { is_expected.not_to be_allowed(:change_commit_committer_check) }
+        it { is_expected.to be_allowed(:read_commit_committer_check) }
+      end
+
+      context 'when the user is a developer' do
+        let(:current_user) { developer }
+
+        it { is_expected.not_to be_allowed(:change_commit_committer_check) }
+        it { is_expected.to be_allowed(:read_commit_committer_check) }
+      end
+    end
+
+    context 'it is enabled on group level' do
+      let(:push_rule) { create(:push_rule, commit_committer_check: true) }
+      let(:group) { create(:group, push_rule: push_rule) }
+      let(:project) { create(:project, namespace_id: group.id) }
+
+      context 'when the user is a maintainer' do
+        let(:current_user) { maintainer }
+
+        it { is_expected.not_to be_allowed(:change_commit_committer_check) }
+        it { is_expected.to be_allowed(:read_commit_committer_check) }
+      end
+
+      context 'when the user is a developer' do
+        let(:current_user) { developer }
+
+        it { is_expected.not_to be_allowed(:change_commit_committer_check) }
+        it { is_expected.to be_allowed(:read_commit_committer_check) }
+      end
+    end
   end
 
   context 'reject_unsigned_commits is not enabled by the current license' do
@@ -1144,19 +1184,59 @@ describe ProjectPolicy do
       stub_licensed_features(reject_unsigned_commits: true)
     end
 
-    context 'the user is a maintainer' do
+    context 'when the user is a maintainer' do
       let(:current_user) { maintainer }
 
       it { is_expected.to be_allowed(:change_reject_unsigned_commits) }
       it { is_expected.to be_allowed(:read_reject_unsigned_commits) }
     end
 
-    context 'the user is a developer' do
+    context 'when the user is a developer' do
       let(:current_user) { developer }
 
       it { is_expected.not_to be_allowed(:change_reject_unsigned_commits) }
       it { is_expected.to be_allowed(:read_reject_unsigned_commits) }
     end
+
+    context 'it is enabled on global level' do
+      before do
+        create(:push_rule_sample, reject_unsigned_commits: true)
+      end
+
+      context 'when the user is a maintainer' do
+        let(:current_user) { maintainer }
+
+        it { is_expected.not_to be_allowed(:change_reject_unsigned_commits) }
+        it { is_expected.to be_allowed(:read_reject_unsigned_commits) }
+      end
+
+      context 'when the user is a developer' do
+        let(:current_user) { developer }
+
+        it { is_expected.not_to be_allowed(:change_reject_unsigned_commits) }
+        it { is_expected.to be_allowed(:read_reject_unsigned_commits) }
+      end
+    end
+
+    context 'it is enabled on group level' do
+      let(:push_rule) { create(:push_rule_without_project, reject_unsigned_commits: true) }
+      let(:group) { create(:group, push_rule: push_rule) }
+      let(:project) { create(:project, namespace_id: group.id) }
+
+      context 'when the user is a maintainer' do
+        let(:current_user) { maintainer }
+
+        it { is_expected.not_to be_allowed(:change_reject_unsigned_commits) }
+        it { is_expected.to be_allowed(:read_reject_unsigned_commits) }
+      end
+
+      context 'when the user is a developer' do
+        let(:current_user) { developer }
+
+        it { is_expected.not_to be_allowed(:change_reject_unsigned_commits) }
+        it { is_expected.to be_allowed(:read_reject_unsigned_commits) }
+      end
+    end
   end
 
   context 'when timelogs report feature is enabled' do

ee/spec/services/groups/create_service_spec.rb

@@ -85,6 +85,61 @@ describe Groups::CreateService, '#execute' do
     end
   end
 
+  context 'creating group push rule' do
+    context 'when feature is available' do
+      before do
+        stub_licensed_features(push_rules: true)
+      end
+
+      context 'when there are push rules settings' do
+        let!(:sample) { create(:push_rule_sample) }
+
+        it 'uses the configured push rules settings' do
+          group = create_group(user, group_params)
+
+          expect(group.reload.push_rule).to have_attributes(
+            force_push_regex: sample.force_push_regex,
+            deny_delete_tag: sample.deny_delete_tag,
+            delete_branch_regex: sample.delete_branch_regex,
+            commit_message_regex: sample.commit_message_regex
+          )
+        end
+
+        context 'when feature flag is switched off' do
+          before do
+            stub_feature_flags(group_push_rules: false)
+          end
+
+          it 'does not create push rule' do
+            group = create_group(user, group_params)
+
+            expect(group.push_rule).to be_nil
+          end
+        end
+      end
+
+      context 'when there are not push rules settings' do
+        it 'is not creating the group push rule' do
+          group = create_group(user, group_params)
+
+          expect(group.push_rule).to be_nil
+        end
+      end
+    end
+
+    context 'when feature not is available' do
+      before do
+        stub_licensed_features(push_rules: false)
+      end
+
+      it 'ignores the group push rule' do
+        group = create_group(user, group_params)
+
+        expect(group.push_rule).to be_nil
+      end
+    end
+  end
+
   def create_group(user, opts)
     described_class.new(user, opts).execute
   end

ee/spec/services/projects/create_service_spec.rb

@@ -186,33 +186,120 @@ describe Projects::CreateService, '#execute' do
     end
   end
 
-  context 'push rules sample' do
-    let!(:sample) { create(:push_rule_sample) }
+  context 'push rules' do
+    context 'with sample' do
+      let!(:sample) { create(:push_rule_sample) }
 
-    subject(:push_rule) { create_project(user, opts).push_rule }
+      before do
+        stub_licensed_features(push_rules: true)
+      end
 
-    it 'creates push rule from sample' do
-      is_expected.to have_attributes(
-        force_push_regex: sample.force_push_regex,
-        deny_delete_tag: sample.deny_delete_tag,
-        delete_branch_regex: sample.delete_branch_regex,
-        commit_message_regex: sample.commit_message_regex
-      )
-    end
+      subject(:push_rule) { create_project(user, opts).push_rule }
 
-    it 'creates association between project settings and push rule' do
-      project_setting = subject.project.project_setting
+      it 'creates push rule from sample' do
+        is_expected.to have_attributes(
+          force_push_regex: sample.force_push_regex,
+          deny_delete_tag: sample.deny_delete_tag,
+          delete_branch_regex: sample.delete_branch_regex,
+          commit_message_regex: sample.commit_message_regex
+        )
+      end
 
-      expect(project_setting.push_rule_id).to eq(subject.id)
+      it 'creates association between project settings and push rule' do
+        project_setting = subject.project.project_setting
+
+        expect(project_setting.push_rule_id).to eq(subject.id)
+      end
+
+      context 'push rules unlicensed' do
+        before do
+          stub_licensed_features(push_rules: false)
+        end
+
+        subject(:push_rule) { create_project(user, opts).push_rule }
+
+        it 'ignores the push rule sample' do
+          is_expected.to be_nil
+        end
+      end
     end
 
-    context 'push rules unlicensed' do
+    context 'group push rules' do
       before do
-        stub_licensed_features(push_rules: false)
+        stub_licensed_features(push_rules: true)
+      end
+
+      context 'project created within a group' do
+        let(:group) { create(:group) }
+        let(:opts) do
+          {
+            name: "GitLab",
+            namespace_id: group.id
+          }
+        end
+
+        before do
+          group.add_owner(user)
+        end
+
+        context 'when group has push rule defined' do
+          let(:push_rule) { create(:push_rule_without_project, force_push_regex: 'testing me') }
+
+          before do
+            group.update!(push_rule: push_rule)
+            group.add_owner(user)
+          end
+
+          it 'creates push rule from group push rule' do
+            project = create_project(user, opts)
+            project_push_rule = project.push_rule
+
+            expect(project_push_rule).to have_attributes(
+              force_push_regex: push_rule.force_push_regex,
+              deny_delete_tag: push_rule.deny_delete_tag,
+              delete_branch_regex: push_rule.delete_branch_regex,
+              commit_message_regex: push_rule.commit_message_regex,
+              is_sample: false
+            )
+            expect(project.project_setting.push_rule_id).to eq(project_push_rule.id)
+          end
+
+          context 'when feature flag is switched off' do
+            let!(:sample) { create(:push_rule_sample) }
+
+            before do
+              stub_feature_flags(group_push_rules: false)
+            end
+
+            it 'creates push rule from sample' do
+              expect(create_project(user, opts).push_rule).to have_attributes(
+                force_push_regex: sample.force_push_regex,
+                deny_delete_tag: sample.deny_delete_tag,
+                delete_branch_regex: sample.delete_branch_regex,
+                commit_message_regex: sample.commit_message_regex
+              )
+            end
+          end
+        end
+
+        context 'when group has not push rule defined' do
+          let!(:sample) { create(:push_rule_sample) }
+
+          it 'creates push rule from sample' do
+            expect(create_project(user, opts).push_rule).to have_attributes(
+              force_push_regex: sample.force_push_regex,
+              deny_delete_tag: sample.deny_delete_tag,
+              delete_branch_regex: sample.delete_branch_regex,
+              commit_message_regex: sample.commit_message_regex
+            )
+          end
+        end
       end
+    end
 
-      it 'ignores the push rule sample' do
-        is_expected.to be_nil
+    context 'when there are no push rules' do
+      it 'does not create push rule' do
+        expect(create_project(user, opts).push_rule).to be_nil
       end
     end
   end

locale/gitlab.pot

@@ -15703,6 +15703,9 @@ msgstr ""
 msgid "Point to any links you like: documentation, built binaries, or other related materials. These can be internal or external links from your GitLab instance. Duplicate URLs are not allowed."
 msgstr ""
 
+msgid "Pre-defined push rules."
+msgstr ""
+
 msgid "Preferences"
 msgstr ""
 
@@ -18277,6 +18280,9 @@ msgstr ""
 msgid "Rook"
 msgstr ""
 
+msgid "Rules that define what git pushes are accepted for a project in this group. All newly created projects in this group will use these settings."
+msgstr ""
+
 msgid "Rules that define what git pushes are accepted for a project. All newly created projects will use these settings."
 msgstr ""
 

spec/features/groups/navbar_spec.rb

@@ -10,7 +10,42 @@ describe 'Group navbar' do
   let_it_be(:user) { create(:user) }
   let_it_be(:group) { create(:group) }
 
+  let(:structure) do
+    [
+      {
+        nav_item: _('Group overview'),
+        nav_sub_items: [
+          _('Details'),
+          _('Activity')
+        ]
+      },
+      {
+        nav_item: _('Issues'),
+        nav_sub_items: [
+          _('List'),
+          _('Board'),
+          _('Labels'),
+          _('Milestones')
+        ]
+      },
+      {
+        nav_item: _('Merge Requests'),
+        nav_sub_items: []
+      },
+      {
+        nav_item: _('Kubernetes'),
+        nav_sub_items: []
+      },
+      (analytics_nav_item if Gitlab.ee?),
+      {
+        nav_item: _('Members'),
+        nav_sub_items: []
+      }
+    ]
+  end
+
   before do
+    stub_feature_flags(group_push_rules: false)
     group.add_maintainer(user)
     sign_in(user)
   end