From 62f492abd3790d4611f58854b06f9ae674afc6ba Mon Sep 17 00:00:00 2001
From: Eugie Limpin <elimpin@gitlab.com>
Date: Thu, 9 Jan 2025 16:43:08 +0800
Subject: [PATCH 01/13] Create read_admin_cicd custom ability

---
 .../json_schemas/member_role_permissions.json       |  3 +++
 doc/api/graphql/reference/index.md                  |  2 ++
 doc/user/custom_roles/abilities.md                  |  1 +
 ee/app/models/members/member_role.rb                |  5 ++++-
 ee/config/custom_abilities/read_admin_cicd.yml      | 13 +++++++++++++
 .../wip/custom_ability_read_admin_cicd.yml          |  9 +++++++++
 ee/spec/models/members/member_role_spec.rb          |  6 ++++++
 7 files changed, 38 insertions(+), 1 deletion(-)
 create mode 100644 ee/config/custom_abilities/read_admin_cicd.yml
 create mode 100644 ee/config/feature_flags/wip/custom_ability_read_admin_cicd.yml

diff --git a/app/validators/json_schemas/member_role_permissions.json b/app/validators/json_schemas/member_role_permissions.json
index cf42ece8ebffb8..f66d982a30ec37 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 b1fb1f542e7c07..3a33a9ef9ed3bd 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 |
 | ----- | ----------- |
+| <a id="memberroleadminpermissionread_admin_cicd"></a>`READ_ADMIN_CICD` | Read-only access to admin runners settings page. |
 | <a id="memberroleadminpermissionread_admin_dashboard"></a>`READ_ADMIN_DASHBOARD` | Read-only access to admin dashboard. |
 
 ### `MemberRolePermission`
@@ -40435,6 +40436,7 @@ Member role permission.
 | <a id="memberrolepermissionmanage_merge_request_settings"></a>`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. |
 | <a id="memberrolepermissionmanage_project_access_tokens"></a>`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. |
 | <a id="memberrolepermissionmanage_security_policy_link"></a>`MANAGE_SECURITY_POLICY_LINK` | Allows linking security policy projects. |
+| <a id="memberrolepermissionread_admin_cicd"></a>`READ_ADMIN_CICD` | Read-only access to admin runners settings page. |
 | <a id="memberrolepermissionread_admin_dashboard"></a>`READ_ADMIN_DASHBOARD` | Read-only access to admin dashboard. |
 | <a id="memberrolepermissionread_code"></a>`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. |
 | <a id="memberrolepermissionread_compliance_dashboard"></a>`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 d19bac3ba811aa..3a89d3786ac6db 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/models/members/member_role.rb b/ee/app/models/members/member_role.rb
index d331a18bcf6955..3fdcfae092e2ed 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/config/custom_abilities/read_admin_cicd.yml b/ee/config/custom_abilities/read_admin_cicd.yml
new file mode 100644
index 00000000000000..2b06f3df8a9c35
--- /dev/null
+++ b/ee/config/custom_abilities/read_admin_cicd.yml
@@ -0,0 +1,13 @@
+---
+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: []
+available_from_access_level:
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 00000000000000..55913013fafafa
--- /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:
+milestone: '17.8'
+group: group::authorization
+type: wip
+default_enabled: false
diff --git a/ee/spec/models/members/member_role_spec.rb b/ee/spec/models/members/member_role_spec.rb
index 4a950c6944f31b..f9cd60887a0bef 100644
--- a/ee/spec/models/members/member_role_spec.rb
+++ b/ee/spec/models/members/member_role_spec.rb
@@ -55,6 +55,12 @@
 
           expect(admin_role).to be_valid
         end
+
+        it 'is valid with read_admin_cicd without base_access_level' do
+          admin_role = build(:member_role, :instance, read_admin_cicd: true, base_access_level: nil)
+
+          expect(admin_role).to be_valid
+        end
       end
 
       context 'when updating an old record' do
-- 
GitLab


From cf37fb75103a7345d47edf6e8d9a3a149b102010 Mon Sep 17 00:00:00 2001
From: Eugie Limpin <elimpin@gitlab.com>
Date: Thu, 9 Jan 2025 14:24:58 +0800
Subject: [PATCH 02/13] Show admin area link and admin CICD menu when
 read_admin_cicd is enabled

