Restrict access to confidential issues on Elasticsearch results

app/services/search/global_service.rb

@@ -12,7 +12,7 @@ module Search
       projects = projects.in_namespace(group.id) if group
 
       if Gitlab.config.elasticsearch.enabled
-        Gitlab::Elastic::SearchResults.new(projects.pluck(:id), params[:search])
+        Gitlab::Elastic::SearchResults.new(current_user, projects.pluck(:id), params[:search])
       else
         Gitlab::SearchResults.new(current_user, projects, params[:search])
       end

app/services/search/project_service.rb

@@ -8,9 +8,10 @@ module Search
 
     def execute
       if Gitlab.config.elasticsearch.enabled
-        Gitlab::Elastic::ProjectSearchResults.new(project.id,
-                                         params[:search],
-                                         params[:repository_ref])
+        Gitlab::Elastic::ProjectSearchResults.new(current_user,
+                                                  project.id,
+                                                  params[:search],
+                                                  params[:repository_ref])
       else
         Gitlab::ProjectSearchResults.new(current_user,
                                          project,

lib/elastic/issues_search.rb

@@ -23,6 +23,8 @@ module Elastic
         indexes :project,     type: :nested
         indexes :author,      type: :nested
 
+        indexes :confidential, type: :boolean
+
         indexes :updated_at_sort, type: :date,   index: :not_analyzed
       end
 
@@ -48,10 +50,38 @@ module Elastic
           query_hash = basic_query_hash(%w(title^2 description), query)
         end
 
-        query_hash = project_ids_filter(query_hash, options[:project_ids])
+        query_hash = project_ids_filter(query_hash, options[:projects_ids])
+        query_hash = confidentiality_filter(query_hash, options[:user_id], options[:authorized_projects_ids])
 
         self.__elasticsearch__.search(query_hash)
       end
+
+      def self.confidentiality_filter(query_hash, user_id, projects_ids)
+        if user_id
+          query_hash[:query][:filtered][:filter] = {
+            bool: {
+              should: [
+                { term: { confidential: false } },
+                { bool: {
+                    must: [
+                      { term: { confidential: true } },
+                      { bool: {
+                          should: [
+                            { term: { author_id: user_id } },
+                            { terms: { project_id: projects_ids } }
+                          ]
+                        }
+                      }
+                    ]
+                  }
+                }
+              ]
+            }
+          }
+        end
+
+        query_hash
+      end
     end
   end
 end

lib/gitlab/elastic/project_search_results.rb

@@ -3,7 +3,8 @@ module Gitlab
     class ProjectSearchResults < SearchResults
       attr_reader :project, :repository_ref
 
-      def initialize(project_id, query, repository_ref = nil)
+      def initialize(user, project_id, query, repository_ref = nil)
+        @user = user
         @project = Project.find(project_id)
 
         @repository_ref = if repository_ref.present?
@@ -106,6 +107,25 @@ module Gitlab
         end
       end
 
+      def issues
+        opt = {
+          projects_ids: limit_project_ids
+        }
+
+        unless user.admin? || project.team.member?(user.id)
+          opt.merge!(
+            authorized_projects_ids: user.authorized_projects.pluck(:id),
+            user_id: user.id
+          )
+        end
+
+        if query =~ /#(\d+)\z/
+          Issue.in_projects(limit_project_ids).where(iid: $1)
+        else
+          Issue.elastic_search(query, options: opt).records
+        end
+      end
+
       def limit_project_ids
         [project.id]
       end

lib/gitlab/elastic/search_results.rb

 module Gitlab
   module Elastic
     class SearchResults
-      attr_reader :query
+      attr_reader :user, :query
 
       # Limit search results by passed project ids
       # It allows us to search only for projects user has access to
       attr_reader :limit_project_ids
 
-      def initialize(limit_project_ids, query)
+      def initialize(user, limit_project_ids, query)
+        @user = user
         @limit_project_ids = limit_project_ids || Project.all
         @query = Shellwords.shellescape(query) if query.present?
       end
@@ -66,6 +67,13 @@ module Gitlab
           project_ids: limit_project_ids
         }
 
