Merge branch '31157-search-security-fix' into 'security'

Respect project features when searching alternative branches with elasticsearch enabled

See merge request !508

changelogs/unreleased-ee/31157-search-security-fix.yml

+---
+title: Respect project features when searching alternative branches with elasticsearch
+  enabled
+merge_request:
+author:

lib/gitlab/elastic/project_search_results.rb

@@ -48,6 +48,8 @@ module Gitlab
       private
 
       def blobs
+        return Kaminari.paginate_array([]) unless Ability.allowed?(@current_user, :download_code, project)
+
         if project.empty_repo? || query.blank?
           Kaminari.paginate_array([])
         else
@@ -89,6 +91,8 @@ module Gitlab
       end
 
       def commits(page: 1, per_page: 20)
+        return Kaminari.paginate_array([]) unless Ability.allowed?(@current_user, :download_code, project)
+
         if project.empty_repo? || query.blank?
           Kaminari.paginate_array([])
         else

spec/lib/gitlab/elastic/project_search_results_spec.rb

@@ -34,8 +34,8 @@ describe Gitlab::Elastic::ProjectSearchResults, lib: true do
 
   describe "search" do
     it "returns correct amounts" do
-      project = create :project
-      project1 = create :project
+      project = create :project, :public
+      project1 = create :project, :public
 
       project.repository.index_blobs
       project.repository.index_commits
@@ -64,30 +64,67 @@ describe Gitlab::Elastic::ProjectSearchResults, lib: true do
   end
 
   describe "search for commits in non-default branch" do
-    it 'finds needed commit' do
-      project = create :project
+    let(:project) { create(:project, :public, visibility) }
+    let(:visibility) { :repository_enabled } 
+    let(:result) { described_class.new(user, 'initial', project.id, 'test') }
+
+    subject(:commits) { result.objects('commits') }
 
-      result = Gitlab::Elastic::ProjectSearchResults.new(user, 'initial', project.id, 'test')
+    it 'finds needed commit' do
       expect(result.commits_count).to eq(1)
     end
 
     it 'responds to total_pages method' do
-      project = create :project
+      expect(commits.total_pages).to eq(1)
+    end
+
+    context 'disabled repository' do
+      let(:visibility) { :repository_disabled }
+
+      it 'hides commits from members' do
+        project.add_reporter(user)
+
+        is_expected.to be_empty
+      end
+
+      it 'hides commits from non-members' do
+        is_expected.to be_empty
+      end
+    end
+
+    context 'private repository' do
+      let(:visibility) { :repository_private }
+
+      it 'shows commits to members' do
+        project.add_reporter(user)
+
+        is_expected.not_to be_empty
+      end
 
-      result = Gitlab::Elastic::ProjectSearchResults.new(user, 'initial', project.id, 'test')
-      expect(result.objects('commits').total_pages).to eq(1)
+      it 'hides commits from non-members' do
+        is_expected.to be_empty
+      end
     end
   end
 
   describe 'search for blobs in non-default branch' do
-    it 'users FileFinder instead of ES search' do
-      project = create :project
+    let(:project) { create(:project, :public, :repository_private) }
+    let(:result) { Gitlab::Elastic::ProjectSearchResults.new(user, 'initial', project.id, 'test') }
+
+    subject(:blobs) { result.objects('blobs') }
+
+    it 'uses FileFinder instead of ES search' do
+      project.add_reporter(user)
 
       expect_any_instance_of(Gitlab::FileFinder).to receive(:find).with('initial').and_return([])
 
-      result = Gitlab::Elastic::ProjectSearchResults.new(user, 'initial', project.id, 'test')
+      _ = blobs
+    end
+
+    it 'respects project visibility' do
+      expect_any_instance_of(Gitlab::FileFinder).to receive(:find).never
 
-      result.blobs_count
+      is_expected.to be_empty
     end
   end