---
 app/helpers/sidebars_helper.rb                |  6 ++-
 ee/app/helpers/ee/sidebars_helper.rb          | 17 ++++++-
 ee/app/policies/ee/global_policy.rb           | 20 +++++++-
 ee/lib/ee/sidebars/admin/menus/ci_cd_menu.rb  | 22 +++++++++
 ee/spec/helpers/sidebars_helper_spec.rb       | 48 ++++++++++---------
 .../sidebars/admin/menus/ci_cd_menu_spec.rb   | 48 +++++++++++++++++++
 ee/spec/policies/global_policy_spec.rb        | 33 ++++++++-----
 lib/sidebars/admin/menus/ci_cd_menu.rb        |  2 +
 8 files changed, 157 insertions(+), 39 deletions(-)
 create mode 100644 ee/lib/ee/sidebars/admin/menus/ci_cd_menu.rb
 create mode 100644 ee/spec/lib/ee/sidebars/admin/menus/ci_cd_menu_spec.rb

diff --git a/app/helpers/sidebars_helper.rb b/app/helpers/sidebars_helper.rb
index 862e727e0d1baa..78f89d1aea7373 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/ee/app/helpers/ee/sidebars_helper.rb b/ee/app/helpers/ee/sidebars_helper.rb
index bf513d27ea2304..78cfe0b1ca0771 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/policies/ee/global_policy.rb b/ee/app/policies/ee/global_policy.rb
index 528afd56db71b3..e5bd07f89b558f 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/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 00000000000000..b5988f0bbf2765
--- /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 unless ::Feature.enabled?(: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/helpers/sidebars_helper_spec.rb b/ee/spec/helpers/sidebars_helper_spec.rb
index 665f264a83e897..3ce6fe3ac1a6aa 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 00000000000000..e38375948f393d
--- /dev/null
+++ b/ee/spec/lib/ee/sidebars/admin/menus/ci_cd_menu_spec.rb
@@ -0,0 +1,48 @@
+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe Sidebars::Admin::Menus::CiCdMenu, feature_category: :navigation do
+  let(:user) { build_stubbed(:user) }
+  let(:context) { Sidebars::Context.new(current_user: user, container: nil) }
+
+  describe '#render' do
+    context 'with a regular user' do
+      subject(:menu) { described_class.new(context) }
+
+      context 'when user is allowed to access_admin_area' do
+        before do
+          allow(user).to receive(:can?).with(:access_admin_area).and_return(true)
+          allow(user).to receive(:can_admin_all_resources?).and_return(false)
+        end
+
+        context 'when custom_ability_read_admin_cicd FF is enabled' do
+          it 'renders' do
+            expect(menu.render?).to be(true)
+          end
+        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 'does not render' do
+            expect(menu.render?).to be(false)
+          end
+        end
+      end
+
+      context 'when user can not access admin area' do
+        before do
+          allow(user).to receive(:can?).with(:access_admin_area).and_return(false)
+          allow(user).to receive(:can_admin_all_resources?).and_return(false)
+        end
+
+        it 'does not render' do
+          expect(menu.render?).to be(false)
+        end
+      end
+    end
+  end
+end
diff --git a/ee/spec/policies/global_policy_spec.rb b/ee/spec/policies/global_policy_spec.rb
index 147d966a519560..8bb05bcbc19a83 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, :admin, 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/lib/sidebars/admin/menus/ci_cd_menu.rb b/lib/sidebars/admin/menus/ci_cd_menu.rb
index e6e8e77a4488cb..84d78b314e8001 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
-- 
GitLab


From 0bff9c06cbaa3bd0ab5f1253666bf93541e4d969 Mon Sep 17 00:00:00 2001
From: Eugie Limpin <elimpin@gitlab.com>
Date: Fri, 10 Jan 2025 14:39:34 +0800
Subject: [PATCH 03/13] Allow access to /admin/runners when user has
 read_admin_cicd permission

---
 .../ee/admin/runners_controller.rb            | 10 +++++++
 .../requests/admin/runners_controller_spec.rb | 30 +++++++++++++++++++
 2 files changed, 40 insertions(+)
 create mode 100644 ee/spec/requests/admin/runners_controller_spec.rb

diff --git a/ee/app/controllers/ee/admin/runners_controller.rb b/ee/app/controllers/ee/admin/runners_controller.rb
index 29a06e89608e79..3c87bdee3fba44 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,15 @@ module RunnersController
       def dashboard
         render_404 unless License.feature_available?(:runner_performance_insights)
       end
+
+      private
+
+      override :authenticate_admin!
+      def authenticate_admin!
+        return if can?(current_user, :read_admin_cicd)
+
+        super
+      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 00000000000000..b94571b35993cd
--- /dev/null
+++ b/ee/spec/requests/admin/runners_controller_spec.rb
@@ -0,0 +1,30 @@
+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe Admin::RunnersController, feature_category: :fleet_visibility do
+  let_it_be(:user) { create(:user) }
+
+  describe 'GET #index' do
+    before do
+      stub_licensed_features(custom_roles: true)
+
+      sign_in(user)
+
+      get admin_runners_path
+    end
+
+    subject { response }
+
+    context 'with a non-admin user' do
+      it { is_expected.to have_gitlab_http_status(:not_found) }
+    end
+
+    context 'with a non-admin user assigned an admin custom role with read_admin_cicd enabled' do
+      let_it_be(:role) { create(:member_role, :admin, :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
-- 
GitLab


From 17cebb09307ad5270af5c8481268690d956883d7 Mon Sep 17 00:00:00 2001
From: Eugie Limpin <elimpin@gitlab.com>
Date: Mon, 13 Jan 2025 14:44:56 +0800
Subject: [PATCH 04/13] Set namespace and base_access_level to nil for admin
 member role traits

---
 ee/spec/factories/member_roles.rb               |  6 ++++++
 ee/spec/models/members/member_role_spec.rb      | 17 +++++++++--------
 ...ser_member_roles_for_admin_preloader_spec.rb |  5 +----
 ee/spec/policies/global_policy_spec.rb          |  2 +-
 .../requests/admin/runners_controller_spec.rb   |  2 +-
 5 files changed, 18 insertions(+), 14 deletions(-)

diff --git a/ee/spec/factories/member_roles.rb b/ee/spec/factories/member_roles.rb
index d83438cdf651d0..6b2b1c30643cd0 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/models/members/member_role_spec.rb b/ee/spec/models/members/member_role_spec.rb
index f9cd60887a0bef..1dbfc6927c1b79 100644
--- a/ee/spec/models/members/member_role_spec.rb
+++ b/ee/spec/models/members/member_role_spec.rb
@@ -50,16 +50,17 @@
           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 }
 
-          expect(admin_role).to be_valid
-        end
-
-        it 'is valid with read_admin_cicd without base_access_level' do
-          admin_role = build(:member_role, :instance, read_admin_cicd: true, base_access_level: nil)
+          with_them do
+            it "is valid when admin ability #{params[:admin_ability]} is enabled" do
+              admin_role = build(:member_role, admin_ability)
 
-          expect(admin_role).to be_valid
+              expect(admin_role.base_access_level).to be_nil
+              expect(admin_role).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 75806bf0738160..85d112e230a7b9 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 8bb05bcbc19a83..9f6449aba3114e 100644
--- a/ee/spec/policies/global_policy_spec.rb
+++ b/ee/spec/policies/global_policy_spec.rb
@@ -888,7 +888,7 @@
 
     with_them do
       context 'when a user is assigned an admin custom role' do
-        let!(:role) { create(:member_role, :admin, custom_ability) }
+        let!(:role) { create(:member_role, custom_ability) }
         let!(:user_member_role) { create(:user_member_role, member_role: role, user: current_user) }
 
         context 'when custom_roles feature is enabled' do
diff --git a/ee/spec/requests/admin/runners_controller_spec.rb b/ee/spec/requests/admin/runners_controller_spec.rb
index b94571b35993cd..c0f3d1b1f7e53f 100644
--- a/ee/spec/requests/admin/runners_controller_spec.rb
+++ b/ee/spec/requests/admin/runners_controller_spec.rb
@@ -21,7 +21,7 @@
     end
 
     context 'with a non-admin user assigned an admin custom role with read_admin_cicd enabled' do
-      let_it_be(:role) { create(:member_role, :admin, :read_admin_cicd) }
+      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) }
-- 
GitLab


