Don't allow a project to be shared with an ancestor of the group it is in

2 files changed, 25 insertions, 3 deletions

diff --git a/app/models/project_group_link.rb b/app/models/project_group_link.rb
index 5cb6b0c527d..ac1e9ab2b0b 100644
--- a/app/models/project_group_link.rb
+++ b/app/models/project_group_link.rb
@@ -33,8 +33,15 @@ class ProjectGroupLink < ActiveRecord::Base
   private
 
   def different_group
-    if self.group && self.project && self.project.group == self.group
-      errors.add(:base, "Project cannot be shared with the project it is in.")
+    return unless self.group && self.project
+
+    project_group = self.project.group
+    return unless project_group
+
+    group_ids = project_group.ancestors.map(&:id).push(project_group.id)
+
+    if group_ids.include?(self.group.id)
+      errors.add(:base, "Project cannot be shared with the group it is in or one of its ancestors.")
     end
   end
 
diff --git a/spec/models/project_group_link_spec.rb b/spec/models/project_group_link_spec.rb
index 59a4ae1b799..9b711bfc007 100644
--- a/spec/models/project_group_link_spec.rb
+++ b/spec/models/project_group_link_spec.rb
@@ -7,12 +7,27 @@ describe ProjectGroupLink do
   end
 
   describe "Validation" do
-    let!(:project_group_link) { create(:project_group_link) }
+    let(:parent_group) { create(:group) }
+    let(:group) { create(:group, parent: parent_group) }
+    let(:project) { create(:project, group: group) }
+    let!(:project_group_link) { create(:project_group_link, project: project) }
 
     it { should validate_presence_of(:project_id) }
     it { should validate_uniqueness_of(:group_id).scoped_to(:project_id).with_message(/already shared/) }
     it { should validate_presence_of(:group) }
     it { should validate_presence_of(:group_access) }
+
+    it "doesn't allow a project to be shared with the group it is in" do
+      project_group_link.group = group
+
+      expect(project_group_link).not_to be_valid
+    end
+
+    it "doesn't allow a project to be shared with an ancestor of the group it is in" do
+      project_group_link.group = parent_group
+
+      expect(project_group_link).not_to be_valid
+    end
   end
 
   describe "destroying a record", truncate: true do