Merge branch 'ajk-219247-fix-leak-of-vuln-counts' into 'master'

219247: Fix leak of number of vulnerabilities

See merge request gitlab-org/gitlab!57037

ee/app/graphql/resolvers/vulnerability_severities_count_resolver.rb

@@ -3,8 +3,11 @@
 module Resolvers
   class VulnerabilitySeveritiesCountResolver < VulnerabilitiesBaseResolver
     include Gitlab::Utils::StrongMemoize
+    include Gitlab::Graphql::Authorize::AuthorizeResource
 
     type Types::VulnerabilitySeveritiesCountType, null: true
+    authorize :read_vulnerability
+    authorizes_object!
 
     argument :project_id, [GraphQL::ID_TYPE],
              required: false,

ee/app/policies/ee/group_policy.rb

@@ -282,7 +282,10 @@ module EE
 
       rule { security_dashboard_enabled & developer }.enable :read_group_security_dashboard
 
-      rule { can?(:read_group_security_dashboard) }.enable :create_vulnerability_export
+      rule { can?(:read_group_security_dashboard) }.policy do
+        enable :create_vulnerability_export
+        enable :read_vulnerability
+      end
 
       rule { admin | owner }.policy do
         enable :read_group_compliance_dashboard

ee/app/policies/instance_security_dashboard_policy.rb

@@ -6,6 +6,10 @@ class InstanceSecurityDashboardPolicy < BasePolicy
     License.feature_available?(:security_dashboard)
   end
 
-  rule { ~anonymous }.enable :read_instance_security_dashboard
+  rule { ~anonymous }.policy do
+    enable :read_instance_security_dashboard
+    enable :read_vulnerability
+  end
+
   rule { security_dashboard_enabled & can?(:read_instance_security_dashboard) }.enable :create_vulnerability_export
 end

ee/changelogs/unreleased/ajk-219247-fix-leak-of-vuln-counts.yml

+---
+title: Require permissions to read vulnerability counts
+merge_request: 57037
+author:
+type: fixed

ee/spec/graphql/resolvers/vulnerability_severities_count_resolver_spec.rb

@@ -27,50 +27,64 @@ RSpec.describe Resolvers::VulnerabilitySeveritiesCountResolver do
     let(:filters) { {} }
     let(:vulnerable) { project }
 
-    context 'when given severities' do
-      let(:filters) { { severity: ['low'] } }
-
-      it 'only returns count for low severity vulnerability' do
-        is_expected.to eq('low' => 1)
+    context 'when the user does not have access' do
+      it 'is redacted' do
+        is_expected.to be_nil
       end
     end
 
-    context 'when given states' do
-      let(:filters) { { state: ['dismissed'] } }
+    context 'when the user has access' do
+      before do
+        stub_licensed_features(security_dashboard: true)
+        project.add_developer(current_user)
+      end
+
+      context 'when given severities' do
+        let(:filters) { { severity: ['low'] } }
 
-      it 'only returns count for high severity vulnerability' do
-        is_expected.to eq('high' => 1)
+        it 'only returns count for low severity vulnerability' do
+          is_expected.to eq('low' => 1)
+        end
       end
-    end
 
-    context 'when given scanner' do
-      let(:filters) { { scanner: [high_vulnerability.finding_scanner_external_id] } }
+      context 'when given states' do
+        let(:filters) { { state: ['dismissed'] } }
 
-      it 'only returns count for high severity vulnerability' do
-        is_expected.to eq('high' => 1)
+        it 'only returns count for high severity vulnerability' do
+          is_expected.to eq('high' => 1)
+        end
       end
-    end
 
-    context 'when given report types' do
-      let(:filters) { { report_type: %i[dast sast] } }
+      context 'when given scanner' do
+        let(:filters) { { scanner: [high_vulnerability.finding_scanner_external_id] } }
 
-      it 'only returns count for vulnerabilities of the given report types' do
-        is_expected.to eq('critical' => 1, 'low' => 1)
+        it 'only returns count for high severity vulnerability' do
+          is_expected.to eq('high' => 1)
+        end
+      end
+
+      context 'when given report types' do
+        let(:filters) { { report_type: %i[dast sast] } }
+
+        it 'only returns count for vulnerabilities of the given report types' do
+          is_expected.to eq('critical' => 1, 'low' => 1)
+        end
       end
-    end
 
-    context 'when resolving vulnerabilities for a project' do
-      it "returns the project's vulnerabilities" do
-        is_expected.to eq('critical' => 1, 'high' => 1, 'low' => 1)
+      context 'when resolving vulnerabilities for a project' do
+        it "returns the project's vulnerabilities" do
+          is_expected.to eq('critical' => 1, 'high' => 1, 'low' => 1)
+        end
       end
     end
 
     context 'when resolving vulnerabilities for an instance security dashboard' do
       before do
+        stub_licensed_features(security_dashboard: true)
         project.add_developer(user)
       end
 
-      let(:vulnerable) { nil }
+      let(:vulnerable) { InstanceSecurityDashboard.new(current_user) }
 
       context 'when there is a current user' do
         it "returns vulnerabilities for all projects on the current user's instance security dashboard" do
@@ -78,11 +92,11 @@ RSpec.describe Resolvers::VulnerabilitySeveritiesCountResolver do
         end
       end
 
-      context 'and there is no current user' do
+      context 'without a current user' do
         let(:current_user) { nil }
 
         it 'returns no vulnerabilities' do
-          is_expected.to be_empty
+          is_expected.to be_blank
         end
       end
     end

ee/spec/policies/group_policy_spec.rb

@@ -846,7 +846,9 @@ RSpec.describe GroupPolicy do
   end
 
   describe 'read_group_security_dashboard & create_vulnerability_export' do
-    let(:abilities) { %i(read_group_security_dashboard create_vulnerability_export) }
+    let(:abilities) do
+      %i[read_group_security_dashboard create_vulnerability_export read_vulnerability]
+    end
 
     before do
       stub_licensed_features(security_dashboard: true)

ee/spec/policies/instance_security_dashboard_policy_spec.rb

@@ -13,14 +13,16 @@ RSpec.describe InstanceSecurityDashboardPolicy do
   subject { described_class.new(current_user, [user]) }
 
   describe 'read_instance_security_dashboard' do
+    let(:abilities) { %i[read_instance_security_dashboard read_vulnerability] }
+
     context 'when the user is not logged in' do
       let(:current_user) { nil }
 
-      it { is_expected.not_to be_allowed(:read_instance_security_dashboard) }
+      it { is_expected.not_to be_allowed(*abilities) }
     end
 
     context 'when the user is logged in' do
-      it { is_expected.to be_allowed(:read_instance_security_dashboard) }
+      it { is_expected.to be_allowed(*abilities) }
     end
   end