Add validation for visibility level of sub groups

Sub groups should not have a visibility level higher than its parent.

2 files changed, 42 insertions, 0 deletions

diff --git a/app/models/group.rb b/app/models/group.rb
index 2816a68257c..15355418d05 100644
--- a/app/models/group.rb
+++ b/app/models/group.rb
@@ -26,6 +26,7 @@ class Group < Namespace
 
   validate :avatar_type, if: ->(user) { user.avatar.present? && user.avatar_changed? }
   validate :visibility_level_allowed_by_projects
+  validate :visibility_level_allowed_by_parent
 
   validates :avatar, file_size: { maximum: 200.kilobytes.to_i }
 
@@ -102,6 +103,14 @@ class Group < Namespace
     full_name
   end
 
+  def visibility_level_allowed_by_parent
+    return if parent_id.blank?
+
+    if parent && (visibility_level > parent.visibility_level)
+      errors.add(:visibility_level, "#{visibility} is not allowed since the parent group has a #{parent.visibility} visibility.")
+    end
+  end
+
   def visibility_level_allowed_by_projects
     allowed_by_projects = self.projects.where('visibility_level > ?', self.visibility_level).none?
 
diff --git a/spec/models/group_spec.rb b/spec/models/group_spec.rb
index c5bfae47606..a3310cf1dce 100644
--- a/spec/models/group_spec.rb
+++ b/spec/models/group_spec.rb
@@ -84,6 +84,39 @@ describe Group do
         expect(group).not_to be_valid
       end
     end
+
+    describe '#visibility_level_allowed_by_parent' do
+      let(:parent) { create(:group, :internal) }
+      let(:sub_group) { build(:group, parent_id: parent.id) }
+
+      context 'without a parent' do
+        it 'is valid' do
+          sub_group.parent_id = nil
+
+          expect(sub_group).to be_valid
+        end
+      end
+
+      context 'with a parent' do
+        context 'when visibility of sub group is greater than the parent' do
+          it 'is invalid' do
+            sub_group.visibility_level = Gitlab::VisibilityLevel::PUBLIC
+
+            expect(sub_group).to be_invalid
+          end
+        end
+
+        context 'when visibility of sub group is lower or equal to the parent' do
+          [Gitlab::VisibilityLevel::INTERNAL, Gitlab::VisibilityLevel::PRIVATE].each do |level|
+            it 'is valid' do
+              sub_group.visibility_level = level
+
+              expect(sub_group).to be_valid
+            end
+          end
+        end
+      end
+    end
   end
 
   describe '.visible_to_user' do