kotovalexarian-likes-gitlab/gitlab-org--gitlab-foss
1
0
Fork 0
Code Releases Activity

Merge branch '66803-fix-uploads-relative-link-filter' into 'master'

Browse source 
Fix permissions check in `RelativeLinkFilter`

See merge request gitlab-org/gitlab-ce!32448
This commit is contained in:
Grzegorz Bizon 2019-09-02 12:05:33 +00:00
commit 60fd42172f
4 changed files with 49 additions and 106 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

5
changelogs/unreleased/66803-fix-uploads-relative-link-filter.yml Normal file
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

13
lib/banzai/filter/relative_link_filter.rb
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

6
spec/helpers/markup_helper_spec.rb
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

131
spec/lib/banzai/filter/relative_link_filter_spec.rb
View file

@ -289,121 +289,72 @@ 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
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 }
it 'does not rewrite the link' do
it 'rewrites the link correctly' do
doc = filter(link(upload_path))
expect(doc.at_css('a')['href']).to eq(upload_path)
expect(doc.at_css('a')['href']).to eq(absolute_path)
end
end
end
context 'with an absolute URL' do
let(:absolute_path) { Gitlab.config.gitlab.url + relative_path }
let(:only_path) { false }
it 'rewrites the link correctly' do
it 'rebuilds relative URL for a link' do
doc = filter(link(upload_path))
expect(doc.at_css('a')['href']).to eq(relative_path)
expect(doc.at_css('a')['href']).to eq(absolute_path)
doc = filter(nested(link(upload_path)))
expect(doc.at_css('a')['href']).to eq(relative_path)
end
it 'rebuilds relative URL for an image' do
doc = filter(image(upload_path))
expect(doc.at_css('img')['src']).to eq(relative_path)
doc = filter(nested(image(upload_path)))
expect(doc.at_css('img')['src']).to eq(relative_path)
end
it 'does not modify absolute URL' do
doc = filter(link('http://example.com'))
expect(doc.at_css('a')['href']).to eq 'http://example.com'
end
it 'supports unescaped Unicode filenames' do
path = '/uploads/한글.png'
doc = filter(link(path))
expect(doc.at_css('a')['href']).to eq("/#{project.full_path}/uploads/%ED%95%9C%EA%B8%80.png")
end
it 'supports escaped Unicode filenames' do
path = '/uploads/한글.png'
escaped = Addressable::URI.escape(path)
doc = filter(image(escaped))
expect(doc.at_css('img')['src']).to eq("/#{project.full_path}/uploads/%ED%95%9C%EA%B8%80.png")
end
end
it 'rebuilds relative URL for a link' do
doc = filter(link(upload_path))
expect(doc.at_css('a')['href']).to eq(relative_path)
context 'without project repository access' do
let(:project) { create(:project, :repository, repository_access_level: ProjectFeature::PRIVATE) }
doc = filter(nested(link(upload_path)))
expect(doc.at_css('a')['href']).to eq(relative_path)
it_behaves_like 'rewrite project uploads'
end
it 'rebuilds relative URL for an image' do
doc = filter(image(upload_path))
expect(doc.at_css('img')['src']).to eq(relative_path)
doc = filter(nested(image(upload_path)))
expect(doc.at_css('img')['src']).to eq(relative_path)
end
it 'does not modify absolute URL' do
doc = filter(link('http://example.com'))
expect(doc.at_css('a')['href']).to eq 'http://example.com'
end
it 'supports unescaped Unicode filenames' do
path = '/uploads/한글.png'
doc = filter(link(path))
expect(doc.at_css('a')['href']).to eq("/#{project.full_path}/uploads/%ED%95%9C%EA%B8%80.png")
end
it 'supports escaped Unicode filenames' do
path = '/uploads/한글.png'
escaped = Addressable::URI.escape(path)
doc = filter(image(escaped))
expect(doc.at_css('img')['src']).to eq("/#{project.full_path}/uploads/%ED%95%9C%EA%B8%80.png")
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 }