Restrict access to references for confidential issues

10 files changed, 102 insertions, 5 deletions

diff --git a/lib/banzai/filter/issue_reference_filter.rb b/lib/banzai/filter/issue_reference_filter.rb
index 9f08aa36e8b..2732e0b5145 100644
--- a/lib/banzai/filter/issue_reference_filter.rb
+++ b/lib/banzai/filter/issue_reference_filter.rb
@@ -9,6 +9,11 @@ module Banzai
         Issue
       end
 
+      def self.user_can_see_reference?(user, node, context)
+        issue = Issue.find(node.attr('data-issue')) rescue nil
+        Ability.abilities.allowed?(user, :read_issue, issue)
+      end
+
       def find_object(project, id)
         project.get_issue(id)
       end
diff --git a/spec/features/issues/new_branch_button_spec.rb b/spec/features/issues/new_branch_button_spec.rb
index 1f3bd915f48..9219b767547 100644
--- a/spec/features/issues/new_branch_button_spec.rb
+++ b/spec/features/issues/new_branch_button_spec.rb
@@ -24,7 +24,7 @@ feature 'Start new branch from an issue', feature: true do
       end
       let(:referenced_mr) do
         create(:merge_request, :simple, source_project: project, target_project: project,
-                                        description: "Fixes ##{issue.iid}")
+                                        description: "Fixes ##{issue.iid}", author: user)
       end
 
       before do
diff --git a/spec/lib/banzai/filter/redactor_filter_spec.rb b/spec/lib/banzai/filter/redactor_filter_spec.rb
index e9bb388e361..9acf6304bcb 100644
--- a/spec/lib/banzai/filter/redactor_filter_spec.rb
+++ b/spec/lib/banzai/filter/redactor_filter_spec.rb
@@ -44,8 +44,78 @@ describe Banzai::Filter::RedactorFilter, lib: true do
     end
   end
 
-  context "for user references" do
+  context 'with data-issue' do
+    context 'for confidential issues' do
+      it 'removes references for non project members' do
+        non_member = create(:user)
+        project = create(:empty_project, :public)
+        issue = create(:issue, :confidential, project: project)
+
+        link = reference_link(project: project.id, issue: issue.id, reference_filter: 'IssueReferenceFilter')
+        doc = filter(link, current_user: non_member)
+
+        expect(doc.css('a').length).to eq 0
+      end
+
+      it 'allows references for author' do
+        author = create(:user)
+        project = create(:empty_project, :public)
+        issue = create(:issue, :confidential, project: project, author: author)
+
+        link = reference_link(project: project.id, issue: issue.id, reference_filter: 'IssueReferenceFilter')
+        doc = filter(link, current_user: author)
+
+        expect(doc.css('a').length).to eq 1
+      end
+
+      it 'allows references for assignee' do
+        assignee = create(:user)
+        project = create(:empty_project, :public)
+        issue = create(:issue, :confidential, project: project, assignee: assignee)
+
+        link = reference_link(project: project.id, issue: issue.id, reference_filter: 'IssueReferenceFilter')
+        doc = filter(link, current_user: assignee)
 
+        expect(doc.css('a').length).to eq 1
+      end
+
+      it 'allows references for project members' do
+        member = create(:user)
+        project = create(:empty_project, :public)
+        project.team << [member, :developer]
+        issue = create(:issue, :confidential, project: project)
+
+        link = reference_link(project: project.id, issue: issue.id, reference_filter: 'IssueReferenceFilter')
+        doc = filter(link, current_user: member)
+
+        expect(doc.css('a').length).to eq 1
+      end
+
+      it 'allows references for admin' do
+        admin = create(:admin)
+        project = create(:empty_project, :public)
+        issue = create(:issue, :confidential, project: project)
+
+        link = reference_link(project: project.id, issue: issue.id, reference_filter: 'IssueReferenceFilter')
+        doc = filter(link, current_user: admin)
+
+        expect(doc.css('a').length).to eq 1
+      end
+    end
+
+    it 'allows references for non confidential issues' do
+      user = create(:user)
+      project = create(:empty_project, :public)
+      issue = create(:issue, project: project)
+
+      link = reference_link(project: project.id, issue: issue.id, reference_filter: 'IssueReferenceFilter')
+      doc = filter(link, current_user: user)
+
+      expect(doc.css('a').length).to eq 1
+    end
+  end
+
+  context "for user references" do
     context 'with data-group' do
       it 'removes unpermitted Group references' do
         user = create(:user)
diff --git a/spec/lib/gitlab/closing_issue_extractor_spec.rb b/spec/lib/gitlab/closing_issue_extractor_spec.rb
index 04cf11fc6f1..844fd79c991 100644
--- a/spec/lib/gitlab/closing_issue_extractor_spec.rb
+++ b/spec/lib/gitlab/closing_issue_extractor_spec.rb
@@ -11,6 +11,7 @@ describe Gitlab::ClosingIssueExtractor, lib: true do
   subject { described_class.new(project, project.creator) }
 
   before do
