Merge branch 'issue_23548_dev' into 'master'

disable markdown in comments when referencing disabled features

fixes https://gitlab.com/gitlab-org/gitlab-ce/issues/23548

This MR prevents the following references when tool is disabled:

- issues
- snippets
- commits - when repo is disabled
- commit range - when repo is disabled
- milestones

This MR does not prevent references to repository files, since they are just markdown links and don't leak
information.

See merge request !2011

Signed-off-by: Rémy Coutable <remy@rymai.me>

9 files changed, 78 insertions, 14 deletions

diff --git a/lib/banzai/reference_parser/base_parser.rb b/lib/banzai/reference_parser/base_parser.rb
index f5d110e987b..d8a855ec1fe 100644
--- a/lib/banzai/reference_parser/base_parser.rb
+++ b/lib/banzai/reference_parser/base_parser.rb
@@ -63,12 +63,7 @@ module Banzai
         nodes.select do |node|
           if node.has_attribute?(project_attr)
             node_id = node.attr(project_attr).to_i
-
-            if project && project.id == node_id
-              true
-            else
-              can?(user, :read_project, projects[node_id])
-            end
+            can_read_reference?(user, projects[node_id])
           else
             true
           end
@@ -226,6 +221,15 @@ module Banzai
 
       attr_reader :current_user, :project
 
+      # When a feature is disabled or visible only for
+      # team members we should not allow team members
+      # see reference comments.
+      # Override this method on subclasses
+      # to check if user can read resource
+      def can_read_reference?(user, ref_project)
+        raise NotImplementedError
+      end
+
       def lazy(&block)
         Gitlab::Lazy.new(&block)
       end
diff --git a/lib/banzai/reference_parser/commit_parser.rb b/lib/banzai/reference_parser/commit_parser.rb
index 0fee9d267de..8c54a041cb8 100644
--- a/lib/banzai/reference_parser/commit_parser.rb
+++ b/lib/banzai/reference_parser/commit_parser.rb
@@ -29,6 +29,12 @@ module Banzai
 
         commits
       end
+
+      private
+
+      def can_read_reference?(user, ref_project)
+        can?(user, :download_code, ref_project)
+      end
     end
   end
 end
diff --git a/lib/banzai/reference_parser/commit_range_parser.rb b/lib/banzai/reference_parser/commit_range_parser.rb
index 69d01f8db15..0878b6afba3 100644
--- a/lib/banzai/reference_parser/commit_range_parser.rb
+++ b/lib/banzai/reference_parser/commit_range_parser.rb
@@ -33,6 +33,12 @@ module Banzai
 
         range.valid_commits? ? range : nil
       end
+
+      private
+
+      def can_read_reference?(user, ref_project)
+        can?(user, :download_code, ref_project)
+      end
     end
   end
 end
diff --git a/lib/banzai/reference_parser/external_issue_parser.rb b/lib/banzai/reference_parser/external_issue_parser.rb
index a1264db2111..6e7b7669578 100644
--- a/lib/banzai/reference_parser/external_issue_parser.rb
+++ b/lib/banzai/reference_parser/external_issue_parser.rb
@@ -20,6 +20,12 @@ module Banzai
       def issue_ids_per_project(nodes)
         gather_attributes_per_project(nodes, self.class.data_attribute)
       end
+
+      private
+
+      def can_read_reference?(user, ref_project)
+        can?(user, :read_issue, ref_project)
+      end
     end
   end
 end
diff --git a/lib/banzai/reference_parser/label_parser.rb b/lib/banzai/reference_parser/label_parser.rb
index e5d1eb11d7f..aa76c64ac5f 100644
--- a/lib/banzai/reference_parser/label_parser.rb
+++ b/lib/banzai/reference_parser/label_parser.rb
@@ -6,6 +6,12 @@ module Banzai
       def references_relation
         Label
       end
+
+      private
+
+      def can_read_reference?(user, ref_project)
+        can?(user, :read_label, ref_project)
+      end
     end
   end
 end
diff --git a/lib/banzai/reference_parser/merge_request_parser.rb b/lib/banzai/reference_parser/merge_request_parser.rb
index c9a9ca79c09..40451947e6c 100644
--- a/lib/banzai/reference_parser/merge_request_parser.rb
+++ b/lib/banzai/reference_parser/merge_request_parser.rb
@@ -6,6 +6,12 @@ module Banzai
       def references_relation
         MergeRequest.includes(:author, :assignee, :target_project)
       end
+
+      private
+
+      def can_read_reference?(user, ref_project)
+        can?(user, :read_merge_request, ref_project)
+      end
     end
   end
 end
diff --git a/lib/banzai/reference_parser/milestone_parser.rb b/lib/banzai/reference_parser/milestone_parser.rb
index a000ac61e5c..d3968d6b229 100644
--- a/lib/banzai/reference_parser/milestone_parser.rb
+++ b/lib/banzai/reference_parser/milestone_parser.rb
@@ -6,6 +6,12 @@ module Banzai
       def references_relation
         Milestone
       end
+
+      private
+
+      def can_read_reference?(user, ref_project)
+        can?(user, :read_milestone, ref_project)
+      end
     end
   end
 end
diff --git a/lib/banzai/reference_parser/snippet_parser.rb b/lib/banzai/reference_parser/snippet_parser.rb
index fa71b3c952a..63b592137bb 100644
--- a/lib/banzai/reference_parser/snippet_parser.rb
+++ b/lib/banzai/reference_parser/snippet_parser.rb
@@ -6,6 +6,12 @@ module Banzai
       def references_relation
         Snippet
       end
+
+      private
+
+      def can_read_reference?(user, ref_project)
+        can?(user, :read_project_snippet, ref_project)
+      end
     end
   end
 end
diff --git a/lib/banzai/reference_parser/user_parser.rb b/lib/banzai/reference_parser/user_parser.rb
index 863f5725d3b..7adaffa19c1 100644
--- a/lib/banzai/reference_parser/user_parser.rb
+++ b/lib/banzai/reference_parser/user_parser.rb
@@ -30,22 +30,36 @@ module Banzai
 
         nodes.each do |node|
           if node.has_attribute?(group_attr)
-            node_group = groups[node.attr(group_attr).to_i]
-
-            if node_group &&
-              can?(user, :read_group, node_group)
-              visible << node
-            end
-          # Remaining nodes will be processed by the parent class'
-          # implementation of this method.
+            next unless can_read_group_reference?(node, user, groups)
+            visible << node
+          elsif can_read_project_reference?(node)
+            visible << node
           else
             remaining << node
           end
         end
 
+        # If project does not belong to a group
+        # and does not have the same project id as the current project
+        # base class will check if user can read the project that contains
+        # the user reference.
         visible + super(current_user, remaining)
       end
 
+      # Check if project belongs to a group which
+      # user can read.
+      def can_read_group_reference?(node, user, groups)
+        node_group = groups[node.attr('data-group').to_i]
+
+        node_group && can?(user, :read_group, node_group)
+      end
+
+      def can_read_project_reference?(node)
+        node_id = node.attr('data-project').to_i
+
+        project && project.id == node_id
+      end
+
       def nodes_user_can_reference(current_user, nodes)
         project_attr = 'data-project'
         author_attr = 'data-author'
@@ -88,6 +102,10 @@ module Banzai
         collection_objects_for_ids(Project, ids).
           flat_map { |p| p.team.members.to_a }
       end
+
+      def can_read_reference?(user, ref_project)
+        can?(user, :read_project, ref_project)
+      end
     end
   end
 end