Hide system notes with invalid references

Merge branch 'security-hide-system-notes-with-invalid-references-17-4' into '17-4-stable-ee'

See merge request gitlab-org/security/gitlab!4482

Changelog: security

app/models/note.rb

@@ -765,13 +765,11 @@ def all_referenced_mentionables_allowed?(user)
     if user_visible_reference_count.present? && total_reference_count.present?
       # if they are not equal, then there are private/confidential references as well
-      total_reference_count == 0 ||
-        (user_visible_reference_count > 0 && user_visible_reference_count == total_reference_count)
+      user_visible_reference_count > 0 && user_visible_reference_count == total_reference_count
     else
       refs = all_references(user)
-      refs.all
-      refs.all_visible?
+      refs.all.present? && refs.all_visible?
     end
   end

ee/spec/services/issuable/discussions_list_service_spec.rb

@@ -6,9 +6,9 @@
   let_it_be(:current_user) { create(:user) }
   let_it_be(:group) { create(:group, :private) }
   let_it_be(:project) { create(:project, :repository, :private, group: group) }
-  let_it_be(:milestone) { create(:milestone, project: project) }
-  let_it_be(:label) { create(:label, project: project) }
-  let_it_be(:label_2) { create(:label, project: project) }
+  let_it_be(:milestone) { create(:milestone, group: group) }
+  let_it_be(:label) { create(:group_label, group: group) }
+  let_it_be(:label_2) { create(:group_label, group: group) }
   let(:finder_params_for_issuable) { {} }

spec/controllers/projects/issues_controller_spec.rb

@@ -1862,6 +1862,16 @@ def get_service_desk(extra_params = {})
             get :discussions, params: { namespace_id: project.namespace, project_id: project, id: issue.iid }
           end.not_to exceed_query_limit(control)
         end
+
+        context 'when reference is invalid' do
+          let(:cross_reference) { "mentioned in some/invalid/project#123" }
+
+          it 'does not include the system note' do
+            get :discussions, params: { namespace_id: project.namespace, project_id: project, id: issue.iid }
+
+            expect(json_response.count).to eq(1)
+          end
+        end
       end
       context 'private project' do

spec/models/note_spec.rb

@@ -972,16 +972,16 @@ def retrieve_participants
         create :note, noteable: ext_issue, project: ext_proj, note: "mentioned in merge request !1", system: true
       end
-      it "returns true for other users" do
-        expect(note.system_note_visible_for?(private_user)).to be_truthy
+      it "returns false" do
+        expect(note.system_note_visible_for?(private_user)).to be_falsey
       end
-      it "returns true if user visible reference count set" do
+      it "returns false if user visible reference count set" do
         note.user_visible_reference_count = 0
         note.total_reference_count = 0
         expect(note).not_to receive(:reference_mentionables)
-        expect(note.system_note_visible_for?(ext_issue.author)).to be_truthy
+        expect(note.system_note_visible_for?(ext_issue.author)).to be_falsey
       end
     end
   end

spec/services/issuable/discussions_list_service_spec.rb

@@ -6,9 +6,9 @@
   let_it_be(:current_user) { create(:user) }
   let_it_be(:group) { create(:group, :private) }
   let_it_be(:project) { create(:project, :repository, :private, group: group) }
-  let_it_be(:milestone) { create(:milestone, project: project) }
-  let_it_be(:label) { create(:label, project: project) }
-  let_it_be(:label_2) { create(:label, project: project) }
+  let_it_be(:milestone) { create(:milestone, group: group) }
+  let_it_be(:label) { create(:group_label, group: group) }
+  let_it_be(:label_2) { create(:group_label, group: group) }
   let(:finder_params_for_issuable) { {} }