+    project.team  << [project.creator, :developer]
     project2.team << [project.creator, :master]
   end
 
diff --git a/spec/lib/gitlab/reference_extractor_spec.rb b/spec/lib/gitlab/reference_extractor_spec.rb
index 7d963795e17..65af37e24f1 100644
--- a/spec/lib/gitlab/reference_extractor_spec.rb
+++ b/spec/lib/gitlab/reference_extractor_spec.rb
@@ -2,6 +2,7 @@ require 'spec_helper'
 
 describe Gitlab::ReferenceExtractor, lib: true do
   let(:project) { create(:project) }
+
   subject { Gitlab::ReferenceExtractor.new(project, project.creator) }
 
   it 'accesses valid user objects' do
@@ -41,6 +42,7 @@ 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)
 
diff --git a/spec/models/commit_spec.rb b/spec/models/commit_spec.rb
index 253902512c3..0e9111c8029 100644
--- a/spec/models/commit_spec.rb
+++ b/spec/models/commit_spec.rb
@@ -86,10 +86,21 @@ eos
     let(:issue) { create :issue, project: project }
     let(:other_project) { create :project, :public }
     let(:other_issue) { create :issue, project: other_project }
+    let(:commiter) { create :user }
+
+    before do
+      project.team << [commiter, :developer]
+      other_project.team << [commiter, :developer]
+    end
 
     it 'detects issues that this commit is marked as closing' do
       ext_ref = "#{other_project.path_with_namespace}##{other_issue.iid}"
-      allow(commit).to receive(:safe_message).and_return("Fixes ##{issue.iid} and #{ext_ref}")
+
+      allow(commit).to receive_messages(
+        safe_message: "Fixes ##{issue.iid} and #{ext_ref}",
+        committer_email: commiter.email
+      )
+
       expect(commit.closes_issues).to include(issue)
       expect(commit.closes_issues).to include(other_issue)
     end
diff --git a/spec/models/concerns/mentionable_spec.rb b/spec/models/concerns/mentionable_spec.rb
index 20f0c561e44..cb33edde820 100644
--- a/spec/models/concerns/mentionable_spec.rb
+++ b/spec/models/concerns/mentionable_spec.rb
@@ -48,7 +48,8 @@ describe Issue, "Mentionable" do
 
   describe '#create_new_cross_references!' do
     let(:project) { create(:project) }
-    let(:issues)  { create_list(:issue, 2, project: project) }
+    let(:author)  { create(:author) }
+    let(:issues)  { create_list(:issue, 2, project: project, author: author) }
 
     context 'before changes are persisted' do
       it 'ignores pre-existing references' do
@@ -91,7 +92,7 @@ describe Issue, "Mentionable" do
     end
 
     def create_issue(description:)
-      create(:issue, project: project, description: description)
+      create(:issue, project: project, description: description, author: author)
     end
   end
 end
diff --git a/spec/models/merge_request_spec.rb b/spec/models/merge_request_spec.rb
index 8bf68013fd2..633a16b59c2 100644
--- a/spec/models/merge_request_spec.rb
+++ b/spec/models/merge_request_spec.rb
@@ -150,6 +150,7 @@ describe MergeRequest, models: true do
     let(:commit2) { double('commit2', safe_message: "Fixes #{issue1.to_reference}") }
 
     before do
+      subject.project.team << [subject.author, :developer]
       allow(subject).to receive(:commits).and_return([commit0, commit1, commit2])
     end
 
diff --git a/spec/services/git_push_service_spec.rb b/spec/services/git_push_service_spec.rb
index b49ca96e8e8..8490a729e51 100644
--- a/spec/services/git_push_service_spec.rb
+++ b/spec/services/git_push_service_spec.rb
@@ -215,12 +215,16 @@ describe GitPushService, services: true do
     let(:commit) { project.commit }
 
     before do
+      project.team << [commit_author, :developer]
+      project.team << [user, :developer]
+
       allow(commit).to receive_messages(
         safe_message: "this commit \n mentions #{issue.to_reference}",
         references: [issue],
         author_name: commit_author.name,
         author_email: commit_author.email
       )
+
       allow(project.repository).to receive(:commits_between).and_return([commit])
     end
 
diff --git a/spec/support/mentionable_shared_examples.rb b/spec/support/mentionable_shared_examples.rb
index fce91015fd4..e876d44c166 100644
--- a/spec/support/mentionable_shared_examples.rb
+++ b/spec/support/mentionable_shared_examples.rb
@@ -52,6 +52,8 @@ shared_context 'mentionable context' do
     end
 
     set_mentionable_text.call(ref_string)
+
+    project.team << [author, :developer]
   end
 end