From 25290749681722d8a8fd1e0aed0b0bf8d1377350 Mon Sep 17 00:00:00 2001
From: Eugie Limpin <elimpin@gitlab.com>
Date: Mon, 13 Jan 2025 15:01:35 +0800
Subject: [PATCH 05/13] Update feature flag definition file: milestone and
 rollout issue

---
 .../feature_flags/wip/custom_ability_read_admin_cicd.yml      | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

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
index 55913013fafafa..bb8fa312742de5 100644
--- a/ee/config/feature_flags/wip/custom_ability_read_admin_cicd.yml
+++ b/ee/config/feature_flags/wip/custom_ability_read_admin_cicd.yml
@@ -2,8 +2,8 @@
 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:
-milestone: '17.8'
+rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/512831
+milestone: '17.9'
 group: group::authorization
 type: wip
 default_enabled: false
-- 
GitLab


From f16403fc6cbc4b5882bd88ea7757cfb16e13dfc6 Mon Sep 17 00:00:00 2001
From: Eugie Limpin <elimpin@gitlab.com>
Date: Mon, 13 Jan 2025 15:42:12 +0800
Subject: [PATCH 06/13] Only allow access to runners index page when
 read_admin_cicd is enabled

---
 .../ee/admin/runners_controller.rb            |  1 +
 .../requests/admin/runners_controller_spec.rb | 24 +++++++++++++++----
 2 files changed, 20 insertions(+), 5 deletions(-)

