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>

app/models/issue.rb

@@ -241,10 +241,27 @@ class Issue < ActiveRecord::Base
   # Returns `true` if the current issue can be viewed by either a logged in User
   # or an anonymous user.
   def visible_to_user?(user = nil)
+    return false unless project.feature_available?(:issues, user)
+
     user ? readable_by?(user) : publicly_visible?
   end
 
+  def overdue?
+    due_date.try(:past?) || false
+  end
+
+  # Only issues on public projects should be checked for spam
+  def check_for_spam?
+    project.public?
+  end
+
+  private
+
   # Returns `true` if the given User can read the current Issue.
+  #
+  # This method duplicates the same check of issue_policy.rb
+  # for performance reasons, check commit: 002ad215818450d2cbbc5fa065850a953dc7ada8
+  # Make sure to sync this method with issue_policy.rb
   def readable_by?(user)
     if user.admin?
       true
@@ -265,13 +282,4 @@ class Issue < ActiveRecord::Base
   def publicly_visible?
     project.public? && !confidential?
   end
-
-  def overdue?
-    due_date.try(:past?) || false
-  end
-
-  # Only issues on public projects should be checked for spam
-  def check_for_spam?
-    project.public?
-  end
 end

app/policies/issue_policy.rb

 class IssuePolicy < IssuablePolicy
+  # This class duplicates the same check of Issue#readable_by? for performance reasons
+  # Make sure to sync this class checks with issue.rb to avoid security problems.
+  # Check commit 002ad215818450d2cbbc5fa065850a953dc7ada8 for more information.
+
   def issue
     @subject
   end

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
@@ -222,6 +217,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

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

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

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

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

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

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

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

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)
+            next unless can_read_group_reference?(node, user, groups)
+            visible << node
+          elsif can_read_project_reference?(node)
             visible << node
-            end
-          # Remaining nodes will be processed by the parent class'
-          # implementation of this method.
           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

spec/features/issues/new_branch_button_spec.rb

@@ -22,6 +22,7 @@ feature 'Start new branch from an issue', feature: true do
         create(:note, :on_issue, :system, project: project,
                                           note: "Mentioned in !#{referenced_mr.iid}")
       end
+
       let(:referenced_mr) do
         create(:merge_request, :simple, source_project: project, target_project: project,
                                         description: "Fixes ##{issue.iid}", author: user)

spec/lib/banzai/filter/redactor_filter_spec.rb

@@ -28,33 +28,41 @@ describe Banzai::Filter::RedactorFilter, lib: true do
         and_return(parser_class)
     end
 
-    it 'removes unpermitted Project references' do
+    context 'valid projects' do
+      before { allow_any_instance_of(Banzai::ReferenceParser::BaseParser).to receive(:can_read_reference?).and_return(true) }
+
+      it 'allows permitted Project references' do
         user = create(:user)
         project = create(:empty_project)
+        project.team << [user, :master]
 
         link = reference_link(project: project.id, reference_type: 'test')
         doc = filter(link, current_user: user)
 
-      expect(doc.css('a').length).to eq 0
+        expect(doc.css('a').length).to eq 1
+      end
     end
 
-    it 'allows permitted Project references' do
+    context 'invalid projects' do
+      before { allow_any_instance_of(Banzai::ReferenceParser::BaseParser).to receive(:can_read_reference?).and_return(false) }
+
+      it 'removes unpermitted references' do
         user = create(:user)
         project = create(:empty_project)
-      project.team << [user, :master]
 
         link = reference_link(project: project.id, reference_type: 'test')
         doc = filter(link, current_user: user)
 
-      expect(doc.css('a').length).to eq 1
+        expect(doc.css('a').length).to eq 0
       end
 
-    it 'handles invalid Project references' do
+      it 'handles invalid references' do
         link = reference_link(project: 12345, reference_type: 'test')
 
         expect { filter(link) }.not_to raise_error
       end
     end
+  end
 
   context 'with data-issue' do
     context 'for confidential issues' do

spec/lib/banzai/reference_parser/base_parser_spec.rb

@@ -27,41 +27,12 @@ describe Banzai::ReferenceParser::BaseParser, lib: true do
     let(:link) { empty_html_link }
 
     context 'when the link has a data-project attribute' do
