diff --git a/app/controllers/projects/environments_controller.rb b/app/controllers/projects/environments_controller.rb
index 5f4d88c57e9b61314c9b4ff7925ab289f20cd23b..4d774123ef1a488b24be4d10bbef1c967901ed75 100644
--- a/app/controllers/projects/environments_controller.rb
+++ b/app/controllers/projects/environments_controller.rb
@@ -10,7 +10,7 @@ class Projects::EnvironmentsController < Projects::ApplicationController
push_frontend_feature_flag(:prometheus_computed_alerts)
end
- before_action :authorize_read_environment!
+ before_action :authorize_read_environment!, except: [:metrics, :additional_metrics, :metrics_dashboard, :metrics_redirect]
before_action :authorize_create_environment!, only: [:new, :create]
before_action :authorize_stop_environment!, only: [:stop]
before_action :authorize_update_environment!, only: [:edit, :update, :cancel_auto_stop]
diff --git a/app/helpers/projects_helper.rb b/app/helpers/projects_helper.rb
index d743ea6aeea9ee774ee56b38357517735e373ec6..79546212bccc829ecdbd485a6f70b017c33b740e 100644
--- a/app/helpers/projects_helper.rb
+++ b/app/helpers/projects_helper.rb
@@ -410,7 +410,7 @@ def get_project_nav_tabs(project, current_user)
nav_tabs << :pipelines
end
- if can?(current_user, :read_environment, project) || can?(current_user, :read_cluster, project)
+ if can_view_operations_tab?(current_user, project)
nav_tabs << :operations
end
@@ -438,22 +438,29 @@ def external_nav_tabs(project)
def tab_ability_map
{
- environments: :read_environment,
- milestones: :read_milestone,
- snippets: :read_snippet,
- settings: :admin_project,
- builds: :read_build,
- clusters: :read_cluster,
- serverless: :read_cluster,
- error_tracking: :read_sentry_issue,
- alert_management: :read_alert_management_alert,
- labels: :read_label,
- issues: :read_issue,
- project_members: :read_project_member,
- wiki: :read_wiki
+ environments: :read_environment,
+ metrics_dashboards: :metrics_dashboard,
+ milestones: :read_milestone,
+ snippets: :read_snippet,
+ settings: :admin_project,
+ builds: :read_build,
+ clusters: :read_cluster,
+ serverless: :read_cluster,
+ error_tracking: :read_sentry_issue,
+ alert_management: :read_alert_management_alert,
+ labels: :read_label,
+ issues: :read_issue,
+ project_members: :read_project_member,
+ wiki: :read_wiki
}
end
+ def can_view_operations_tab?(current_user, project)
+ [:read_environment, :read_cluster, :metrics_dashboard].any? do |ability|
+ can?(current_user, ability, project)
+ end
+ end
+
def search_tab_ability_map
@search_tab_ability_map ||= tab_ability_map.merge(
blobs: :download_code,
diff --git a/app/policies/project_policy.rb b/app/policies/project_policy.rb
index a24c0471d6ced6d20cdf8edbf48fcbfd67c06bca..44de17121deb854ffba320d4968650de5e9f784e 100644
--- a/app/policies/project_policy.rb
+++ b/app/policies/project_policy.rb
@@ -278,7 +278,6 @@ class ProjectPolicy < BasePolicy
rule { can?(:metrics_dashboard) }.policy do
enable :read_prometheus
- enable :read_environment
enable :read_deployment
end
@@ -429,27 +428,11 @@ class ProjectPolicy < BasePolicy
rule { builds_disabled | repository_disabled }.policy do
prevent(*create_read_update_admin_destroy(:build))
prevent(*create_read_update_admin_destroy(:pipeline_schedule))
+ prevent(*create_read_update_admin_destroy(:environment))
prevent(*create_read_update_admin_destroy(:cluster))
prevent(*create_read_update_admin_destroy(:deployment))
end
- # Enabling `read_environment` specifically for the condition of `metrics_dashboard_allowed` is
- # necessary due to the route for metrics dashboard requiring an environment id.
- # This will be addressed in https://gitlab.com/gitlab-org/gitlab/-/issues/213833 when
- # environments and metrics are decoupled and these rules will be removed.
-
- rule { (builds_disabled | repository_disabled) & ~metrics_dashboard_allowed}.policy do
- prevent(*create_read_update_admin_destroy(:environment))
- end
-
- rule { (builds_disabled | repository_disabled) & metrics_dashboard_allowed}.policy do
- prevent :create_environment
- prevent :update_environment
- prevent :admin_environment
- prevent :destroy_environment
- enable :read_environment
- end
-
# There's two separate cases when builds_disabled is true:
# 1. When internal CI is disabled - builds_disabled && internal_builds_disabled
# - We do not prevent the user from accessing Pipelines to allow them to access external CI
diff --git a/app/views/layouts/nav/sidebar/_project.html.haml b/app/views/layouts/nav/sidebar/_project.html.haml
index a67860e8e2eb3128c61ea3c951ca90f93a4f8f0b..c738493507f493633dc5c9d2c0977dfb4bd68b6b 100644
--- a/app/views/layouts/nav/sidebar/_project.html.haml
+++ b/app/views/layouts/nav/sidebar/_project.html.haml
@@ -216,7 +216,7 @@
= _('Operations')
%li.divider.fly-out-top-item
- - if project_nav_tab? :environments
+ - if project_nav_tab? :metrics_dashboards
= nav_link(controller: :environments, action: [:metrics, :metrics_redirect]) do
= link_to metrics_project_environments_path(@project), title: _('Metrics'), class: 'shortcuts-metrics', data: { qa_selector: 'operations_metrics_link' } do
%span
diff --git a/spec/controllers/projects/environments_controller_spec.rb b/spec/controllers/projects/environments_controller_spec.rb
index 56fff2771ec92bf69d82cfec01d426de1d5f2eb8..cd4896ede43281da7663850c0d5152c4077a7f91 100644
--- a/spec/controllers/projects/environments_controller_spec.rb
+++ b/spec/controllers/projects/environments_controller_spec.rb
@@ -354,6 +354,19 @@
expect(response).to redirect_to(environment_metrics_path(environment))
end
+ context 'with anonymous user and public dashboard visibility' do
+ let(:project) { create(:project, :public) }
+ let(:user) { create(:user) }
+
+ it 'redirects successfully' do
+ project.project_feature.update!(metrics_dashboard_access_level: ProjectFeature::ENABLED)
+
+ get :metrics_redirect, params: { namespace_id: project.namespace, project_id: project }
+
+ expect(response).to redirect_to(environment_metrics_path(environment))
+ end
+ end
+
context 'when there are no environments' do
let(:environment) { }
@@ -422,6 +435,19 @@
get :metrics, params: environment_params
end
end
+
+ context 'with anonymous user and public dashboard visibility' do
+ let(:project) { create(:project, :public) }
+ let(:user) { create(:user) }
+
+ it 'returns success' do
+ project.project_feature.update!(metrics_dashboard_access_level: ProjectFeature::ENABLED)
+
+ get :metrics, params: environment_params
+
+ expect(response).to have_gitlab_http_status(:ok)
+ end
+ end
end
describe 'GET #additional_metrics' do
@@ -497,6 +523,26 @@
get :metrics, params: environment_params
end
end
+
+ context 'with anonymous user and public dashboard visibility' do
+ let(:project) { create(:project, :public) }
+ let(:user) { create(:user) }
+
+ it 'does not fail' do
+ allow(environment)
+ .to receive(:additional_metrics)
+ .and_return({
+ success: true,
+ data: {},
+ last_update: 42
+ })
+ project.project_feature.update!(metrics_dashboard_access_level: ProjectFeature::ENABLED)
+
+ additional_metrics(window_params)
+
+ expect(response).to have_gitlab_http_status(:ok)
+ end
+ end
end
describe 'GET #metrics_dashboard' do
@@ -673,6 +719,17 @@
it_behaves_like 'dashboard can be specified'
it_behaves_like 'dashboard can be embedded'
+ context 'with anonymous user and public dashboard visibility' do
+ let(:project) { create(:project, :public) }
+ let(:user) { create(:user) }
+
+ before do
+ project.project_feature.update!(metrics_dashboard_access_level: ProjectFeature::ENABLED)
+ end
+
+ it_behaves_like 'the default dashboard'
+ end
+
context 'permissions' do
before do
allow(controller).to receive(:can?).and_return true
diff --git a/spec/helpers/projects_helper_spec.rb b/spec/helpers/projects_helper_spec.rb
index 189ab1a8354f8787f8e97404271ef99864e9a986..27dc8707bd8a9d9bd40436921f406c1d27ca529b 100644
--- a/spec/helpers/projects_helper_spec.rb
+++ b/spec/helpers/projects_helper_spec.rb
@@ -5,6 +5,9 @@
describe ProjectsHelper do
include ProjectForksHelper
+ let_it_be(:project) { create(:project) }
+ let_it_be(:user) { create(:user) }
+
describe '#project_incident_management_setting' do
let(:project) { create(:project) }
@@ -500,6 +503,23 @@
end
end
+ describe '#can_view_operations_tab?' do
+ before do
+ allow(helper).to receive(:current_user).and_return(user)
+ end
+
+ subject { helper.send(:can_view_operations_tab?, user, project) }
+
+ [:read_environment, :read_cluster, :metrics_dashboard].each do |ability|
+ it 'includes operations tab' do
+ allow(helper).to receive(:can?).and_return(false)
+ allow(helper).to receive(:can?).with(user, ability, project).and_return(true)
+
+ is_expected.to be(true)
+ end
+ end
+ end
+
describe '#show_projects' do
let(:projects) do
create(:project)
diff --git a/spec/policies/project_policy_spec.rb b/spec/policies/project_policy_spec.rb
index 09d54eb9df63d3a4ae82b3370f53b441beda1726..7c98d8cc856d55db6f8d2afacf59709ecb37cba2 100644
--- a/spec/policies/project_policy_spec.rb
+++ b/spec/policies/project_policy_spec.rb
@@ -218,41 +218,16 @@ def set_access_level(access_level)
project.project_feature.update(builds_access_level: ProjectFeature::DISABLED)
end
- context 'without metrics_dashboard_allowed' do
- before do
- project.project_feature.update(metrics_dashboard_access_level: ProjectFeature::DISABLED)
- end
-
- it 'disallows all permissions except pipeline when the feature is disabled' do
- builds_permissions = [
- :create_build, :read_build, :update_build, :admin_build, :destroy_build,
- :create_pipeline_schedule, :read_pipeline_schedule, :update_pipeline_schedule, :admin_pipeline_schedule, :destroy_pipeline_schedule,
- :create_environment, :read_environment, :update_environment, :admin_environment, :destroy_environment,
- :create_cluster, :read_cluster, :update_cluster, :admin_cluster, :destroy_cluster,
- :create_deployment, :read_deployment, :update_deployment, :admin_deployment, :destroy_deployment
- ]
-
- expect_disallowed(*builds_permissions)
- end
- end
-
- context 'with metrics_dashboard_allowed' do
- before do
- project.project_feature.update(metrics_dashboard_access_level: ProjectFeature::ENABLED)
- end
+ it 'disallows all permissions except pipeline when the feature is disabled' do
+ builds_permissions = [
+ :create_build, :read_build, :update_build, :admin_build, :destroy_build,
+ :create_pipeline_schedule, :read_pipeline_schedule, :update_pipeline_schedule, :admin_pipeline_schedule, :destroy_pipeline_schedule,
+ :create_environment, :read_environment, :update_environment, :admin_environment, :destroy_environment,
+ :create_cluster, :read_cluster, :update_cluster, :admin_cluster, :destroy_cluster,
+ :create_deployment, :read_deployment, :update_deployment, :admin_deployment, :destroy_deployment
+ ]
- it 'disallows all permissions except pipeline and read_environment when the feature is disabled' do
- builds_permissions = [
- :create_build, :read_build, :update_build, :admin_build, :destroy_build,
- :create_pipeline_schedule, :read_pipeline_schedule, :update_pipeline_schedule, :admin_pipeline_schedule, :destroy_pipeline_schedule,
- :create_environment, :update_environment, :admin_environment, :destroy_environment,
- :create_cluster, :read_cluster, :update_cluster, :admin_cluster, :destroy_cluster,
- :create_deployment, :read_deployment, :update_deployment, :admin_deployment, :destroy_deployment
- ]
-
- expect_disallowed(*builds_permissions)
- expect_allowed(:read_environment)
- end
+ expect_disallowed(*builds_permissions)
end
end
@@ -277,49 +252,20 @@ def set_access_level(access_level)
context 'repository feature' do
subject { described_class.new(owner, project) }
- before do
+ it 'disallows all permissions when the feature is disabled' do
project.project_feature.update(repository_access_level: ProjectFeature::DISABLED)
- end
-
- context 'without metrics_dashboard_allowed' do
- before do
- project.project_feature.update(metrics_dashboard_access_level: ProjectFeature::DISABLED)
- end
- it 'disallows all permissions when the feature is disabled' do
- repository_permissions = [
- :create_pipeline, :update_pipeline, :admin_pipeline, :destroy_pipeline,
- :create_build, :read_build, :update_build, :admin_build, :destroy_build,
- :create_pipeline_schedule, :read_pipeline_schedule, :update_pipeline_schedule, :admin_pipeline_schedule, :destroy_pipeline_schedule,
- :create_environment, :read_environment, :update_environment, :admin_environment, :destroy_environment,
- :create_cluster, :read_cluster, :update_cluster, :admin_cluster,
- :create_deployment, :read_deployment, :update_deployment, :admin_deployment, :destroy_deployment,
- :destroy_release
- ]
-
- expect_disallowed(*repository_permissions)
- end
- end
-
- context 'with metrics_dashboard_allowed' do
- before do
- project.project_feature.update(metrics_dashboard_access_level: ProjectFeature::ENABLED)
- end
-
- it 'disallows all permissions when the feature is disabled' do
- repository_permissions = [
- :create_pipeline, :update_pipeline, :admin_pipeline, :destroy_pipeline,
- :create_build, :read_build, :update_build, :admin_build, :destroy_build,
- :create_pipeline_schedule, :read_pipeline_schedule, :update_pipeline_schedule, :admin_pipeline_schedule, :destroy_pipeline_schedule,
- :create_environment, :update_environment, :admin_environment, :destroy_environment,
- :create_cluster, :read_cluster, :update_cluster, :admin_cluster,
- :create_deployment, :read_deployment, :update_deployment, :admin_deployment, :destroy_deployment,
- :destroy_release
- ]
-
- expect_disallowed(*repository_permissions)
- expect_allowed(:read_environment)
- end
+ repository_permissions = [
+ :create_pipeline, :update_pipeline, :admin_pipeline, :destroy_pipeline,
+ :create_build, :read_build, :update_build, :admin_build, :destroy_build,
+ :create_pipeline_schedule, :read_pipeline_schedule, :update_pipeline_schedule, :admin_pipeline_schedule, :destroy_pipeline_schedule,
+ :create_environment, :read_environment, :update_environment, :admin_environment, :destroy_environment,
+ :create_cluster, :read_cluster, :update_cluster, :admin_cluster,
+ :create_deployment, :read_deployment, :update_deployment, :admin_deployment, :destroy_deployment,
+ :destroy_release
+ ]
+
+ expect_disallowed(*repository_permissions)
end
end