Commit 592ff4e9 authored by Bob Van Landuyt's avatar Bob Van Landuyt

Merge branch 'master' of dev.gitlab.org:gitlab/gitlab-ee

parents 88145e75 531b60ff
Please view this file on the master branch, on stable branches it's out of date. Please view this file on the master branch, on stable branches it's out of date.
## 11.3.4 (2018-10-05)
### Security (1 change)
- Properly filter private references from system notes.
## 11.3.3 (2018-10-04) ## 11.3.3 (2018-10-04)
- No changes. - No changes.
...@@ -95,6 +102,13 @@ Please view this file on the master branch, on stable branches it's out of date. ...@@ -95,6 +102,13 @@ Please view this file on the master branch, on stable branches it's out of date.
- Remove differences between CE and EE settings panel component. - Remove differences between CE and EE settings panel component.
## 11.2.5 (2018-10-05)
### Security (1 change)
- Properly filter private references from system notes.
## 11.2.4 (2018-09-26) ## 11.2.4 (2018-09-26)
### Security (2 changes) ### Security (2 changes)
......
...@@ -2,6 +2,15 @@ ...@@ -2,6 +2,15 @@
documentation](doc/development/changelog.md) for instructions on adding your own documentation](doc/development/changelog.md) for instructions on adding your own
entry. entry.
## 11.3.4 (2018-10-05)
### Security (3 changes)
- Filter user sensitive data from discussions JSON. !2537
- Properly filter private references from system notes.
- Markdown API no longer displays confidential title references unless authorized.
## 11.3.3 (2018-10-04) ## 11.3.3 (2018-10-04)
- No changes. - No changes.
...@@ -279,6 +288,15 @@ entry. ...@@ -279,6 +288,15 @@ entry.
- Creates Vue component for artifacts block on job page. - Creates Vue component for artifacts block on job page.
## 11.2.5 (2018-10-05)
### Security (3 changes)
- Filter user sensitive data from discussions JSON. !2538
- Properly filter private references from system notes.
- Markdown API no longer displays confidential title references unless authorized.
## 11.2.4 (2018-09-26) ## 11.2.4 (2018-09-26)
### Security (6 changes) ### Security (6 changes)
......
...@@ -45,10 +45,12 @@ class Note < ActiveRecord::Base ...@@ -45,10 +45,12 @@ class Note < ActiveRecord::Base
# Banzai::ObjectRenderer. # Banzai::ObjectRenderer.
attr_accessor :redacted_note_html attr_accessor :redacted_note_html
# An Array containing the number of visible references as generated by # Number of user visible references as generated by Banzai::ObjectRenderer
# Banzai::ObjectRenderer
attr_accessor :user_visible_reference_count attr_accessor :user_visible_reference_count
# Total of all references as generated by Banzai::ObjectRenderer
attr_accessor :total_reference_count
# Attribute used to store the attributes that have been changed by quick actions. # Attribute used to store the attributes that have been changed by quick actions.
attr_accessor :commands_changes attr_accessor :commands_changes
...@@ -296,15 +298,7 @@ class Note < ActiveRecord::Base ...@@ -296,15 +298,7 @@ class Note < ActiveRecord::Base
end end
def cross_reference_not_visible_for?(user) def cross_reference_not_visible_for?(user)
cross_reference? && !has_referenced_mentionables?(user) cross_reference? && !all_referenced_mentionables_allowed?(user)
end
def has_referenced_mentionables?(user)
if user_visible_reference_count.present?
user_visible_reference_count > 0
else
referenced_mentionables(user).any?
end
end end
def award_emoji? def award_emoji?
...@@ -474,9 +468,18 @@ class Note < ActiveRecord::Base ...@@ -474,9 +468,18 @@ class Note < ActiveRecord::Base
self.discussion_id ||= discussion_class.discussion_id(self) self.discussion_id ||= discussion_class.discussion_id(self)
end 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? def force_cross_reference_regex_check?
return unless system? 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
end end
...@@ -12,6 +12,7 @@ class SystemNoteMetadata < ActiveRecord::Base ...@@ -12,6 +12,7 @@ class SystemNoteMetadata < ActiveRecord::Base
commit cross_reference commit cross_reference
close duplicate close duplicate
relate unrelate relate unrelate
moved
].freeze ].freeze
ICON_TYPES = %w[ ICON_TYPES = %w[
...@@ -29,4 +30,8 @@ class SystemNoteMetadata < ActiveRecord::Base ...@@ -29,4 +30,8 @@ class SystemNoteMetadata < ActiveRecord::Base
def icon_types def icon_types
ICON_TYPES ICON_TYPES
end end
def cross_reference_types
TYPES_WITH_CROSS_REFERENCES
end
end end
...@@ -8,9 +8,20 @@ module EE ...@@ -8,9 +8,20 @@ module EE
epic_issue_moved issue_changed_epic epic_date_changed epic_issue_moved issue_changed_epic epic_date_changed
].freeze ].freeze
EE_TYPES_WITH_CROSS_REFERENCES = %w[
relate unrelate
epic_issue_added issue_added_to_epic epic_issue_removed issue_removed_from_epic
epic_issue_moved issue_changed_epic
].freeze
override :icon_types override :icon_types
def icon_types def icon_types
@icon_types ||= (super + EE_ICON_TYPES).freeze @icon_types ||= (super + EE_ICON_TYPES).freeze
end end
override :cross_reference_types
def cross_reference_types
@cross_reference_types ||= (super + EE_TYPES_WITH_CROSS_REFERENCES).freeze
end
end end
end end
---
title: Properly filter private references from system notes
merge_request:
author:
type: security
...@@ -38,6 +38,7 @@ module Banzai ...@@ -38,6 +38,7 @@ module Banzai
redacted_data = redacted[index] redacted_data = redacted[index]
object.__send__("redacted_#{attribute}_html=", redacted_data[:document].to_html(save_options).html_safe) # rubocop:disable GitlabSecurity/PublicSend 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.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
end end
......
...@@ -37,7 +37,13 @@ module Banzai ...@@ -37,7 +37,13 @@ module Banzai
all_document_nodes.each do |entry| all_document_nodes.each do |entry|
nodes_for_document = entry[:nodes] 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 metadata << doc_data
nodes_for_document.each do |node| nodes_for_document.each do |node|
......
...@@ -231,33 +231,60 @@ describe Note do ...@@ -231,33 +231,60 @@ describe Note do
let(:ext_proj) { create(:project, :public) } let(:ext_proj) { create(:project, :public) }
let(:ext_issue) { create(:issue, project: ext_proj) } let(:ext_issue) { create(:issue, project: ext_proj) }
let(:note) do shared_examples "checks references" do
create :note, it "returns true" do
noteable: ext_issue, project: ext_proj, expect(note.cross_reference_not_visible_for?(ext_issue.author)).to be_truthy
note: "mentioned in issue #{private_issue.to_reference(ext_proj)}", end
system: true
end
it "returns true" do it "returns false" do
expect(note.cross_reference_not_visible_for?(ext_issue.author)).to be_truthy expect(note.cross_reference_not_visible_for?(private_user)).to be_falsy
end end
it "returns false" do it "returns false if user visible reference count set" do
expect(note.cross_reference_not_visible_for?(private_user)).to be_falsy 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 end
it "returns false if user visible reference count set" do context "when there is one reference in note" do
note.user_visible_reference_count = 1 let(:note) do
create :note,
noteable: ext_issue, project: ext_proj,
note: "mentioned in issue #{private_issue.to_reference(ext_proj)}",
system: true
end
expect(note).not_to receive(:reference_mentionables) it_behaves_like "checks references"
expect(note.cross_reference_not_visible_for?(ext_issue.author)).to be_falsy
end end
it "returns true if ref count is 0" do context "when there are two references in note" do
note.user_visible_reference_count = 0 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_behaves_like "checks references"
expect(note).not_to receive(:reference_mentionables) it "returns true if user visible reference count set and there is a private reference" do
expect(note.cross_reference_not_visible_for?(ext_issue.author)).to be_truthy note.user_visible_reference_count = 1
note.total_reference_count = 2
expect(note).not_to receive(:reference_mentionables)
expect(note.cross_reference_not_visible_for?(ext_issue.author)).to be_truthy
end
end end
end end
...@@ -269,7 +296,7 @@ describe Note do ...@@ -269,7 +296,7 @@ describe Note do
end end
context 'when the note might contain cross references' do 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(:note) { create(:note, :system) }
let!(:metadata) { create(:system_note_metadata, note: note, action: type) } let!(:metadata) { create(:system_note_metadata, note: note, action: type) }
......
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