-      it 'returns the nodes if the attribute value equals the current project ID' do
+      it 'checks if user can read the resource' do
         link['data-project'] = project.id.to_s
 
-        expect(Ability).not_to receive(:allowed?)
-        expect(subject.nodes_visible_to_user(user, [link])).to eq([link])
-      end
-
-      it 'returns the nodes if the user can read the project' do
-        other_project = create(:empty_project, :public)
-
-        link['data-project'] = other_project.id.to_s
-
-        expect(Ability).to receive(:allowed?).
-          with(user, :read_project, other_project).
-          and_return(true)
-
-        expect(subject.nodes_visible_to_user(user, [link])).to eq([link])
-      end
-
-      it 'returns an empty Array when the attribute value is empty' do
-        link['data-project'] = ''
-
-        expect(subject.nodes_visible_to_user(user, [link])).to eq([])
-      end
-
-      it 'returns an empty Array when the user can not read the project' do
-        other_project = create(:empty_project, :public)
-
-        link['data-project'] = other_project.id.to_s
-
-        expect(Ability).to receive(:allowed?).
-          with(user, :read_project, other_project).
-          and_return(false)
+        expect(subject).to receive(:can_read_reference?).with(user, project)
 
-        expect(subject.nodes_visible_to_user(user, [link])).to eq([])
+        subject.nodes_visible_to_user(user, [link])
       end
     end
 

spec/lib/banzai/reference_parser/commit_parser_spec.rb

@@ -8,6 +8,14 @@ describe Banzai::ReferenceParser::CommitParser, lib: true do
   subject { described_class.new(project, user) }
   let(:link) { empty_html_link }
 
+  describe '#nodes_visible_to_user' do
+    context 'when the link has a data-issue attribute' do
+      before { link['data-commit'] = 123 }
+
+      it_behaves_like "referenced feature visibility", "repository"
+    end
+  end
+
   describe '#referenced_by' do
     context 'when the link has a data-project attribute' do
       before do

spec/lib/banzai/reference_parser/commit_range_parser_spec.rb

@@ -8,6 +8,14 @@ describe Banzai::ReferenceParser::CommitRangeParser, lib: true do
   subject { described_class.new(project, user) }
   let(:link) { empty_html_link }
 
+  describe '#nodes_visible_to_user' do
+    context 'when the link has a data-issue attribute' do
+      before { link['data-commit-range'] = '123..456' }
+
+      it_behaves_like "referenced feature visibility", "repository"
+    end
+  end
+
   describe '#referenced_by' do
     context 'when the link has a data-project attribute' do
       before do

spec/lib/banzai/reference_parser/external_issue_parser_spec.rb

@@ -8,6 +8,14 @@ describe Banzai::ReferenceParser::ExternalIssueParser, lib: true do
   subject { described_class.new(project, user) }
   let(:link) { empty_html_link }
 
+  describe '#nodes_visible_to_user' do
+    context 'when the link has a data-issue attribute' do
+      before { link['data-external-issue'] = 123 }
+
+      it_behaves_like "referenced feature visibility", "issues"
+    end
+  end
+
   describe '#referenced_by' do
     context 'when the link has a data-project attribute' do
       before do

spec/lib/banzai/reference_parser/issue_parser_spec.rb

@@ -6,8 +6,8 @@ describe Banzai::ReferenceParser::IssueParser, lib: true do
   let(:project) { create(:empty_project, :public) }
   let(:user)    { create(:user) }
   let(:issue)   { create(:issue, project: project) }
-  subject { described_class.new(project, user) }
   let(:link)    { empty_html_link }
+  subject       { described_class.new(project, user) }
 
   describe '#nodes_visible_to_user' do
     context 'when the link has a data-issue attribute' do
@@ -15,6 +15,8 @@ describe Banzai::ReferenceParser::IssueParser, lib: true do
         link['data-issue'] = issue.id.to_s
       end
 
+      it_behaves_like "referenced feature visibility", "issues"
+
       it 'returns the nodes when the user can read the issue' do
         expect(Ability).to receive(:issues_readable_by_user).
           with([issue], user).

