diff options
-rw-r--r-- | app/models/project_services/hipchat_service.rb | 2 | ||||
-rw-r--r-- | changelogs/unreleased/security-bw-confidential-titles-through-markdown-api.yml | 5 | ||||
-rw-r--r-- | lib/api/markdown.rb | 7 | ||||
-rw-r--r-- | lib/banzai.rb | 7 | ||||
-rw-r--r-- | spec/requests/api/markdown_spec.rb | 46 |
5 files changed, 62 insertions, 5 deletions
diff --git a/app/models/project_services/hipchat_service.rb b/app/models/project_services/hipchat_service.rb index dce878e485f..6fb4edb91ad 100644 --- a/app/models/project_services/hipchat_service.rb +++ b/app/models/project_services/hipchat_service.rb @@ -147,7 +147,7 @@ class HipchatService < Service context.merge!(options) - html = Banzai.post_process(Banzai.render(text, context), context) + html = Banzai.render_and_post_process(text, context) sanitized_html = sanitize(html, tags: HIPCHAT_ALLOWED_TAGS, attributes: %w[href title alt]) sanitized_html.truncate(200, separator: ' ', omission: '...') diff --git a/changelogs/unreleased/security-bw-confidential-titles-through-markdown-api.yml b/changelogs/unreleased/security-bw-confidential-titles-through-markdown-api.yml new file mode 100644 index 00000000000..e0231b7962f --- /dev/null +++ b/changelogs/unreleased/security-bw-confidential-titles-through-markdown-api.yml @@ -0,0 +1,5 @@ +--- +title: Markdown API no longer displays confidential title references unless authorized +merge_request: +author: +type: security diff --git a/lib/api/markdown.rb b/lib/api/markdown.rb index 5d55224c1a7..09a8c34c5c0 100644 --- a/lib/api/markdown.rb +++ b/lib/api/markdown.rb @@ -10,7 +10,8 @@ module API detail "This feature was introduced in GitLab 11.0." end post do - context = { only_path: false } + context = { only_path: false, current_user: current_user } + context[:pipeline] = params[:gfm] ? :full : :plain_markdown if params[:project] project = Project.find_by_full_path(params[:project]) @@ -22,9 +23,7 @@ module API context[:skip_project_check] = true end - context[:pipeline] = params[:gfm] ? :full : :plain_markdown - - { html: Banzai.render(params[:text], context) } + { html: Banzai.render_and_post_process(params[:text], context) } end end end diff --git a/lib/banzai.rb b/lib/banzai.rb index 5df98f66f3b..788f29a6c08 100644 --- a/lib/banzai.rb +++ b/lib/banzai.rb @@ -1,4 +1,11 @@ module Banzai + # if you need to render markdown, then you probably need to post_process as well, + # such as removing references that the current user doesn't have + # permission to make + def self.render_and_post_process(text, context = {}) + post_process(render(text, context), context) + end + def self.render(text, context = {}) Renderer.render(text, context) end diff --git a/spec/requests/api/markdown_spec.rb b/spec/requests/api/markdown_spec.rb index a55796cf343..e369c1435f0 100644 --- a/spec/requests/api/markdown_spec.rb +++ b/spec/requests/api/markdown_spec.rb @@ -106,6 +106,52 @@ describe API::Markdown do .and include("#1</a>") end end + + context 'with a public project and confidential issue' do + let(:public_project) { create(:project, :public) } + let(:confidential_issue) { create(:issue, :confidential, project: public_project, title: 'Confidential title') } + + let(:text) { ":tada: Hello world! :100: #{confidential_issue.to_reference}" } + let(:params) { { text: text, gfm: true, project: public_project.full_path } } + + shared_examples 'user without proper access' do + it 'does not render the title or link' do + expect(response).to have_http_status(201) + expect(json_response["html"]).not_to include('Confidential title') + expect(json_response["html"]).not_to include('<a href=') + expect(json_response["html"]).to include('Hello world!') + .and include('data-name="tada"') + .and include('data-name="100"') + .and include('#1</p>') + end + end + + context 'when not logged in' do + let(:user) { } + + it_behaves_like 'user without proper access' + end + + context 'when logged in as user without access' do + let(:user) { create(:user) } + + it_behaves_like 'user without proper access' + end + + context 'when logged in as author' do + let(:user) { confidential_issue.author } + + it 'renders the title or link' do + expect(response).to have_http_status(201) + expect(json_response["html"]).to include('Confidential title') + expect(json_response["html"]).to include('Hello world!') + .and include('data-name="tada"') + .and include('data-name="100"') + .and include("<a href=\"#{IssuesHelper.url_for_issue(confidential_issue.iid, public_project)}\"") + .and include("#1</a>") + end + end + end end end end |