From 9154586ce5c46dfac83a1ed1e4beac1940913f16 Mon Sep 17 00:00:00 2001
From: Valery Sizov <valery@gitlab.com>
Date: Thu, 26 May 2016 14:12:43 +0300
Subject: [PATCH] Confidential notes data leak

---
 CHANGELOG                            |  1 +
 app/models/note.rb                   | 22 +++++++++++++++++++---
 lib/gitlab/project_search_results.rb |  2 +-
 spec/models/note_spec.rb             | 19 +++++++++++++++++++
 4 files changed, 40 insertions(+), 4 deletions(-)

diff --git a/CHANGELOG b/CHANGELOG
index 5dc56d18c3a..c6226aee049 100644
--- a/CHANGELOG
+++ b/CHANGELOG
@@ -17,6 +17,7 @@ v 8.9.0 (unreleased)
 v 8.8.3
   - Fix gitlab importer failing to import new projects due to missing credentials
   - Fix import URL migration not rescuing with the correct Error
+  - In search results, only show notes on confidential issues that the user has access to
 
 v 8.8.2
   - Added remove due date button. !4209
diff --git a/app/models/note.rb b/app/models/note.rb
index 55b98557244..29f38539116 100644
--- a/app/models/note.rb
+++ b/app/models/note.rb
@@ -77,14 +77,30 @@ class Note < ActiveRecord::Base
     #
     # This method uses ILIKE on PostgreSQL and LIKE on MySQL.
     #
-    # query - The search query as a String.
+    # query   - The search query as a String.
+    # as_user - Limit results to those viewable by a specific user
     #
     # Returns an ActiveRecord::Relation.
-    def search(query)
+    def search(query, as_user: nil)
       table   = arel_table
       pattern = "%#{query}%"
 
-      where(table[:note].matches(pattern))
+      found_notes = joins('LEFT JOIN issues ON issues.id = noteable_id').
+        where(table[:note].matches(pattern))
+
+      if as_user
+        found_notes.where('
+          issues.confidential IS NULL
+          OR issues.confidential IS FALSE
+          OR (issues.confidential IS TRUE
+            AND (issues.author_id = :user_id
+            OR issues.assignee_id = :user_id
+            OR issues.project_id IN(:project_ids)))',
+          user_id: as_user.id,
+          project_ids: as_user.authorized_projects.select(:id))
+      else
+        found_notes.where('issues.confidential IS NULL OR issues.confidential IS FALSE')
+      end
     end
 
     def grouped_awards
diff --git a/lib/gitlab/project_search_results.rb b/lib/gitlab/project_search_results.rb
index 71c5b6801fb..183bd10d6a3 100644
--- a/lib/gitlab/project_search_results.rb
+++ b/lib/gitlab/project_search_results.rb
@@ -74,7 +74,7 @@ module Gitlab
     end
 
     def notes
-      project.notes.user.search(query).order('updated_at DESC')
+      project.notes.user.search(query, as_user: @current_user).order('updated_at DESC')
     end
 
     def commits
diff --git a/spec/models/note_spec.rb b/spec/models/note_spec.rb
index 5d916f0e6a6..64a5bab7cb1 100644
--- a/spec/models/note_spec.rb
+++ b/spec/models/note_spec.rb
@@ -111,6 +111,25 @@ describe Note, models: true do
     it 'returns notes with matching content regardless of the casing' do
       expect(described_class.search('WOW')).to eq([note])
     end
+
+    context "confidential issues" do
+      let(:user) { create :user }
+      let(:confidential_issue) { create(:issue, :confidential, author: user) }
+      let(:confidential_note) { create :note, note: "Random", noteable: confidential_issue }
+
+      it "returns notes with matching content if user can see the issue" do
+        expect(described_class.search(confidential_note.note, as_user: user)).to eq([confidential_note])
+      end
+
+      it "does not return notes with matching content if user can not see the issue" do
+        user = create :user
+        expect(described_class.search(confidential_note.note, as_user: user)).to be_empty
+      end
+
+      it "does not return notes with matching content for unauthenticated users" do
+        expect(described_class.search(confidential_note.note)).to be_empty
+      end
+    end
   end
 
   describe '.grouped_awards' do