spec/lib/banzai/reference_parser/label_parser_spec.rb

@@ -9,6 +9,14 @@ describe Banzai::ReferenceParser::LabelParser, lib: true do
   subject { described_class.new(project, user) }
   let(:link) { empty_html_link }
 
+  describe '#nodes_visible_to_user' do
+    context 'when the link has a data-issue attribute' do
+      before { link['data-label'] = label.id.to_s }
+
+      it_behaves_like "referenced feature visibility", "issues", "merge_requests"
+    end
+  end
+
   describe '#referenced_by' do
     describe 'when the link has a data-label attribute' do
       context 'using an existing label ID' do

spec/lib/banzai/reference_parser/merge_request_parser_spec.rb

@@ -8,6 +8,19 @@ describe Banzai::ReferenceParser::MergeRequestParser, lib: true do
   subject { described_class.new(merge_request.target_project, user) }
   let(:link) { empty_html_link }
 
+  describe '#nodes_visible_to_user' do
+    context 'when the link has a data-issue attribute' do
+      let(:project) { merge_request.target_project }
+
+      before do
+        project.update_attribute(:visibility_level, Gitlab::VisibilityLevel::PUBLIC)
+        link['data-merge-request'] = merge_request.id.to_s
+      end
+
+      it_behaves_like "referenced feature visibility", "merge_requests"
+    end
+  end
+
   describe '#referenced_by' do
     describe 'when the link has a data-merge-request attribute' do
       context 'using an existing merge request ID' do

spec/lib/banzai/reference_parser/milestone_parser_spec.rb

@@ -9,6 +9,14 @@ describe Banzai::ReferenceParser::MilestoneParser, lib: true do
   subject { described_class.new(project, user) }
   let(:link) { empty_html_link }
 
+  describe '#nodes_visible_to_user' do
+    context 'when the link has a data-issue attribute' do
+      before { link['data-milestone'] = milestone.id.to_s }
+
+      it_behaves_like "referenced feature visibility", "issues", "merge_requests"
+    end
+  end
+
   describe '#referenced_by' do
     describe 'when the link has a data-milestone attribute' do
       context 'using an existing milestone ID' do

spec/lib/banzai/reference_parser/snippet_parser_spec.rb

@@ -9,6 +9,14 @@ describe Banzai::ReferenceParser::SnippetParser, lib: true do
   subject { described_class.new(project, user) }
   let(:link) { empty_html_link }
 
+  describe '#nodes_visible_to_user' do
+    context 'when the link has a data-issue attribute' do
+      before { link['data-snippet'] = snippet.id.to_s }
+
+      it_behaves_like "referenced feature visibility", "snippets"
+    end
+  end
+
   describe '#referenced_by' do
     describe 'when the link has a data-snippet attribute' do
       context 'using an existing snippet ID' do

spec/lib/banzai/reference_parser/user_parser_spec.rb

@@ -103,6 +103,8 @@ describe Banzai::ReferenceParser::UserParser, lib: true do
           it 'returns the nodes if the attribute value equals the current project ID' do
             link['data-project'] = project.id.to_s
 
+            # Ensure that we dont call for Ability.allowed?
+            # When project_id in the node is equal to current project ID
             expect(Ability).not_to receive(:allowed?)
 
             expect(subject.nodes_visible_to_user(user, [link])).to eq([link])

spec/lib/gitlab/closing_issue_extractor_spec.rb

@@ -257,8 +257,9 @@ describe Gitlab::ClosingIssueExtractor, lib: true do
       context 'with an external issue tracker reference' do
         it 'extracts the referenced issue' do
           jira_project = create(:jira_project, name: 'JIRA_EXT1')
+          jira_project.team << [jira_project.creator, :master]
           jira_issue = ExternalIssue.new("#{jira_project.name}-1", project: jira_project)
-          closing_issue_extractor = described_class.new jira_project
+          closing_issue_extractor = described_class.new(jira_project, jira_project.creator)
           message = "Resolve #{jira_issue.to_reference}"
 
           expect(closing_issue_extractor.closed_by_message(message)).to eq([jira_issue])

spec/lib/gitlab/gfm/reference_rewriter_spec.rb

