Merge branch 'security-fix-leaking-private-project-namespace' into 'master'
[master] Fix leaking private project namespace Closes #2708 See merge request gitlab/gitlabhq!2529
This commit is contained in:
commit
d26bf613b4
|
@ -38,10 +38,12 @@ class Note < ActiveRecord::Base
|
|||
alias_attribute :last_edited_at, :updated_at
|
||||
alias_attribute :last_edited_by, :updated_by
|
||||
|
||||
# Attribute containing rendered and redacted Markdown as generated by
|
||||
# Banzai::ObjectRenderer.
|
||||
# Number of user visible references as generated by Banzai::ObjectRenderer
|
||||
attr_accessor :redacted_note_html
|
||||
|
||||
# Total of all references as generated by Banzai::ObjectRenderer
|
||||
attr_accessor :total_reference_count
|
||||
|
||||
# An Array containing the number of visible references as generated by
|
||||
# Banzai::ObjectRenderer
|
||||
attr_accessor :user_visible_reference_count
|
||||
|
@ -288,15 +290,7 @@ class Note < ActiveRecord::Base
|
|||
end
|
||||
|
||||
def cross_reference_not_visible_for?(user)
|
||||
cross_reference? && !has_referenced_mentionables?(user)
|
||||
end
|
||||
|
||||
def has_referenced_mentionables?(user)
|
||||
if user_visible_reference_count.present?
|
||||
user_visible_reference_count > 0
|
||||
else
|
||||
referenced_mentionables(user).any?
|
||||
end
|
||||
cross_reference? && !all_referenced_mentionables_allowed?(user)
|
||||
end
|
||||
|
||||
def award_emoji?
|
||||
|
@ -466,9 +460,18 @@ class Note < ActiveRecord::Base
|
|||
self.discussion_id ||= discussion_class.discussion_id(self)
|
||||
end
|
||||
|
||||
def all_referenced_mentionables_allowed?(user)
|
||||
if user_visible_reference_count.present? && total_reference_count.present?
|
||||
# if they are not equal, then there are private/confidential references as well
|
||||
user_visible_reference_count > 0 && user_visible_reference_count == total_reference_count
|
||||
else
|
||||
referenced_mentionables(user).any?
|
||||
end
|
||||
end
|
||||
|
||||
def force_cross_reference_regex_check?
|
||||
return unless system?
|
||||
|
||||
SystemNoteMetadata::TYPES_WITH_CROSS_REFERENCES.include?(system_note_metadata&.action)
|
||||
system_note_metadata&.cross_reference_types&.include?(system_note_metadata&.action)
|
||||
end
|
||||
end
|
||||
|
|
|
@ -9,6 +9,7 @@ class SystemNoteMetadata < ActiveRecord::Base
|
|||
TYPES_WITH_CROSS_REFERENCES = %w[
|
||||
commit cross_reference
|
||||
close duplicate
|
||||
moved
|
||||
].freeze
|
||||
|
||||
ICON_TYPES = %w[
|
||||
|
@ -26,4 +27,8 @@ class SystemNoteMetadata < ActiveRecord::Base
|
|||
def icon_types
|
||||
ICON_TYPES
|
||||
end
|
||||
|
||||
def cross_reference_types
|
||||
TYPES_WITH_CROSS_REFERENCES
|
||||
end
|
||||
end
|
||||
|
|
|
@ -0,0 +1,5 @@
|
|||
---
|
||||
title: Properly filter private references from system notes
|
||||
merge_request:
|
||||
author:
|
||||
type: security
|
|
@ -38,6 +38,7 @@ module Banzai
|
|||
redacted_data = redacted[index]
|
||||
object.__send__("redacted_#{attribute}_html=", redacted_data[:document].to_html(save_options).html_safe) # rubocop:disable GitlabSecurity/PublicSend
|
||||
object.user_visible_reference_count = redacted_data[:visible_reference_count] if object.respond_to?(:user_visible_reference_count)
|
||||
object.total_reference_count = redacted_data[:total_reference_count] if object.respond_to?(:total_reference_count)
|
||||
end
|
||||
end
|
||||
|
||||
|
|
|
@ -37,7 +37,13 @@ module Banzai
|
|||
|
||||
all_document_nodes.each do |entry|
|
||||
nodes_for_document = entry[:nodes]
|
||||
doc_data = { document: entry[:document], visible_reference_count: nodes_for_document.count }
|
||||
|
||||
doc_data = {
|
||||
document: entry[:document],
|
||||
total_reference_count: nodes_for_document.count,
|
||||
visible_reference_count: nodes_for_document.count
|
||||
}
|
||||
|
||||
metadata << doc_data
|
||||
|
||||
nodes_for_document.each do |node|
|
||||
|
|
|
@ -231,33 +231,60 @@ describe Note do
|
|||
let(:ext_proj) { create(:project, :public) }
|
||||
let(:ext_issue) { create(:issue, project: ext_proj) }
|
||||
|
||||
let(:note) do
|
||||
create :note,
|
||||
noteable: ext_issue, project: ext_proj,
|
||||
note: "mentioned in issue #{private_issue.to_reference(ext_proj)}",
|
||||
system: true
|
||||
shared_examples "checks references" do
|
||||
it "returns true" do
|
||||
expect(note.cross_reference_not_visible_for?(ext_issue.author)).to be_truthy
|
||||
end
|
||||
|
||||
it "returns false" do
|
||||
expect(note.cross_reference_not_visible_for?(private_user)).to be_falsy
|
||||
end
|
||||
|
||||
it "returns false if user visible reference count set" do
|
||||
note.user_visible_reference_count = 1
|
||||
note.total_reference_count = 1
|
||||
|
||||
expect(note).not_to receive(:reference_mentionables)
|
||||
expect(note.cross_reference_not_visible_for?(ext_issue.author)).to be_falsy
|
||||
end
|
||||
|
||||
it "returns true if ref count is 0" do
|
||||
note.user_visible_reference_count = 0
|
||||
|
||||
expect(note).not_to receive(:reference_mentionables)
|
||||
expect(note.cross_reference_not_visible_for?(ext_issue.author)).to be_truthy
|
||||
end
|
||||
end
|
||||
|
||||
it "returns true" do
|
||||
expect(note.cross_reference_not_visible_for?(ext_issue.author)).to be_truthy
|
||||
context "when there is one reference in note" do
|
||||
let(:note) do
|
||||
create :note,
|
||||
noteable: ext_issue, project: ext_proj,
|
||||
note: "mentioned in issue #{private_issue.to_reference(ext_proj)}",
|
||||
system: true
|
||||
end
|
||||
|
||||
it_behaves_like "checks references"
|
||||
end
|
||||
|
||||
it "returns false" do
|
||||
expect(note.cross_reference_not_visible_for?(private_user)).to be_falsy
|
||||
end
|
||||
context "when there are two references in note" do
|
||||
let(:note) do
|
||||
create :note,
|
||||
noteable: ext_issue, project: ext_proj,
|
||||
note: "mentioned in issue #{private_issue.to_reference(ext_proj)} and " \
|
||||
"public issue #{ext_issue.to_reference(ext_proj)}",
|
||||
system: true
|
||||
end
|
||||
|
||||
it "returns false if user visible reference count set" do
|
||||
note.user_visible_reference_count = 1
|
||||
it_behaves_like "checks references"
|
||||
|
||||
expect(note).not_to receive(:reference_mentionables)
|
||||
expect(note.cross_reference_not_visible_for?(ext_issue.author)).to be_falsy
|
||||
end
|
||||
it "returns true if user visible reference count set and there is a private reference" do
|
||||
note.user_visible_reference_count = 1
|
||||
note.total_reference_count = 2
|
||||
|
||||
it "returns true if ref count is 0" do
|
||||
note.user_visible_reference_count = 0
|
||||
|
||||
expect(note).not_to receive(:reference_mentionables)
|
||||
expect(note.cross_reference_not_visible_for?(ext_issue.author)).to be_truthy
|
||||
expect(note).not_to receive(:reference_mentionables)
|
||||
expect(note.cross_reference_not_visible_for?(ext_issue.author)).to be_truthy
|
||||
end
|
||||
end
|
||||
end
|
||||
|
||||
|
@ -269,7 +296,7 @@ describe Note do
|
|||
end
|
||||
|
||||
context 'when the note might contain cross references' do
|
||||
SystemNoteMetadata::TYPES_WITH_CROSS_REFERENCES.each do |type|
|
||||
SystemNoteMetadata.new.cross_reference_types.each do |type|
|
||||
let(:note) { create(:note, :system) }
|
||||
let!(:metadata) { create(:system_note_metadata, note: note, action: type) }
|
||||
|
||||
|
|
Loading…
Reference in New Issue