+        unless user.admin?
+          opt.merge!(
+            authorized_projects_ids: user.authorized_projects.pluck(:id),
+            user_id: user.id
+          )
+        end
+
         Issue.elastic_search(query, options: opt)
       end
 

spec/lib/gitlab/elastic/project_search_results_spec.rb

 require 'spec_helper'
 
 describe Gitlab::Elastic::ProjectSearchResults, lib: true do
+  let(:user) { create(:user) }
+  let(:project) { create(:project) }
+  let(:query) { 'hello world' }
+
   before do
     allow(Gitlab.config.elasticsearch).to receive(:enabled).and_return(true)
     Project.__elasticsearch__.create_index!
-
-    @project = create(:project)
+    Issue.__elasticsearch__.create_index!
   end
 
   after do
     allow(Gitlab.config.elasticsearch).to receive(:enabled).and_return(false)
     Project.__elasticsearch__.delete_index!
+    Issue.__elasticsearch__.delete_index!
   end
 
-  let(:query) { 'hello world' }
-
   describe 'initialize with empty ref' do
-    let(:results) { Gitlab::Elastic::ProjectSearchResults.new(@project.id, query, '') }
+    subject(:results) { described_class.new(user, project.id, query, '') }
 
-    it { expect(results.project).to eq(@project) }
+    it { expect(results.project).to eq(project) }
     it { expect(results.repository_ref).to be_nil }
     it { expect(results.query).to eq('hello world') }
   end
 
   describe 'initialize with ref' do
     let(:ref) { 'refs/heads/test' }
-    let(:results) { Gitlab::Elastic::ProjectSearchResults.new(@project.id, query, ref) }
+    subject(:results) { described_class.new(user, project.id, query, ref) }
 
-    it { expect(results.project).to eq(@project) }
+    it { expect(results.project).to eq(project) }
     it { expect(results.repository_ref).to eq(ref) }
     it { expect(results.query).to eq('hello world') }
   end
@@ -39,7 +41,7 @@ describe Gitlab::Elastic::ProjectSearchResults, lib: true do
 
       project.repository.index_blobs
       project.repository.index_commits
-      
+
       # Notes
       create :note, note: 'bla-bla term', project: project
       # The note in the project you have no access to
@@ -53,13 +55,70 @@ describe Gitlab::Elastic::ProjectSearchResults, lib: true do
 
       Project.__elasticsearch__.refresh_index!
 
-      result = Gitlab::Elastic::ProjectSearchResults.new(project.id, "term")
+      result = Gitlab::Elastic::ProjectSearchResults.new(user, project.id, "term")
       expect(result.notes_count).to eq(1)
       expect(result.wiki_blobs_count).to eq(1)
       expect(result.blobs_count).to eq(1)
 
-      result1 = Gitlab::Elastic::ProjectSearchResults.new(project.id, "initial")
+      result1 = Gitlab::Elastic::ProjectSearchResults.new(user, project.id, "initial")
       expect(result1.commits_count).to eq(1)
     end
   end
