Merge branch 'rc/add_metrics_dashboard_policy' into 'master'

Rc/add metrics dashboard policy

See merge request gitlab-org/gitlab!29634

app/assets/javascripts/pages/projects/shared/permissions/components/settings_panel.vue

@@ -12,6 +12,7 @@ import {
   visibilityLevelDescriptions,
   featureAccessLevelMembers,
   featureAccessLevelEveryone,
+  featureAccessLevel,
 } from '../constants';
 import { toggleHiddenClassBySelector } from '../external';
 
@@ -127,7 +128,7 @@ export default {
       wikiAccessLevel: 20,
       snippetsAccessLevel: 20,
       pagesAccessLevel: 20,
-      metricsAccessLevel: visibilityOptions.PRIVATE,
+      metricsDashboardAccessLevel: featureAccessLevel.PROJECT_MEMBERS,
       containerRegistryEnabled: true,
       lfsEnabled: true,
       requestAccessEnabled: true,
@@ -174,6 +175,10 @@ export default {
       return options;
     },
 
+    metricsOptionsDropdownEnabled() {
+      return this.featureAccessLevelOptions.length < 2;
+    },
+
     repositoryEnabled() {
       return this.repositoryAccessLevel > 0;
     },
@@ -211,6 +216,7 @@ export default {
         this.buildsAccessLevel = Math.min(10, this.buildsAccessLevel);
         this.wikiAccessLevel = Math.min(10, this.wikiAccessLevel);
         this.snippetsAccessLevel = Math.min(10, this.snippetsAccessLevel);
+        this.metricsDashboardAccessLevel = Math.min(10, this.metricsDashboardAccessLevel);
         if (this.pagesAccessLevel === 20) {
           // When from Internal->Private narrow access for only members
           this.pagesAccessLevel = 10;
@@ -225,6 +231,7 @@ export default {
         if (this.wikiAccessLevel > 0) this.wikiAccessLevel = 20;
         if (this.snippetsAccessLevel > 0) this.snippetsAccessLevel = 20;
         if (this.pagesAccessLevel === 10) this.pagesAccessLevel = 20;
+        if (this.metricsDashboardAccessLevel === 10) this.metricsDashboardAccessLevel = 20;
         this.highlightChanges();
       }
     },
@@ -485,17 +492,18 @@ export default {
         <div class="project-feature-controls">
           <div class="select-wrapper">
             <select
-              v-model="metricsAccessLevel"
+              v-model="metricsDashboardAccessLevel"
+              :disabled="metricsOptionsDropdownEnabled"
               name="project[project_feature_attributes][metrics_dashboard_access_level]"
-              class="form-control select-control"
+              class="form-control project-repo-select select-control"
             >
               <option
-                :value="visibilityOptions.PRIVATE"
-                :disabled="!visibilityAllowed(visibilityOptions.PRIVATE)"
+                :value="featureAccessLevelMembers[0]"
+                :disabled="!visibilityAllowed(visibilityOptions.INTERNAL)"
                 >{{ featureAccessLevelMembers[1] }}</option
               >
               <option
-                :value="visibilityOptions.PUBLIC"
+                :value="featureAccessLevelEveryone[0]"
                 :disabled="!visibilityAllowed(visibilityOptions.PUBLIC)"
                 >{{ featureAccessLevelEveryone[1] }}</option
               >

app/controllers/projects/environments_controller.rb

@@ -4,6 +4,13 @@ class Projects::EnvironmentsController < Projects::ApplicationController
   include MetricsDashboard
 
   layout 'project'
+
+  before_action only: [:metrics, :additional_metrics, :metrics_dashboard] do
+    authorize_metrics_dashboard!
+
+    push_frontend_feature_flag(:prometheus_computed_alerts)
+    push_frontend_feature_flag(:metrics_dashboard_annotations, project)
+  end
   before_action :authorize_read_environment!
   before_action :authorize_create_environment!, only: [:new, :create]
   before_action :authorize_stop_environment!, only: [:stop]
@@ -12,10 +19,6 @@ class Projects::EnvironmentsController < Projects::ApplicationController
   before_action :environment, only: [:show, :edit, :update, :stop, :terminal, :terminal_websocket_authorize, :metrics, :cancel_auto_stop]
   before_action :verify_api_request!, only: :terminal_websocket_authorize
   before_action :expire_etag_cache, only: [:index], unless: -> { request.format.json? }
-  before_action only: [:metrics, :additional_metrics, :metrics_dashboard] do
-    push_frontend_feature_flag(:prometheus_computed_alerts)
-    push_frontend_feature_flag(:metrics_dashboard_annotations, project)
-  end
   after_action :expire_etag_cache, only: [:cancel_auto_stop]
 
   def index

app/controllers/projects_controller.rb

@@ -403,6 +403,7 @@ class ProjectsController < Projects::ApplicationController
         snippets_access_level
         wiki_access_level
         pages_access_level
+        metrics_dashboard_access_level
       ]
     ]
   end

app/helpers/projects_helper.rb

@@ -589,7 +589,8 @@ module ProjectsHelper
       pagesAccessLevel: feature.pages_access_level,
       containerRegistryEnabled: !!project.container_registry_enabled,
       lfsEnabled: !!project.lfs_enabled,
-      emailsDisabled: project.emails_disabled?
+      emailsDisabled: project.emails_disabled?,
+      metricsDashboardAccessLevel: feature.metrics_dashboard_access_level
     }
   end
 

