From 727ec95528c3b928992406e570427728e7186fd4 Mon Sep 17 00:00:00 2001
From: Mark Chao <mchao@gitlab.com>
Date: Mon, 11 Mar 2019 16:52:40 +0800
Subject: Hide related branches when user does not have permission

Guest user of a project should not see branches
---
 app/assets/javascripts/issue.js                    |  4 ++-
 app/controllers/projects/issues_controller.rb      |  1 +
 app/views/projects/issues/show.html.haml           |  5 +--
 changelogs/unreleased/security-56224.yml           |  5 +++
 .../user_creates_branch_and_merge_request_spec.rb  | 36 +++++++++++++++++++++-
 5 files changed, 47 insertions(+), 4 deletions(-)
 create mode 100644 changelogs/unreleased/security-56224.yml

diff --git a/app/assets/javascripts/issue.js b/app/assets/javascripts/issue.js
index 94b78907d9a..b3508f36cf9 100644
--- a/app/assets/javascripts/issue.js
+++ b/app/assets/javascripts/issue.js
@@ -16,7 +16,9 @@ export default class Issue {
     Issue.createMrDropdownWrap = document.querySelector('.create-mr-dropdown-wrap');
 
     Issue.initMergeRequests();
-    Issue.initRelatedBranches();
+    if (document.querySelector('#related-branches')) {
+      Issue.initRelatedBranches();
+    }
 
     this.closeButtons = $('a.btn-close');
     this.reopenButtons = $('a.btn-reopen');
diff --git a/app/controllers/projects/issues_controller.rb b/app/controllers/projects/issues_controller.rb
index b9d02a62fc3..2cb40697b5c 100644
--- a/app/controllers/projects/issues_controller.rb
+++ b/app/controllers/projects/issues_controller.rb
@@ -39,6 +39,7 @@ class Projects::IssuesController < Projects::ApplicationController
   before_action :authorize_create_merge_request_from!, only: [:create_merge_request]
 
   before_action :authorize_import_issues!, only: [:import_csv]
+  before_action :authorize_download_code!, only: [:related_branches]
 
   before_action :set_suggested_issues_feature_flags, only: [:new]
 
diff --git a/app/views/projects/issues/show.html.haml b/app/views/projects/issues/show.html.haml
index 3a674da6e87..819d3c4ec76 100644
--- a/app/views/projects/issues/show.html.haml
+++ b/app/views/projects/issues/show.html.haml
@@ -80,8 +80,9 @@
     #merge-requests{ data: { url: referenced_merge_requests_project_issue_path(@project, @issue) } }
       // This element is filled in using JavaScript.
 
-    #related-branches{ data: { url: related_branches_project_issue_path(@project, @issue) } }
-      // This element is filled in using JavaScript.
+    - if can?(current_user, :download_code, @project)
+      #related-branches{ data: { url: related_branches_project_issue_path(@project, @issue) } }
+        // This element is filled in using JavaScript.
 
   .content-block.emoji-block.emoji-block-sticky
     .row
diff --git a/changelogs/unreleased/security-56224.yml b/changelogs/unreleased/security-56224.yml
new file mode 100644
index 00000000000..a4e274e6ca5
--- /dev/null
+++ b/changelogs/unreleased/security-56224.yml
@@ -0,0 +1,5 @@
+---
+title: Hide "related branches" when user does not have permission
+merge_request:
+author:
+type: security
diff --git a/spec/features/issues/user_creates_branch_and_merge_request_spec.rb b/spec/features/issues/user_creates_branch_and_merge_request_spec.rb
index 693ad89069c..0a006011c89 100644
--- a/spec/features/issues/user_creates_branch_and_merge_request_spec.rb
+++ b/spec/features/issues/user_creates_branch_and_merge_request_spec.rb
@@ -1,6 +1,7 @@
 require 'rails_helper'
 
 describe 'User creates branch and merge request on issue page', :js do
+  let(:membership_level) { :developer }
   let(:user) { create(:user) }
   let!(:project) { create(:project, :repository) }
   let(:issue) { create(:issue, project: project, title: 'Cherry-Coloured Funk') }
@@ -17,7 +18,7 @@ describe 'User creates branch and merge request on issue page', :js do
 
   context 'when signed in' do
     before do
-      project.add_developer(user)
+      project.add_user(user, membership_level)
 
       sign_in(user)
     end
@@ -167,6 +168,39 @@ describe 'User creates branch and merge request on issue page', :js do
         expect(page).not_to have_css('.create-mr-dropdown-wrap')
       end
     end
+
+    context 'when related branch exists' do
+      let!(:project) { create(:project, :repository, :private) }
+      let(:branch_name) { "#{issue.iid}-foo" }
+
+      before do
+        project.repository.create_branch(branch_name, 'master')
+
+        visit project_issue_path(project, issue)
+      end
+
+      context 'when user is developer' do
+        it 'shows related branches' do
+          expect(page).to have_css('#related-branches')
+
+          wait_for_requests
+
+          expect(page).to have_content(branch_name)
+        end
+      end
+
+      context 'when user is guest' do
+        let(:membership_level) { :guest }
+
+        it 'does not show related branches' do
+          expect(page).not_to have_css('#related-branches')
+
+          wait_for_requests
+
+          expect(page).not_to have_content(branch_name)
+        end
+      end
+    end
   end
 
   private
-- 
cgit v1.2.1