summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorBrett Walker <bwalker@gitlab.com>2018-09-19 14:58:43 -0500
committerBrett Walker <bwalker@gitlab.com>2018-09-29 11:29:32 -0500
commit829c9c65f9b730b3ecad7d3ba222e3dcd6489b85 (patch)
tree2a9ce8bd65b2bc8291b7c5e63f746c39034ae288
parente5d3a75aac4f0bb287699b21f3a56b8bfe499665 (diff)
downloadgitlab-ce-829c9c65f9b730b3ecad7d3ba222e3dcd6489b85.tar.gz
post_process markdown redered by API
-rw-r--r--app/models/project_services/hipchat_service.rb2
-rw-r--r--changelogs/unreleased/security-bw-confidential-titles-through-markdown-api.yml5
-rw-r--r--lib/api/markdown.rb7
-rw-r--r--lib/banzai.rb7
-rw-r--r--spec/requests/api/markdown_spec.rb46
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 66012f0da99..a69b7b4c4b6 100644
--- a/app/models/project_services/hipchat_service.rb
+++ b/app/models/project_services/hipchat_service.rb
@@ -149,7 +149,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