Limit labels returned for a specific project as an administratorrs-issue-24527

Prior, an administrator viewing a project's Labels page would see _all_
labels from every project they had access to, rather than only the
labels of that specific project (if any).

This was not an information disclosure, as admins have access to
everything, but it was a performance issue.

3 files changed, 42 insertions, 24 deletions

diff --git a/app/finders/labels_finder.rb b/app/finders/labels_finder.rb
index 865f093f04a..111efb1d9d4 100644
--- a/app/finders/labels_finder.rb
+++ b/app/finders/labels_finder.rb
@@ -18,9 +18,11 @@ class LabelsFinder < UnionFinder
   def label_ids
     label_ids = []
 
-    if project
-      label_ids << project.group.labels if project.group.present?
-      label_ids << project.labels
+    if project?
+      if project
+        label_ids << project.group.labels if project.group.present?
+        label_ids << project.labels
+      end
     else
       label_ids << Label.where(group_id: projects.group_ids)
       label_ids << Label.where(project_id: projects.select(:id))
@@ -30,6 +32,8 @@ class LabelsFinder < UnionFinder
   end
 
   def sort(items)
+    return Label.none if items.blank?
+
     items.reorder(title: :asc)
   end
 
@@ -40,16 +44,16 @@ class LabelsFinder < UnionFinder
     items.where(title: title)
   end
 
-  def group_id
-    params[:group_id].presence
+  def group?
+    params[:group_id].present?
   end
 
-  def project_id
-    params[:project_id].presence
+  def project?
+    params[:project_id].present?
   end
 
-  def projects_ids
-    params[:project_ids]
+  def projects?
+    params[:project_ids].present?
   end
 
   def title
@@ -59,8 +63,9 @@ class LabelsFinder < UnionFinder
   def project
     return @project if defined?(@project)
 
-    if project_id
-      @project = find_project
+    if project?
+      @project = Project.find(params[:project_id])
+      @project = nil unless authorized_to_read_labels?(@project)
     else
       @project = nil
     end
@@ -68,26 +73,20 @@ class LabelsFinder < UnionFinder
     @project
   end
 
-  def find_project
-    if skip_authorization
-      Project.find_by(id: project_id)
-    else
-      available_projects.find_by(id: project_id)
-    end
-  end
-
   def projects
     return @projects if defined?(@projects)
 
-    @projects = skip_authorization ? Project.all : available_projects
-    @projects = @projects.in_namespace(group_id) if group_id
-    @projects = @projects.where(id: projects_ids) if projects_ids
+    @projects = skip_authorization ? Project.all : ProjectsFinder.new.execute(current_user)
+    @projects = @projects.in_namespace(params[:group_id]) if group?
+    @projects = @projects.where(id: params[:projects_ids]) if projects?
     @projects = @projects.reorder(nil)
 
     @projects
   end
 
-  def available_projects
-    @available_projects ||= ProjectsFinder.new.execute(current_user)
+  def authorized_to_read_labels?(project)
+    return true if skip_authorization
+
+    Ability.allowed?(current_user, :read_label, project)
   end
 end
diff --git a/changelogs/unreleased/rs-issue-24527.yml b/changelogs/unreleased/rs-issue-24527.yml
new file mode 100644
index 00000000000..a7b6358e60e
--- /dev/null
+++ b/changelogs/unreleased/rs-issue-24527.yml
@@ -0,0 +1,4 @@
+---
+title: Limit labels returned for a specific project as an administrator
+merge_request: 7496
+author: 
diff --git a/spec/finders/labels_finder_spec.rb b/spec/finders/labels_finder_spec.rb
index 10cfb66ec1c..9085cc8debf 100644
--- a/spec/finders/labels_finder_spec.rb
+++ b/spec/finders/labels_finder_spec.rb
@@ -64,6 +64,21 @@ describe LabelsFinder do
 
         expect(finder.execute).to eq [group_label_2, project_label_1, group_label_1]
       end
+
+      context 'as an administrator' do
+        it 'does not return labels from another project' do
+          # Purposefully creating a project with _nothing_ associated to it
+          isolated_project = create(:empty_project)
+          admin = create(:admin)
+
+          # project_3 has a label associated to it, which we don't want coming
+          # back when we ask for the isolated project's labels
+          project_3.team << [admin, :reporter]
+          finder = described_class.new(admin, project_id: isolated_project.id)
+
+          expect(finder.execute).to be_empty
+        end
+      end
     end
 
     context 'filtering by title' do