Merge branch 'es_data_leak' into 'master'

ES: Protecting notes for confidential issue in search result page

https://gitlab.com/gitlab-org/gitlab-ee/issues/575

If it goes to the patch release, please include following to the release blog post:
--------------------------------------------------
If you use Elasticsearch please run following command after upgrade:

```
# Omnibus installations
sudo gitlab-rake gitlab:elastic:reindex_model MODEL=Note

# Installations from source
bundle exec rake gitlab:elastic:reindex_model MODEL=Note RAILS_ENV=production
```
----------------------------------------------------

See merge request !500

CHANGELOG-EE

@@ -6,6 +6,7 @@ v 8.8.3
   - Gracefully handle malformed DNs in LDAP group sync
   - Make it clear the license overusage is visible only to admins
   - Reduce load on DB for license upgrade check
+  - [Elastic] In search results, only show notes on confidential issues that the user has access to
 
 v 8.8.2
   - Fix repository mirror updates for new imports stuck in started

app/models/concerns/elastic/application_search.rb

@@ -38,7 +38,12 @@ module Elastic
 
       after_commit on: :update do
         if Gitlab.config.elasticsearch.enabled && self.searchable?
-          ElasticIndexerWorker.perform_async(:update, self.class.to_s, self.id)
+          ElasticIndexerWorker.perform_async(
+            :update,
+            self.class.to_s,
+            self.id,
+            changed_fields: self.previous_changes.keys
+          )
         end
       end
 

app/models/concerns/elastic/notes_search.rb

@@ -12,6 +12,12 @@ module Elastic
         indexes :project_id,  type: :integer
         indexes :created_at,  type: :date
 
+        indexes :issue do
+          indexes :assignee_id, type: :integer
+          indexes :author_id, type: :integer
+          indexes :confidential, type: :boolean
+        end
+
         indexes :updated_at_sort, type: :string, index: 'not_analyzed'
       end
 
@@ -24,27 +30,36 @@ module Elastic
           data[attr.to_s] = self.send(attr)
         end
 
+        if noteable.is_a?(Issue)
+          data['issue'] = {
+            assignee_id: noteable.assignee_id,
+            author_id: noteable.author_id,
+            confidential: noteable.confidential
+          }
+        end
+
         data['updated_at_sort'] = updated_at
         data
       end
 
       def self.elastic_search(query, options: {})
-        options[:in] = ["note"]
+        options[:in] = ['note']
 
         query_hash = {
           query: {
             bool: {
-              must: { match: { note: query } },
+              must: [{ match: { note: query } }],
             },
           }
         }
 
         if query.blank?
-          query_hash[:query][:bool][:must] = { match_all: {} }
+          query_hash[:query][:bool][:must] = [{ match_all: {} }]
           query_hash[:track_scores] = true
         end
 
         query_hash = project_ids_filter(query_hash, options[:project_ids])
+        query_hash = confidentiality_filter(query_hash, options[:current_user])
 
         query_hash[:sort] = [
           { updated_at_sort: { order: :desc, mode: :min } },
@@ -55,6 +70,40 @@ module Elastic
 
         self.__elasticsearch__.search(query_hash)
       end
+
+      def self.confidentiality_filter(query_hash, current_user)
+        return query_hash if current_user && current_user.admin?
+
+        filter = {
+          bool: {
+            should: [
+              { bool: { must_not: [{ exists: { field: :issue } }] } },
+              { term: { "issue.confidential" => false } }
+            ]
+          }
+        }
+
+        if current_user
+          filter[:bool][:should] << {
+            bool: {
+              must: [
+                { term: { "issue.confidential" => true } },
+                { bool: {
+                    should: [
+                      { term: { "issue.author_id" => current_user.id } },
+                      { term: { "issue.assignee_id" => current_user.id } },
+                      { terms: { "issue.project_id" => current_user.authorized_projects.pluck(:id) } }
+                    ]
+                  }
+                }
+              ]
+            }
+          }
+        end
+
+        query_hash[:query][:bool][:must] << filter
+        query_hash
+      end
     end
   end
 end

app/workers/elastic_indexer_worker.rb

@@ -3,6 +3,8 @@ class ElasticIndexerWorker
 
   sidekiq_options queue: :elasticsearch
 
+  ISSUE_TRACKED_FIELDS = %w(assignee_id author_id confidential)
+
   Client = Elasticsearch::Client.new(host: Gitlab.config.elasticsearch.host,
                                      port: Gitlab.config.elasticsearch.port)
 
@@ -14,6 +16,8 @@ class ElasticIndexerWorker
       record = klass.find(record_id)
       record.__elasticsearch__.client = Client
       record.__elasticsearch__.__send__ "#{operation}_document"
+
+      update_issue_notes(record, options["changed_fields"]) if klass == Issue
     when /delete/
       Client.delete index: klass.index_name, type: klass.document_type, id: record_id
 
@@ -23,6 +27,12 @@ class ElasticIndexerWorker
     true # Less work to do!
   end
 
+  def update_issue_notes(record, changed_fields)
+    if changed_fields && (changed_fields & ISSUE_TRACKED_FIELDS).any?
+      Note.import query: -> { where(noteable: record) }
+    end
+  end
+
   def clear_project_indexes(record_id)
     # Remove repository index
     Client.delete_by_query({

lib/gitlab/elastic/project_search_results.rb

@@ -86,7 +86,8 @@ module Gitlab
 
       def notes
         opt = {
-          project_ids: limit_project_ids
+          project_ids: limit_project_ids,
+          current_user: @current_user
         }
 
         Note.elastic_search(query, options: opt)

spec/models/concerns/elastic/note_spec.rb

@@ -48,4 +48,33 @@ describe "Note", elastic: true do
     create :note, :system, project: project
     create :note, :award, project: project
   end
+
+  context 'notes to confidential issues' do
+    it "does not find note" do
+      issue = create :issue, :confidential
+
+      create :note, note: 'bla-bla term', project: issue.project, noteable: issue
+      create :note, project: issue.project, noteable: issue
+
+      Note.__elasticsearch__.refresh_index!
+
+      options = { project_ids: [issue.project.id] }
+
+      expect(Note.elastic_search('term', options: options).total_count).to eq(0)
+    end
+
+    it "finds note when user is authorized to see it" do
+      user = create :user
+      issue = create :issue, :confidential, author: user
+
+      create :note, note: 'bla-bla term', project: issue.project, noteable: issue
+      create :note, project: issue.project, noteable: issue
+
+      Note.__elasticsearch__.refresh_index!
+
+      options = { project_ids: [issue.project.id], current_user: user }
+
+      expect(Note.elastic_search('term', options: options).total_count).to eq(1)
+    end
+  end
 end