diff --git a/ee/app/controllers/ee/admin/runners_controller.rb b/ee/app/controllers/ee/admin/runners_controller.rb
index 3c87bdee3fba44..3aad8185bdea39 100644
--- a/ee/app/controllers/ee/admin/runners_controller.rb
+++ b/ee/app/controllers/ee/admin/runners_controller.rb
@@ -22,6 +22,7 @@ def dashboard
 
       override :authenticate_admin!
       def authenticate_admin!
+        return super unless action_name == 'index'
         return if can?(current_user, :read_admin_cicd)
 
         super
diff --git a/ee/spec/requests/admin/runners_controller_spec.rb b/ee/spec/requests/admin/runners_controller_spec.rb
index c0f3d1b1f7e53f..94cb75183fc728 100644
--- a/ee/spec/requests/admin/runners_controller_spec.rb
+++ b/ee/spec/requests/admin/runners_controller_spec.rb
@@ -5,17 +5,31 @@
 RSpec.describe Admin::RunnersController, feature_category: :fleet_visibility do
   let_it_be(:user) { create(:user) }
 
-  describe 'GET #index' do
+  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
-      stub_licensed_features(custom_roles: true)
+      get edit_admin_runner_path(runner)
+    end
 
-      sign_in(user)
+    context 'with a non-admin user' do
+      it { is_expected.to have_gitlab_http_status(:not_found) }
+    end
+  end
 
+  describe 'GET #index' do
+    before do
       get admin_runners_path
     end
 
-    subject { response }
-
     context 'with a non-admin user' do
       it { is_expected.to have_gitlab_http_status(:not_found) }
     end
-- 
GitLab


From c8368e2393b76cdccba9be3d796113a7841c0eaa Mon Sep 17 00:00:00 2001
From: Eugie Limpin <elimpin@gitlab.com>
Date: Mon, 13 Jan 2025 17:10:00 +0800
Subject: [PATCH 07/13] Remove available_from_access_level field for
 read_admin_cicd ability

---
 ee/config/custom_abilities/read_admin_cicd.yml | 1 -
 1 file changed, 1 deletion(-)

diff --git a/ee/config/custom_abilities/read_admin_cicd.yml b/ee/config/custom_abilities/read_admin_cicd.yml
index 2b06f3df8a9c35..913fde2b296bd4 100644
--- a/ee/config/custom_abilities/read_admin_cicd.yml
+++ b/ee/config/custom_abilities/read_admin_cicd.yml
@@ -10,4 +10,3 @@ group_ability: false
 project_ability: false
 admin_ability: true
 requirements: []
-available_from_access_level:
-- 
GitLab


From 8667b409b5261f2a61e6f8aa58ae67d76e06900f Mon Sep 17 00:00:00 2001
From: Eugie Limpin <elimpin@gitlab.com>
Date: Mon, 13 Jan 2025 18:53:55 +0800
Subject: [PATCH 08/13] Add spec for when no custom admin ability feature flag
 is enabled

---
 ee/spec/models/members/member_role_spec.rb | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/ee/spec/models/members/member_role_spec.rb b/ee/spec/models/members/member_role_spec.rb
index 1dbfc6927c1b79..13251ae7d5bdce 100644
--- a/ee/spec/models/members/member_role_spec.rb
+++ b/ee/spec/models/members/member_role_spec.rb
@@ -61,6 +61,17 @@
               expect(admin_role).to be_valid
             end
           end
