Merge branch 'security-bw-confidential-titles-through-markdown-api' into 'master'

[master] Confidential issue/private snippet titles can be read by unauthenticated user through GFM markdown API

Closes #2706

See merge request gitlab/gitlabhq!2507

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: '...')

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

lib/api/markdown.rb

@@ -12,7 +12,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])
@@ -24,9 +25,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

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

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