Fix upload URLs in Markdown

Fixes RelativeLinkFilter for users that don't have access
to the project's repository
This commit is contained in:
Heinrich Lee Yu 2019-08-30 17:44:30 +08:00
parent a1a6def114
commit be0f039d9c
4 changed files with 49 additions and 106 deletions

View file

@ -0,0 +1,5 @@
---
title: Fix upload URLs in Markdown for users without access to project repository
merge_request: 32448
author:
type: fixed

View file

@ -19,7 +19,6 @@ module Banzai
def call
return doc if context[:system_note]
return doc unless visible_to_user?
@uri_types = {}
clear_memoization(:linkable_files)
@ -50,7 +49,7 @@ module Banzai
if html_attr.value.start_with?('/uploads/')
process_link_to_upload_attr(html_attr)
elsif linkable_files?
elsif linkable_files? && repo_visible_to_user?
process_link_to_repository_attr(html_attr)
end
end
@ -168,14 +167,8 @@ module Banzai
Gitlab.config.gitlab.relative_url_root.presence || '/'
end
def visible_to_user?
if project
Ability.allowed?(current_user, :download_code, project)
elsif group
Ability.allowed?(current_user, :read_group, group)
else # Objects detached from projects or groups, e.g. Personal Snippets.
true
end
def repo_visible_to_user?
project && Ability.allowed?(current_user, :download_code, project)
end
def ref

View file

@ -65,9 +65,6 @@ describe MarkupHelper do
describe 'inside a group' do
before do
# Ensure the generated reference links aren't redacted
group.add_maintainer(user)
helper.instance_variable_set(:@group, group)
helper.instance_variable_set(:@project, nil)
end
@ -81,9 +78,6 @@ describe MarkupHelper do
let(:project_in_group) { create(:project, group: group) }
before do
# Ensure the generated reference links aren't redacted
project_in_group.add_maintainer(user)
helper.instance_variable_set(:@group, group)
helper.instance_variable_set(:@project, project_in_group)
end

View file

@ -289,37 +289,7 @@ describe Banzai::Filter::RelativeLinkFilter do
let(:relative_path) { "/#{project.full_path}#{upload_path}" }
context 'to a project upload' do
context 'without project repository access' do
let(:project) { create(:project, :repository, repository_access_level: ProjectFeature::PRIVATE) }
it 'does not rebuild relative URL for a link' do
doc = filter(link(upload_path))
expect(doc.at_css('a')['href']).to eq(upload_path)
doc = filter(nested(link(upload_path)))
expect(doc.at_css('a')['href']).to eq(upload_path)
end
it 'does not rebuild relative URL for an image' do
doc = filter(image(upload_path))
expect(doc.at_css('img')['src']).to eq(upload_path)
doc = filter(nested(image(upload_path)))
expect(doc.at_css('img')['src']).to eq(upload_path)
end
context 'with an absolute URL' do
let(:absolute_path) { Gitlab.config.gitlab.url + relative_path }
let(:only_path) { false }
it 'does not rewrite the link' do
doc = filter(link(upload_path))
expect(doc.at_css('a')['href']).to eq(upload_path)
end
end
end
shared_examples 'rewrite project uploads' do
context 'with an absolute URL' do
let(:absolute_path) { Gitlab.config.gitlab.url + relative_path }
let(:only_path) { false }
@ -368,42 +338,23 @@ describe Banzai::Filter::RelativeLinkFilter do
end
end
context 'without project repository access' do
let(:project) { create(:project, :repository, repository_access_level: ProjectFeature::PRIVATE) }
it_behaves_like 'rewrite project uploads'
end
context 'with project repository access' do
it_behaves_like 'rewrite project uploads'
end
end
context 'to a group upload' do
let(:upload_path) { '/uploads/e90decf88d8f96fe9e1389afc2e4a91f/test.jpg' }
let(:upload_link) { link(upload_path) }
let(:upload_link) { link('/uploads/e90decf88d8f96fe9e1389afc2e4a91f/test.jpg') }
let(:group) { create(:group) }
let(:project) { nil }
let(:relative_path) { "/groups/#{group.full_path}/-/uploads/e90decf88d8f96fe9e1389afc2e4a91f/test.jpg" }
context 'without group read access' do
let(:group) { create(:group, :private) }
it 'does not rewrite the link' do
doc = filter(upload_link)
expect(doc.at_css('a')['href']).to eq(upload_path)
end
it 'does not rewrite the link for subgroup' do
group.update!(parent: create(:group))
doc = filter(upload_link)
expect(doc.at_css('a')['href']).to eq(upload_path)
end
context 'with an absolute URL' do
let(:absolute_path) { Gitlab.config.gitlab.url + relative_path }
let(:only_path) { false }
it 'does not rewrite the link' do
doc = filter(upload_link)
expect(doc.at_css('a')['href']).to eq(upload_path)
end
end
end
context 'with an absolute URL' do
let(:absolute_path) { Gitlab.config.gitlab.url + relative_path }
let(:only_path) { false }