From 5216e8f044c457f4d506d1b5425043b2ef5a3f05 Mon Sep 17 00:00:00 2001
From: imand3r <ianderson@gitlab.com>
Date: Thu, 16 Jan 2025 21:25:36 +0000
Subject: [PATCH 1/5] Create read_admin_subscription custom admin ability

---
 .../json_schemas/member_role_permissions.json |  3 ++
 doc/user/custom_roles/abilities.md            |  8 ++++
 .../admin/subscriptions_controller.rb         | 13 +++++++
 ee/app/helpers/ee/sidebars_helper.rb          | 33 +++++++++-------
 ee/app/helpers/license_helper.rb              |  2 +-
 ee/app/policies/ee/global_policy.rb           |  6 +++
 .../read_admin_subscription.yml               | 12 ++++++
 .../sidebars/admin/menus/subscription_menu.rb |  5 +++
 ee/spec/helpers/license_helper_spec.rb        | 14 +++++++
 ee/spec/helpers/sidebars_helper_spec.rb       | 38 +++++++++----------
 .../admin/menus/subscription_menu_spec.rb     |  3 +-
 ee/spec/policies/global_policy_spec.rb        |  5 ++-
 .../read_admin_subscription/request_spec.rb   | 26 +++++++++++++
 lib/sidebars/admin/base_menu.rb               |  9 ++++-
 .../menus/admin_menus_shared_examples.rb      | 14 ++++++-
 15 files changed, 151 insertions(+), 40 deletions(-)
 create mode 100644 ee/config/custom_abilities/read_admin_subscription.yml
 create mode 100644 ee/spec/requests/custom_roles/read_admin_subscription/request_spec.rb

diff --git a/app/validators/json_schemas/member_role_permissions.json b/app/validators/json_schemas/member_role_permissions.json
index ea67ed8c1a7dbd..94da66e9b2f160 100644
--- a/app/validators/json_schemas/member_role_permissions.json
+++ b/app/validators/json_schemas/member_role_permissions.json
@@ -70,6 +70,9 @@
     "read_admin_monitoring": {
       "type": "boolean"
     },
+    "read_admin_subscription": {
+      "type": "boolean"
+    },
     "read_code": {
       "type": "boolean"
     },
diff --git a/doc/user/custom_roles/abilities.md b/doc/user/custom_roles/abilities.md
index 58fd6de1cd29b2..e942c8935901a2 100644
--- a/doc/user/custom_roles/abilities.md
+++ b/doc/user/custom_roles/abilities.md
@@ -23,6 +23,14 @@ Some permissions depend on other permissions.
 For example, the `admin_vulnerability` permission requires you to also include the `read_vulnerability` permission.
 Any dependencies are noted in the `Description` column for each permission.
 
+## Admin
+
+| 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) |
+| View subscription details | Read subscription details in the Admin Area | [`read_admin_subscription`](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/178230) | Instance | GitLab [17.9](https://gitlab.com/gitlab-org/gitlab/-/issues/507961) |
+
 ## Code review workflow
 
 | Permission | Description | API Attribute | Scope | Introduced |
diff --git a/ee/app/controllers/admin/subscriptions_controller.rb b/ee/app/controllers/admin/subscriptions_controller.rb
index aeb4adb3e25e87..005b7bc6d9d4b7 100644
--- a/ee/app/controllers/admin/subscriptions_controller.rb
+++ b/ee/app/controllers/admin/subscriptions_controller.rb
@@ -2,8 +2,21 @@
 
 # EE:Self Managed
 class Admin::SubscriptionsController < Admin::ApplicationController
+  extend ::Gitlab::Utils::Override
+
   respond_to :html
 
   feature_category :plan_provisioning
   urgency :low
+
+  private
+
+  override :authenticate_admin!
+  def authenticate_admin!
+    return super unless action_name == 'show'
+
+    return if current_user.can?(:read_admin_subscription)
+
+    super
+  end
 end
diff --git a/ee/app/helpers/ee/sidebars_helper.rb b/ee/app/helpers/ee/sidebars_helper.rb
index 78cfe0b1ca0771..df72662dd72f83 100644
--- a/ee/app/helpers/ee/sidebars_helper.rb
+++ b/ee/app/helpers/ee/sidebars_helper.rb
@@ -3,6 +3,7 @@
 module EE
   module SidebarsHelper
     extend ::Gitlab::Utils::Override
+    include ::Gitlab::Utils::StrongMemoize
 
     override :project_sidebar_context_data
     def project_sidebar_context_data(project, user, current_ref, **args)
@@ -75,28 +76,34 @@ def super_sidebar_context(user, group:, project:, panel:, panel_type:)
 
     private
 
+    def custom_role_grants_admin_access?
+      return false unless current_user
+      return false unless ::Feature.enabled?(:custom_ability_read_admin_dashboard, current_user)
+
+      ::Authz::Admin.new(current_user).permitted.any?
+    end
+    strong_memoize_attr :custom_role_grants_admin_access?
+
     override :display_admin_area_link?
     def display_admin_area_link?
       return true if super
 
-      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)
+      custom_role_grants_admin_access?
     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
+      return super unless custom_role_grants_admin_access?
+
+      # If user does not have access to /admin (dashboard) but has access to other admin resources
+      # then link them to the first one they have access to
+      if current_user.can?(:read_admin_cicd)
+        admin_runners_path
+      elsif current_user.can?(:read_admin_subscription)
+        admin_subscription_path
+      else
+        super
       end
-
-      super
     end
 
     def super_sidebar_default_pins(panel_type)
