Merge branch '343494-change-ci-minutes-policy' into 'master'

Only allow owners & admins to access CI minutes

See merge request gitlab-org/gitlab!77118

ee/app/models/ci/minutes/context.rb

@@ -3,15 +3,13 @@
 module Ci
   module Minutes
     class Context
-      delegate :shared_runners_minutes_limit_enabled?, to: :level
+      delegate :shared_runners_minutes_limit_enabled?, to: :namespace
       delegate :name, to: :namespace, prefix: true
 
-      attr_reader :level
+      attr_reader :namespace
 
       def initialize(project, namespace, tracking_strategy: nil)
-        @project = project
         @namespace = project&.shared_runners_limit_namespace || namespace
-        @level = project || namespace
         @tracking_strategy = tracking_strategy
       end
 
@@ -21,8 +19,6 @@ module Ci
 
       private
 
-      attr_reader :project, :namespace
-
       def quota
         @quota ||= ::Ci::Minutes::Quota.new(namespace, tracking_strategy: @tracking_strategy)
       end

ee/app/models/ci/minutes/notification.rb

@@ -17,10 +17,10 @@ module Ci
 
       def show?(current_user, cookies = false)
         return false unless @stage
-        return false unless @context.level
+        return false unless @context.namespace
         return false if alert_has_been_dismissed?(cookies)
 
-        Ability.allowed?(current_user, :read_ci_minutes_quota, @context.level)
+        Ability.allowed?(current_user, :read_ci_minutes_quota, @context.namespace)
       end
 
       def text

ee/app/policies/ee/group_policy.rb

@@ -298,7 +298,6 @@ module EE
       rule { developer }.policy do
         enable :create_wiki
         enable :admin_merge_request
-        enable :read_ci_minutes_quota
         enable :read_group_audit_events
       end
 
@@ -316,6 +315,7 @@ module EE
         enable :read_group_compliance_dashboard
         enable :read_group_credentials_inventory
         enable :admin_group_credentials_inventory
+        enable :read_ci_minutes_quota
       end
 
       rule { (admin | owner) & group_merge_request_approval_settings_enabled }.policy do

ee/app/policies/ee/project_policy.rb

@@ -183,11 +183,14 @@ module EE
         enable :create_vulnerability_feedback
         enable :destroy_vulnerability_feedback
         enable :update_vulnerability_feedback
-        enable :read_ci_minutes_quota
         enable :admin_feature_flags_issue_links
         enable :read_project_audit_events
       end
 
+      rule { can?(:owner_access) }.policy do
+        enable :read_ci_minutes_quota
+      end
+
       rule { can?(:developer_access) & iterations_available }.policy do
         enable :create_iteration
         enable :admin_iteration

ee/spec/features/ci_shared_runner_warnings_spec.rb

@@ -13,153 +13,166 @@ RSpec.describe 'CI shared runner limits' do
   let!(:job) { create(:ci_build, pipeline: pipeline) }
 
   before do
+    group.add_user(user, membership_level)
     sign_in(user)
   end
 
-  context 'when on a project related page' do
-    before do
-      group.add_developer(user)
-    end
+  where(:membership_level, :visible) do
+    :owner | true
+    :developer | false
+  end
 
-    where(:case_name, :percent_threshold, :minutes_limit, :minutes_used) do
-      'warning level' | 30 | 1000 | 800
-      'danger level'  | 5  | 1000 | 980
-    end
+  with_them do
+    context 'when on a project related page' do
+      where(:membership_level, :visible) do
+        :owner | true
+        :developer | false
+      end
 
-    with_them do
-      context "when there is a notification and minutes still exist", :js do
-        let(:message) do
-          "#{group.name} namespace has #{percent_threshold}% or less Shared Runner Pipeline minutes remaining. " \
-            "After it runs out, no new jobs or pipelines in its projects will run."
-        end
+      before do
+        group.add_user(user, membership_level)
+      end
 
-        before do
-          group.update!(shared_runners_minutes_limit: minutes_limit)
-          allow_any_instance_of(::Ci::Minutes::Quota).to receive(:total_minutes_used).and_return(minutes_used)
-        end
+      where(:case_name, :percent_threshold, :minutes_limit, :minutes_used) do
+        'warning level' | 30 | 1000 | 800
+        'danger level'  | 5  | 1000 | 980
+      end
 
-        it 'displays a warning message on pipelines page' do
-          visit project_pipelines_path(project)
+      with_them do
+        context "when there is a notification and minutes still exist", :js do
+          let(:message) do
+            "#{group.name} namespace has #{percent_threshold}% or less Shared Runner Pipeline minutes remaining. " \
+              "After it runs out, no new jobs or pipelines in its projects will run."
+          end
+
+          before do
+            group.update!(shared_runners_minutes_limit: minutes_limit)
+            allow_any_instance_of(::Ci::Minutes::Quota).to receive(:total_minutes_used).and_return(minutes_used)
+          end
+
+          it 'displays a warning message on pipelines page' do
+            visit project_pipelines_path(project)
+
+            alerts_according_to_role(visible: visible, message: message)
+          end
+
+          it 'displays a warning message on project homepage' do
+            visit project_path(project)
 