@@ -6,7 +6,7 @@ describe Gitlab::Gfm::ReferenceRewriter do
   let(:new_project) { create(:project) }
   let(:user) { create(:user) }
 
-  before { old_project.team << [user, :guest] }
+  before { old_project.team << [user, :reporter] }
 
   describe '#rewrite' do
     subject do

spec/lib/gitlab/reference_extractor_spec.rb

@@ -3,6 +3,8 @@ require 'spec_helper'
 describe Gitlab::ReferenceExtractor, lib: true do
   let(:project) { create(:project) }
 
+  before { project.team << [project.creator, :developer] }
+
   subject { Gitlab::ReferenceExtractor.new(project, project.creator) }
 
   it 'accesses valid user objects' do
@@ -42,7 +44,6 @@ describe Gitlab::ReferenceExtractor, lib: true do
   end
 
   it 'accesses valid issue objects' do
-    project.team << [project.creator, :developer]
     @i0 = create(:issue, project: project)
     @i1 = create(:issue, project: project)
 

spec/models/issue_spec.rb

@@ -22,7 +22,7 @@ describe Issue, models: true do
     it { is_expected.to have_db_index(:deleted_at) }
   end
 
-  describe 'visible_to_user' do
+  describe '.visible_to_user' do
     let(:user) { create(:user) }
     let(:authorized_user) { create(:user) }
     let(:project) { create(:project, namespace: authorized_user.namespace) }
@@ -102,11 +102,17 @@ describe Issue, models: true do
     it 'returns the merge request to close this issue' do
       allow(mr).to receive(:closes_issue?).with(issue).and_return(true)
 
-      expect(issue.closed_by_merge_requests).to eq([mr])
+      expect(issue.closed_by_merge_requests(mr.author)).to eq([mr])
+    end
+
+    it "returns an empty array when the merge request is closed already" do
+      closed_mr
+
+      expect(issue.closed_by_merge_requests(closed_mr.author)).to eq([])
     end
 
     it "returns an empty array when the current issue is closed already" do
-      expect(closed_issue.closed_by_merge_requests).to eq([])
+      expect(closed_issue.closed_by_merge_requests(closed_issue.author)).to eq([])
     end
   end
 
@@ -212,7 +218,7 @@ describe Issue, models: true do
                                                source_project: subject.project,
                                                source_branch: "#{subject.iid}-branch" })
       merge_request.create_cross_references!(user)
-      expect(subject.referenced_merge_requests).not_to be_empty
+      expect(subject.referenced_merge_requests(user)).not_to be_empty
       expect(subject.related_branches(user)).to eq([subject.to_branch_name])
     end
 
@@ -308,6 +314,22 @@ describe Issue, models: true do
   end
 
   describe '#visible_to_user?' do
+    context 'without a user' do
+      let(:issue) { build(:issue) }
+
+      it 'returns true when the issue is publicly visible' do
+        expect(issue).to receive(:publicly_visible?).and_return(true)
+
+        expect(issue.visible_to_user?).to eq(true)
+      end
+
+      it 'returns false when the issue is not publicly visible' do
+        expect(issue).to receive(:publicly_visible?).and_return(false)
+
+        expect(issue.visible_to_user?).to eq(false)
+      end
+    end
+
     context 'with a user' do
       let(:user) { build(:user) }
       let(:issue) { build(:issue) }
@@ -323,26 +345,24 @@ describe Issue, models: true do
 
         expect(issue.visible_to_user?(user)).to eq(false)
       end
-    end
 
-    context 'without a user' do
-      let(:issue) { build(:issue) }
+      it 'returns false when feature is disabled' do
+        expect(issue).not_to receive(:readable_by?)
 
-      it 'returns true when the issue is publicly visible' do
-        expect(issue).to receive(:publicly_visible?).and_return(true)
+        issue.project.project_feature.update_attribute(:issues_access_level, ProjectFeature::DISABLED)
 
-        expect(issue.visible_to_user?).to eq(true)
+        expect(issue.visible_to_user?(user)).to eq(false)
       end
 
-      it 'returns false when the issue is not publicly visible' do
-        expect(issue).to receive(:publicly_visible?).and_return(false)
+      it 'returns false when restricted for members' do
+        expect(issue).not_to receive(:readable_by?)
 