app/models/project_feature.rb

@@ -23,7 +23,7 @@ class ProjectFeature < ApplicationRecord
   PUBLIC   = 30
 
   FEATURES = %i(issues forking merge_requests wiki snippets builds repository pages metrics_dashboard).freeze
-  PRIVATE_FEATURES_MIN_ACCESS_LEVEL = { merge_requests: Gitlab::Access::REPORTER }.freeze
+  PRIVATE_FEATURES_MIN_ACCESS_LEVEL = { merge_requests: Gitlab::Access::REPORTER, metrics_dashboard: Gitlab::Access::REPORTER }.freeze
   PRIVATE_FEATURES_MIN_ACCESS_LEVEL_FOR_PRIVATE_PROJECT = { repository: Gitlab::Access::REPORTER }.freeze
   STRING_OPTIONS = HashWithIndifferentAccess.new({
     'disabled' => DISABLED,

app/policies/project_policy.rb

@@ -88,6 +88,11 @@ class ProjectPolicy < BasePolicy
     @subject.feature_available?(:forking, @user)
   end
 
+  with_scope :subject
+  condition(:metrics_dashboard_allowed) do
+    feature_available?(:metrics_dashboard)
+  end
+
   with_scope :global
   condition(:mirror_available, score: 0) do
     ::Gitlab::CurrentSettings.current_application_settings.mirror_available
@@ -134,6 +139,7 @@ class ProjectPolicy < BasePolicy
     wiki
     builds
     pages
+    metrics_dashboard
   ]
 
   features.each do |f|
@@ -227,6 +233,7 @@ class ProjectPolicy < BasePolicy
     enable :read_prometheus
     enable :read_metrics_dashboard_annotation
     enable :read_alert_management_alerts
+    enable :metrics_dashboard
   end
 
   # We define `:public_user_access` separately because there are cases in gitlab-ee
@@ -249,6 +256,16 @@ class ProjectPolicy < BasePolicy
     enable :fork_project
   end
 
+  rule { metrics_dashboard_disabled }.policy do
+    prevent(:metrics_dashboard)
+  end
+
+  rule { can?(:metrics_dashboard) }.policy do
+    enable :read_prometheus
+    enable :read_environment
+    enable :read_deployment
+  end
+
   rule { owner | admin | guest | group_member }.prevent :request_access
   rule { ~request_access_enabled }.prevent :request_access
 
@@ -327,6 +344,14 @@ class ProjectPolicy < BasePolicy
     enable :admin_terraform_state
   end
 
+  rule { public_project & metrics_dashboard_allowed }.policy do
+    enable :metrics_dashboard
+  end
+
+  rule { internal_access & metrics_dashboard_allowed }.policy do
+    enable :metrics_dashboard
+  end
+
   rule { (mirror_available & can?(:admin_project)) | admin }.enable :admin_remote_mirror
   rule { can?(:push_code) }.enable :admin_tag
 

app/services/metrics/dashboard/base_service.rb

@@ -42,7 +42,7 @@ module Metrics
       def allowed?
         return false unless params[:environment]
 
-        Ability.allowed?(current_user, :read_environment, project)
+        project&.feature_available?(:metrics_dashboard, current_user)
       end
 
       # Returns a new dashboard Hash, supplemented with DB info

changelogs/unreleased/rc-add_metrics_dashboard_policy.yml

+---
+title: Add ability to change metrics dashboard visibility
+merge_request: 29634
+author:
+type: added

doc/user/project/settings/index.md

@@ -61,6 +61,7 @@ Use the switches to enable or disable the following features:
 | **Wiki**                          | ✓                         | Enables a separate system for [documentation](../wiki/)                                                                                                                                        |
 | **Snippets**                      | ✓                         | Enables [sharing of code and text](../../snippets.md)                                                                                                                                          |
 | **Pages**                         | ✓                         | Allows you to [publish static websites](../pages/)                                                                                                                                             |
+| **Metrics Dashboard**             | ✓                         | Control access to [metrics dashboard](../integrations/prometheus.md)
 
 Some features depend on others:
 
@@ -80,13 +81,15 @@ Some features depend on others:
 
 - If you disable **Repository** functionality, GitLab also disables the following
   features for your project:
-
   - **Merge Requests**
   - **Pipelines**
   - **Container Registry**
   - **Git Large File Storage**
   - **Packages**
 
+- Metrics dashboard access requires reading both project environments and deployments.
+  Users with access to the metrics dashboard can also access environments and deployments.
+
 #### Disabling email notifications
 
 Project owners can disable all [email notifications](../../profile/notifications.md#gitlab-notification-emails)

spec/controllers/projects/environments_controller_spec.rb

@@ -410,6 +410,18 @@ describe Projects::EnvironmentsController do
         expect(json_response['last_update']).to eq(42)
       end
     end
+
+    context 'permissions' do
+      before do
+        allow(controller).to receive(:can?).and_return true
+      end
+
+      it 'checks :metrics_dashboard ability' do
+        expect(controller).to receive(:can?).with(anything, :metrics_dashboard, anything)
+
+        get :metrics, params: environment_params
+      end
+    end
   end
 
   describe 'GET #additional_metrics' do
@@ -473,6 +485,18 @@ describe Projects::EnvironmentsController do
           .to raise_error(ActionController::ParameterMissing)
       end
     end
+
+    context 'permissions' do
+      before do
+        allow(controller).to receive(:can?).and_return true
+      end
+
+      it 'checks :metrics_dashboard ability' do
+        expect(controller).to receive(:can?).with(anything, :metrics_dashboard, anything)
+
+        get :metrics, params: environment_params
+      end
+    end
   end
 
   describe 'GET #metrics_dashboard' do
@@ -648,6 +672,18 @@ describe Projects::EnvironmentsController do
     it_behaves_like 'the default dashboard'
     it_behaves_like 'dashboard can be specified'
     it_behaves_like 'dashboard can be embedded'
+
+    context 'permissions' do
+      before do
+        allow(controller).to receive(:can?).and_return true
+      end
+
+      it 'checks :metrics_dashboard ability' do
+        expect(controller).to receive(:can?).with(anything, :metrics_dashboard, anything)
+
+        get :metrics, params: environment_params
+      end
+    end
   end
 
   describe 'GET #search' do

spec/frontend/pages/projects/shared/permissions/components/settings_panel_spec.js

@@ -489,15 +489,22 @@ describe('Settings Panel', () => {
         .find('[name="project[project_feature_attributes][metrics_dashboard_access_level]"]')
         .setValue(visibilityOptions.PUBLIC);
 
-      expect(wrapper.vm.metricsAccessLevel).toBe(visibilityOptions.PUBLIC);
+      expect(wrapper.vm.metricsDashboardAccessLevel).toBe(visibilityOptions.PUBLIC);
     });
 
     it('should contain help text', () => {
-      wrapper = overrideCurrentSettings({ visibilityLevel: visibilityOptions.PRIVATE });
-
       expect(wrapper.find({ ref: 'metrics-visibility-settings' }).props().helpText).toEqual(
         'With Metrics Dashboard you can visualize this project performance metrics',
       );
     });
+
+    it('should disable the metrics visibility dropdown when the project visibility level changes to private', () => {
+      wrapper = overrideCurrentSettings({ visibilityLevel: visibilityOptions.PRIVATE });
+
+      const metricsSettingsRow = wrapper.find({ ref: 'metrics-visibility-settings' });
+
+      expect(wrapper.vm.metricsOptionsDropdownEnabled).toBe(true);
+      expect(metricsSettingsRow.find('select').attributes('disabled')).toEqual('disabled');
+    });
   });
 });

