Merge branch 'ac-fix-graphql-project-stats' into 'master'

Fixes graphql project stats

Closes #63275

See merge request gitlab-org/gitlab-ce!29708

5 files changed, 93 insertions, 3 deletions

diff --git a/app/graphql/types/project_statistics_type.rb b/app/graphql/types/project_statistics_type.rb
index 62537361918..4000c6db280 100644
--- a/app/graphql/types/project_statistics_type.rb
+++ b/app/graphql/types/project_statistics_type.rb
@@ -4,6 +4,8 @@ module Types
   class ProjectStatisticsType < BaseObject
     graphql_name 'ProjectStatistics'
 
+    authorize :read_statistics
+
     field :commit_count, GraphQL::INT_TYPE, null: false
 
     field :storage_size, GraphQL::INT_TYPE, null: false
diff --git a/app/graphql/types/project_type.rb b/app/graphql/types/project_type.rb
index 2236ffa394d..81914b70c7f 100644
--- a/app/graphql/types/project_type.rb
+++ b/app/graphql/types/project_type.rb
@@ -70,7 +70,7 @@ module Types
     field :group, Types::GroupType, null: true
 
     field :statistics, Types::ProjectStatisticsType,
-          null: false,
+          null: true,
           resolve: -> (obj, _args, _ctx) { Gitlab::Graphql::Loaders::BatchProjectStatisticsLoader.new(obj.id).find }
 
     field :repository, Types::RepositoryType, null: false
diff --git a/app/policies/project_statistics_policy.rb b/app/policies/project_statistics_policy.rb
new file mode 100644
index 00000000000..c0592f1ea13
--- /dev/null
+++ b/app/policies/project_statistics_policy.rb
@@ -0,0 +1,5 @@
+# frozen_string_literal: true
+
+class ProjectStatisticsPolicy < BasePolicy
+  delegate { @subject.project }
+end
diff --git a/spec/policies/project_statistics_policy_spec.rb b/spec/policies/project_statistics_policy_spec.rb
new file mode 100644
index 00000000000..50dfbf7291b
--- /dev/null
+++ b/spec/policies/project_statistics_policy_spec.rb
@@ -0,0 +1,83 @@
+# frozen_string_literal: true
+
+require 'spec_helper'
+
+describe ProjectStatisticsPolicy do
+  using RSpec::Parameterized::TableSyntax
+
+  describe '#rules' do
+    let(:external)   { create(:user, :external) }
+    let(:guest)      { create(:user) }
+    let(:reporter)   { create(:user) }
+    let(:developer)  { create(:user) }
+    let(:maintainer) { create(:user) }
+
+    let(:users) do
+      {
+        unauthenticated: nil,
+        non_member: create(:user),
+        guest: guest,
+        reporter: reporter,
+        developer: developer,
+        maintainer: maintainer
+      }
+    end
+
+    where(:project_type, :user_type, :outcome) do
+      [
+        # Public projects
+        [:public, :unauthenticated, false],
+        [:public, :non_member, false],
+        [:public, :guest, false],
+        [:public, :reporter, true],
+        [:public, :developer, true],
+        [:public, :maintainer, true],
+
+        # Private project
+        [:private, :unauthenticated, false],
+        [:private, :non_member, false],
+        [:private, :guest, false],
+        [:private, :reporter, true],
+        [:private, :developer, true],
+        [:private, :maintainer, true],
+
+        # Internal projects
+        [:internal, :unauthenticated, false],
+        [:internal, :non_member, false],
+        [:internal, :guest, false],
+        [:internal, :reporter, true],
+        [:internal, :developer, true],
+        [:internal, :maintainer, true]
+      ]
+    end
+
+    with_them do
+      let(:user) { users[user_type] }
+      let(:project) { create(:project, visibility_level: Gitlab::VisibilityLevel.level_value(project_type.to_s)) }
+      let(:project_statistics) { create(:project_statistics, project: project) }
+
+      subject { Ability.allowed?(user, :read_statistics, project_statistics) }
+
+      before do
+        project.add_guest(guest)
+        project.add_reporter(reporter)
+        project.add_developer(developer)
+        project.add_maintainer(maintainer)
+      end
+
+      it { is_expected.to eq(outcome) }
+
+      context 'when the user is external' do
+        let(:user) { external }
+
+        before do
+          unless [:unauthenticated, :non_member].include?(user_type)
+            project.add_user(external, user_type)
+          end
+        end
+
+        it { is_expected.to eq(outcome) }
+      end
+    end
+  end
+end
diff --git a/spec/requests/api/graphql/project/project_statistics_spec.rb b/spec/requests/api/graphql/project/project_statistics_spec.rb
index 8683fa1f390..14a3f37b779 100644
--- a/spec/requests/api/graphql/project/project_statistics_spec.rb
+++ b/spec/requests/api/graphql/project/project_statistics_spec.rb
@@ -34,10 +34,10 @@ describe 'rendering namespace statistics' do
   context 'when the project is public' do
     let(:project) { create(:project, :public) }
 
-    it 'includes the statistics regardless of the user' do
+    it 'hides statistics for unauthenticated requests' do
       post_graphql(query, current_user: nil)
 
-      expect(graphql_data['project']['statistics']).to be_present
+      expect(graphql_data['project']['statistics']).to be_blank
     end
   end
 end