Automatic merge of gitlab-org/gitlab-ce master

changelogs/unreleased/add-metrics-dashboard-permission-check.yml

+---
+title: Add permission check to metrics dashboards endpoint
+merge_request: 30017
+author:
+type: added

lib/gitlab/metrics/dashboard/base_service.rb

@@ -10,6 +10,8 @@ module Gitlab
         NOT_FOUND_ERROR = Gitlab::Template::Finders::RepoTemplateFinder::FileNotFoundError
 
         def get_dashboard
+          return error('Insufficient permissions.', :unauthorized) unless allowed?
+
           success(dashboard: process_dashboard)
         rescue NOT_FOUND_ERROR
           error("#{dashboard_path} could not be found.", :not_found)
@@ -30,6 +32,12 @@ module Gitlab
 
         private
 
+        # Determines whether users should be able to view
+        # dashboards at all.
+        def allowed?
+          Ability.allowed?(current_user, :read_environment, project)
+        end
+
         # Returns a new dashboard Hash, supplemented with DB info
         def process_dashboard
           Gitlab::Metrics::Dashboard::Processor

spec/lib/gitlab/metrics/dashboard/dynamic_dashboard_service_spec.rb

@@ -6,13 +6,19 @@ describe Gitlab::Metrics::Dashboard::DynamicDashboardService, :use_clean_rails_m
   include MetricsDashboardHelpers
 
   set(:project) { build(:project) }
+  set(:user) { create(:user) }
   set(:environment) { create(:environment, project: project) }
 
+  before do
+    project.add_maintainer(user)
+  end
+
   describe '#get_dashboard' do
-    let(:service_params) { [project, nil, { environment: environment, dashboard_path: nil }] }
+    let(:service_params) { [project, user, { environment: environment, dashboard_path: nil }] }
     let(:service_call) { described_class.new(*service_params).get_dashboard }
 
     it_behaves_like 'valid embedded dashboard service response'
+    it_behaves_like 'raises error for users with insufficient permissions'
 
     it 'caches the unprocessed dashboard for subsequent calls' do
       expect(YAML).to receive(:safe_load).once.and_call_original

spec/lib/gitlab/metrics/dashboard/finder_spec.rb

@@ -6,12 +6,17 @@ describe Gitlab::Metrics::Dashboard::Finder, :use_clean_rails_memory_store_cachi
   include MetricsDashboardHelpers
 
   set(:project) { build(:project) }
+  set(:user) { create(:user) }
   set(:environment) { create(:environment, project: project) }
   let(:system_dashboard_path) { Gitlab::Metrics::Dashboard::SystemDashboardService::SYSTEM_DASHBOARD_PATH}
 
+  before do
+    project.add_maintainer(user)
+  end
+
   describe '.find' do
     let(:dashboard_path) { '.gitlab/dashboards/test.yml' }
-    let(:service_call) { described_class.find(project, nil, environment, dashboard_path: dashboard_path) }
+    let(:service_call) { described_class.find(project, user, environment, dashboard_path: dashboard_path) }
 
     it_behaves_like 'misconfigured dashboard service response', :not_found
 
@@ -41,13 +46,13 @@ describe Gitlab::Metrics::Dashboard::Finder, :use_clean_rails_memory_store_cachi
     end
 
     context 'when no dashboard is specified' do
-      let(:service_call) { described_class.find(project, nil, environment) }
+      let(:service_call) { described_class.find(project, user, environment) }
 
       it_behaves_like 'valid dashboard service response'
     end
 
     context 'when the dashboard is expected to be embedded' do
-      let(:service_call) { described_class.find(project, nil, environment, dashboard_path: nil, embedded: true) }
+      let(:service_call) { described_class.find(project, user, environment, dashboard_path: nil, embedded: true) }
 
       it_behaves_like 'valid embedded dashboard service response'
     end

spec/lib/gitlab/metrics/dashboard/project_dashboard_service_spec.rb

@@ -5,8 +5,8 @@ require 'rails_helper'
 describe Gitlab::Metrics::Dashboard::ProjectDashboardService, :use_clean_rails_memory_store_caching do
   include MetricsDashboardHelpers
 
-  set(:user) { build(:user) }
-  set(:project) { build(:project) }
+  set(:user) { create(:user) }
+  set(:project) { create(:project) }
   set(:environment) { create(:environment, project: project) }
 
   before do
@@ -22,6 +22,8 @@ describe Gitlab::Metrics::Dashboard::ProjectDashboardService, :use_clean_rails_m
       it_behaves_like 'misconfigured dashboard service response', :not_found
     end
 
+    it_behaves_like 'raises error for users with insufficient permissions'
+
     context 'when the dashboard exists' do
       let(:project) { project_with_dashboard(dashboard_path) }
 

spec/lib/gitlab/metrics/dashboard/system_dashboard_service_spec.rb

@@ -5,15 +5,21 @@ require 'spec_helper'
 describe Gitlab::Metrics::Dashboard::SystemDashboardService, :use_clean_rails_memory_store_caching do
   include MetricsDashboardHelpers
 
-  set(:project) { build(:project) }
+  set(:user) { create(:user) }
+  set(:project) { create(:project) }
   set(:environment) { create(:environment, project: project) }
 
+  before do
+    project.add_maintainer(user)
+  end
+
   describe 'get_dashboard' do
     let(:dashboard_path) { described_class::SYSTEM_DASHBOARD_PATH }
-    let(:service_params) { [project, nil, { environment: environment, dashboard_path: dashboard_path }] }
+    let(:service_params) { [project, user, { environment: environment, dashboard_path: dashboard_path }] }
     let(:service_call) { described_class.new(*service_params).get_dashboard }
 
     it_behaves_like 'valid dashboard service response'
+    it_behaves_like 'raises error for users with insufficient permissions'
 
     it 'caches the unprocessed dashboard for subsequent calls' do
       expect(YAML).to receive(:safe_load).once.and_call_original

spec/support/helpers/metrics_dashboard_helpers.rb

@@ -50,4 +50,12 @@ module MetricsDashboardHelpers
 
     it_behaves_like 'valid dashboard service response for schema'
   end
+
+  shared_examples_for 'raises error for users with insufficient permissions' do
+    context 'when the user does not have sufficient access' do
+      let(:user) { build(:user) }
+
+      it_behaves_like 'misconfigured dashboard service response', :unauthorized
+    end
+  end
 end