All ancestor groups should be returned by GroupsFinder

And not only the group in which the authorized project is located, but every
group that is a parent or grandparent of the group in which the project
resides.

5 files changed, 39 insertions, 5 deletions

diff --git a/app/finders/groups_finder.rb b/app/finders/groups_finder.rb
index e0adc32bd95..03d145c4ccd 100644
--- a/app/finders/groups_finder.rb
+++ b/app/finders/groups_finder.rb
@@ -18,7 +18,7 @@ class GroupsFinder < UnionFinder
 
     if current_user
       groups << Group.member_self_and_descendants(current_user.id)
-      groups << Group.where(id: current_user.authorized_projects.select(:namespace_id))
+      groups << current_user.groups_through_project_authorizations
     end
     groups << Group.unscoped.public_to_user(current_user)
 
diff --git a/app/models/user.rb b/app/models/user.rb
index ad24ec0ec08..e2d840a1fe0 100644
--- a/app/models/user.rb
+++ b/app/models/user.rb
@@ -492,6 +492,18 @@ class User < ActiveRecord::Base
     Group.where("namespaces.id IN (#{union.to_sql})")
   end
 
+  def groups_through_project_authorizations
+    paths = Project.member_self_and_descendants(id).pluck('routes.path')
+
+    return Group.none if paths.empty?
+
+    wheres = paths.map do |path|
+      "#{ActiveRecord::Base.connection.quote(path)} LIKE CONCAT(routes.path, '/%')"
+    end
+
+    Group.joins(:route).where(wheres.join(' OR '))
+  end
+
   def nested_groups
     Group.member_descendants(id)
   end
diff --git a/spec/controllers/groups_controller_spec.rb b/spec/controllers/groups_controller_spec.rb
index 6b65e1c6cc5..3b4cb2357a0 100644
--- a/spec/controllers/groups_controller_spec.rb
+++ b/spec/controllers/groups_controller_spec.rb
@@ -2,7 +2,7 @@ require 'rails_helper'
 
 describe GroupsController do
   let(:user) { create(:user) }
-  let(:group) { create(:group) }
+  let(:group) { create(:group, :public) }
   let(:project) { create(:empty_project, namespace: group) }
   let!(:group_member) { create(:group_member, group: group, user: user) }
 
@@ -35,14 +35,15 @@ describe GroupsController do
         sign_in(user)
       end
 
-      it 'shows the public subgroups' do
+      it 'shows all subgroups' do
         get :subgroups, id: group.to_param
 
-        expect(assigns(:nested_groups)).to contain_exactly(public_subgroup)
+        expect(assigns(:nested_groups)).to contain_exactly(public_subgroup, private_subgroup)
       end
 
-      context 'being member' do
+      context 'being member of private subgroup' do
         it 'shows public and private subgroups the user is member of' do
+          group_member.destroy!
           private_subgroup.add_guest(user)
 
           get :subgroups, id: group.to_param
diff --git a/spec/finders/groups_finder_spec.rb b/spec/finders/groups_finder_spec.rb
index e50870f63b5..31f1f753d4d 100644
--- a/spec/finders/groups_finder_spec.rb
+++ b/spec/finders/groups_finder_spec.rb
@@ -85,6 +85,14 @@ describe GroupsFinder do
 
               is_expected.to include(private_subgroup)
             end
+
+            it 'returns all the parent groups if project is several levels deep' do
+              private_subsubgroup = create(:group, :private, parent: private_subgroup)
+              subsubproject = create(:empty_project, :private, namespace: private_subsubgroup)
+              subsubproject.add_guest(user)
+
+              is_expected.to include(private_subsubgroup)
+            end
           end
         end
       end
diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb
index 9be4996192b..d78325b5ef8 100644
--- a/spec/models/user_spec.rb
+++ b/spec/models/user_spec.rb
@@ -1251,6 +1251,19 @@ describe User, models: true do
     it { is_expected.to eq([private_group]) }
   end
 
+  describe '#groups_through_project_authorizations' do
+    it 'returns all groups being ancestor of the authorized project' do
+      user = create(:user)
+      group = create(:group, :private)
+      subgroup = create(:group, :private, parent: group)
+      subsubgroup = create(:group, :private, parent: subgroup)
+      project = create(:empty_project, :private, namespace: subsubgroup)
+      project.add_guest(user)
+
+      expect(user.groups_through_project_authorizations).to contain_exactly(group, subgroup, subsubgroup)
+    end
+  end
+
   describe '#authorized_projects', truncate: true do
     context 'with a minimum access level' do
       it 'includes projects for which the user is an owner' do