-        expect(issue.visible_to_user?).to eq(false)
-      end
+        issue.project.project_feature.update_attribute(:issues_access_level, ProjectFeature::PRIVATE)
+
+        expect(issue.visible_to_user?(user)).to eq(false)
       end
     end
 
-  describe '#readable_by?' do
     describe 'with a regular user that is not a team member' do
       let(:user) { create(:user) }
 
@@ -352,13 +372,13 @@ describe Issue, models: true do
         it 'returns true for a regular issue' do
           issue = build(:issue, project: project)
 
-          expect(issue).to be_readable_by(user)
+          expect(issue.visible_to_user?(user)).to eq(true)
         end
 
         it 'returns false for a confidential issue' do
           issue = build(:issue, project: project, confidential: true)
 
-          expect(issue).not_to be_readable_by(user)
+          expect(issue.visible_to_user?(user)).to eq(false)
         end
       end
 
@@ -369,13 +389,13 @@ describe Issue, models: true do
           it 'returns true for a regular issue' do
             issue = build(:issue, project: project)
 
-            expect(issue).to be_readable_by(user)
+            expect(issue.visible_to_user?(user)).to eq(true)
           end
 
           it 'returns false for a confidential issue' do
             issue = build(:issue, :confidential, project: project)
 
-            expect(issue).not_to be_readable_by(user)
+            expect(issue.visible_to_user?(user)).to eq(false)
           end
         end
 
@@ -387,13 +407,13 @@ describe Issue, models: true do
           it 'returns false for a regular issue' do
             issue = build(:issue, project: project)
 
-            expect(issue).not_to be_readable_by(user)
+            expect(issue.visible_to_user?(user)).to eq(false)
           end
 
           it 'returns false for a confidential issue' do
             issue = build(:issue, :confidential, project: project)
 
-            expect(issue).not_to be_readable_by(user)
+            expect(issue.visible_to_user?(user)).to eq(false)
           end
         end
       end
@@ -404,26 +424,28 @@ describe Issue, models: true do
         it 'returns false for a regular issue' do
           issue = build(:issue, project: project)
 
-          expect(issue).not_to be_readable_by(user)
+          expect(issue.visible_to_user?(user)).to eq(false)
         end
 
         it 'returns false for a confidential issue' do
           issue = build(:issue, :confidential, project: project)
 
-          expect(issue).not_to be_readable_by(user)
+          expect(issue.visible_to_user?(user)).to eq(false)
         end
 
         context 'when the user is the project owner' do
+          before { project.team << [user, :master] }
+
           it 'returns true for a regular issue' do
             issue = build(:issue, project: project)
 
-            expect(issue).not_to be_readable_by(user)
+            expect(issue.visible_to_user?(user)).to eq(true)
           end
 
           it 'returns true for a confidential issue' do
             issue = build(:issue, :confidential, project: project)
 
-            expect(issue).not_to be_readable_by(user)
+            expect(issue.visible_to_user?(user)).to eq(true)
           end
         end
       end
@@ -441,13 +463,13 @@ describe Issue, models: true do
         it 'returns true for a regular issue' do
           issue = build(:issue, project: project)
 
-          expect(issue).to be_readable_by(user)
+          expect(issue.visible_to_user?(user)).to eq(true)
         end
 
         it 'returns true for a confidential issue' do
           issue = build(:issue, :confidential, project: project)
 
-          expect(issue).to be_readable_by(user)
+          expect(issue.visible_to_user?(user)).to eq(true)
         end
       end
 
@@ -461,13 +483,13 @@ describe Issue, models: true do
         it 'returns true for a regular issue' do
           issue = build(:issue, project: project)
 
-          expect(issue).to be_readable_by(user)
+          expect(issue.visible_to_user?(user)).to eq(true)
         end
 
         it 'returns true for a confidential issue' do
           issue = build(:issue, :confidential, project: project)
 
-          expect(issue).to be_readable_by(user)
+          expect(issue.visible_to_user?(user)).to eq(true)
         end
       end
 
@@ -481,13 +503,13 @@ describe Issue, models: true do
         it 'returns true for a regular issue' do
           issue = build(:issue, project: project)
 