-          expect_quota_exceeded_alert(message)
+            alerts_according_to_role(visible: visible, message: message)
+          end
+
+          it 'displays a warning message on a job page' do
+            visit project_job_path(project, job)
+
+            alerts_according_to_role(visible: visible, message: message)
+          end
+        end
+      end
+
+      context 'when limit is exceeded', :js do
+        let(:group) { create(:group, :with_used_build_minutes_limit) }
+        let(:message) do
+          "#{group.name} namespace has exceeded its pipeline minutes quota. " \
+            "Buy additional pipeline minutes, or no new jobs or pipelines in its projects will run."
         end
 
         it 'displays a warning message on project homepage' do
           visit project_path(project)
 
-          expect_quota_exceeded_alert(message)
+          alerts_according_to_role(visible: visible, message: message)
+        end
+
+        it 'displays a warning message on pipelines page' do
+          visit project_pipelines_path(project)
+
+          alerts_according_to_role(visible: visible, message: message)
         end
 
         it 'displays a warning message on a job page' do
           visit project_job_path(project, job)
 
-          expect_quota_exceeded_alert(message)
+          alerts_according_to_role(visible: visible, message: message)
         end
       end
-    end
 
-    context 'when limit is exceeded', :js do
-      let(:group) { create(:group, :with_used_build_minutes_limit) }
-      let(:message) do
-        "#{group.name} namespace has exceeded its pipeline minutes quota. " \
-          "Buy additional pipeline minutes, or no new jobs or pipelines in its projects will run."
-      end
+      context 'when limit not yet exceeded' do
+        let(:group) { create(:group, :with_not_used_build_minutes_limit) }
 
-      it 'displays a warning message on project homepage' do
-        visit project_path(project)
+        it 'does not display a warning message on project homepage' do
+          visit project_path(project)
 
-        expect_quota_exceeded_alert(message)
-      end
+          expect_no_quota_exceeded_alert
+        end
 
-      it 'displays a warning message on pipelines page' do
-        visit project_pipelines_path(project)
+        it 'does not display a warning message on pipelines page' do
+          visit project_pipelines_path(project)
 
-        expect_quota_exceeded_alert(message)
-      end
+          expect_no_quota_exceeded_alert
+        end
 
-      it 'displays a warning message on a job page' do
-        visit project_job_path(project, job)
+        it 'displays a warning message on a job page' do
+          visit project_job_path(project, job)
 
-        expect_quota_exceeded_alert(message)
+          expect_no_quota_exceeded_alert
+        end
       end
     end
 
-    context 'when limit not yet exceeded' do
-      let(:group) { create(:group, :with_not_used_build_minutes_limit) }
-
-      it 'does not display a warning message on project homepage' do
-        visit project_path(project)
-
-        expect_no_quota_exceeded_alert
+    context 'when on a group related page' do
+      where(:case_name, :percent_threshold, :minutes_limit, :minutes_used) do
+        'warning level' | 30 | 1000 | 800
+        'danger level'  | 5  | 1000 | 980
       end
 
-      it 'does not display a warning message on pipelines page' do
-        visit project_pipelines_path(project)
+      with_them do
+        context "when there is a notification and minutes still exist", :js do
+          let(:message) do
+            "#{group.name} namespace has #{percent_threshold}% or less Shared Runner Pipeline minutes remaining. " \
+              "After it runs out, no new jobs or pipelines in its projects will run."
+          end
 
-        expect_no_quota_exceeded_alert
-      end
+          before do
+            group.update!(shared_runners_minutes_limit: minutes_limit)
+            allow_any_instance_of(::Ci::Minutes::Quota).to receive(:total_minutes_used).and_return(minutes_used)
+          end
 
-      it 'displays a warning message on a job page' do
-        visit project_job_path(project, job)
+          it 'displays a warning message on group information page' do
+            visit group_path(group)
 
-        expect_no_quota_exceeded_alert
+            alerts_according_to_role(visible: visible, message: message)
+          end
+        end
       end
-    end
-  end
 
-  context 'when on a group related page' do
-    before do
-      group.add_owner(user)
-    end
-
-    where(:case_name, :percent_threshold, :minutes_limit, :minutes_used) do
-      'warning level' | 30 | 1000 | 800
-      'danger level'  | 5  | 1000 | 980
-    end
-
-    with_them do
-      context "when there is a notification and minutes still exist", :js do
+      context 'when limit is exceeded', :js do
+        let(:group) { create(:group, :with_used_build_minutes_limit) }
         let(:message) do
