Merge branch 'port-ee-3435' into 'security-10-0'

[10.0 CE] Prevent "Related Issues" from leaking confidential issues

See merge request gitlab/gitlabhq!2193

4 files changed, 11 insertions, 31 deletions

diff --git a/app/models/note.rb b/app/models/note.rb
index f44590e2144..b1eb3e0c366 100644
--- a/app/models/note.rb
+++ b/app/models/note.rb
@@ -161,7 +161,7 @@ class Note < ActiveRecord::Base
   end
 
   def cross_reference?
-    system? && SystemNoteService.cross_reference?(note)
+    system? && matches_cross_reference_regex?
   end
 
   def diff_note?
diff --git a/app/services/system_note_service.rb b/app/services/system_note_service.rb
index 1f66a2668f9..7cf03726174 100644
--- a/app/services/system_note_service.rb
+++ b/app/services/system_note_service.rb
@@ -162,7 +162,6 @@ module SystemNoteService
   #   "changed time estimate to 3d 5h"
   #
   # Returns the created Note object
-
   def change_time_estimate(noteable, project, author)
     parsed_time = Gitlab::TimeTrackingFormatter.output(noteable.time_estimate)
     body = if noteable.time_estimate == 0
@@ -188,7 +187,6 @@ module SystemNoteService
   #   "added 2h 30m of time spent"
   #
   # Returns the created Note object
-
   def change_time_spent(noteable, project, author)
     time_spent = noteable.time_spent
 
@@ -451,10 +449,6 @@ module SystemNoteService
     end
   end
 
-  def cross_reference?(note_text)
-    note_text =~ /\A#{cross_reference_note_prefix}/i
-  end
-
   # Check if a cross-reference is disallowed
   #
   # This method prevents adding a "mentioned in !1" note on every single commit
@@ -484,7 +478,6 @@ module SystemNoteService
   # mentioner - Mentionable object
   #
   # Returns Boolean
-
   def cross_reference_exists?(noteable, mentioner)
     # Initial scope should be system notes of this noteable type
     notes = Note.system.where(noteable_type: noteable.class)
diff --git a/spec/controllers/projects/issues_controller_spec.rb b/spec/controllers/projects/issues_controller_spec.rb
index b4a22a46b51..e62ac4d4569 100644
--- a/spec/controllers/projects/issues_controller_spec.rb
+++ b/spec/controllers/projects/issues_controller_spec.rb
@@ -226,7 +226,7 @@ describe Projects::IssuesController do
           id: issue.iid,
           issue: { assignee_ids: [assignee.id] },
           format: :json
-        body = JSON.parse(response.body)
+        body = json_response
 
         expect(body['assignees'].first.keys)
           .to match_array(%w(id name username avatar_url state web_url))
@@ -889,16 +889,17 @@ describe Projects::IssuesController do
 
   describe 'GET #discussions' do
     let!(:discussion) { create(:discussion_note_on_issue, noteable: issue, project: issue.project) }
+    context 'when authenticated' do
+      before do
+        project.add_developer(user)
+        sign_in(user)
+      end
 
-    before do
-      project.add_developer(user)
-      sign_in(user)
-    end
-
-    it 'returns discussion json' do
-      get :discussions, namespace_id: project.namespace, project_id: project, id: issue.iid
+      it 'returns discussion json' do
+        get :discussions, namespace_id: project.namespace, project_id: project, id: issue.iid
 
-      expect(JSON.parse(response.body).first.keys).to match_array(%w[id reply_id expanded notes individual_note])
+        expect(json_response.first.keys).to match_array(%w[id reply_id expanded notes individual_note])
+      end
     end
 
     context 'with cross-reference system note', :request_store do
diff --git a/spec/services/system_note_service_spec.rb b/spec/services/system_note_service_spec.rb
index b1241cd8d0b..7129d80284b 100644
--- a/spec/services/system_note_service_spec.rb
+++ b/spec/services/system_note_service_spec.rb
@@ -502,20 +502,6 @@ describe SystemNoteService do
     end
   end
 
-  describe '.cross_reference?' do
-    it 'is truthy when text begins with expected text' do
-      expect(described_class.cross_reference?('mentioned in something')).to be_truthy
-    end
-
-    it 'is truthy when text begins with legacy capitalized expected text' do
-      expect(described_class.cross_reference?('mentioned in something')).to be_truthy
-    end
-
-    it 'is falsey when text does not begin with expected text' do
-      expect(described_class.cross_reference?('this is a note')).to be_falsey
-    end
-  end
-
   describe '.cross_reference_disallowed?' do
     context 'when mentioner is not a MergeRequest' do
       it 'is falsey' do