diff --git a/ee/app/helpers/license_helper.rb b/ee/app/helpers/license_helper.rb
index 4c3e451f302843..67c95dc073694a 100644
--- a/ee/app/helpers/license_helper.rb
+++ b/ee/app/helpers/license_helper.rb
@@ -61,7 +61,7 @@ def cloud_license_view_data
       customers_portal_url: subscription_portal_manage_url,
       free_trial_path: new_trial_url,
       has_active_license: (has_active_license? ? 'true' : 'false'),
-      license_remove_path: admin_license_path,
+      license_remove_path: (current_user.can?(:destroy_licenses) ? admin_license_path : ''),
       subscription_sync_path: sync_seat_link_admin_license_path,
       congratulation_svg_path: image_path('illustrations/cloud-check-sm.svg'),
       license_usage_file_path: admin_license_usage_export_path(format: :csv)
diff --git a/ee/app/policies/ee/global_policy.rb b/ee/app/policies/ee/global_policy.rb
index 53b4066efc666e..23fb4164a56d6d 100644
--- a/ee/app/policies/ee/global_policy.rb
+++ b/ee/app/policies/ee/global_policy.rb
@@ -228,6 +228,12 @@ module GlobalPolicy
         enable :access_admin_area
         enable :read_admin_cicd
       end
+
+      rule { custom_role_enables_read_admin_subscription }.policy do
+        enable :read_admin_subscription
+        enable :read_billable_member
+        enable :read_licenses
+      end
     end
 
     def duo_chat
diff --git a/ee/config/custom_abilities/read_admin_subscription.yml b/ee/config/custom_abilities/read_admin_subscription.yml
new file mode 100644
index 00000000000000..627dea65424744
--- /dev/null
+++ b/ee/config/custom_abilities/read_admin_subscription.yml
@@ -0,0 +1,12 @@
+---
+title: View subscription details
+name: read_admin_subscription
+description: Read subscription details in the Admin Area
+introduced_by_issue: https://gitlab.com/gitlab-org/gitlab/-/issues/507961
+introduced_by_mr: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/178230
+feature_category: admin
+milestone: '17.9'
+admin_ability: true
+group_ability: false
+project_ability: false
+requirements: []
diff --git a/ee/lib/sidebars/admin/menus/subscription_menu.rb b/ee/lib/sidebars/admin/menus/subscription_menu.rb
index 32ec5f68eab808..0a1117a5eb67ca 100644
--- a/ee/lib/sidebars/admin/menus/subscription_menu.rb
+++ b/ee/lib/sidebars/admin/menus/subscription_menu.rb
@@ -28,6 +28,11 @@ def extra_container_html_options
         def active_routes
           { controller: :subscriptions }
         end
+
+        override :render_with_custom_abilities
+        def render_with_custom_abilities
+          %i[read_admin_subscription]
+        end
       end
     end
   end
diff --git a/ee/spec/helpers/license_helper_spec.rb b/ee/spec/helpers/license_helper_spec.rb
index bb9d74cb42ac74..606c882af600da 100644
--- a/ee/spec/helpers/license_helper_spec.rb
+++ b/ee/spec/helpers/license_helper_spec.rb
@@ -80,9 +80,13 @@ def stub_default_url_options(host: "localhost", protocol: "http", port: nil, scr
   end
 
   describe '#cloud_license_view_data' do
+    let(:current_user) { build(:user) }
+
     before do
       allow(helper).to receive(:subscription_portal_manage_url).and_return('subscriptions_manage_url')
       allow(helper).to receive(:new_trial_url).and_return('new_trial_url')
+      allow(helper).to receive(:current_user).and_return(current_user)
+      allow(current_user).to receive(:can?).with(:destroy_licenses).and_return(true)
     end
 
     context 'when there is a current license' do
@@ -116,6 +120,16 @@ def stub_default_url_options(host: "localhost", protocol: "http", port: nil, scr
                                                        license_usage_file_path: admin_license_usage_export_path(format: :csv) })
       end
     end
+
+    context 'when the current user cannot destroy licenses' do
+      before do
+        allow(current_user).to receive(:can?).with(:destroy_licenses).and_return(false)
+      end
+
+      it 'returns the data for the view without the license_remove_path set' do
+        expect(helper.cloud_license_view_data).to include(license_remove_path: '')
+      end
+    end
   end
 
   describe '#show_promotions?' do
diff --git a/ee/spec/helpers/sidebars_helper_spec.rb b/ee/spec/helpers/sidebars_helper_spec.rb
index 7bc1c16fea0261..a526ade4093552 100644
--- a/ee/spec/helpers/sidebars_helper_spec.rb
+++ b/ee/spec/helpers/sidebars_helper_spec.rb
@@ -292,14 +292,6 @@
       ]
     end
 
-    let_it_be(:link_to_admin_dashboard) do
-      { title: s_('Navigation|Admin area'), link: '/admin', icon: 'admin' }
-    end
-
-    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
@@ -317,30 +309,34 @@
       end
 
       context 'when user is allowed to access_admin_area' do
-        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)
+        where(:read_admin_dashboard_ff, :admin_ability, :link) do
+          false | nil                      | nil
+          true  | nil                      | nil
+          true  | :admin_unknown           | '/admin'
+          false | :admin_unknown           | nil
+          true  | :read_admin_cicd         | '/admin/runners'
+          true  | :read_admin_subscription | '/admin/subscription'
         end
 
         with_them do
           before do