-          "#{group.name} namespace has #{percent_threshold}% or less Shared Runner Pipeline minutes remaining. " \
-            "After it runs out, no new jobs or pipelines in its projects will run."
-        end
-
-        before do
-          group.update!(shared_runners_minutes_limit: minutes_limit)
-          allow_any_instance_of(::Ci::Minutes::Quota).to receive(:total_minutes_used).and_return(minutes_used)
+          "#{group.name} namespace has exceeded its pipeline minutes quota. " \
+            "Buy additional pipeline minutes, or no new jobs or pipelines in its projects will run."
         end
 
         it 'displays a warning message on group information page' do
           visit group_path(group)
 
-          expect_quota_exceeded_alert(message)
+          alerts_according_to_role(visible: visible, message: message)
         end
       end
-    end
 
-    context 'when limit is exceeded', :js do
-      let(:group) { create(:group, :with_used_build_minutes_limit) }
-      let(:message) do
-        "#{group.name} namespace has exceeded its pipeline minutes quota. " \
-          "Buy additional pipeline minutes, or no new jobs or pipelines in its projects will run."
-      end
+      context 'when limit not yet exceeded' do
+        let(:group) { create(:group, :with_not_used_build_minutes_limit) }
 
-      it 'displays a warning message on group information page' do
-        visit group_path(group)
+        it 'does not display a warning message on group information page' do
+          visit group_path(group)
 
-        expect_quota_exceeded_alert(message)
+          expect_no_quota_exceeded_alert
+        end
       end
     end
+  end
 
-    context 'when limit not yet exceeded' do
-      let(:group) { create(:group, :with_not_used_build_minutes_limit) }
-
-      it 'does not display a warning message on group information page' do
-        visit group_path(group)
-
-        expect_no_quota_exceeded_alert
-      end
-    end
+  def alerts_according_to_role(visible: false, message: '')
+    visible ? expect_quota_exceeded_alert(message) : expect_no_quota_exceeded_alert
   end
 
   def expect_quota_exceeded_alert(message)

ee/spec/models/ci/minutes/context_spec.rb

@@ -9,7 +9,7 @@ RSpec.describe Ci::Minutes::Context do
   describe 'delegation' do
     subject { described_class.new(project, group) }
 
-    it { is_expected.to delegate_method(:shared_runners_minutes_limit_enabled?).to(:level) }
+    it { is_expected.to delegate_method(:shared_runners_minutes_limit_enabled?).to(:namespace) }
     it { is_expected.to delegate_method(:name).to(:namespace).with_prefix }
   end
 end

ee/spec/models/ci/minutes/notification_spec.rb

@@ -189,7 +189,7 @@ RSpec.describe Ci::Minutes::Notification do
   context 'when at project level' do
     context 'when eligible to see notifications' do
       before do
-        group.add_developer(user)
+        group.add_owner(user)
       end
 
       describe '#show?' do
@@ -217,12 +217,22 @@ RSpec.describe Ci::Minutes::Notification do
         subject { described_class.new(injected_project, nil) }
       end
     end
+
+    context 'when user is not in the correct role' do
+      before do
+        group.add_developer user
+      end
+
+      it_behaves_like 'not eligible to see notifications' do
+        subject { described_class.new(injected_project, nil) }
+      end
+    end
   end
 
   context 'when at namespace level' do
     context 'when eligible to see notifications' do
       before do
-        group.add_developer(user)
+        group.add_owner(user)
       end
 
       context 'with a project that has runners enabled inside namespace' do
@@ -259,5 +269,15 @@ RSpec.describe Ci::Minutes::Notification do
         subject { described_class.new(injected_project, nil) }
       end
     end
+
+    context 'when user is not in the correct role' do
+      before do
+        group.add_developer user
+      end
+
+      it_behaves_like 'not eligible to see notifications' do
+        subject { described_class.new(injected_project, nil) }
+      end
+    end
   end
 end

ee/spec/policies/group_policy_spec.rb

@@ -1349,8 +1349,8 @@ RSpec.describe GroupPolicy do
     where(:role, :admin_mode, :allowed) do
       :guest      | nil   | false
       :reporter   | nil   | false
-      :developer  | nil   | true
-      :maintainer | nil   | true
+      :developer  | nil   | false
+      :maintainer | nil   | false
       :owner      | nil   | true
       :admin      | true  | true
       :admin      | false | false

ee/spec/policies/project_policy_spec.rb

@@ -1542,8 +1542,8 @@ RSpec.describe ProjectPolicy do
     where(:role, :admin_mode, :allowed) do
       :guest      | nil   | false
       :reporter   | nil   | false
-      :developer  | nil   | true
-      :maintainer | nil   | true
+      :developer  | nil   | false
+      :maintainer | nil   | false
       :owner      | nil   | true
       :admin      | false | false
       :admin      | true  | true