+
+  describe 'confidential issues' do
+    let(:query) { 'issue' }
+    let(:author) { create(:user) }
+    let(:non_member) { create(:user) }
+    let(:member) { create(:user) }
+    let(:admin) { create(:admin) }
+    let!(:issue) { create(:issue, project: project, title: 'Issue 1') }
+    let!(:security_issue_1) { create(:issue, :confidential, project: project, title: 'Security issue 1', author: author) }
+    let!(:security_issue_2) { create(:issue, :confidential, title: 'Security issue 2', project: project) }
+
+    before do
+      Issue.__elasticsearch__.refresh_index!
+    end
+
+    it 'should not list project confidential issues for non project members' do
+      results = described_class.new(non_member, project.id, query)
+      issues = results.objects('issues')
+
+      expect(issues).to include issue
+      expect(issues).not_to include security_issue_1
+      expect(issues).not_to include security_issue_2
+      expect(results.issues_count).to eq 1
+    end
+
+    it 'should list project confidential issues for author' do
+      results = described_class.new(author, project.id, query)
+      issues = results.objects('issues')
+
+      expect(issues).to include issue
+      expect(issues).to include security_issue_1
+      expect(issues).not_to include security_issue_2
+      expect(results.issues_count).to eq 2
+    end
+
+    it 'should list project confidential issues for project members' do
+      project.team << [member, :developer]
+
+      results = described_class.new(member, project.id, query)
+      issues = results.objects('issues')
+
+      expect(issues).to include issue
+      expect(issues).to include security_issue_1
+      expect(issues).to include security_issue_2
+      expect(results.issues_count).to eq 3
+    end
+
+    it 'should list all project issues for admin' do
+      results = described_class.new(admin, project.id, query)
+      issues = results.objects('issues')
+
+      expect(issues).to include issue
+      expect(issues).to include security_issue_1
+      expect(issues).to include security_issue_2
+      expect(results.issues_count).to eq 3
+    end
+  end
 end

spec/lib/gitlab/elastic/search_results_spec.rb

@@ -9,6 +9,7 @@ describe Gitlab::Elastic::SearchResults, lib: true do
     allow(Gitlab.config.elasticsearch).to receive(:enabled).and_return(false)
   end
 
+  let(:user) { create(:user) }
   let(:project_1) { create(:project) }
   let(:project_2) { create(:project) }
   let(:limit_project_ids) { [project_1.id] }
@@ -45,7 +46,7 @@ describe Gitlab::Elastic::SearchResults, lib: true do
     end
 
     it 'should list issues that title or description contain the query' do
-      results = described_class.new(limit_project_ids, 'hello world')
+      results = described_class.new(user, limit_project_ids, 'hello world')
       issues = results.objects('issues')
 
       expect(issues).to include @issue_1
@@ -55,14 +56,14 @@ describe Gitlab::Elastic::SearchResults, lib: true do
     end
 
     it 'should return empty list when issues title or description does not contain the query' do
-      results = described_class.new(limit_project_ids, 'security')
+      results = described_class.new(user, limit_project_ids, 'security')
 
       expect(results.objects('issues')).to be_empty
       expect(results.issues_count).to eq 0
     end
 
     it 'should list issue when search by a valid iid' do
-      results = described_class.new(limit_project_ids, '#2')
+      results = described_class.new(user, limit_project_ids, '#2')
       issues = results.objects('issues')
 
       expect(issues).not_to include @issue_1
@@ -72,13 +73,89 @@ describe Gitlab::Elastic::SearchResults, lib: true do
     end
 
     it 'should return empty list when search by invalid iid' do
-      results = described_class.new(limit_project_ids, '#222')
+      results = described_class.new(user, limit_project_ids, '#222')
 
       expect(results.objects('issues')).to be_empty
       expect(results.issues_count).to eq 0
     end
   end
 
