Add User#nested_groups and User#nested_projects methodsdz-nested-groups-access-improvements

Signed-off-by: Dmitriy Zaporozhets <dmitriy.zaporozhets@gmail.com>

6 files changed, 71 insertions, 2 deletions

diff --git a/app/models/concerns/routable.rb b/app/models/concerns/routable.rb
index 1108a64c59e..2b93aa30c0f 100644
--- a/app/models/concerns/routable.rb
+++ b/app/models/concerns/routable.rb
@@ -60,6 +60,21 @@ module Routable
         joins(:route).where(wheres.join(' OR '))
       end
     end
+
+    # Builds a relation to find multiple objects that are nested under user membership
+    #
+    # Usage:
+    #
+    #     Klass.member_descendants(1)
+    #
+    # Returns an ActiveRecord::Relation.
+    def member_descendants(user_id)
+      joins(:route).
+        joins("INNER JOIN routes r2 ON routes.path LIKE CONCAT(r2.path, '/%')
+               INNER JOIN members ON members.source_id = r2.source_id
+               AND members.source_type = r2.source_type").
+        where('members.user_id = ?', user_id)
+    end
   end
 
   private
diff --git a/app/models/route.rb b/app/models/route.rb
index ebd18dce737..dd171fdb069 100644
--- a/app/models/route.rb
+++ b/app/models/route.rb
@@ -15,8 +15,9 @@ class Route < ActiveRecord::Base
     # rubocop:disable Rails/FindEach
     Route.where('path LIKE ?', "#{path_was}/%").each do |route|
       # Note that update column skips validation and callbacks.
-      # We need this to avoid recursive call of rename_children method
+      # We need this to avoid recursive call of rename_descendants method
       route.update_column(:path, route.path.sub(path_was, path))
     end
+    # rubocop:enable Rails/FindEach
   end
 end
diff --git a/app/models/user.rb b/app/models/user.rb
index 06dd98a3188..f294b9f77c1 100644
--- a/app/models/user.rb
+++ b/app/models/user.rb
@@ -439,6 +439,15 @@ class User < ActiveRecord::Base
     Group.where("namespaces.id IN (#{union.to_sql})")
   end
 
+  def nested_groups
+    Group.member_descendants(id)
+  end
+
+  def nested_projects
+    Project.joins(:namespace).where('namespaces.parent_id IS NOT NULL').
+      member_descendants(id)
+  end
+
   def refresh_authorized_projects
     Users::RefreshAuthorizedProjectsService.new(self).execute
   end
diff --git a/app/services/users/refresh_authorized_projects_service.rb b/app/services/users/refresh_authorized_projects_service.rb
index 21ec1bd9e65..84d3c1c4373 100644
--- a/app/services/users/refresh_authorized_projects_service.rb
+++ b/app/services/users/refresh_authorized_projects_service.rb
@@ -119,7 +119,8 @@ module Users
         user.personal_projects.select("#{user.id} AS user_id, projects.id AS project_id, #{Gitlab::Access::MASTER} AS access_level"),
         user.groups_projects.select_for_project_authorization,
         user.projects.select_for_project_authorization,
-        user.groups.joins(:shared_projects).select_for_project_authorization
+        user.groups.joins(:shared_projects).select_for_project_authorization,
+        user.nested_projects.select_for_project_authorization
       ]
 
       Gitlab::SQL::Union.new(relations)
diff --git a/spec/models/concerns/routable_spec.rb b/spec/models/concerns/routable_spec.rb
index b556135532f..30443534cca 100644
--- a/spec/models/concerns/routable_spec.rb
+++ b/spec/models/concerns/routable_spec.rb
@@ -68,4 +68,14 @@ describe Group, 'Routable' do
       end
     end
   end
+
+  describe '.member_descendants' do
+    let!(:user) { create(:user) }
+    let!(:nested_group) { create(:group, parent: group) }
+
+    before { group.add_owner(user) }
+    subject { described_class.member_descendants(user.id) }
+
+    it { is_expected.to eq([nested_group]) }
+  end
 end
diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb
index 8b20ee81614..f2ed2d45dca 100644
--- a/spec/models/user_spec.rb
+++ b/spec/models/user_spec.rb
@@ -1356,6 +1356,39 @@ describe User, models: true do
     end
   end
 
+  describe '#nested_groups' do
+    let!(:user) { create(:user) }
+    let!(:group) { create(:group) }
+    let!(:nested_group) { create(:group, parent: group) }
+
+    before do
+      group.add_owner(user)
+
+      # Add more data to ensure method does not include wrong groups
+      create(:group).add_owner(create(:user))
+    end
+
+    it { expect(user.nested_groups).to eq([nested_group]) }
+  end
+
+  describe '#nested_projects' do
+    let!(:user) { create(:user) }
+    let!(:group) { create(:group) }
+    let!(:nested_group) { create(:group, parent: group) }
+    let!(:project) { create(:project, namespace: group) }
+    let!(:nested_project) { create(:project, namespace: nested_group) }
+
+    before do
+      group.add_owner(user)
+
+      # Add more data to ensure method does not include wrong projects
+      other_project = create(:project, namespace: create(:group, :nested))
+      other_project.add_developer(create(:user))
+    end
+
+    it { expect(user.nested_projects).to eq([nested_project]) }
+  end
+
   describe '#refresh_authorized_projects', redis: true do
     let(:project1) { create(:empty_project) }
     let(:project2) { create(:empty_project) }