Make the GroupFinder specs more strict

Ensure the results match exactly and project authorizations do allow access to
sibling groups/projects deeper down.

Also apply WHERE scopes before running the UNION, to increase performance.

1 files changed, 12 insertions, 4 deletions

diff --git a/app/finders/groups_finder.rb b/app/finders/groups_finder.rb
index cb4ab6eacc1..e6fb112e7f2 100644
--- a/app/finders/groups_finder.rb
+++ b/app/finders/groups_finder.rb
@@ -5,8 +5,10 @@ class GroupsFinder < UnionFinder
   end
 
   def execute
-    groups = find_union(all_groups, Group).with_route.order_id_desc
-    by_parent(groups)
+    items = all_groups.map do |item|
+      by_parent(item)
+    end
+    find_union(items, Group).with_route.order_id_desc
   end
 
   private
@@ -17,8 +19,6 @@ class GroupsFinder < UnionFinder
     groups = []
 
     if current_user
-      groups_for_ancestors = find_union([current_user.authorized_groups, authorized_project_groups], Group)
-      groups_for_descendants = current_user.authorized_groups
       groups << Gitlab::GroupHierarchy.new(groups_for_ancestors, groups_for_descendants).all_groups
     end
     groups << Group.unscoped.public_to_user(current_user)
@@ -26,6 +26,14 @@ class GroupsFinder < UnionFinder
     groups
   end
 
+  def groups_for_ancestors
+    current_user.authorized_groups
+  end
+
+  def groups_for_descendants
+    current_user.groups
+  end
+
   def by_parent(groups)
     return groups unless params[:parent]