Commit 3799edd7 authored by Robert Speicher's avatar Robert Speicher Committed by Yorick Peterse

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
parent e30c651b
...@@ -15,6 +15,7 @@ v 8.8.3 ...@@ -15,6 +15,7 @@ v 8.8.3
- Fix incorrect links on pipeline page when merge request created from fork. !4376 - 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 - Use downcased path to container repository as this is expected path by Docker. !4420
- Fix wiki project clone address error (chujinjin). !4429 - 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 v 8.8.2
- Added remove due date button. !4209 - Added remove due date button. !4209
......
...@@ -77,14 +77,30 @@ class Note < ActiveRecord::Base ...@@ -77,14 +77,30 @@ class Note < ActiveRecord::Base
# #
# This method uses ILIKE on PostgreSQL and LIKE on MySQL. # 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. # Returns an ActiveRecord::Relation.
def search(query) def search(query, as_user: nil)
table = arel_table table = arel_table
pattern = "%#{query}%" 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 end
def grouped_awards def grouped_awards
......
...@@ -74,7 +74,7 @@ module Gitlab ...@@ -74,7 +74,7 @@ module Gitlab
end end
def notes 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 end
def commits def commits
......
...@@ -111,6 +111,25 @@ describe Note, models: true do ...@@ -111,6 +111,25 @@ describe Note, models: true do
it 'returns notes with matching content regardless of the casing' do it 'returns notes with matching content regardless of the casing' do
expect(described_class.search('WOW')).to eq([note]) expect(described_class.search('WOW')).to eq([note])
end 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 end
describe '.grouped_awards' do describe '.grouped_awards' do
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment