summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorRémy Coutable <remy@rymai.me>2019-06-27 10:54:30 +0000
committerRémy Coutable <remy@rymai.me>2019-06-27 10:54:30 +0000
commit47089be063b754b3cc76c47bda7d26876889e5de (patch)
tree8c0c12fd13fba5585ad66033cc52bf6d343a512d
parente7193f235643a4b53d82206a7771586bcc46c88a (diff)
parent9c50cf03f5cfe5df5eff28edb80aa9fc1ab9fe6e (diff)
downloadgitlab-ce-47089be063b754b3cc76c47bda7d26876889e5de.tar.gz
Merge branch 'add-metrics-dashboard-permission-check' into 'master'
Add permission check to metrics dashboards See merge request gitlab-org/gitlab-ce!30017
-rw-r--r--changelogs/unreleased/add-metrics-dashboard-permission-check.yml5
-rw-r--r--lib/gitlab/metrics/dashboard/base_service.rb8
-rw-r--r--spec/lib/gitlab/metrics/dashboard/dynamic_dashboard_service_spec.rb8
-rw-r--r--spec/lib/gitlab/metrics/dashboard/finder_spec.rb11
-rw-r--r--spec/lib/gitlab/metrics/dashboard/project_dashboard_service_spec.rb6
-rw-r--r--spec/lib/gitlab/metrics/dashboard/system_dashboard_service_spec.rb10
-rw-r--r--spec/support/helpers/metrics_dashboard_helpers.rb8
7 files changed, 48 insertions, 8 deletions
diff --git a/changelogs/unreleased/add-metrics-dashboard-permission-check.yml b/changelogs/unreleased/add-metrics-dashboard-permission-check.yml
new file mode 100644
index 00000000000..0ea2c4c8e41
--- /dev/null
+++ b/changelogs/unreleased/add-metrics-dashboard-permission-check.yml
@@ -0,0 +1,5 @@
+---
+title: Add permission check to metrics dashboards endpoint
+merge_request: 30017
+author:
+type: added
diff --git a/lib/gitlab/metrics/dashboard/base_service.rb b/lib/gitlab/metrics/dashboard/base_service.rb
index 90895eb237a..0628e82e592 100644
--- a/lib/gitlab/metrics/dashboard/base_service.rb
+++ b/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
diff --git a/spec/lib/gitlab/metrics/dashboard/dynamic_dashboard_service_spec.rb b/spec/lib/gitlab/metrics/dashboard/dynamic_dashboard_service_spec.rb
index eecd257b38d..79a78df44ae 100644
--- a/spec/lib/gitlab/metrics/dashboard/dynamic_dashboard_service_spec.rb
+++ b/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
diff --git a/spec/lib/gitlab/metrics/dashboard/finder_spec.rb b/spec/lib/gitlab/metrics/dashboard/finder_spec.rb
index b9a5ee9c2b3..d8ed54c0248 100644
--- a/spec/lib/gitlab/metrics/dashboard/finder_spec.rb
+++ b/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
diff --git a/spec/lib/gitlab/metrics/dashboard/project_dashboard_service_spec.rb b/spec/lib/gitlab/metrics/dashboard/project_dashboard_service_spec.rb
index 57d82421b5d..468e8ec9ef2 100644
--- a/spec/lib/gitlab/metrics/dashboard/project_dashboard_service_spec.rb
+++ b/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) }
diff --git a/spec/lib/gitlab/metrics/dashboard/system_dashboard_service_spec.rb b/spec/lib/gitlab/metrics/dashboard/system_dashboard_service_spec.rb
index 2af745bd4d7..13f22dd01c5 100644
--- a/spec/lib/gitlab/metrics/dashboard/system_dashboard_service_spec.rb
+++ b/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
diff --git a/spec/support/helpers/metrics_dashboard_helpers.rb b/spec/support/helpers/metrics_dashboard_helpers.rb
index 6de00eea474..1511a2f6b49 100644
--- a/spec/support/helpers/metrics_dashboard_helpers.rb
+++ b/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