Merge branch 'security-search-by-iid-leaks-data' into 'master'

Fix search by IID reveals private data

See merge request gitlab/gitlab-ee!1354

changelogs/unreleased/security-search-by-iid-leaks-data.yml

+---
+title: Limit search for IID to a type to avoid leaking records with the same IID that
+  the user does not have access to
+merge_request:
+author:
+type: security

ee/lib/elastic/latest/application_class_proxy.rb

@@ -82,7 +82,10 @@ module Elastic
         {
           query: {
             bool: {
-              filter: [{ term: { iid: iid } }]
+              filter: [
+                { term: { iid: iid } },
+                { term: { type: self.es_type } }
+              ]
             }
           }
         }

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

@@ -75,6 +75,28 @@ describe Issue, :elastic do
     expect(described_class.elastic_search('bla-bla', options: { project_ids: :any, public_and_internal_projects: true }).total_count).to eq(3)
   end
 
+  it "searches by iid and scopes to type: issue only" do
+    issue = nil
+
+    Sidekiq::Testing.inline! do
+      issue = create :issue, title: 'bla-bla issue', project: project
+      create :issue, description: 'term2 in description', project: project
+
+      # MergeRequest with the same iid should not be found in Issue search
+      create :merge_request, title: 'bla-bla', source_project: project, iid: issue.iid
+
+      Gitlab::Elastic::Helper.refresh_index
+    end
+
+    # User needs to be admin or the MergeRequest would just be filtered by
+    # confidential: false
+    options = { project_ids: [project.id], current_user: admin }
+
+    results = described_class.elastic_search("##{issue.iid}", options: options)
+    expect(results.total_count).to eq(1)
+    expect(results.first.title).to eq('bla-bla issue')
+  end
+
   it "returns json with all needed elements" do
     assignee = create(:user)
     issue = create :issue, project: project, assignees: [assignee]

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

@@ -40,6 +40,27 @@ describe MergeRequest, :elastic do
     expect(described_class.elastic_search('term3', options: { project_ids: :any, public_and_internal_projects: true }).total_count).to eq(1)
   end
 
+  it "searches by iid and scopes to type: merge_request only" do
+    project = create :project, :public, :repository
+    merge_request = nil
+
+    Sidekiq::Testing.inline! do
+      merge_request = create :merge_request, title: 'bla-bla merge request', source_project: project
+      create :merge_request, description: 'term2 in description', source_project: project, target_branch: "feature2"
+
+      # Issue with the same iid should not be found in MergeRequest search
+      create :issue, project: project, iid: merge_request.iid
+
+      Gitlab::Elastic::Helper.refresh_index
+    end
+
+    options = { project_ids: [project.id] }
+
+    results = described_class.elastic_search("!#{merge_request.iid}", options: options)
+    expect(results.total_count).to eq(1)
+    expect(results.first.title).to eq('bla-bla merge request')
+  end
+
   it "returns json with all needed elements" do
     merge_request = create :merge_request