+
+          context 'when no custom admin ability feature flag is enabled' do
+            it "is invalid" do
+              stub_feature_flags(custom_ability_read_admin_dashboard: false, custom_ability_read_admin_cicd: false)
+
+              admin_role = build(:member_role, :read_admin_dashboard)
+
+              expect(admin_role.base_access_level).to be_nil
+              expect(admin_role).not_to be_valid
+            end
+          end
         end
       end
 
-- 
GitLab


From 8806cba2b4773acb7d4499af72d2abd0a2a75244 Mon Sep 17 00:00:00 2001
From: Eugie Limpin <elimpin@gitlab.com>
Date: Wed, 15 Jan 2025 10:47:57 +0800
Subject: [PATCH 09/13] Refactor specs to improve readability

---
 .../requests/admin/runners_controller_spec.rb    | 16 ++++++++++------
 1 file changed, 10 insertions(+), 6 deletions(-)

diff --git a/ee/spec/requests/admin/runners_controller_spec.rb b/ee/spec/requests/admin/runners_controller_spec.rb
index 94cb75183fc728..0fa171e3818729 100644
--- a/ee/spec/requests/admin/runners_controller_spec.rb
+++ b/ee/spec/requests/admin/runners_controller_spec.rb
@@ -3,7 +3,7 @@
 require 'spec_helper'
 
 RSpec.describe Admin::RunnersController, feature_category: :fleet_visibility do
-  let_it_be(:user) { create(:user) }
+  let_it_be(:non_admin_user) { create(:user) }
 
   subject { response }
 
@@ -21,6 +21,8 @@
     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
@@ -31,14 +33,16 @@
     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
 
-    context 'with a non-admin user 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) }
+      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) }
+        it { is_expected.to have_gitlab_http_status(:ok) }
+      end
     end
   end
 end
-- 
GitLab


From 83e17b4fe5665462f6b82403f71edb67defed875 Mon Sep 17 00:00:00 2001
From: Eugie Limpin <elimpin@gitlab.com>
Date: Wed, 15 Jan 2025 10:51:54 +0800
Subject: [PATCH 10/13] Refactor specs to improve readability

---
 ee/spec/models/members/member_role_spec.rb | 18 +++++++++++-------
 1 file changed, 11 insertions(+), 7 deletions(-)

diff --git a/ee/spec/models/members/member_role_spec.rb b/ee/spec/models/members/member_role_spec.rb
index 13251ae7d5bdce..965853851acd7a 100644
--- a/ee/spec/models/members/member_role_spec.rb
+++ b/ee/spec/models/members/member_role_spec.rb
@@ -54,20 +54,24 @@
           where(:admin_ability) { MemberRole.all_customizable_admin_permission_keys }
 
           with_them do
-            it "is valid when admin ability #{params[:admin_ability]} is enabled" do
-              admin_role = build(:member_role, admin_ability)
+            context "when admin ability #{params[:admin_ability]} is enabled" do
+              let(:admin_role) { build(:member_role, admin_ability) }
 
-              expect(admin_role.base_access_level).to be_nil
-              expect(admin_role).to be_valid
+              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
-            it "is invalid" do
-              stub_feature_flags(custom_ability_read_admin_dashboard: false, custom_ability_read_admin_cicd: false)
+            let(:admin_role) { build(:member_role, :read_admin_dashboard) }
 
-              admin_role = build(:member_role, :read_admin_dashboard)
+            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
-- 
GitLab


From 125915c87258bd7c7e9f6664f6567b7ce8440ccb Mon Sep 17 00:00:00 2001
From: Eugie Limpin <elimpin@gitlab.com>
Date: Wed, 15 Jan 2025 10:56:16 +0800
Subject: [PATCH 11/13] Use `if Feature.disabled?` to improve readability

---
 ee/lib/ee/sidebars/admin/menus/ci_cd_menu.rb | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/ee/lib/ee/sidebars/admin/menus/ci_cd_menu.rb b/ee/lib/ee/sidebars/admin/menus/ci_cd_menu.rb
index b5988f0bbf2765..9f2a586b42e016 100644
--- a/ee/lib/ee/sidebars/admin/menus/ci_cd_menu.rb
+++ b/ee/lib/ee/sidebars/admin/menus/ci_cd_menu.rb
@@ -9,7 +9,7 @@ module CiCdMenu
 
           override :render?
           def render?