-          expect(issue).to be_readable_by(user)
+          expect(issue.visible_to_user?(user)).to eq(true)
         end
 
         it 'returns true for a confidential issue' do
           issue = build(:issue, :confidential, project: project)
 
-          expect(issue).to be_readable_by(user)
+          expect(issue.visible_to_user?(user)).to eq(true)
         end
       end
     end
@@ -499,13 +521,13 @@ describe Issue, models: true do
       it 'returns true for a regular issue' do
         issue = build(:issue, project: project)
 
-        expect(issue).to be_readable_by(user)
+        expect(issue.visible_to_user?(user)).to eq(true)
       end
 
       it 'returns true for a confidential issue' do
         issue = build(:issue, :confidential, project: project)
 
-        expect(issue).to be_readable_by(user)
+        expect(issue.visible_to_user?(user)).to eq(true)
       end
     end
   end
@@ -517,13 +539,13 @@ describe Issue, models: true do
       it 'returns true for a regular issue' do
         issue = build(:issue, project: project)
 
-        expect(issue).to be_publicly_visible
+        expect(issue).to be_truthy
       end
 
       it 'returns false for a confidential issue' do
         issue = build(:issue, :confidential, project: project)
 
-        expect(issue).not_to be_publicly_visible
+        expect(issue).not_to be_falsy
       end
     end
 
@@ -533,13 +555,13 @@ describe Issue, models: true do
       it 'returns false for a regular issue' do
         issue = build(:issue, project: project)
 
-        expect(issue).not_to be_publicly_visible
+        expect(issue).not_to be_falsy
       end
 
       it 'returns false for a confidential issue' do
         issue = build(:issue, :confidential, project: project)
 
-        expect(issue).not_to be_publicly_visible
+        expect(issue).not_to be_falsy
       end
     end
 
@@ -549,13 +571,13 @@ describe Issue, models: true do
       it 'returns false for a regular issue' do
         issue = build(:issue, project: project)
 
-        expect(issue).not_to be_publicly_visible
+        expect(issue).not_to be_falsy
       end
 
       it 'returns false for a confidential issue' do
         issue = build(:issue, :confidential, project: project)
 
-        expect(issue).not_to be_publicly_visible
+        expect(issue).not_to be_falsy
       end
     end
   end

spec/support/cycle_analytics_helpers.rb

@@ -49,7 +49,8 @@ module CycleAnalyticsHelpers
   end
 
   def merge_merge_requests_closing_issue(issue)
-    merge_requests = issue.closed_by_merge_requests
+    merge_requests = issue.closed_by_merge_requests(user)
+
     merge_requests.each { |merge_request| MergeRequests::MergeService.new(project, user).execute(merge_request) }
   end
 

spec/support/reference_parser_shared_examples.rb

+RSpec.shared_examples "referenced feature visibility" do |*related_features|
+  let(:feature_fields) do
+    related_features.map { |feature| (feature + "_access_level").to_sym }
+  end
+
+  before { link['data-project'] = project.id.to_s }
+
+  context "when feature is disabled" do
+    it "does not create reference" do
+      set_features_fields_to(ProjectFeature::DISABLED)
+      expect(subject.nodes_visible_to_user(user, [link])).to eq([])
+    end
+  end
+
+  context "when feature is enabled only for team members" do
+    before { set_features_fields_to(ProjectFeature::PRIVATE) }
+
+    it "does not create reference for non member" do
+      non_member = create(:user)
+
+      expect(subject.nodes_visible_to_user(non_member, [link])).to eq([])
+    end
+
+    it "creates reference for member" do
+      project.team << [user, :developer]
+
+      expect(subject.nodes_visible_to_user(user, [link])).to eq([link])
+    end
+  end
+
+  context "when feature is enabled" do
+    # The project is public
+    it "creates reference" do
+      set_features_fields_to(ProjectFeature::ENABLED)
+
+      expect(subject.nodes_visible_to_user(user, [link])).to eq([link])
+    end
+  end
+
+  def set_features_fields_to(visibility_level)
+    feature_fields.each { |field| project.project_feature.update_attribute(field, visibility_level) }
+  end
+end