-            allow(user).to receive(:can?).and_call_original
+            allow_next_instance_of(::Authz::Admin) do |instance|
+              allow(instance).to receive(:permitted).and_return([admin_ability]) if admin_ability
+            end
 
-            allow(user).to receive(:can?).with(:access_admin_area).and_return(true)
+            allow(user).to receive(:can?).and_call_original
+            allow(user).to receive(:can?).with(admin_ability).and_return(true) if admin_ability
             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 the correct links' do
-            expect(sidebar_context[:context_switcher_links]).to eq(links)
+            if link
+              expect(sidebar_context[:context_switcher_links]).to include(hash_including(link: link))
+            else
+              expect(sidebar_context[:context_switcher_links]).not_to include(hash_including(link: '/admin'))
+            end
           end
         end
       end
diff --git a/ee/spec/lib/sidebars/admin/menus/subscription_menu_spec.rb b/ee/spec/lib/sidebars/admin/menus/subscription_menu_spec.rb
index 972d120f4f88de..7e9e2651abf603 100644
--- a/ee/spec/lib/sidebars/admin/menus/subscription_menu_spec.rb
+++ b/ee/spec/lib/sidebars/admin/menus/subscription_menu_spec.rb
@@ -6,7 +6,8 @@
   it_behaves_like 'Admin menu',
     link: '/admin/subscription',
     title: s_('Admin|Subscription'),
-    icon: 'license'
+    icon: 'license',
+    custom_ability: :read_admin_subscription
 
   it_behaves_like 'Admin menu without sub menus', active_routes: { controller: :subscriptions }
 end
diff --git a/ee/spec/policies/global_policy_spec.rb b/ee/spec/policies/global_policy_spec.rb
index 5bce129e4196cd..3fef3b89f4ac3f 100644
--- a/ee/spec/policies/global_policy_spec.rb
+++ b/ee/spec/policies/global_policy_spec.rb
@@ -882,8 +882,9 @@
 
   context 'custom permissions' do
     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]
+      :read_admin_cicd         | %i[read_admin_cicd access_admin_area]
+      :read_admin_dashboard    | %i[read_admin_dashboard access_admin_area]
+      :read_admin_subscription | %i[read_admin_subscription read_billable_member read_licenses]
     end
 
     with_them do
diff --git a/ee/spec/requests/custom_roles/read_admin_subscription/request_spec.rb b/ee/spec/requests/custom_roles/read_admin_subscription/request_spec.rb
new file mode 100644
index 00000000000000..c2b6a2e9a13ae9
--- /dev/null
+++ b/ee/spec/requests/custom_roles/read_admin_subscription/request_spec.rb
@@ -0,0 +1,26 @@
+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe 'User with read_admin_subscription custom role', feature_category: :system_access do
+  let_it_be(:user) { create(:user) }
+  let_it_be(:role) { create(:member_role, :read_admin_subscription) }
+  let_it_be(:user_member_role) { create(:user_member_role, member_role: role, user: user) }
+
+  before do
+    stub_licensed_features(custom_roles: true)
+
+    sign_in(user)
+  end
+
+  describe Admin::SubscriptionsController do
+    describe "#show" do
+      it 'user has access via a custom role' do
+        get admin_subscription_path
+
+        expect(response).to have_gitlab_http_status(:ok)
+        expect(response).to render_template(:show)
+      end
+    end
+  end
+end
diff --git a/lib/sidebars/admin/base_menu.rb b/lib/sidebars/admin/base_menu.rb
index 897a193f67236a..e1eb13e3f95122 100644
--- a/lib/sidebars/admin/base_menu.rb
+++ b/lib/sidebars/admin/base_menu.rb
@@ -6,8 +6,15 @@ class BaseMenu < ::Sidebars::Menu
       override :render?
       def render?
         return false unless context.current_user
+        return true if context.current_user.can_admin_all_resources?
 
-        context.current_user.can_admin_all_resources?
+        render_with_custom_abilities.any? { |ability| context.current_user.can?(ability) }
+      end
+
+      private
+
+      def render_with_custom_abilities
+        []
       end
     end
   end
diff --git a/spec/support/shared_examples/lib/sidebars/admin/menus/admin_menus_shared_examples.rb b/spec/support/shared_examples/lib/sidebars/admin/menus/admin_menus_shared_examples.rb
index 4168d8675356d8..1a7176bdab8bd1 100644
--- a/spec/support/shared_examples/lib/sidebars/admin/menus/admin_menus_shared_examples.rb
+++ b/spec/support/shared_examples/lib/sidebars/admin/menus/admin_menus_shared_examples.rb
@@ -1,6 +1,6 @@
 # frozen_string_literal: true
 
-RSpec.shared_examples 'Admin menu' do |link:, title:, icon:, separated: false|
+RSpec.shared_examples 'Admin menu' do |link:, title:, icon:, separated: false, custom_ability: nil|
   let_it_be(:user) { build(:user, :admin) }
 
   before do
@@ -39,6 +39,18 @@
         expect(described_class.new(Sidebars::Context.new(current_user: build(:user),
           container: nil)).render?).to be false
       end
+
+      if custom_ability
+        context 'when a custom ability allows access' do
+          let_it_be(:user) { create(:user) }
+          let_it_be(:role) { create(:member_role, custom_ability) }
+          let_it_be(:user_member_role) { create(:user_member_role, member_role: role, user: user) }
+
+          it 'renders' do
+            expect(subject.render?).to be true
+          end
+        end
+      end
     end
 
     context 'when user is not logged in' do
-- 
GitLab


