Commit 613bcdc6 authored by Robert Speicher's avatar Robert Speicher

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
parents ef4fedc1 9154586c
...@@ -21,6 +21,7 @@ v 8.9.0 (unreleased) ...@@ -21,6 +21,7 @@ v 8.9.0 (unreleased)
v 8.8.3 v 8.8.3
- Fix gitlab importer failing to import new projects due to missing credentials - Fix gitlab importer failing to import new projects due to missing credentials
- Fix import URL migration not rescuing with the correct Error - 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 v 8.8.2
- Added remove due date button. !4209 - Added remove due date button. !4209
......
...@@ -85,13 +85,29 @@ class Note < ActiveRecord::Base ...@@ -85,13 +85,29 @@ 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}%"
found_notes = joins('LEFT JOIN issues ON issues.id = noteable_id').
where(table[:note].matches(pattern)) 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
......
...@@ -139,6 +139,25 @@ describe Note, models: true do ...@@ -139,6 +139,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