diff --git a/app/helpers/sidebars_helper.rb b/app/helpers/sidebars_helper.rb index 862e727e0d1baaddc52061291a6a7970004d1860..78f89d1aea73733103c23a29360130e2dc99f707 100644 --- a/app/helpers/sidebars_helper.rb +++ b/app/helpers/sidebars_helper.rb @@ -416,7 +416,7 @@ def context_switcher_links if display_admin_area_link? links.append( - { title: s_('Navigation|Admin area'), link: admin_root_path, icon: 'admin' } + { title: s_('Navigation|Admin area'), link: admin_area_link, icon: 'admin' } ) end @@ -504,6 +504,10 @@ def terms_link Gitlab::CurrentSettings.terms ? '/-/users/terms' : nil end + def admin_area_link + admin_root_path + end + def display_admin_area_link? current_user&.can_admin_all_resources? end diff --git a/app/validators/json_schemas/member_role_permissions.json b/app/validators/json_schemas/member_role_permissions.json index cf42ece8ebffb896475c70e34fce3edb80d08c03..f66d982a30ec374b259501d6f0d70d0e5c5bbe8f 100644 --- a/app/validators/json_schemas/member_role_permissions.json +++ b/app/validators/json_schemas/member_role_permissions.json @@ -55,6 +55,9 @@ "manage_security_policy_link": { "type": "boolean" }, + "read_admin_cicd": { + "type": "boolean" + }, "read_admin_dashboard": { "type": "boolean" }, diff --git a/doc/api/graphql/reference/index.md b/doc/api/graphql/reference/index.md index b1fb1f542e7c07b5a1b0a924878176e2cbc6ec0e..3a33a9ef9ed3bd7f3f3945245d77603ae1a4fa17 100644 --- a/doc/api/graphql/reference/index.md +++ b/doc/api/graphql/reference/index.md @@ -40410,6 +40410,7 @@ Member role admin permission. | Value | Description | | ----- | ----------- | +| `READ_ADMIN_CICD` | Read-only access to admin runners settings page. | | `READ_ADMIN_DASHBOARD` | Read-only access to admin dashboard. | ### `MemberRolePermission` @@ -40435,6 +40436,7 @@ Member role permission. | `MANAGE_MERGE_REQUEST_SETTINGS` | Configure merge request settings at the group or project level. Group actions include managing merge checks and approval settings. Project actions include managing MR configurations, approval rules and settings, and branch targets. In order to enable Suggested reviewers, the "Manage project access tokens" custom permission needs to be enabled. | | `MANAGE_PROJECT_ACCESS_TOKENS` | Create, read, update, and delete project access tokens. When creating a token, users with this custom permission must select a role for that token that has the same or fewer permissions as the default role used as the base for the custom role. | | `MANAGE_SECURITY_POLICY_LINK` | Allows linking security policy projects. | +| `READ_ADMIN_CICD` | Read-only access to admin runners settings page. | | `READ_ADMIN_DASHBOARD` | Read-only access to admin dashboard. | | `READ_CODE` | Allows read-only access to the source code in the user interface. Does not allow users to edit or download repository archives, clone or pull repositories, view source code in an IDE, or view merge requests for private projects. You can download individual files because read-only access inherently grants the ability to make a local copy of the file. | | `READ_COMPLIANCE_DASHBOARD` | Read compliance capabilities including adherence, violations, and frameworks for groups and projects. | diff --git a/doc/user/custom_roles/abilities.md b/doc/user/custom_roles/abilities.md index d19bac3ba811aa25064edea97dbfe893d3a20424..3a89d3786ac6db40c47e073deb4854b174372c7c 100644 --- a/doc/user/custom_roles/abilities.md +++ b/doc/user/custom_roles/abilities.md @@ -27,6 +27,7 @@ Any dependencies are noted in the `Description` column for each permission. | Permission | Description | API Attribute | Scope | Introduced | |:-----------|:------------|:--------------|:------|:-----------| +| Read-only access to admin runners settings page | Read-only access to admin runners settings page | [`read_admin_cicd`](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/177233) | Instance | GitLab [17.9](https://gitlab.com/gitlab-org/gitlab/-/issues/507960) | | Read-only access to admin dashboard | Read-only access to admin dashboard | [`read_admin_dashboard`](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/171581) | Instance | GitLab [17.6](https://gitlab.com/gitlab-org/gitlab/-/issues/501549) | ## Code review workflow diff --git a/ee/app/controllers/ee/admin/runners_controller.rb b/ee/app/controllers/ee/admin/runners_controller.rb index 29a06e89608e7969e2a79a5df2048ff562889f29..3aad8185bdea3913dfbf99d4b2d2f3969bcfe42a 100644 --- a/ee/app/controllers/ee/admin/runners_controller.rb +++ b/ee/app/controllers/ee/admin/runners_controller.rb @@ -4,6 +4,7 @@ module EE module Admin module RunnersController extend ActiveSupport::Concern + extend ::Gitlab::Utils::Override prepended do before_action(only: [:index]) { push_licensed_feature(:runner_performance_insights) } @@ -16,6 +17,16 @@ module RunnersController def dashboard render_404 unless License.feature_available?(:runner_performance_insights) end + + private + + override :authenticate_admin! + def authenticate_admin! + return super unless action_name == 'index' + return if can?(current_user, :read_admin_cicd) + + super + end end end end diff --git a/ee/app/helpers/ee/sidebars_helper.rb b/ee/app/helpers/ee/sidebars_helper.rb index bf513d27ea230422590f5a9709c8ac0a539ed52d..78cfe0b1ca077119e0c769e26a2b254b058f8a35 100644 --- a/ee/app/helpers/ee/sidebars_helper.rb +++ b/ee/app/helpers/ee/sidebars_helper.rb @@ -79,11 +79,26 @@ def super_sidebar_context(user, group:, project:, panel:, panel_type:) def display_admin_area_link? return true if super - return false unless ::Feature.enabled?(:custom_ability_read_admin_dashboard, current_user) + if ::Feature.disabled?(:custom_ability_read_admin_dashboard, current_user) && + ::Feature.disabled?(:custom_ability_read_admin_cicd, current_user) + return false + end current_user&.can?(:access_admin_area) end + override :admin_area_link + def admin_area_link + has_access_to_dashboard = ::Feature.enabled?(:custom_ability_read_admin_dashboard, current_user) + + # if user does not have access to /admin (dashboard) but has access to /admin/runners then link them there + if ::Feature.enabled?(:custom_ability_read_admin_cicd, current_user) && !has_access_to_dashboard + return admin_runners_path + end + + super + end + def super_sidebar_default_pins(panel_type) case panel_type when 'group' diff --git a/ee/app/models/members/member_role.rb b/ee/app/models/members/member_role.rb index d331a18bcf695529b9454919e88be620b52c1bdf..3fdcfae092e2edd06edc3f7bdb93319aabd49933 100644 --- a/ee/app/models/members/member_role.rb +++ b/ee/app/models/members/member_role.rb @@ -140,7 +140,10 @@ def admin_related_permissions end def admin_related_role? - return false unless Feature.enabled?(:custom_ability_read_admin_dashboard, Feature.current_request) + if Feature.disabled?(:custom_ability_read_admin_dashboard, Feature.current_request) && + Feature.disabled?(:custom_ability_read_admin_cicd, Feature.current_request) + return false + end admin_related_permissions.present? end diff --git a/ee/app/policies/ee/global_policy.rb b/ee/app/policies/ee/global_policy.rb index 528afd56db71b37f57be5323457d2adb01f0cde0..e5bd07f89b558f0e80b8168e7ad5a5fe6aba9392 100644 --- a/ee/app/policies/ee/global_policy.rb +++ b/ee/app/policies/ee/global_policy.rb @@ -5,6 +5,8 @@ module GlobalPolicy extend ActiveSupport::Concern prepended do + include ::Gitlab::Utils::StrongMemoize + condition(:operations_dashboard_available) do License.feature_available?(:operations_dashboard) end @@ -124,8 +126,11 @@ module GlobalPolicy License.feature_available?(:remote_development) end - condition(:custom_role_enables_read_admin_dashboard) do - ::Authz::CustomAbility.allowed?(@user, :read_admin_dashboard) + MemberRole.all_customizable_admin_permission_keys.each do |ability| + desc "Admin custom role that enables #{ability.to_s.tr('_', ' ')}" + condition(:"custom_role_enables_#{ability}") do + custom_role_ability(@user).allowed?(ability) + end end rule { ~anonymous & remote_development_feature_licensed }.policy do @@ -214,6 +219,11 @@ module GlobalPolicy enable :access_admin_area enable :read_admin_dashboard end + + rule { custom_role_enables_read_admin_cicd }.policy do + enable :access_admin_area + enable :read_admin_cicd + end end def duo_chat @@ -228,5 +238,11 @@ def duo_chat_self_hosted? def self_hosted_models_available_for?(user) CloudConnector::AvailableServices.find_by_name(:self_hosted_models).allowed_for?(user) end + + def custom_role_ability(user) + strong_memoize_with(:custom_role_ability, user) do + ::Authz::CustomAbility.new(user) + end + end end end diff --git a/ee/config/custom_abilities/read_admin_cicd.yml b/ee/config/custom_abilities/read_admin_cicd.yml new file mode 100644 index 0000000000000000000000000000000000000000..913fde2b296bd47504474103aa85b91abc7d96f3 --- /dev/null +++ b/ee/config/custom_abilities/read_admin_cicd.yml @@ -0,0 +1,12 @@ +--- +title: Read-only access to admin runners settings page +name: read_admin_cicd +description: Read-only access to admin runners settings page +introduced_by_issue: https://gitlab.com/gitlab-org/gitlab/-/issues/507960 +introduced_by_mr: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/177233 +feature_category: admin +milestone: '17.9' +group_ability: false +project_ability: false +admin_ability: true +requirements: [] diff --git a/ee/config/feature_flags/wip/custom_ability_read_admin_cicd.yml b/ee/config/feature_flags/wip/custom_ability_read_admin_cicd.yml new file mode 100644 index 0000000000000000000000000000000000000000..bb8fa312742de58f2fde33d5ca907740bdcdc21a --- /dev/null +++ b/ee/config/feature_flags/wip/custom_ability_read_admin_cicd.yml @@ -0,0 +1,9 @@ +--- +name: custom_ability_read_admin_cicd +feature_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/507960 +introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/177233 +rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/512831 +milestone: '17.9' +group: group::authorization +type: wip +default_enabled: false diff --git a/ee/lib/ee/sidebars/admin/menus/ci_cd_menu.rb b/ee/lib/ee/sidebars/admin/menus/ci_cd_menu.rb new file mode 100644 index 0000000000000000000000000000000000000000..9f2a586b42e0164220251a6430594309223168bd --- /dev/null +++ b/ee/lib/ee/sidebars/admin/menus/ci_cd_menu.rb @@ -0,0 +1,22 @@ +# frozen_string_literal: true + +module EE + module Sidebars # rubocop:disable Gitlab/BoundedContexts -- overridden class is not inside a bounded context namespace + module Admin + module Menus + module CiCdMenu + extend ::Gitlab::Utils::Override + + override :render? + def render? + return super if ::Feature.disabled?(:custom_ability_read_admin_cicd, context.current_user) + + return true if context.current_user&.can?(:access_admin_area) + + super + end + end + end + end + end +end diff --git a/ee/spec/factories/member_roles.rb b/ee/spec/factories/member_roles.rb index d83438cdf651d00aef26cf7e58519caa17bd3251..6b2b1c30643cd0f03074d1ab299ddf1726f6919d 100644 --- a/ee/spec/factories/member_roles.rb +++ b/ee/spec/factories/member_roles.rb @@ -15,9 +15,15 @@ Gitlab::CustomRoles::Definition.all.each_value do |attributes| trait attributes[:name].to_sym do send(attributes[:name].to_sym) { true } + attributes.fetch(:requirements, []).each do |requirement| send(requirement.to_sym) { true } end + + if attributes[:admin_ability] + namespace { nil } + base_access_level { nil } + end end end diff --git a/ee/spec/helpers/sidebars_helper_spec.rb b/ee/spec/helpers/sidebars_helper_spec.rb index 665f264a83e897912b12c92e6521fc6f62a5aa0a..3ce6fe3ac1a6aa583b53c6d7ac2fbafa5731dd52 100644 --- a/ee/spec/helpers/sidebars_helper_spec.rb +++ b/ee/spec/helpers/sidebars_helper_spec.rb @@ -286,11 +286,15 @@ ] end - let_it_be(:admin_area_link) do + let_it_be(:link_to_admin_dashboard) do { title: s_('Navigation|Admin area'), link: '/admin', icon: 'admin' } end - subject(:super_sidebar_context) do + let_it_be(:link_to_admin_cicd) do + { title: s_('Navigation|Admin area'), link: '/admin/runners', icon: 'admin' } + end + + subject(:sidebar_context) do helper.super_sidebar_context(user, group: nil, project: nil, panel: panel, panel_type: panel_type) end @@ -303,34 +307,34 @@ context 'when user is not an admin' do it 'returns only the public links for a user' do - expect(super_sidebar_context[:context_switcher_links]).to eq(public_links_for_user) + expect(sidebar_context[:context_switcher_links]).to eq(public_links_for_user) end context 'when user is allowed to access_admin_area' do - before do - allow(user).to receive(:can?).and_call_original - - allow(user).to receive(:can?).with(:access_admin_area).and_return(true) - allow(user).to receive(:can_admin_all_resources?).and_return(false) + let(:with_link_to_admin_dashboard) { [*public_links_for_user, link_to_admin_dashboard] } + let(:with_link_to_admin_cicd) { [*public_links_for_user, link_to_admin_cicd] } + let(:without_link_to_admin_area) { public_links_for_user } + + where(:read_admin_dashboard_ff, :read_admin_cicd_ff, :links) do + false | false | ref(:without_link_to_admin_area) + true | false | ref(:with_link_to_admin_dashboard) + false | true | ref(:with_link_to_admin_cicd) + true | true | ref(:with_link_to_admin_dashboard) end - context 'when custom_ability_read_admin_dashboard FF is enabled' do - it 'returns public links and enter admin mode link' do - expect(super_sidebar_context[:context_switcher_links]).to eq([ - *public_links_for_user, admin_area_link - ]) - end - end - - context 'when custom_ability_read_admin_dashboard FF is disabled' do + with_them do before do - stub_feature_flags(custom_ability_read_admin_dashboard: false) + allow(user).to receive(:can?).and_call_original + + allow(user).to receive(:can?).with(:access_admin_area).and_return(true) + allow(user).to receive(:can_admin_all_resources?).and_return(false) + + stub_feature_flags(custom_ability_read_admin_dashboard: read_admin_dashboard_ff) + stub_feature_flags(custom_ability_read_admin_cicd: read_admin_cicd_ff) end - it 'returns only public links' do - expect(super_sidebar_context[:context_switcher_links]).to eq([ - *public_links_for_user - ]) + it 'returns the correct links' do + expect(sidebar_context[:context_switcher_links]).to eq(links) end end end diff --git a/ee/spec/lib/ee/sidebars/admin/menus/ci_cd_menu_spec.rb b/ee/spec/lib/ee/sidebars/admin/menus/ci_cd_menu_spec.rb new file mode 100644 index 0000000000000000000000000000000000000000..0c24a73dbc6ad87409c45968af9be8713e0367a1 --- /dev/null +++ b/ee/spec/lib/ee/sidebars/admin/menus/ci_cd_menu_spec.rb @@ -0,0 +1,41 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Sidebars::Admin::Menus::CiCdMenu, feature_category: :navigation do + let_it_be(:user) { build_stubbed(:user) } + + let(:context) { Sidebars::Context.new(current_user: user, container: nil) } + let(:menu) { described_class.new(context) } + + describe '#render?' do + subject(:render?) { menu.render? } + + before do + allow(user).to receive(:can_admin_all_resources?).and_return(false) + allow(user).to receive(:can?).with(:access_admin_area).and_return(can_access_admin_area) + end + + context 'when user is allowed to access_admin_area' do + let(:can_access_admin_area) { true } + + context 'when custom_ability_read_admin_cicd FF is enabled' do + it { is_expected.to be(true) } + end + + context 'when custom_ability_read_admin_cicd FF is disabled' do + before do + stub_feature_flags(custom_ability_read_admin_cicd: false) + end + + it { is_expected.to be(false) } + end + end + + context 'when user is not allowed to access_admin_area' do + let(:can_access_admin_area) { false } + + it { is_expected.to be(false) } + end + end +end diff --git a/ee/spec/models/members/member_role_spec.rb b/ee/spec/models/members/member_role_spec.rb index 4a950c6944f31b103b7a27b0b2b98978bc2694ab..965853851acd7a0dfd1e3e1e17f2ba807360505b 100644 --- a/ee/spec/models/members/member_role_spec.rb +++ b/ee/spec/models/members/member_role_spec.rb @@ -50,10 +50,32 @@ expect(member_role).not_to be_valid end - it 'is valid with read_admin_dashboard without base_access_level' do - admin_role = build(:member_role, :instance, read_admin_dashboard: true, base_access_level: nil) + context 'with nil base_access_level' do + where(:admin_ability) { MemberRole.all_customizable_admin_permission_keys } + + with_them do + context "when admin ability #{params[:admin_ability]} is enabled" do + let(:admin_role) { build(:member_role, admin_ability) } + + it "is valid" do + expect(admin_role.base_access_level).to be_nil + expect(admin_role).to be_valid + end + end + end + + context 'when no custom admin ability feature flag is enabled' do + let(:admin_role) { build(:member_role, :read_admin_dashboard) } - expect(admin_role).to be_valid + before do + stub_feature_flags(custom_ability_read_admin_dashboard: false, custom_ability_read_admin_cicd: false) + end + + it "is invalid" do + expect(admin_role.base_access_level).to be_nil + expect(admin_role).not_to be_valid + end + end end end diff --git a/ee/spec/models/preloaders/user_member_roles_for_admin_preloader_spec.rb b/ee/spec/models/preloaders/user_member_roles_for_admin_preloader_spec.rb index 75806bf0738160807b9da33c1b43739d73881172..85d112e230a7b9e951457a4728885f80f063ba0e 100644 --- a/ee/spec/models/preloaders/user_member_roles_for_admin_preloader_spec.rb +++ b/ee/spec/models/preloaders/user_member_roles_for_admin_preloader_spec.rb @@ -9,10 +9,7 @@ subject(:result) { described_class.new(user: user).execute } def create_member_role(ability, user) - create(:member_role, :admin).tap do |record| - record.assign_attributes(ability => true) - record.save! - + create(:member_role, ability).tap do |record| create(:user_member_role, user: user, member_role: record) end end diff --git a/ee/spec/policies/global_policy_spec.rb b/ee/spec/policies/global_policy_spec.rb index 147d966a5195604748f7ae6fa1cdf423976cfec1..9f6449aba3114e3c387ea5295287ad850de795fa 100644 --- a/ee/spec/policies/global_policy_spec.rb +++ b/ee/spec/policies/global_policy_spec.rb @@ -881,24 +881,31 @@ end context 'custom permissions' do - context 'when a user is assigned custom role with :read_admin_dashboard true' do - let_it_be(:role) { create(:member_role, :admin) } - let_it_be(:user_member_role) { create(:user_member_role, member_role: role, user: current_user) } + where(:custom_ability, :enabled_permissions) do + :read_admin_dashboard | %i[read_admin_dashboard access_admin_area] + :read_admin_cicd | %i[read_admin_cicd access_admin_area] + end - context 'when custom_roles feature is enabled' do - before do - stub_licensed_features(custom_roles: true) - end + with_them do + context 'when a user is assigned an admin custom role' do + let!(:role) { create(:member_role, custom_ability) } + let!(:user_member_role) { create(:user_member_role, member_role: role, user: current_user) } - it { is_expected.to be_allowed(:read_admin_dashboard, :access_admin_area) } - end + context 'when custom_roles feature is enabled' do + before do + stub_licensed_features(custom_roles: true) + end - context 'when custom_roles feature is disabled' do - before do - stub_licensed_features(custom_roles: false) + it { is_expected.to be_allowed(*enabled_permissions) } end - it { is_expected.to be_disallowed(:read_admin_dashboard, :access_admin_area) } + context 'when custom_roles feature is disabled' do + before do + stub_licensed_features(custom_roles: false) + end + + it { is_expected.to be_disallowed(*enabled_permissions) } + end end end end diff --git a/ee/spec/requests/admin/runners_controller_spec.rb b/ee/spec/requests/admin/runners_controller_spec.rb new file mode 100644 index 0000000000000000000000000000000000000000..0fa171e381872929fad93f2794689779b41a4e90 --- /dev/null +++ b/ee/spec/requests/admin/runners_controller_spec.rb @@ -0,0 +1,48 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Admin::RunnersController, feature_category: :fleet_visibility do + let_it_be(:non_admin_user) { create(:user) } + + subject { response } + + before do + stub_licensed_features(custom_roles: true) + + sign_in(user) + end + + describe 'GET #edit' do + let_it_be(:runner) { create(:ci_runner) } + + before do + get edit_admin_runner_path(runner) + end + + context 'with a non-admin user' do + let_it_be(:user) { non_admin_user } + + it { is_expected.to have_gitlab_http_status(:not_found) } + end + end + + describe 'GET #index' do + before do + get admin_runners_path + end + + context 'with a non-admin user' do + let_it_be(:user) { non_admin_user } + + it { is_expected.to have_gitlab_http_status(:not_found) } + + context 'when assigned an admin custom role with read_admin_cicd enabled' do + let_it_be(:role) { create(:member_role, :read_admin_cicd) } + let_it_be(:user_member_role) { create(:user_member_role, member_role: role, user: user) } + + it { is_expected.to have_gitlab_http_status(:ok) } + end + end + end +end diff --git a/lib/sidebars/admin/menus/ci_cd_menu.rb b/lib/sidebars/admin/menus/ci_cd_menu.rb index e6e8e77a4488cbb1161a69bf4ce2d81d75a524a0..84d78b314e8001feead6dcecadcea9fb2ae2000c 100644 --- a/lib/sidebars/admin/menus/ci_cd_menu.rb +++ b/lib/sidebars/admin/menus/ci_cd_menu.rb @@ -45,3 +45,5 @@ def jobs_menu_item end end end + +Sidebars::Admin::Menus::CiCdMenu.prepend_mod