From 41fa04a1776c4e4c21580e3a858fbc5706cf69eb Mon Sep 17 00:00:00 2001
From: imand3r <ianderson@gitlab.com>
Date: Sat, 18 Jan 2025 22:22:38 +0000
Subject: [PATCH 2/5] Add feature flag

---
 .../controllers/admin/subscriptions_controller.rb  | 14 +++++---------
 ee/app/helpers/ee/sidebars_helper.rb               |  1 -
 ee/app/policies/ee/global_policy.rb                |  7 ++++---
 .../wip/custom_ability_read_admin_subscription.yml |  9 +++++++++
 ee/spec/helpers/sidebars_helper_spec.rb            | 14 +++++---------
 ee/spec/policies/global_policy_spec.rb             |  3 +++
 6 files changed, 26 insertions(+), 22 deletions(-)
 create mode 100644 ee/config/feature_flags/wip/custom_ability_read_admin_subscription.yml

diff --git a/ee/app/controllers/admin/subscriptions_controller.rb b/ee/app/controllers/admin/subscriptions_controller.rb
index 005b7bc6d9d4b7..c218c64059526a 100644
--- a/ee/app/controllers/admin/subscriptions_controller.rb
+++ b/ee/app/controllers/admin/subscriptions_controller.rb
@@ -2,21 +2,17 @@
 
 # EE:Self Managed
 class Admin::SubscriptionsController < Admin::ApplicationController
-  extend ::Gitlab::Utils::Override
-
   respond_to :html
 
   feature_category :plan_provisioning
   urgency :low
 
-  private
-
-  override :authenticate_admin!
-  def authenticate_admin!
-    return super unless action_name == 'show'
+  skip_before_action :authenticate_admin!, only: :show
+  before_action :authenticate_read_subscription!, only: :show
 
-    return if current_user.can?(:read_admin_subscription)
+  private
 
-    super
+  def authenticate_read_subscription!
+    render_404 unless current_user.can?(:read_admin_subscription)
   end
 end
diff --git a/ee/app/helpers/ee/sidebars_helper.rb b/ee/app/helpers/ee/sidebars_helper.rb
index df72662dd72f83..dd7c83bf97e5d9 100644
--- a/ee/app/helpers/ee/sidebars_helper.rb
+++ b/ee/app/helpers/ee/sidebars_helper.rb
@@ -78,7 +78,6 @@ def super_sidebar_context(user, group:, project:, panel:, panel_type:)
 
     def custom_role_grants_admin_access?
       return false unless current_user
-      return false unless ::Feature.enabled?(:custom_ability_read_admin_dashboard, current_user)
 
       ::Authz::Admin.new(current_user).permitted.any?
     end
diff --git a/ee/app/policies/ee/global_policy.rb b/ee/app/policies/ee/global_policy.rb
index 23fb4164a56d6d..a2d8a2d8130c9d 100644
--- a/ee/app/policies/ee/global_policy.rb
+++ b/ee/app/policies/ee/global_policy.rb
@@ -147,14 +147,15 @@ module GlobalPolicy
       end
 
       rule { admin }.policy do
-        enable :read_licenses
         enable :destroy_licenses
+        enable :manage_subscription
+        enable :read_admin_subscription
         enable :read_all_geo
         enable :read_all_workspaces
-        enable :manage_subscription
+        enable :read_cloud_connector_status
         enable :read_jobs_statistics
+        enable :read_licenses
         enable :read_runner_usage
-        enable :read_cloud_connector_status
       end
 
       rule { admin & user_allowed_to_manage_self_hosted_models_settings }.policy do
diff --git a/ee/config/feature_flags/wip/custom_ability_read_admin_subscription.yml b/ee/config/feature_flags/wip/custom_ability_read_admin_subscription.yml
new file mode 100644
index 00000000000000..143e554b0b0fda
--- /dev/null
+++ b/ee/config/feature_flags/wip/custom_ability_read_admin_subscription.yml
@@ -0,0 +1,9 @@
+---
+name: custom_ability_read_admin_subscription
+feature_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/507961
+introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/178230
+rollout_issue_url:
+milestone: '17.9'
+group: group::authorization
+type: wip
+default_enabled: false
diff --git a/ee/spec/helpers/sidebars_helper_spec.rb b/ee/spec/helpers/sidebars_helper_spec.rb
index a526ade4093552..0ec522902c5fdb 100644
--- a/ee/spec/helpers/sidebars_helper_spec.rb
+++ b/ee/spec/helpers/sidebars_helper_spec.rb
@@ -309,13 +309,11 @@
       end
 
       context 'when user is allowed to access_admin_area' do
-        where(:read_admin_dashboard_ff, :admin_ability, :link) do
-          false | nil                      | nil
-          true  | nil                      | nil
-          true  | :admin_unknown           | '/admin'
-          false | :admin_unknown           | nil
-          true  | :read_admin_cicd         | '/admin/runners'
-          true  | :read_admin_subscription | '/admin/subscription'
+        where(:admin_ability, :link) do
+          nil                      | nil
+          :admin_unknown           | '/admin'
+          :read_admin_cicd         | '/admin/runners'
+          :read_admin_subscription | '/admin/subscription'
         end
 
         with_them do
@@ -327,8 +325,6 @@
             allow(user).to receive(:can?).and_call_original
             allow(user).to receive(:can?).with(admin_ability).and_return(true) if admin_ability
             allow(user).to receive(:can_admin_all_resources?).and_return(false)
-
-            stub_feature_flags(custom_ability_read_admin_dashboard: read_admin_dashboard_ff)
           end
 
           it 'returns the correct links' do
