Merge branch 'issue_208662' into 'master'

Index note confidential attribute on elastic search

See merge request gitlab-org/gitlab!30774

ee/lib/elastic/latest/note_class_proxy.rb

@@ -32,19 +32,44 @@ module Elastic
         return query_hash if current_user&.can_read_all_resources?
 
         filter = {
+          bool: {
+            should: [
+              bool: {
+                must: [
+                  {
                     bool: {
                     should: [
                       { bool: { must_not: [{ exists: { field: :issue } }] } },
                       { term: { "issue.confidential" => false } }
                     ]
                     }
+                  },
+                  {
+                    bool: {
+                    should: [
+                      { bool: { must_not: [{ exists: { field: :confidential } }] } },
+                      { term: { "confidential" => false } }
+                    ]
+                    }
+                  }
+                ]
+              }
+            ]
+          }
         }
 
         if current_user
           filter[:bool][:should] << {
             bool: {
               must: [
+                {
+                  bool: {
+                    should: [
                       { term: { "issue.confidential" => true } },
+                      { term: { "confidential" => true } }
+                    ]
+                  }
+                },
                 {
                   bool: {
                     should: [

ee/lib/elastic/latest/note_instance_proxy.rb

@@ -10,7 +10,7 @@ module Elastic
 
         # We don't use as_json(only: ...) because it calls all virtual and serialized attributtes
         # https://gitlab.com/gitlab-org/gitlab/issues/349
-        [:id, :note, :project_id, :noteable_type, :noteable_id, :created_at, :updated_at].each do |attr|
+        [:id, :note, :project_id, :noteable_type, :noteable_id, :created_at, :updated_at, :confidential].each do |attr|
           data[attr.to_s] = safely_read_attribute_for_elasticsearch(attr)
         end
 

ee/spec/models/concerns/elastic/note_spec.rb

@@ -89,7 +89,8 @@ describe Note, :elastic do
       'noteable_type',
       'noteable_id',
       'created_at',
-      'updated_at'
+      'updated_at',
+      'confidential'
     ).merge({
       'issue' => {
         'assignee_id' => issue.assignee_ids,

ee/spec/services/search/global_service_spec.rb

@@ -187,4 +187,26 @@ describe Search::GlobalService do
       end
     end
   end
+
+  context 'confidential notes' do
+    let(:project) { create(:project, :public) }
+
+    context 'with notes on issues' do
+      it_behaves_like 'search notes shared examples' do
+        let(:noteable) { create :issue, project: project }
+      end
+    end
+
+    context 'with notes on merge requests' do
+      it_behaves_like 'search notes shared examples' do
+        let(:noteable) { create :merge_request, target_project: project, source_project: project }
+      end
+    end
+
+    context 'with notes on commits' do
+      it_behaves_like 'search notes shared examples' do
+        let(:noteable) { create(:commit, project: project) }
+      end
+    end
+  end
 end

ee/spec/support/shared_examples/services/search_notes_shared_examples.rb

+# frozen_string_literal: true
+
+RSpec.shared_examples 'search notes shared examples' do
+  context 'notes confidentiality', :elastic, :sidekiq_inline do
+    let_it_be(:user) { create(:user) }
+    let(:commit_id) { noteable.is_a?(Commit) ? noteable.id : nil }
+    let(:noteable_id) { noteable.is_a?(Issuable) ? noteable.id : nil }
+    let!(:not_confidential_note) { create(:note, confidential: false, noteable_id: noteable_id, commit_id: commit_id, noteable_type: noteable.class.name, project: noteable.project, note: 'note 1') }
+    let!(:nil_confidential_note) { create(:note, noteable_id: noteable_id, commit_id: commit_id, noteable_type: noteable.class.name, project: noteable.project, note: 'note 2') }
+    let!(:confidential_note) { create(:note, confidential: true, noteable_id: noteable_id, commit_id: commit_id, noteable_type: noteable.class.name, project: noteable.project, note: 'note 2') }
+
+    before do
+      ensure_elasticsearch_index!
+    end
+
+    context 'for anonymous user' do
+      it 'filters confidential notes' do
+        expect_search_results(nil, 'notes', expected_objects: []) do |user|
+          described_class.new(nil, search: 'note').execute
+        end
+      end
+    end
+
+    context 'when user cannot read confidential notes' do
+      it 'filters confidential notes' do
+        noteable.project.add_guest(user)
+
+        expect_search_results(user, 'notes', expected_objects: [not_confidential_note, nil_confidential_note]) do |user|
+          described_class.new(user, search: 'note').execute
+        end
+      end
+    end
+
+    context 'when user can read confidential notes' do
+      it 'does not filter confidental notes' do
+        noteable.project.add_reporter(user)
+
+        expect_search_results(user, 'notes', expected_objects: [not_confidential_note, nil_confidential_note, confidential_note]) do |user|
+          described_class.new(user, search: 'note').execute
+        end
+      end
+    end
+
+    # For now only issues can be confidential and have confidential notes,
+    # these specs are here to make sure not confidential notes on confidential issues
+    # does not get leaked when mixed with other issuable notes.
+    context 'with additional notes on a confidential issue' do
+      let!(:not_confidential_note_on_confidential_issue) { create(:note, project: noteable.project, noteable: confidential_issue, note: 'note 4') }
+      let!(:confidential_note_on_confidential_issue) { create(:note, confidential: true, project: noteable.project, noteable: confidential_issue, note: 'note 5') }
+
+      context 'when user cannot read confidential' do
+        let(:confidential_issue) { create :issue, confidential: true, project: noteable.project }
+
+        it 'filters all notes from confidential issue' do
+          confidential_issue.project.add_guest(user)
+
+          ensure_elasticsearch_index!
+
+          expect_search_results(user, 'notes', expected_objects: [not_confidential_note, nil_confidential_note]) do |user|
+            described_class.new(user, search: 'note').execute
+          end
+        end
+      end
+
+      context 'when user can read confidential' do
+        context 'when user is project reporter' do
+          let(:confidential_issue) { create :issue, confidential: true, project: noteable.project}
+
+          it 'does not filter confidential issue notes' do
+            confidential_issue.project.add_reporter(user)
+
+            ensure_elasticsearch_index!
+
+            expected_objects = [
+                not_confidential_note, nil_confidential_note, confidential_note,
+                not_confidential_note_on_confidential_issue, confidential_note_on_confidential_issue
+            ]
+            expect_search_results(user, 'notes', expected_objects: expected_objects) do |user|
+              described_class.new(user, search: 'note').execute
+            end
+          end
+        end
+
+        context 'when user is a participant' do
+          let(:expected_objects) do
+            [
+              not_confidential_note, nil_confidential_note,
+              not_confidential_note_on_confidential_issue,
+              confidential_note_on_confidential_issue
+            ]
+          end
+
+          context 'as issue author' do
+            let(:confidential_issue) { create :issue, confidential: true, project: noteable.project, author: user }
+
+            it 'does not filter confidential issue notes' do
+              ensure_elasticsearch_index!
+
+              expect_search_results(user, 'notes', expected_objects: expected_objects) do |user|
+                described_class.new(user, search: 'note').execute
+              end
+            end
+          end
+
+          context 'as issue assignee' do
+            let(:confidential_issue) { create :issue, confidential: true, project: noteable.project, assignees: [user] }
+
+            it 'does not filter confidential issue notes' do
+              ensure_elasticsearch_index!
+
+              expect_search_results(user, 'notes', expected_objects: expected_objects) do |user|
+                described_class.new(user, search: 'note').execute
+              end
+            end
+          end
+        end
+      end
+    end
+  end
+end