Merge branch 'add-hierarchy-share-setting-to-ui' into 'master'

Add Prevent Sharing Outside Hierarchy to Group Settings UI

See merge request gitlab-org/gitlab!63810

app/controllers/groups_controller.rb

@@ -265,7 +265,8 @@ class GroupsController < Groups::ApplicationController
       :default_branch_protection,
       :default_branch_name,
       :allow_mfa_for_subgroups,
-      :resource_access_token_creation_allowed
+      :resource_access_token_creation_allowed,
+      :prevent_sharing_groups_outside_hierarchy
     ]
   end
 

app/helpers/groups_helper.rb

@@ -77,6 +77,10 @@ module GroupsHelper
     can?(current_user, :change_share_with_group_lock, group)
   end
 
+  def can_change_prevent_sharing_groups_outside_hierarchy?(group)
+    can?(current_user, :change_prevent_sharing_groups_outside_hierarchy, group)
+  end
+
   def can_disable_group_emails?(group)
     can?(current_user, :set_emails_disabled, group) && !group.parent&.emails_disabled?
   end
@@ -188,6 +192,14 @@ module GroupsHelper
     end
   end
 
+  def link_to_group(group)
+    link_to(group.name, group_path(group))
+  end
+
+  def prevent_sharing_groups_outside_hierarchy_help_text(group)
+    s_("GroupSettings|This setting is only available on the top-level group and it applies to all subgroups. Groups that have already been shared with a group outside %{group} will still be shared, and this access will have to be revoked manually.").html_safe % { group: link_to_group(group) }
+  end
+
   def parent_group_options(current_group)
     exclude_groups = current_group.self_and_descendants.pluck_primary_key
     exclude_groups << current_group.parent_id if current_group.parent_id

app/models/group.rb

@@ -80,6 +80,8 @@ class Group < Namespace
   # debian_distributions and associated component_files must be destroyed by ruby code in order to properly remove carrierwave uploads
   has_many :debian_distributions, class_name: 'Packages::Debian::GroupDistribution', dependent: :destroy # rubocop:disable Cop/ActiveRecordDependent
 
+  delegate :prevent_sharing_groups_outside_hierarchy, to: :namespace_settings
+
   accepts_nested_attributes_for :variables, allow_destroy: true
 
   validate :visibility_level_allowed_by_projects

app/models/namespace_setting.rb

@@ -14,7 +14,8 @@ class NamespaceSetting < ApplicationRecord
   before_validation :normalize_default_branch_name
 
   NAMESPACE_SETTINGS_PARAMS = [:default_branch_name, :delayed_project_removal,
-                               :lock_delayed_project_removal, :resource_access_token_creation_allowed].freeze
+                               :lock_delayed_project_removal, :resource_access_token_creation_allowed,
+                               :prevent_sharing_groups_outside_hierarchy].freeze
 
   self.primary_key = :namespace_id
 

app/policies/group_policy.rb

@@ -155,6 +155,7 @@ class GroupPolicy < BasePolicy
 
     enable :set_note_created_at
     enable :set_emails_disabled
+    enable :change_prevent_sharing_groups_outside_hierarchy
     enable :update_default_branch_protection
     enable :create_deploy_token
     enable :destroy_deploy_token

app/services/namespace_settings/update_service.rb

@@ -14,6 +14,7 @@ module NamespaceSettings
 
     def execute
       validate_resource_access_token_creation_allowed_param
+      validate_prevent_sharing_groups_outside_hierarchy_param
 
       if group.namespace_settings
         group.namespace_settings.attributes = settings_params
@@ -32,6 +33,15 @@ module NamespaceSettings
         group.namespace_settings.errors.add(:resource_access_token_creation_allowed, _('can only be changed by a group admin.'))
       end
     end
+
+    def validate_prevent_sharing_groups_outside_hierarchy_param
+      return if settings_params[:prevent_sharing_groups_outside_hierarchy].nil?
+
+      unless can?(current_user, :change_prevent_sharing_groups_outside_hierarchy, group)
+        settings_params.delete(:prevent_sharing_groups_outside_hierarchy)
+        group.namespace_settings.errors.add(:prevent_sharing_groups_outside_hierarchy, _('can only be changed by a group admin.'))
+      end
+    end
   end
 end
 

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

@@ -7,13 +7,21 @@
     .form-group
       = render 'shared/allow_request_access', form: f
 
+    - if @group.root?
+      .form-group.gl-mb-3
+        .gl-form-checkbox.custom-control.custom-checkbox
+          = f.check_box :prevent_sharing_groups_outside_hierarchy, disabled: !can_change_prevent_sharing_groups_outside_hierarchy?(@group), class: 'custom-control-input'
+          = f.label :prevent_sharing_groups_outside_hierarchy, class: 'custom-control-label' do
+            %span
+              = s_('GroupSettings|Prevent members from sending invitations to groups outside of %{group} and its subgroups.').html_safe % { group: link_to_group(@group) }
+            %p.js-descr.help-text= prevent_sharing_groups_outside_hierarchy_help_text(@group)
+
     .form-group.gl-mb-3
       .gl-form-checkbox.custom-control.custom-checkbox
         = f.check_box :share_with_group_lock, disabled: !can_change_share_with_group_lock?(@group), class: 'custom-control-input'
         = f.label :share_with_group_lock, class: 'custom-control-label' do
           %span
-            - group_link = link_to @group.name, group_path(@group)
-            = s_('GroupSettings|Prevent sharing a project within %{group} with other groups').html_safe % { group: group_link }
+            = s_('GroupSettings|Prevent sharing a project within %{group} with other groups').html_safe % { group: link_to_group(@group) }
           %p.js-descr.help-text= share_with_group_lock_help_text(@group)
 
     .form-group.gl-mb-3

