From 3799edd78c4806e4fda788247053a7ee10142e64 Mon Sep 17 00:00:00 2001 From: Robert Speicher Date: Tue, 31 May 2016 19:35:13 +0000 Subject: Merge branch 'data_leak' into 'master' Confidential notes data leak Fixes part of https://gitlab.com/gitlab-org/gitlab-ee/issues/575 See merge request !1967 --- 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 0ed726f0696..d00d3fab9a4 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -15,6 +15,7 @@ v 8.8.3 - Fix incorrect links on pipeline page when merge request created from fork. !4376 - Use downcased path to container repository as this is expected path by Docker. !4420 - Fix wiki project clone address error (chujinjin). !4429 + - 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 -- cgit v1.2.3