Merge branch 'security-59581-related-merge-requests-count' into 'master'

Expose merge requests count based on user access

See merge request gitlab/gitlabhq!3157

8 files changed, 44 insertions, 13 deletions

diff --git a/app/controllers/concerns/issuable_collections.rb b/app/controllers/concerns/issuable_collections.rb
index 88a0690938a..21b3949e361 100644
--- a/app/controllers/concerns/issuable_collections.rb
+++ b/app/controllers/concerns/issuable_collections.rb
@@ -42,7 +42,7 @@ module IssuableCollections
 
     @issuables          = @issuables.page(params[:page])
     @issuables          = per_page_for_relative_position if params[:sort] == 'relative_position'
-    @issuable_meta_data = issuable_meta_data(@issuables, collection_type)
+    @issuable_meta_data = issuable_meta_data(@issuables, collection_type, current_user)
     @total_pages        = issuable_page_count
   end
   # rubocop:enable Gitlab/ModuleWithInstanceVariables
diff --git a/app/controllers/concerns/issuable_collections_action.rb b/app/controllers/concerns/issuable_collections_action.rb
index 18ed4027eac..4ad287c4a13 100644
--- a/app/controllers/concerns/issuable_collections_action.rb
+++ b/app/controllers/concerns/issuable_collections_action.rb
@@ -11,7 +11,7 @@ module IssuableCollectionsAction
               .non_archived
               .page(params[:page])
 
-    @issuable_meta_data = issuable_meta_data(@issues, collection_type)
+    @issuable_meta_data = issuable_meta_data(@issues, collection_type, current_user)
 
     respond_to do |format|
       format.html
@@ -22,7 +22,7 @@ module IssuableCollectionsAction
   def merge_requests
     @merge_requests = issuables_collection.page(params[:page])
 
-    @issuable_meta_data = issuable_meta_data(@merge_requests, collection_type)
+    @issuable_meta_data = issuable_meta_data(@merge_requests, collection_type, current_user)
   end
   # rubocop:enable Gitlab/ModuleWithInstanceVariables
 
diff --git a/app/controllers/projects_controller.rb b/app/controllers/projects_controller.rb
index 12db493978b..330e2d0f8a5 100644
--- a/app/controllers/projects_controller.rb
+++ b/app/controllers/projects_controller.rb
@@ -298,7 +298,7 @@ class ProjectsController < Projects::ApplicationController
       elsif @project.feature_available?(:issues, current_user)
         @issues = issuables_collection.page(params[:page])
         @collection_type = 'Issue'
-        @issuable_meta_data = issuable_meta_data(@issues, @collection_type)
+        @issuable_meta_data = issuable_meta_data(@issues, @collection_type, current_user)
       end
 
       render :show
diff --git a/app/helpers/issuables_helper.rb b/app/helpers/issuables_helper.rb
index 045de105b77..cd2669ef6ad 100644
--- a/app/helpers/issuables_helper.rb
+++ b/app/helpers/issuables_helper.rb
@@ -280,7 +280,7 @@ module IssuablesHelper
       initialTaskStatus: issuable.task_status
     }
 
-    data[:hasClosingMergeRequest] = issuable.merge_requests_count != 0 if issuable.is_a?(Issue)
+    data[:hasClosingMergeRequest] = issuable.merge_requests_count(current_user) != 0 if issuable.is_a?(Issue)
 
     if parent.is_a?(Group)
       data[:groupPath] = parent.path
diff --git a/app/models/concerns/issuable.rb b/app/models/concerns/issuable.rb
index 127430cc68f..299e413321d 100644
--- a/app/models/concerns/issuable.rb
+++ b/app/models/concerns/issuable.rb
@@ -29,7 +29,11 @@ module Issuable
   # This object is used to gather issuable meta data for displaying
   # upvotes, downvotes, notes and closing merge requests count for issues and merge requests
   # lists avoiding n+1 queries and improving performance.