diff --git a/ee/spec/policies/global_policy_spec.rb b/ee/spec/policies/global_policy_spec.rb
index 3fef3b89f4ac3f..6109f2959b6d71 100644
--- a/ee/spec/policies/global_policy_spec.rb
+++ b/ee/spec/policies/global_policy_spec.rb
@@ -76,6 +76,7 @@
   it { is_expected.to be_disallowed(:read_all_workspaces) }
   it { is_expected.to be_disallowed(:manage_subscription) }
   it { is_expected.to be_disallowed(:read_cloud_connector_status) }
+  it { is_expected.to be_disallowed(:read_admin_subscription) }
 
   context 'when admin mode enabled', :enable_admin_mode do
     it { expect(described_class.new(admin, [user])).to be_allowed(:read_licenses) }
@@ -84,6 +85,7 @@
     it { expect(described_class.new(admin, [user])).to be_allowed(:read_all_workspaces) }
     it { expect(described_class.new(admin, [user])).to be_allowed(:manage_subscription) }
     it { expect(described_class.new(admin, [user])).to be_allowed(:read_cloud_connector_status) }
+    it { expect(described_class.new(admin, [user])).to be_allowed(:read_admin_subscription) }
   end
 
   context 'when admin mode disabled' do
@@ -93,6 +95,7 @@
     it { expect(described_class.new(admin, [user])).to be_disallowed(:read_all_workspaces) }
     it { expect(described_class.new(admin, [user])).to be_disallowed(:manage_subscription) }
     it { expect(described_class.new(admin, [user])).to be_disallowed(:read_cloud_connector_status) }
+    it { expect(described_class.new(admin, [user])).to be_disallowed(:read_admin_subscription) }
   end
 
   shared_examples 'analytics policy' do |action|
-- 
GitLab


From 35058792627432fc7a8fef03136ba50b0bdfb616 Mon Sep 17 00:00:00 2001
From: imand3r <ianderson@gitlab.com>
Date: Thu, 23 Jan 2025 21:03:31 +0000
Subject: [PATCH 3/5] Apply suggestions

---
 ee/app/helpers/ee/sidebars_helper.rb          |  1 +
 ...custom_ability_read_admin_subscription.yml |  2 +-
 .../sidebars/admin/menus/subscription_menu.rb |  6 ++---
 ee/spec/helpers/license_helper_spec.rb        |  5 ++---
 ee/spec/helpers/sidebars_helper_spec.rb       |  1 +
 .../admin/menus/admin_overview_menu_spec.rb   |  7 +-----
 .../sidebars/admin/menus/ci_cd_menu_spec.rb   |  2 +-
 .../admin/menus/subscription_menu_spec.rb     |  2 +-
 .../menus/admin_menus_shared_examples.rb      | 22 +++++++++++++++++++
 lib/sidebars/admin/base_menu.rb               |  7 +++---
 spec/helpers/sidebars_helper_spec.rb          |  4 ++--
 .../menus/admin_menus_shared_examples.rb      | 16 ++------------
 12 files changed, 40 insertions(+), 35 deletions(-)
 create mode 100644 ee/spec/support/shared_examples/lib/sidebars/admin/menus/admin_menus_shared_examples.rb

diff --git a/ee/app/helpers/ee/sidebars_helper.rb b/ee/app/helpers/ee/sidebars_helper.rb
index dd7c83bf97e5d9..975253d48b1721 100644
--- a/ee/app/helpers/ee/sidebars_helper.rb
+++ b/ee/app/helpers/ee/sidebars_helper.rb
@@ -93,6 +93,7 @@ def display_admin_area_link?
     override :admin_area_link
     def admin_area_link
       return super unless custom_role_grants_admin_access?
+      return super if current_user.can?(:read_admin_dashboard)
 
       # If user does not have access to /admin (dashboard) but has access to other admin resources
       # then link them to the first one they have access to
diff --git a/ee/config/feature_flags/wip/custom_ability_read_admin_subscription.yml b/ee/config/feature_flags/wip/custom_ability_read_admin_subscription.yml
index 143e554b0b0fda..f3a494188e0819 100644
--- a/ee/config/feature_flags/wip/custom_ability_read_admin_subscription.yml
+++ b/ee/config/feature_flags/wip/custom_ability_read_admin_subscription.yml
@@ -2,7 +2,7 @@
 name: custom_ability_read_admin_subscription
 feature_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/507961
 introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/178230
-rollout_issue_url:
+rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/514810
 milestone: '17.9'
 group: group::authorization
 type: wip
diff --git a/ee/lib/sidebars/admin/menus/subscription_menu.rb b/ee/lib/sidebars/admin/menus/subscription_menu.rb
index 0a1117a5eb67ca..08d1aadd46103b 100644
--- a/ee/lib/sidebars/admin/menus/subscription_menu.rb
+++ b/ee/lib/sidebars/admin/menus/subscription_menu.rb
@@ -29,9 +29,9 @@ def active_routes
           { controller: :subscriptions }
         end
 
-        override :render_with_custom_abilities
-        def render_with_custom_abilities
-          %i[read_admin_subscription]
+        override :render_with_abilities
+        def render_with_abilities
+          super + %i[read_admin_subscription]
         end
       end
     end