doc/user/group/index.md

@@ -425,6 +425,30 @@ To restore a group that is marked for deletion:
 1. Expand the **Path, transfer, remove** section.
 1. In the Restore group section, select **Restore group**.
 
+## Prevent group sharing outside the group hierarchy
+
+This setting is only available on top-level groups. It affects all subgroups.
+
+When checked, any group within the top-level group hierarchy can be shared only with other groups within the hierarchy.
+
+For example, with these groups:
+
+- **Animals > Dogs**
+- **Animals > Cats**
+- **Plants > Trees**
+
+If you select this setting in the **Animals** group:
+
+- **Dogs** can be shared with **Cats**.
+- **Dogs** cannot be shared with **Trees**.
+
+To prevent sharing outside of the group's hierarchy:
+
+1. Go to the group's **Settings > General** page.
+1. Expand the **Permissions, LFS, 2FA** section.
+1. Select **Prevent members from sending invitations to groups outside of `<group_name>` and its subgroups**.
+1. Select **Save changes**.
+
 ## Prevent a project from being shared with groups
 
 Prevent projects in a group from [sharing

locale/gitlab.pot

@@ -15904,6 +15904,9 @@ msgstr ""
 msgid "GroupSettings|Prevent forking setting was not saved"
 msgstr ""
 
+msgid "GroupSettings|Prevent members from sending invitations to groups outside of %{group} and its subgroups."
+msgstr ""
+
 msgid "GroupSettings|Prevent sharing a project within %{group} with other groups"
 msgstr ""
 
@@ -15940,6 +15943,9 @@ msgstr ""
 msgid "GroupSettings|This setting is applied on %{ancestor_group}. You can override the setting or %{remove_ancestor_share_with_group_lock}."
 msgstr ""
 
+msgid "GroupSettings|This setting is only available on the top-level group and it applies to all subgroups. Groups that have already been shared with a group outside %{group} will still be shared, and this access will have to be revoked manually."
+msgstr ""
+
 msgid "GroupSettings|This setting will be applied to all subgroups unless overridden by a group owner. Groups that already have access to the project will continue to have access unless removed manually."
 msgstr ""
 

spec/controllers/groups_controller_spec.rb

@@ -651,6 +651,45 @@ RSpec.describe GroupsController, factory_default: :keep do
     end
   end
 
+  describe 'updating :prevent_sharing_groups_outside_hierarchy' do
+    subject do
+      put :update,
+        params: {
+          id: group.to_param,
+          group: { prevent_sharing_groups_outside_hierarchy: true }
+        }
+    end
+
+    context 'when user is a group owner' do
+      before do
+        group.add_owner(user)
+        sign_in(user)
+      end
+
+      it 'updates the attribute' do
+        expect { subject }
+            .to change { group.namespace_settings.reload.prevent_sharing_groups_outside_hierarchy }
+            .from(false)
+            .to(true)
+
+        expect(response).to have_gitlab_http_status(:found)
+      end
+    end
+
+    context 'when not a group owner' do
+      before do
+        group.add_maintainer(user)
+        sign_in(user)
+      end
+
+      it 'does not update the attribute' do
+        expect { subject }.not_to change { group.namespace_settings.reload.prevent_sharing_groups_outside_hierarchy }
+
+        expect(response).to have_gitlab_http_status(:not_found)
+      end
+    end
+  end
+
   describe '#ensure_canonical_path' do
     before do
       sign_in(user)

spec/features/groups/group_settings_spec.rb

@@ -153,6 +153,26 @@ RSpec.describe 'Edit group settings' do
     end
   end
 
+  describe 'prevent sharing outside group hierarchy setting' do
+    it 'updates the setting' do
+      visit edit_group_path(group)
+
+      check 'group_prevent_sharing_groups_outside_hierarchy'
+
+      expect { save_permissions_group }.to change {
+        group.reload.namespace_settings.prevent_sharing_groups_outside_hierarchy
+      }.to(true)
+    end
+
+    it 'is not present for a subgroup' do
+      subgroup = create(:group, parent: group)
+      visit edit_group_path(subgroup)
+
+      expect(page).to have_text "Permissions"
+      expect(page).not_to have_selector('#group_prevent_sharing_groups_outside_hierarchy')
+    end
+  end
+
   def update_path(new_group_path)
     visit edit_group_path(group)
 

spec/services/namespace_settings/update_service_spec.rb

@@ -75,5 +75,37 @@ RSpec.describe NamespaceSettings::UpdateService do
         end
       end
     end
+
+    context "updating :prevent_sharing_groups_outside_hierarchy" do
+      let(:settings) { { prevent_sharing_groups_outside_hierarchy: true } }
+
+      context 'when user is a group owner' do
+        before do
+          group.add_owner(user)
+        end
+
+        it 'changes settings' do
+          expect { service.execute }
+            .to change { group.namespace_settings.prevent_sharing_groups_outside_hierarchy }
+            .from(false).to(true)
+        end
+      end
+
+      context 'when user is not a group owner' do
+        before do
+          group.add_maintainer(user)
+        end
+
+        it 'does not change settings' do
+          expect { service.execute }.not_to change { group.namespace_settings.prevent_sharing_groups_outside_hierarchy }
+        end
+
+        it 'returns the group owner error' do
+          service.execute
+
+          expect(group.namespace_settings.errors.messages[:prevent_sharing_groups_outside_hierarchy]).to include('can only be changed by a group admin.')
+        end
+      end
+    end
   end
 end