diff options
| -rw-r--r-- | lib/banzai/filter/cross_project_issuable_information_filter.rb | 40 | ||||
| -rw-r--r-- | lib/banzai/pipeline/post_process_pipeline.rb | 1 | ||||
| -rw-r--r-- | lib/banzai/redactor.rb | 21 | ||||
| -rw-r--r-- | spec/lib/banzai/filter/cross_project_issuable_information_filter_spec.rb | 50 | ||||
| -rw-r--r-- | spec/lib/banzai/filter/redactor_filter_spec.rb | 2 | ||||
| -rw-r--r-- | spec/lib/banzai/redactor_spec.rb | 49 |
6 files changed, 70 insertions, 93 deletions
diff --git a/lib/banzai/filter/cross_project_issuable_information_filter.rb b/lib/banzai/filter/cross_project_issuable_information_filter.rb deleted file mode 100644 index c2c08b4fd6a..00000000000 --- a/lib/banzai/filter/cross_project_issuable_information_filter.rb +++ /dev/null @@ -1,40 +0,0 @@ -module Banzai - module Filter - # HTML filter that removes sensitive information from cross project - # issue references. - # - # The link to the issue or merge request is preserved only the IID is shown, - # but all other info is removed. - class CrossProjectIssuableInformationFilter < HTML::Pipeline::Filter - def call - return doc if can_read_cross_project? - - extractor = Banzai::IssuableExtractor.new(project, current_user) - issuables = extractor.extract([doc]) - - issuables.each do |node, issuable| - next if issuable.project == project - - node['class'] = node['class'].gsub('has-tooltip', '') - node['title'] = nil - end - - doc - end - - private - - def project - context[:project] - end - - def can_read_cross_project? - Ability.allowed?(current_user, :read_cross_project) - end - - def current_user - context[:current_user] - end - end - end -end diff --git a/lib/banzai/pipeline/post_process_pipeline.rb b/lib/banzai/pipeline/post_process_pipeline.rb index 4cf97e2d9d2..dcd52bc03c7 100644 --- a/lib/banzai/pipeline/post_process_pipeline.rb +++ b/lib/banzai/pipeline/post_process_pipeline.rb @@ -6,7 +6,6 @@ module Banzai Filter::RedactorFilter, Filter::RelativeLinkFilter, Filter::IssuableStateFilter, - Filter::CrossProjectIssuableInformationFilter, Filter::AbsoluteLinkFilter ] end diff --git a/lib/banzai/redactor.rb b/lib/banzai/redactor.rb index de3ebe72720..827df7c08ae 100644 --- a/lib/banzai/redactor.rb +++ b/lib/banzai/redactor.rb @@ -19,8 +19,9 @@ module Banzai # # Returns the documents passed as the first argument. def redact(documents) - all_document_nodes = document_nodes(documents) + redact_cross_project_references(documents) unless can_read_cross_project? + all_document_nodes = document_nodes(documents) redact_document_nodes(all_document_nodes) end @@ -51,6 +52,18 @@ module Banzai metadata end + def redact_cross_project_references(documents) + extractor = Banzai::IssuableExtractor.new(project, user) + issuables = extractor.extract(documents) + + issuables.each do |node, issuable| + next if issuable.project == project + + node['class'] = node['class'].gsub('has-tooltip', '') + node['title'] = nil + end + end + # Returns the nodes visible to the current user. # # nodes - The input nodes to check. @@ -78,5 +91,11 @@ module Banzai { document: document, nodes: Querying.css(document, 'a.gfm[data-reference-type]') } end end + + private + + def can_read_cross_project? + Ability.allowed?(user, :read_cross_project) + end end end diff --git a/spec/lib/banzai/filter/cross_project_issuable_information_filter_spec.rb b/spec/lib/banzai/filter/cross_project_issuable_information_filter_spec.rb deleted file mode 100644 index 92b8c526e88..00000000000 --- a/spec/lib/banzai/filter/cross_project_issuable_information_filter_spec.rb +++ /dev/null @@ -1,50 +0,0 @@ -require 'spec_helper' - -describe Banzai::Filter::CrossProjectIssuableInformationFilter do - include ActionView::Helpers::UrlHelper - include FilterSpecHelper - - let(:user) { create(:user) } - let(:project) { create(:project, :public) } - let(:context) { { project: project, current_user: user } } - let(:other_project) { create(:project, :public) } - - def create_link(issuable) - type = issuable.class.name.underscore.downcase - link_to(issuable.to_reference, '', - class: 'gfm has-tooltip', - title: issuable.title, - data: { - reference_type: type, - "#{type}": issuable.id - }) - end - - context 'when the user cannot read cross project' do - before do - allow(Ability).to receive(:allowed?) { false } - end - - it 'skips links to issues within the same project' do - issue = create(:issue, project: project) - link = create_link(issue) - doc = filter(link, context) - - result = doc.css('a').last - - expect(result['class']).to include('has-tooltip') - expect(result['title']).to eq(issue.title) - end - - it 'removes info from a cross project reference' do - issue = create(:issue, project: other_project) - link = create_link(issue) - doc = filter(link, context) - - result = doc.css('a').last - - expect(result['class']).not_to include('has-tooltip') - expect(result['title']).to be_empty - end - end -end diff --git a/spec/lib/banzai/filter/redactor_filter_spec.rb b/spec/lib/banzai/filter/redactor_filter_spec.rb index 5a7858e77f3..9a2e521fdcf 100644 --- a/spec/lib/banzai/filter/redactor_filter_spec.rb +++ b/spec/lib/banzai/filter/redactor_filter_spec.rb @@ -6,7 +6,7 @@ describe Banzai::Filter::RedactorFilter do it 'ignores non-GFM links' do html = %(See <a href="https://google.com/">Google</a>) - doc = filter(html, current_user: double) + doc = filter(html, current_user: build(:user)) expect(doc.css('a').length).to eq 1 end diff --git a/spec/lib/banzai/redactor_spec.rb b/spec/lib/banzai/redactor_spec.rb index 893a10d1b90..1fa89137972 100644 --- a/spec/lib/banzai/redactor_spec.rb +++ b/spec/lib/banzai/redactor_spec.rb @@ -88,6 +88,55 @@ describe Banzai::Redactor do end end + context 'when the user cannot read cross project' do + include ActionView::Helpers::UrlHelper + let(:project) { create(:project) } + let(:other_project) { create(:project, :public) } + + def create_link(issuable) + type = issuable.class.name.underscore.downcase + link_to(issuable.to_reference, '', + class: 'gfm has-tooltip', + title: issuable.title, + data: { + reference_type: type, + "#{type}": issuable.id + }) + end + + before do + project.add_developer(user) + + allow(Ability).to receive(:allowed?).and_call_original + allow(Ability).to receive(:allowed?).with(user, :read_cross_project, :global) { false } + allow(Ability).to receive(:allowed?).with(user, :read_cross_project) { false } + end + + it 'skips links to issues within the same project' do + issue = create(:issue, project: project) + link = create_link(issue) + doc = Nokogiri::HTML.fragment(link) + + redactor.redact([doc]) + result = doc.css('a').last + + expect(result['class']).to include('has-tooltip') + expect(result['title']).to eq(issue.title) + end + + it 'removes info from a cross project reference' do + issue = create(:issue, project: other_project) + link = create_link(issue) + doc = Nokogiri::HTML.fragment(link) + + redactor.redact([doc]) + result = doc.css('a').last + + expect(result['class']).not_to include('has-tooltip') + expect(result['title']).to be_empty + end + end + describe '#redact_nodes' do it 'redacts an Array of nodes' do doc = Nokogiri::HTML.fragment('<a href="foo">foo</a>') |