diff --git a/ee/spec/helpers/license_helper_spec.rb b/ee/spec/helpers/license_helper_spec.rb
index 606c882af600da..3ef9b429f24254 100644
--- a/ee/spec/helpers/license_helper_spec.rb
+++ b/ee/spec/helpers/license_helper_spec.rb
@@ -79,14 +79,13 @@ def stub_default_url_options(host: "localhost", protocol: "http", port: nil, scr
     end
   end
 
-  describe '#cloud_license_view_data' do
-    let(:current_user) { build(:user) }
+  describe '#cloud_license_view_data', :enable_admin_mode do
+    let(:current_user) { build(:admin) }
 
     before do
       allow(helper).to receive(:subscription_portal_manage_url).and_return('subscriptions_manage_url')
       allow(helper).to receive(:new_trial_url).and_return('new_trial_url')
       allow(helper).to receive(:current_user).and_return(current_user)
-      allow(current_user).to receive(:can?).with(:destroy_licenses).and_return(true)
     end
 
     context 'when there is a current license' do
diff --git a/ee/spec/helpers/sidebars_helper_spec.rb b/ee/spec/helpers/sidebars_helper_spec.rb
index 0ec522902c5fdb..a75e669d3f0c72 100644
--- a/ee/spec/helpers/sidebars_helper_spec.rb
+++ b/ee/spec/helpers/sidebars_helper_spec.rb
@@ -313,6 +313,7 @@
           nil                      | nil
           :admin_unknown           | '/admin'
           :read_admin_cicd         | '/admin/runners'
+          :read_admin_dashboard    | '/admin'
           :read_admin_subscription | '/admin/subscription'
         end
 
diff --git a/ee/spec/lib/ee/sidebars/admin/menus/admin_overview_menu_spec.rb b/ee/spec/lib/ee/sidebars/admin/menus/admin_overview_menu_spec.rb
index fecf2d02b66ffe..69fd697262ae2f 100644
--- a/ee/spec/lib/ee/sidebars/admin/menus/admin_overview_menu_spec.rb
+++ b/ee/spec/lib/ee/sidebars/admin/menus/admin_overview_menu_spec.rb
@@ -12,8 +12,8 @@
 
       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)
         end
 
         context 'when custom_ability_read_admin_dashboard FF is enabled' do
@@ -34,11 +34,6 @@
       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(admin_overview_menu.render?).to be(false)
         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
index 0c24a73dbc6ad8..35fe1911d21324 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
@@ -12,7 +12,7 @@
     subject(:render?) { menu.render? }
 
     before do
-      allow(user).to receive(:can_admin_all_resources?).and_return(false)
+      allow(user).to receive(:can?).and_call_original
       allow(user).to receive(:can?).with(:access_admin_area).and_return(can_access_admin_area)
     end
 
diff --git a/ee/spec/lib/sidebars/admin/menus/subscription_menu_spec.rb b/ee/spec/lib/sidebars/admin/menus/subscription_menu_spec.rb
index 7e9e2651abf603..fc05509a772591 100644
--- a/ee/spec/lib/sidebars/admin/menus/subscription_menu_spec.rb
+++ b/ee/spec/lib/sidebars/admin/menus/subscription_menu_spec.rb
@@ -3,7 +3,7 @@
 require 'spec_helper'
 
 RSpec.describe Sidebars::Admin::Menus::SubscriptionMenu, feature_category: :navigation do
-  it_behaves_like 'Admin menu',
+  it_behaves_like 'Admin menu with custom ability',
     link: '/admin/subscription',
     title: s_('Admin|Subscription'),
     icon: 'license',
diff --git a/ee/spec/support/shared_examples/lib/sidebars/admin/menus/admin_menus_shared_examples.rb b/ee/spec/support/shared_examples/lib/sidebars/admin/menus/admin_menus_shared_examples.rb
new file mode 100644
index 00000000000000..90a088460873e1
--- /dev/null
+++ b/ee/spec/support/shared_examples/lib/sidebars/admin/menus/admin_menus_shared_examples.rb
@@ -0,0 +1,22 @@
+# frozen_string_literal: true
+
+RSpec.shared_examples 'Admin menu with custom ability' do |link:, title:, icon:, custom_ability:, separated: false|
+  include_examples 'Admin menu', link: link, title: title, icon: icon, separated: separated
+
+  describe '#render?' do
+    let_it_be(:user) { create(:user) }
+    let_it_be(:role) { create(:member_role, custom_ability) }
+    let_it_be(:user_member_role) { create(:user_member_role, member_role: role, user: user) }
+    let(:context) { Sidebars::Context.new(current_user: user, container: nil) }
+
+    subject { described_class.new(context).render? }
+
+    before do
+      stub_licensed_features(custom_roles: true)
+    end
+
+    context 'when a custom ability allows access' do
+      it { is_expected.to be true }
+    end
+  end
+end
diff --git a/lib/sidebars/admin/base_menu.rb b/lib/sidebars/admin/base_menu.rb
index e1eb13e3f95122..c00c248754b9e5 100644
--- a/lib/sidebars/admin/base_menu.rb
+++ b/lib/sidebars/admin/base_menu.rb
@@ -6,15 +6,14 @@ class BaseMenu < ::Sidebars::Menu
       override :render?
       def render?
         return false unless context.current_user
-        return true if context.current_user.can_admin_all_resources?
 
-        render_with_custom_abilities.any? { |ability| context.current_user.can?(ability) }
+        render_with_abilities.any? { |ability| context.current_user.can?(ability) }
       end
 
       private
 
-      def render_with_custom_abilities
-        []
+      def render_with_abilities
+        %i[admin_all_resources]
       end
     end
   end
diff --git a/spec/helpers/sidebars_helper_spec.rb b/spec/helpers/sidebars_helper_spec.rb
index 49e98ad52ac484..4381c6e996041a 100644
--- a/spec/helpers/sidebars_helper_spec.rb
+++ b/spec/helpers/sidebars_helper_spec.rb
@@ -686,9 +686,9 @@
       end
 
       describe 'admin user' do
