Security fix for CI/CD analytics visibility

Merge branch 'security-fix-ci-cd-analytics-visibility-14-9' into '14-9-stable-ee'

See merge request gitlab-org/security/gitlab!2304

Changelog: security

app/policies/project_policy.rb

@@ -600,13 +600,14 @@ class ProjectPolicy < BasePolicy
     enable :read_cycle_analytics
     enable :read_pages_content
     enable :read_analytics
-    enable :read_ci_cd_analytics
     enable :read_insights
     # NOTE: may be overridden by IssuePolicy
     enable :read_issue
   end
+  rule { can?(:public_access) & public_builds }.enable :read_ci_cd_analytics
+
   rule { public_builds }.policy do
     enable :read_build
   end

spec/graphql/resolvers/project_pipeline_statistics_resolver_spec.rb

@@ -11,7 +11,7 @@ RSpec.describe Resolvers::ProjectPipelineStatisticsResolver do
   let(:current_user) { reporter }
-  before_all do
+  before do
     project.add_guest(guest)
     project.add_reporter(reporter)
   end
@@ -20,13 +20,8 @@ RSpec.describe Resolvers::ProjectPipelineStatisticsResolver do
     expect(described_class).to have_nullable_graphql_type(::Types::Ci::AnalyticsType)
   end
-  def resolve_statistics(project, args)
-    ctx = { current_user: current_user }
-    resolve(described_class, obj: project, args: args, ctx: ctx)
-  end
-
-  describe '#resolve' do
-    it 'returns the pipelines statistics for a given project' do
+  shared_examples 'returns the pipelines statistics for a given project' do
+    it do
       result = resolve_statistics(project, {})
       expect(result.keys).to contain_exactly(
         :week_pipelines_labels,
@@ -42,14 +37,67 @@ RSpec.describe Resolvers::ProjectPipelineStatisticsResolver do
         :pipeline_times_values
       )
     end
+  end
+
+  shared_examples 'it returns nils' do
+    it do
+      result = resolve_statistics(project, {})
+
+      expect(result).to be_nil
+    end
+  end
+
+  def resolve_statistics(project, args)
+    ctx = { current_user: current_user }
+    resolve(described_class, obj: project, args: args, ctx: ctx)
+  end
+
+  describe '#resolve' do
+    it_behaves_like 'returns the pipelines statistics for a given project'
     context 'when the user does not have access to the CI/CD analytics data' do
       let(:current_user) { guest }
-      it 'returns nil' do
-        result = resolve_statistics(project, {})
-        expect(result).to be_nil
+      it_behaves_like 'it returns nils'
+    end
+
+    context 'when the project is public' do
+      let_it_be(:project) { create(:project, :public) }
+
+      context 'public pipelines are disabled' do
+        before do
+          project.update!(public_builds: false)
+        end
+
+        context 'user is not a member' do
+          let(:current_user) { create(:user) }
+
+          it_behaves_like 'it returns nils'
+        end
+
+        context 'user is a guest' do
+          let(:current_user) { guest }
+
+          it_behaves_like 'it returns nils'
+        end
+
+        context 'user is a reporter or above' do
+          let(:current_user) { reporter }
+
+          it_behaves_like 'returns the pipelines statistics for a given project'
+        end
+      end
+
+      context 'public pipelines are enabled' do
+        before do
+          project.update!(public_builds: true)
+        end
+
+        context 'user is not a member' do
+          let(:current_user) { create(:user) }
+          it_behaves_like 'returns the pipelines statistics for a given project'
+        end
       end
     end
   end

spec/policies/project_policy_spec.rb

@@ -1316,6 +1316,110 @@ RSpec.describe ProjectPolicy do
     end
   end
+  describe 'read_ci_cd_analytics' do
+    context 'public project' do
+      let(:project) { create(:project, :public, :analytics_enabled) }
+      let(:current_user) { create(:user) }
+
+      context 'when public pipelines are disabled for the project' do
+        before do
+          project.update!(public_builds: false)
+        end
+
+        context 'project member' do
+          %w(guest reporter developer maintainer).each do |role|
+            context role do
+              before do
+                project.add_user(current_user, role.to_sym)
+              end
+
+              if role == 'guest'
+                it { is_expected.to be_disallowed(:read_ci_cd_analytics) }
+              else
+                it { is_expected.to be_allowed(:read_ci_cd_analytics) }
+              end
+            end
+          end
+        end
+
+        context 'non member' do
+          let(:current_user) { non_member }
+
+          it { is_expected.to be_disallowed(:read_ci_cd_analytics) }
+        end
+
+        context 'anonymous' do
+          let(:current_user) { anonymous }
+
+          it { is_expected.to be_disallowed(:read_ci_cd_analytics) }
+        end
+      end
+
+      context 'when public pipelines are enabled for the project' do
+        before do
+          project.update!(public_builds: true)
+        end
+
+        context 'project member' do
+          %w(guest reporter developer maintainer).each do |role|
+            context role do
+              before do
+                project.add_user(current_user, role.to_sym)
+              end
+
+              it { is_expected.to be_allowed(:read_ci_cd_analytics) }
+            end
+          end
+        end
+
+        context 'non member' do
+          let(:current_user) { non_member }
+
+          it { is_expected.to be_allowed(:read_ci_cd_analytics) }
+        end
+
+        context 'anonymous' do
+          let(:current_user) { anonymous }
+
+          it { is_expected.to be_allowed(:read_ci_cd_analytics) }
+        end
+      end
+    end
+
+    context 'private project' do
+      let(:project) { create(:project, :private, :analytics_enabled) }
+      let(:current_user) { create(:user) }
+
+      context 'project member' do
+        %w(guest reporter developer maintainer).each do |role|
+          context role do
+            before do
+              project.add_user(current_user, role.to_sym)
+            end
+
+            if role == 'guest'
+              it { is_expected.to be_disallowed(:read_ci_cd_analytics) }
+            else
+              it { is_expected.to be_allowed(:read_ci_cd_analytics) }
+            end
+          end
+        end
+      end
+
+      context 'non member' do
+        let(:current_user) { non_member }
+
+        it { is_expected.to be_disallowed(:read_ci_cd_analytics) }
+      end
+
+      context 'anonymous' do
+        let(:current_user) { anonymous }
+
+        it { is_expected.to be_disallowed(:read_ci_cd_analytics) }
+      end
+    end
+  end
+
   it_behaves_like 'Self-managed Core resource access tokens'
   describe 'operations feature' do