spec/policies/project_policy_spec.rb

@@ -29,6 +29,7 @@ describe ProjectPolicy do
       admin_issue admin_label admin_list read_commit_status read_build
       read_container_image read_pipeline read_environment read_deployment
       read_merge_request download_wiki_code read_sentry_issue read_metrics_dashboard_annotation
+      metrics_dashboard
     ]
   end
 
@@ -485,4 +486,190 @@ describe ProjectPolicy do
       it { is_expected.to be_disallowed(:read_prometheus_alerts) }
     end
   end
+
+  describe 'metrics_dashboard feature' do
+    subject { described_class.new(current_user, project) }
+
+    context 'public project' do
+      let(:project) { create(:project, :public) }
+
+      context 'feature private' do
+        context 'with reporter' do
+          let(:current_user) { reporter }
+
+          it { is_expected.to be_allowed(:metrics_dashboard) }
+          it { is_expected.to be_allowed(:read_prometheus) }
+          it { is_expected.to be_allowed(:read_deployment) }
+        end
+
+        context 'with guest' do
+          let(:current_user) { guest }
+
+          it { is_expected.to be_disallowed(:metrics_dashboard) }
+        end
+
+        context 'with anonymous' do
+          let(:current_user) { nil }
+
+          it { is_expected.to be_disallowed(:metrics_dashboard) }
+        end
+      end
+
+      context 'feature enabled' do
+        before do
+          project.project_feature.update(metrics_dashboard_access_level: ProjectFeature::ENABLED)
+        end
+
+        context 'with reporter' do
+          let(:current_user) { reporter }
+
+          it { is_expected.to be_allowed(:metrics_dashboard) }
+          it { is_expected.to be_allowed(:read_prometheus) }
+          it { is_expected.to be_allowed(:read_deployment) }
+        end
+
+        context 'with guest' do
+          let(:current_user) { guest }
+
+          it { is_expected.to be_allowed(:metrics_dashboard) }
+          it { is_expected.to be_allowed(:read_prometheus) }
+          it { is_expected.to be_allowed(:read_deployment) }
+        end
+
+        context 'with anonymous' do
+          let(:current_user) { nil }
+
+          it { is_expected.to be_allowed(:metrics_dashboard) }
+          it { is_expected.to be_allowed(:read_prometheus) }
+          it { is_expected.to be_allowed(:read_deployment) }
+        end
+      end
+    end
+
+    context 'internal project' do
+      let(:project) { create(:project, :internal) }
+
+      context 'feature private' do
+        context 'with reporter' do
+          let(:current_user) { reporter }
+
+          it { is_expected.to be_allowed(:metrics_dashboard) }
+          it { is_expected.to be_allowed(:read_prometheus) }
+          it { is_expected.to be_allowed(:read_deployment) }
+        end
+
+        context 'with guest' do
+          let(:current_user) { guest }
+
+          it { is_expected.to be_disallowed(:metrics_dashboard) }
+        end
+
+        context 'with anonymous' do
+          let(:current_user) { nil }
+
+          it { is_expected.to be_disallowed(:metrics_dashboard)}
+        end
+      end
+
+      context 'feature enabled' do
+        before do
+          project.project_feature.update(metrics_dashboard_access_level: ProjectFeature::ENABLED)
+        end
+
+        context 'with reporter' do
+          let(:current_user) { reporter }
+
+          it { is_expected.to be_allowed(:metrics_dashboard) }
+          it { is_expected.to be_allowed(:read_prometheus) }
+          it { is_expected.to be_allowed(:read_deployment) }
+        end
+
+        context 'with guest' do
+          let(:current_user) { guest }
+
+          it { is_expected.to be_allowed(:metrics_dashboard) }
+          it { is_expected.to be_allowed(:read_prometheus) }
+          it { is_expected.to be_allowed(:read_deployment) }
+        end
+
+        context 'with anonymous' do
+          let(:current_user) { nil }
+
+          it { is_expected.to be_disallowed(:metrics_dashboard) }
+        end
+      end
+    end
+
+    context 'private project' do
+      let(:project) { create(:project, :private) }
+
+      context 'feature private' do
+        context 'with reporter' do
+          let(:current_user) { reporter }
+
+          it { is_expected.to be_allowed(:metrics_dashboard) }
+          it { is_expected.to be_allowed(:read_prometheus) }
+          it { is_expected.to be_allowed(:read_deployment) }
+        end
+
+        context 'with guest' do
+          let(:current_user) { guest }
+
+          it { is_expected.to be_disallowed(:metrics_dashboard) }
+        end
+
+        context 'with anonymous' do
+          let(:current_user) { nil }
+
+          it { is_expected.to be_disallowed(:metrics_dashboard) }
+        end
+      end
+
+      context 'feature enabled' do
+        context 'with reporter' do
+          let(:current_user) { reporter }
+
+          it { is_expected.to be_allowed(:metrics_dashboard) }
+          it { is_expected.to be_allowed(:read_prometheus) }
+          it { is_expected.to be_allowed(:read_deployment) }
+        end
+
+        context 'with guest' do
+          let(:current_user) { guest }
+
+          it { is_expected.to be_disallowed(:metrics_dashboard) }
+        end
+
+        context 'with anonymous' do
+          let(:current_user) { nil }
+
+          it { is_expected.to be_disallowed(:metrics_dashboard) }
+        end
+      end
+    end
+
+    context 'feature disabled' do
+      before do
+        project.project_feature.update(metrics_dashboard_access_level: ProjectFeature::DISABLED)
+      end
+
+      context 'with reporter' do
+        let(:current_user) { reporter }
+
+        it { is_expected.to be_disallowed(:metrics_dashboard) }
+      end
+
+      context 'with guest' do
+        let(:current_user) { guest }
+
+        it { is_expected.to be_disallowed(:metrics_dashboard) }
+      end
+
+      context 'with anonymous' do
+        let(:current_user) { nil }
+
+        it { is_expected.to be_disallowed(:metrics_dashboard) }
+      end
+    end
+  end
 end