-        it 'returns Admin Panel for admin nav', :aggregate_failures do
-          allow(user).to receive(:can_admin_all_resources?).and_return(true)
+        let(:user) { build(:admin) }
 
+        it 'returns Admin Panel for admin nav', :enable_admin_mode do
           expect(helper.super_sidebar_nav_panel(nav: 'admin', user: user)).to be_a(Sidebars::Admin::Panel)
         end
       end
diff --git a/spec/support/shared_examples/lib/sidebars/admin/menus/admin_menus_shared_examples.rb b/spec/support/shared_examples/lib/sidebars/admin/menus/admin_menus_shared_examples.rb
index 1a7176bdab8bd1..9077a70a8984c4 100644
--- a/spec/support/shared_examples/lib/sidebars/admin/menus/admin_menus_shared_examples.rb
+++ b/spec/support/shared_examples/lib/sidebars/admin/menus/admin_menus_shared_examples.rb
@@ -1,10 +1,10 @@
 # frozen_string_literal: true
 
-RSpec.shared_examples 'Admin menu' do |link:, title:, icon:, separated: false, custom_ability: nil|
+RSpec.shared_examples 'Admin menu' do |link:, title:, icon:, separated: false|
   let_it_be(:user) { build(:user, :admin) }
 
   before do
-    allow(user).to receive(:can_admin_all_resources?).and_return(true)
+    stub_application_setting(admin_mode: false)
   end
 
   let(:context) { Sidebars::Context.new(current_user: user, container: nil) }
@@ -39,18 +39,6 @@
         expect(described_class.new(Sidebars::Context.new(current_user: build(:user),
           container: nil)).render?).to be false
       end
-
-      if custom_ability
-        context 'when a custom ability allows access' do
-          let_it_be(:user) { create(:user) }
-          let_it_be(:role) { create(:member_role, custom_ability) }
-          let_it_be(:user_member_role) { create(:user_member_role, member_role: role, user: user) }
-
-          it 'renders' do
-            expect(subject.render?).to be true
-          end
-        end
-      end
     end
 
     context 'when user is not logged in' do
-- 
GitLab


From 563231d6fec33d069c0a2c6771928a3d2dd62b73 Mon Sep 17 00:00:00 2001
From: imand3r <ianderson@gitlab.com>
Date: Fri, 24 Jan 2025 19:38:02 +0000
Subject: [PATCH 4/5] Add ability to override authorization when enforcing
 admin authentication

---
 .../concerns/enforces_admin_authentication.rb | 15 ++++++
 .../admin/subscriptions_controller.rb         |  9 +---
 .../enforces_admin_authentication_spec.rb     | 51 ++++++++++++++++---
 3 files changed, 60 insertions(+), 15 deletions(-)

diff --git a/app/controllers/concerns/enforces_admin_authentication.rb b/app/controllers/concerns/enforces_admin_authentication.rb
index 94c0e98c91a7a5..8d365f3e53d81d 100644
--- a/app/controllers/concerns/enforces_admin_authentication.rb
+++ b/app/controllers/concerns/enforces_admin_authentication.rb
@@ -11,6 +11,13 @@ module EnforcesAdminAuthentication
 
   included do
     before_action :authenticate_admin!
+
+    def self.authorize!(ability, only:)
+      actions = Array(only)
+
+      skip_before_action :authenticate_admin!, only: actions
+      before_action -> { authorize_ability!(ability) }, only: actions
+    end
   end
 
   def authenticate_admin!
@@ -27,4 +34,12 @@ def authenticate_admin!
   def storable_location?
     request.path != new_admin_session_path
   end
+
+  private
+
+  def authorize_ability!(ability)
+    return authenticate_admin! if current_user.admin?
+
+    render_404 unless current_user.can?(ability)
+  end
 end
diff --git a/ee/app/controllers/admin/subscriptions_controller.rb b/ee/app/controllers/admin/subscriptions_controller.rb
index c218c64059526a..f34d4e6139e174 100644
--- a/ee/app/controllers/admin/subscriptions_controller.rb
+++ b/ee/app/controllers/admin/subscriptions_controller.rb
@@ -7,12 +7,5 @@ class Admin::SubscriptionsController < Admin::ApplicationController
   feature_category :plan_provisioning
   urgency :low
 
-  skip_before_action :authenticate_admin!, only: :show
-  before_action :authenticate_read_subscription!, only: :show
-
-  private
-
-  def authenticate_read_subscription!
-    render_404 unless current_user.can?(:read_admin_subscription)
-  end
+  authorize! :read_admin_subscription, only: :show
 end
diff --git a/spec/controllers/concerns/enforces_admin_authentication_spec.rb b/spec/controllers/concerns/enforces_admin_authentication_spec.rb
index 106b1d53fd2bd3..331e1ada73b2ab 100644
--- a/spec/controllers/concerns/enforces_admin_authentication_spec.rb
+++ b/spec/controllers/concerns/enforces_admin_authentication_spec.rb
@@ -3,8 +3,6 @@
 require 'spec_helper'
 
 RSpec.describe EnforcesAdminAuthentication do
-  include AdminModeHelper
-
   let(:user) { create(:user) }
 
   before do
@@ -19,6 +17,49 @@ def index
     end
   end
 
