Merge branch 'fix-18997' into 'master'

Fix visibility of snippets when searching using ElasticSearch

See merge request !501

app/models/concerns/elastic/snippets_search.rb

@@ -45,7 +45,7 @@ module Elastic
       def self.elastic_search(query, options: {})
         query_hash = basic_query_hash(%w(title file_name), query)
 
-        query_hash = filter(query_hash, options[:author_id])
+        query_hash = filter(query_hash, options[:user])
 
         self.__elasticsearch__.search(query_hash)
       end
@@ -59,7 +59,7 @@ module Elastic
           }
         }
 
-        query_hash = filter(query_hash, options[:author_id])
+        query_hash = filter(query_hash, options[:user])
 
         query_hash[:sort] = [
           { updated_at_sort: { order: :desc, mode: :min } },
@@ -71,18 +71,24 @@ module Elastic
         self.__elasticsearch__.search(query_hash)
       end
 
-      def self.filter(query_hash, author_id)
-        if author_id
-          query_hash[:query][:bool][:filter] = {
-            bool: {
-              should: [
-                { terms: { visibility_level: [Snippet::PUBLIC, Snippet::INTERNAL] } },
-                { term: { author_id: author_id } }
-              ]
-            }
-          }
-        end
-
+      def self.filter(query_hash, user)
+        return query_hash if user && user.admin?
+
+        filter = if user
+                   {
+                     bool: {
+                       should: [
+                         { terms: { visibility_level: [Snippet::PUBLIC, Snippet::INTERNAL] } },
+                         { term: { author_id: user.id } },
+                         { terms: { project_id: user.authorized_projects.pluck(:id) } },
+                       ]
+                     }
+                   }
+                 else
+                   { term: { visibility_level: Snippet::PUBLIC } }
+                 end
+
+        query_hash[:query][:bool][:filter] = filter
         query_hash
       end
     end

lib/gitlab/elastic/snippet_search_results.rb

@@ -21,7 +21,7 @@ module Gitlab
 
       def snippet_titles
         opt = {
-          author_id: @user.id
+          user: @user
         }
 
         Snippet.elastic_search(query, options: opt)
@@ -29,7 +29,7 @@ module Gitlab
 
       def snippet_blobs
         opt = {
-          author_id: @user.id
+          user: @user
         }
 
         Snippet.elastic_search_code(query, options: opt)

spec/models/concerns/elastic/snippet_spec.rb

@@ -11,45 +11,85 @@ describe Snippet, elastic: true do
     stub_application_setting(elasticsearch_search: false, elasticsearch_indexing: false)
   end
 
-  it "searches snippets by code" do
-    user = create :user
+  context 'searching snippets by code' do
+    let!(:author) { create(:user) }
+    let!(:project) { create(:project) }
 
-    snippet = create :personal_snippet, :private, content: 'genius code', author: user
-    create :personal_snippet, :private, content: 'genius code'
-    create :personal_snippet, :private
+    let!(:public_snippet)   { create(:snippet, :public, content: 'password: XXX') }
+    let!(:internal_snippet) { create(:snippet, :internal, content: 'password: XXX') }
+    let!(:private_snippet)  { create(:snippet, :private, content: 'password: XXX', author: author) }
 
-    snippet3 = create :personal_snippet, :public, content: 'genius code'
+    let!(:project_public_snippet)   { create(:snippet, :public, project: project, content: 'password: XXX') }
+    let!(:project_internal_snippet) { create(:snippet, :internal, project: project, content: 'password: XXX') }
+    let!(:project_private_snippet)  { create(:snippet, :private, project: project, content: 'password: XXX') }
 
-    described_class.__elasticsearch__.refresh_index!
+    before do
+      described_class.__elasticsearch__.refresh_index!
+    end
+
+    it 'returns only public snippets when user is blank' do
+      result = described_class.elastic_search_code('password', options: { user: nil })
+
+      expect(result.total_count).to eq(2)
+      expect(result.records).to match_array [public_snippet, project_public_snippet]
+    end
+
+    it 'returns only public and internal snippets for regular users' do
+      regular_user = create(:user)
+
+      result = described_class.elastic_search_code('password', options: { user: regular_user })
+
+      expect(result.total_count).to eq(4)
+      expect(result.records).to match_array [public_snippet, internal_snippet, project_public_snippet, project_internal_snippet]
+    end
+
+    it 'returns public, internal snippets, and project private snippets for project members' do
+      member = create(:user)
+      project.team << [member, :developer]
+
+      result = described_class.elastic_search_code('password', options: { user: member })
+
+      expect(result.total_count).to eq(5)
+      expect(result.records).to match_array [public_snippet, internal_snippet, project_public_snippet, project_internal_snippet, project_private_snippet]
+    end
+
+    it 'returns private snippets where the user is the author' do
+      result = described_class.elastic_search_code('password', options: { user: author })
+
+      expect(result.total_count).to eq(5)
+      expect(result.records).to match_array [public_snippet, internal_snippet, private_snippet, project_public_snippet, project_internal_snippet]
+    end
 
-    options = { author_id: user.id }
+    it 'returns all snippets for admins' do
+      admin = create(:admin)
 
-    result = described_class.elastic_search_code('genius code', options: options)
+      result = described_class.elastic_search_code('password', options: { user: admin })
 
-    expect(result.total_count).to eq(2)
-    expect(result.records.map(&:id)).to include(snippet.id, snippet3.id)
+      expect(result.total_count).to eq(6)
+      expect(result.records).to match_array [public_snippet, internal_snippet, private_snippet, project_public_snippet, project_internal_snippet, project_private_snippet]
+    end
   end
 
-  it "searches snippets by title and file_name" do
-    user = create :user
+  it 'searches snippets by title and file_name' do
+    user = create(:user)
 
-    create :snippet, :public, title: 'home'
-    create :snippet, :private, title: 'home 1'
-    create :snippet, :public, file_name: 'index.php'
-    create :snippet
+    create(:snippet, :public, title: 'home')
+    create(:snippet, :private, title: 'home 1')
+    create(:snippet, :public, file_name: 'index.php')
+    create(:snippet)
 
     described_class.__elasticsearch__.refresh_index!
 
-    options = { author_id: user.id }
+    options = { user: user }
 
     expect(described_class.elastic_search('home', options: options).total_count).to eq(1)
-    expect(described_class.elastic_search('index.php', options:  options).total_count).to eq(1)
+    expect(described_class.elastic_search('index.php', options: options).total_count).to eq(1)
   end
 
-  it "returns json with all needed elements" do
-    snippet = create :project_snippet
+  it 'returns json with all needed elements' do
+    snippet = create(:project_snippet)
 
-    expected_hash =  snippet.attributes.extract!(
+    expected_hash = snippet.attributes.extract!(
       'id',
       'title',
       'file_name',