From 879ca8ab319a60f114e99a57848467d252fc65c7 Mon Sep 17 00:00:00 2001
From: Aakriti Gupta <agupta@gitlab.com>
Date: Wed, 6 Nov 2019 17:07:11 +0100
Subject: [PATCH] Prevent guests from seeing commits for cycle analytics - if
 the user has access level lower than REPORTER, don't include commit count in
 summary

---
 ...y-ag-cycle-analytics-guest-permissions.yml |  5 +++++
 lib/gitlab/cycle_analytics/stage_summary.rb   | 22 ++++++++++++++++---
 spec/features/cycle_analytics_spec.rb         | 10 ++++++++-
 .../cycle_analytics/stage_summary_spec.rb     | 21 ++++++++++++++++++
 4 files changed, 54 insertions(+), 4 deletions(-)
 create mode 100644 changelogs/unreleased/security-ag-cycle-analytics-guest-permissions.yml

diff --git a/changelogs/unreleased/security-ag-cycle-analytics-guest-permissions.yml b/changelogs/unreleased/security-ag-cycle-analytics-guest-permissions.yml
new file mode 100644
index 000000000000..c7a3b8923cdf
--- /dev/null
+++ b/changelogs/unreleased/security-ag-cycle-analytics-guest-permissions.yml
@@ -0,0 +1,5 @@
+---
+title: Hide commit counts from guest users in Cycle Analytics.
+merge_request:
+author:
+type: security
diff --git a/lib/gitlab/cycle_analytics/stage_summary.rb b/lib/gitlab/cycle_analytics/stage_summary.rb
index 5198dd5b4eb6..c0ee65a106d1 100644
--- a/lib/gitlab/cycle_analytics/stage_summary.rb
+++ b/lib/gitlab/cycle_analytics/stage_summary.rb
@@ -10,13 +10,29 @@ module Gitlab
       end
 
       def data
-        [serialize(Summary::Issue.new(project: @project, from: @from, current_user: @current_user)),
-         serialize(Summary::Commit.new(project: @project, from: @from)),
-         serialize(Summary::Deploy.new(project: @project, from: @from))]
+        summary = [issue_stats]
+        summary << commit_stats if user_has_sufficient_access?
+        summary << deploy_stats
       end
 
       private
 
+      def issue_stats
+        serialize(Summary::Issue.new(project: @project, from: @from, current_user: @current_user))
+      end
+
+      def commit_stats
+        serialize(Summary::Commit.new(project: @project, from: @from))
+      end
+
+      def deploy_stats
+        serialize(Summary::Deploy.new(project: @project, from: @from))
+      end
+
+      def user_has_sufficient_access?
+        @project.team.member?(@current_user, Gitlab::Access::REPORTER)
+      end
+
       def serialize(summary_object)
         AnalyticsSummarySerializer.new.represent(summary_object)
       end
diff --git a/spec/features/cycle_analytics_spec.rb b/spec/features/cycle_analytics_spec.rb
index 07f0864fb3ba..df8d5124f36e 100644
--- a/spec/features/cycle_analytics_spec.rb
+++ b/spec/features/cycle_analytics_spec.rb
@@ -108,6 +108,10 @@ describe 'Cycle Analytics', :js do
       wait_for_requests
     end
 
+    it 'does not show the commit stats' do
+      expect(page).to have_no_selector(:xpath, commits_counter_selector)
+    end
+
     it 'needs permissions to see restricted stages' do
       expect(find('.stage-events')).to have_content(issue.title)
 
@@ -123,8 +127,12 @@ describe 'Cycle Analytics', :js do
     find(:xpath, "//p[contains(text(),'New Issue')]/preceding-sibling::h3")
   end
 
+  def commits_counter_selector
+    "//p[contains(text(),'Commits')]/preceding-sibling::h3"
+  end
+
   def commits_counter
-    find(:xpath, "//p[contains(text(),'Commits')]/preceding-sibling::h3")
+    find(:xpath, commits_counter_selector)
   end
 
   def deploys_counter
diff --git a/spec/lib/gitlab/cycle_analytics/stage_summary_spec.rb b/spec/lib/gitlab/cycle_analytics/stage_summary_spec.rb
index 778c2f479b56..35bfeae5ea24 100644
--- a/spec/lib/gitlab/cycle_analytics/stage_summary_spec.rb
+++ b/spec/lib/gitlab/cycle_analytics/stage_summary_spec.rb
@@ -8,6 +8,10 @@ describe Gitlab::CycleAnalytics::StageSummary do
   let(:user) { create(:user, :admin) }
   subject { described_class.new(project, from: Time.now, current_user: user).data }
 
+  before do
+    project.add_maintainer(user)
+  end
+
   describe "#new_issues" do
     it "finds the number of issues created after the 'from date'" do
       Timecop.freeze(5.days.ago) { create(:issue, project: project) }
@@ -42,6 +46,23 @@ describe Gitlab::CycleAnalytics::StageSummary do
 
       expect(subject.second[:value]).to eq(100)
     end
+
+    context 'when a guest user is signed in' do
+      let(:guest_user) { create(:user) }
+
+      before do
+        project.add_guest(guest_user)
+      end
+
+      it 'does not include commit stats' do
+        data = described_class.new(project, from: from, current_user: guest_user).data
+        expect(includes_commits?(data)).to be_falsy
+      end
+
+      def includes_commits?(data)
+        data.any? { |h| h["title"] == 'Commits' }
+      end
+    end
   end
 
   describe "#deploys" do
-- 
2.22.0