-  IssuableMeta = Struct.new(:upvotes, :downvotes, :user_notes_count, :merge_requests_count)
+  IssuableMeta = Struct.new(:upvotes, :downvotes, :user_notes_count, :mrs_count) do
+    def merge_requests_count(user = nil)
+      mrs_count
+    end
+  end
 
   included do
     cache_markdown_field :title, pipeline: :single_line
diff --git a/app/models/issue.rb b/app/models/issue.rb
index 30e29911758..982a94315bd 100644
--- a/app/models/issue.rb
+++ b/app/models/issue.rb
@@ -250,8 +250,8 @@ class Issue < ApplicationRecord
   end
   # rubocop: enable CodeReuse/ServiceClass
 
-  def merge_requests_count
-    merge_requests_closing_issues.count
+  def merge_requests_count(user = nil)
+    ::MergeRequestsClosingIssues.count_for_issue(self.id, user)
   end
 
   def labels_hook_attrs
diff --git a/app/models/merge_requests_closing_issues.rb b/app/models/merge_requests_closing_issues.rb
index 61af50841ee..22cedf57b86 100644
--- a/app/models/merge_requests_closing_issues.rb
+++ b/app/models/merge_requests_closing_issues.rb
@@ -7,11 +7,38 @@ class MergeRequestsClosingIssues < ApplicationRecord
   validates :merge_request_id, uniqueness: { scope: :issue_id }, presence: true
   validates :issue_id, presence: true
 
+  scope :with_issues, ->(ids) { where(issue_id: ids) }
+  scope :with_merge_requests_enabled, -> do
+    joins(:merge_request)
+      .joins('INNER JOIN project_features ON merge_requests.target_project_id = project_features.project_id')
+      .where('project_features.merge_requests_access_level >= :access', access: ProjectFeature::ENABLED)
+  end
+
+  scope :accessible_by, ->(user) do
+    joins(:merge_request)
+      .joins('INNER JOIN project_features ON merge_requests.target_project_id = project_features.project_id')
+      .where('project_features.merge_requests_access_level >= :access OR EXISTS(:authorizations)',
+             access: ProjectFeature::ENABLED,
+             authorizations: user.authorizations_for_projects(min_access_level: Gitlab::Access::REPORTER, related_project_column: "merge_requests.target_project_id")
+            )
+  end
+
   class << self
-    def count_for_collection(ids)
-      group(:issue_id)
-        .where(issue_id: ids)
-        .pluck('issue_id', 'COUNT(*) as count')
+    def count_for_collection(ids, current_user)
+      closing_merge_requests(ids, current_user).group(:issue_id).pluck('issue_id', 'COUNT(*) as count')
+    end
+
+    def count_for_issue(id, current_user)
+      closing_merge_requests(id, current_user).count
+    end
+
+    private
+
+    def closing_merge_requests(ids, current_user)
+      return with_issues(ids) if current_user&.admin?
+      return with_issues(ids).with_merge_requests_enabled if current_user.blank?
+
+      with_issues(ids).accessible_by(current_user)
     end
   end
 end
diff --git a/app/views/shared/_issuable_meta_data.html.haml b/app/views/shared/_issuable_meta_data.html.haml
index 31a5370a5f8..71b13a5d741 100644
--- a/app/views/shared/_issuable_meta_data.html.haml
+++ b/app/views/shared/_issuable_meta_data.html.haml
@@ -2,7 +2,7 @@
 - issue_votes        = @issuable_meta_data[issuable.id]
 - upvotes, downvotes = issue_votes.upvotes, issue_votes.downvotes
 - issuable_url       = @collection_type == "Issue" ? issue_path(issuable, anchor: 'notes') : merge_request_path(issuable, anchor: 'notes')
-- issuable_mr        = @issuable_meta_data[issuable.id].merge_requests_count
+- issuable_mr        = @issuable_meta_data[issuable.id].merge_requests_count(current_user)
 
 - if issuable_mr > 0
   %li.issuable-mr.d-none.d-sm-block.has-tooltip{ title: _('Related merge requests') }