-            return super unless ::Feature.enabled?(:custom_ability_read_admin_cicd, context.current_user)
+            return super if ::Feature.disabled?(:custom_ability_read_admin_cicd, context.current_user)
 
             return true if context.current_user&.can?(:access_admin_area)
 
-- 
GitLab


From 5d739be6be7110adc0a1ac2a133e9e92a270e9de Mon Sep 17 00:00:00 2001
From: Eugie Limpin <elimpin@gitlab.com>
Date: Wed, 15 Jan 2025 11:14:32 +0800
Subject: [PATCH 12/13] Call render? as part of the subject

---
 .../sidebars/admin/menus/ci_cd_menu_spec.rb   | 20 ++++++++-----------
 1 file changed, 8 insertions(+), 12 deletions(-)

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
index e38375948f393d..42684752f95097 100644
--- 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
@@ -6,10 +6,12 @@
   let(:user) { build_stubbed(:user) }
   let(:context) { Sidebars::Context.new(current_user: user, container: nil) }
 
-  describe '#render' do
-    context 'with a regular user' do
-      subject(:menu) { described_class.new(context) }
+  describe '#render?' do
+    let(:menu) { described_class.new(context) }
+
+    subject(:render?) { menu.render? }
 
+    context 'with a regular user' do
       context 'when user is allowed to access_admin_area' do
         before do
           allow(user).to receive(:can?).with(:access_admin_area).and_return(true)
@@ -17,9 +19,7 @@
         end
 
         context 'when custom_ability_read_admin_cicd FF is enabled' do
-          it 'renders' do
-            expect(menu.render?).to be(true)
-          end
+          it { is_expected.to be(true) }
         end
 
         context 'when custom_ability_read_admin_cicd FF is disabled' do
@@ -27,9 +27,7 @@
             stub_feature_flags(custom_ability_read_admin_cicd: false)
           end
 
-          it 'does not render' do
-            expect(menu.render?).to be(false)
-          end
+          it { is_expected.to be(false) }
         end
       end
 
@@ -39,9 +37,7 @@
           allow(user).to receive(:can_admin_all_resources?).and_return(false)
         end
 
-        it 'does not render' do
-          expect(menu.render?).to be(false)
-        end
+        it { is_expected.to be(false) }
       end
     end
   end
-- 
GitLab


From 4b56e2b3589917873210034dada724cf0855bb6e Mon Sep 17 00:00:00 2001
From: Eugie Limpin <elimpin@gitlab.com>
Date: Wed, 15 Jan 2025 11:22:52 +0800
Subject: [PATCH 13/13] Refactor specs to improve readability

---
 .../sidebars/admin/menus/ci_cd_menu_spec.rb   | 41 +++++++++----------
 1 file changed, 19 insertions(+), 22 deletions(-)

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
index 42684752f95097..0c24a73dbc6ad8 100644
--- 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
@@ -3,42 +3,39 @@
 require 'spec_helper'
 
 RSpec.describe Sidebars::Admin::Menus::CiCdMenu, feature_category: :navigation do
-  let(:user) { build_stubbed(:user) }
+  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
-    let(:menu) { described_class.new(context) }
-
     subject(:render?) { menu.render? }
 
-    context 'with a regular user' do
-      context 'when user is allowed to access_admin_area' do
-        before do
-          allow(user).to receive(:can?).with(:access_admin_area).and_return(true)
-          allow(user).to receive(:can_admin_all_resources?).and_return(false)
-        end
-
-        context 'when custom_ability_read_admin_cicd FF is enabled' do
-          it { is_expected.to be(true) }
-        end
+    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 custom_ability_read_admin_cicd FF is disabled' do
-          before do
-            stub_feature_flags(custom_ability_read_admin_cicd: false)
-          end
+    context 'when user is allowed to access_admin_area' do
+      let(:can_access_admin_area) { true }
 
-          it { is_expected.to be(false) }
-        end
+      context 'when custom_ability_read_admin_cicd FF is enabled' do
+        it { is_expected.to be(true) }
       end
 
-      context 'when user can not access admin area' do
+      context 'when custom_ability_read_admin_cicd FF is disabled' do
         before do
-          allow(user).to receive(:can?).with(:access_admin_area).and_return(false)
-          allow(user).to receive(:can_admin_all_resources?).and_return(false)
+          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
-- 
GitLab