+  describe 'confidential issues' do
+    let(:project_3) { create(:empty_project) }
+    let(:project_4) { create(:empty_project) }
+    let(:query) { 'issue' }
+    let(:limit_project_ids) { [project_1.id, project_2.id, project_3.id] }
+    let(:author) { create(:user) }
+    let(:non_member) { create(:user) }
+    let(:member) { create(:user) }
+    let(:admin) { create(:admin) }
+    let!(:issue) { create(:issue, project: project_1, title: 'Issue 1') }
+    let!(:security_issue_1) { create(:issue, :confidential, project: project_1, title: 'Security issue 1', author: author) }
+    let!(:security_issue_2) { create(:issue, :confidential, title: 'Security issue 2', project: project_1) }
+    let!(:security_issue_3) { create(:issue, :confidential, project: project_2, title: 'Security issue 3', author: author) }
+    let!(:security_issue_4) { create(:issue, :confidential, project: project_3, title: 'Security issue 4') }
+    let!(:security_issue_5) { create(:issue, :confidential, project: project_4, title: 'Security issue 5') }
+
+    before do
+      Issue.__elasticsearch__.refresh_index!
+    end
+
+    it 'should not list confidential issues for non project members' do
+      results = described_class.new(non_member, limit_project_ids, query)
+      issues = results.objects('issues')
+
+      expect(issues).to include issue
+      expect(issues).not_to include security_issue_1
+      expect(issues).not_to include security_issue_2
+      expect(issues).not_to include security_issue_3
+      expect(issues).not_to include security_issue_4
+      expect(issues).not_to include security_issue_5
+      expect(results.issues_count).to eq 1
+    end
+
+    it 'should list confidential issues for author' do
+      results = described_class.new(author, limit_project_ids, query)
+      issues = results.objects('issues')
+
+      expect(issues).to include issue
+      expect(issues).to include security_issue_1
+      expect(issues).not_to include security_issue_2
+      expect(issues).to include security_issue_3
+      expect(issues).not_to include security_issue_4
+      expect(issues).not_to include security_issue_5
+      expect(results.issues_count).to eq 3
+    end
+
+    it 'should list confidential issues for project members' do
+      project_1.team << [member, :developer]
+      project_2.team << [member, :developer]
+
+      results = described_class.new(member, limit_project_ids, query)
+      issues = results.objects('issues')
+
+      expect(issues).to include issue
+      expect(issues).to include security_issue_1
+      expect(issues).to include security_issue_2
+      expect(issues).to include security_issue_3
+      expect(issues).not_to include security_issue_4
+      expect(issues).not_to include security_issue_5
+      expect(results.issues_count).to eq 4
+    end
+
+    it 'should list all issues for admin' do
+      results = described_class.new(admin, limit_project_ids, query)
+      issues = results.objects('issues')
+
+      expect(issues).to include issue
+      expect(issues).to include security_issue_1
+      expect(issues).to include security_issue_2
+      expect(issues).to include security_issue_3
+      expect(issues).to include security_issue_4
+      expect(issues).not_to include security_issue_5
+      expect(results.issues_count).to eq 5
+    end
+  end
+
   describe 'merge requests' do
     before do
       MergeRequest.__elasticsearch__.create_index!
@@ -115,7 +192,7 @@ describe Gitlab::Elastic::SearchResults, lib: true do
     end
 
     it 'should list merge requests that title or description contain the query' do
-      results = described_class.new(limit_project_ids, 'hello world')
+      results = described_class.new(user, limit_project_ids, 'hello world')
       merge_requests = results.objects('merge_requests')
 
       expect(merge_requests).to include @merge_request_1
@@ -125,14 +202,14 @@ describe Gitlab::Elastic::SearchResults, lib: true do
     end
 
     it 'should return empty list when merge requests title or description does not contain the query' do
-      results = described_class.new(limit_project_ids, 'security')
+      results = described_class.new(user, limit_project_ids, 'security')
 
       expect(results.objects('merge_requests')).to be_empty
       expect(results.merge_requests_count).to eq 0
     end
 
     it 'should list merge request when search by a valid iid' do
-      results = described_class.new(limit_project_ids, '#2')
+      results = described_class.new(user, limit_project_ids, '#2')
       merge_requests = results.objects('merge_requests')
 
       expect(merge_requests).not_to include @merge_request_1
@@ -142,7 +219,7 @@ describe Gitlab::Elastic::SearchResults, lib: true do
     end
 
     it 'should return empty list when search by invalid iid' do
-      results = described_class.new(limit_project_ids, '#222')
+      results = described_class.new(user, limit_project_ids, '#222')
 
       expect(results.objects('merge_requests')).to be_empty
       expect(results.merge_requests_count).to eq 0