+  describe '.authorize!' do
+    controller(ApplicationController) do
+      include EnforcesAdminAuthentication
+
+      authorize! :ability, only: :index
+
+      def index
+        head :ok
+      end
+    end
+
+    context 'when the user is an admin', :enable_admin_mode do
+      let(:user) { create(:admin) }
+
+      it 'renders ok' do
+        get :index
+
+        expect(response).to have_gitlab_http_status(:ok)
+      end
+    end
+
+    context 'when the user is a regular user' do
+      it 'renders a 404' do
+        get :index
+
+        expect(response).to have_gitlab_http_status(:not_found)
+      end
+
+      context 'when an ability grants access' do
+        before do
+          allow(Ability).to receive(:allowed?).and_call_original
+          allow(Ability).to receive(:allowed?).with(user, :ability, :global).and_return(true)
+        end
+
+        it 'renders ok' do
+          get :index
+
+          expect(response).to have_gitlab_http_status(:ok)
+        end
+      end
+    end
+  end
+
   context 'application setting :admin_mode is enabled' do
     describe 'authenticate_admin!' do
       context 'as an admin' do
@@ -31,11 +72,7 @@ def index
           expect(assigns(:current_user_mode)&.admin_mode?).to be(false)
         end
 
-        context 'when admin mode is active' do
-          before do
-            enable_admin_mode!(user)
-          end
-
+        context 'when admin mode is active', :enable_admin_mode do
           it 'renders ok' do
             get :index
 
-- 
GitLab


From 6228b59be82458e95fcf7652199d637fbeef266d Mon Sep 17 00:00:00 2001
From: Ian Anderson <ianderson@gitlab.com>
Date: Mon, 27 Jan 2025 22:16:59 +0000
Subject: [PATCH 5/5] Apply 1 suggestion(s) to 1 file(s)

Co-authored-by: Isaac Durham <idurham@gitlab.com>
---
 doc/api/graphql/reference/index.md                     | 2 ++
 doc/user/custom_roles/abilities.md                     | 8 --------
 ee/config/custom_abilities/read_admin_subscription.yml | 2 +-
 3 files changed, 3 insertions(+), 9 deletions(-)

diff --git a/doc/api/graphql/reference/index.md b/doc/api/graphql/reference/index.md
index 54fe39da58a9ce..cc5dcaf4ee4f4f 100644
--- a/doc/api/graphql/reference/index.md
+++ b/doc/api/graphql/reference/index.md
@@ -40948,6 +40948,7 @@ Member role admin permission.
 | <a id="memberroleadminpermissionread_admin_cicd"></a>`READ_ADMIN_CICD` | Read CI/CD details including runners and jobs. |
 | <a id="memberroleadminpermissionread_admin_dashboard"></a>`READ_ADMIN_DASHBOARD` | Read-only access to admin dashboard. |
 | <a id="memberroleadminpermissionread_admin_monitoring"></a>`READ_ADMIN_MONITORING` | Allows read access to system monitoring including system info, background migrations, health checks, audit logs, and gitaly in the Admin Area. |
+| <a id="memberroleadminpermissionread_admin_subscription"></a>`READ_ADMIN_SUBSCRIPTION` | Read subscription details in the Admin area. |
 
 ### `MemberRolePermission`
 
@@ -40977,6 +40978,7 @@ Member role permission.
 | <a id="memberrolepermissionread_admin_cicd"></a>`READ_ADMIN_CICD` | Read CI/CD details including runners and jobs. |
 | <a id="memberrolepermissionread_admin_dashboard"></a>`READ_ADMIN_DASHBOARD` | Read-only access to admin dashboard. |
 | <a id="memberrolepermissionread_admin_monitoring"></a>`READ_ADMIN_MONITORING` | Allows read access to system monitoring including system info, background migrations, health checks, audit logs, and gitaly in the Admin Area. |
+| <a id="memberrolepermissionread_admin_subscription"></a>`READ_ADMIN_SUBSCRIPTION` | Read subscription details in the Admin area. |
 | <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. |
 | <a id="memberrolepermissionread_crm_contact"></a>`READ_CRM_CONTACT` | Read CRM contact. |
diff --git a/doc/user/custom_roles/abilities.md b/doc/user/custom_roles/abilities.md
index e942c8935901a2..58fd6de1cd29b2 100644
--- a/doc/user/custom_roles/abilities.md
+++ b/doc/user/custom_roles/abilities.md
@@ -23,14 +23,6 @@ Some permissions depend on other permissions.
 For example, the `admin_vulnerability` permission requires you to also include the `read_vulnerability` permission.
 Any dependencies are noted in the `Description` column for each permission.
 
-## Admin
-
-| 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) |
-| View subscription details | Read subscription details in the Admin Area | [`read_admin_subscription`](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/178230) | Instance | GitLab [17.9](https://gitlab.com/gitlab-org/gitlab/-/issues/507961) |
-
 ## Code review workflow
 
 | Permission | Description | API Attribute | Scope | Introduced |
diff --git a/ee/config/custom_abilities/read_admin_subscription.yml b/ee/config/custom_abilities/read_admin_subscription.yml
index 627dea65424744..84384cf778c746 100644
--- a/ee/config/custom_abilities/read_admin_subscription.yml
+++ b/ee/config/custom_abilities/read_admin_subscription.yml
@@ -1,7 +1,7 @@
 ---
 title: View subscription details
 name: read_admin_subscription
-description: Read subscription details in the Admin Area
+description: Read subscription details in the Admin area.
 introduced_by_issue: https://gitlab.com/gitlab-org/gitlab/-/issues/507961
 introduced_by_mr: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/178230
 feature_category: admin
-- 
GitLab