From b94d08e3a725a37ab3335d69653f3d09a88df38f Mon Sep 17 00:00:00 2001 From: mo khan Date: Thu, 9 Jan 2025 15:48:43 -0700 Subject: [PATCH 1/3] Add `read_admin_users` permission This permission allows members to view the user management pages in the Admin area. EE: true Changelog: added --- app/helpers/admin/user_actions_helper.rb | 2 + .../json_schemas/member_role_permissions.json | 3 + app/views/admin/users/_head.html.haml | 49 +++++----- app/views/admin/users/_tabs.html.haml | 5 +- app/views/admin/users/index.html.haml | 9 +- app/views/admin/users/show.html.haml | 3 +- doc/api/graphql/reference/index.md | 2 + .../controllers/ee/admin/users_controller.rb | 5 + .../helpers/ee/admin/user_actions_helper.rb | 16 ++++ ee/app/helpers/ee/sidebars_helper.rb | 2 + ee/app/policies/ee/global_policy.rb | 2 + .../custom_abilities/read_admin_users.yml | 12 +++ .../wip/custom_ability_read_admin_users.yml | 9 ++ .../admin/menus/admin_overview_menu.rb | 30 +++++- .../ee/admin/user_actions_helper_spec.rb | 28 ++++++ ee/spec/helpers/sidebars_helper_spec.rb | 7 +- .../admin/menus/admin_overview_menu_spec.rb | 58 +++++++----- ee/spec/policies/global_policy_spec.rb | 1 + .../read_admin_users/request_spec.rb | 93 +++++++++++++++++++ .../views/admin/users/index.html.haml_spec.rb | 5 +- .../helpers/admin/user_actions_helper_spec.rb | 9 +- 21 files changed, 283 insertions(+), 67 deletions(-) create mode 100644 ee/app/helpers/ee/admin/user_actions_helper.rb create mode 100644 ee/config/custom_abilities/read_admin_users.yml create mode 100644 ee/config/feature_flags/wip/custom_ability_read_admin_users.yml create mode 100644 ee/spec/helpers/ee/admin/user_actions_helper_spec.rb create mode 100644 ee/spec/requests/custom_roles/read_admin_users/request_spec.rb diff --git a/app/helpers/admin/user_actions_helper.rb b/app/helpers/admin/user_actions_helper.rb index a54dd02e7c28a6..69dd7ea044832a 100644 --- a/app/helpers/admin/user_actions_helper.rb +++ b/app/helpers/admin/user_actions_helper.rb @@ -81,3 +81,5 @@ def trust_actions end end end + +::Admin::UserActionsHelper.prepend_mod diff --git a/app/validators/json_schemas/member_role_permissions.json b/app/validators/json_schemas/member_role_permissions.json index 94da66e9b2f160..b365412c333be1 100644 --- a/app/validators/json_schemas/member_role_permissions.json +++ b/app/validators/json_schemas/member_role_permissions.json @@ -73,6 +73,9 @@ "read_admin_subscription": { "type": "boolean" }, + "read_admin_users": { + "type": "boolean" + }, "read_code": { "type": "boolean" }, diff --git a/app/views/admin/users/_head.html.haml b/app/views/admin/users/_head.html.haml index 843af3d1da9a24..6e454f3b6e1fea 100644 --- a/app/views/admin/users/_head.html.haml +++ b/app/views/admin/users/_head.html.haml @@ -16,30 +16,33 @@ = render Pajamas::BadgeComponent.new(s_('AdminUsers|Deactivated'), variant: :neutral) - if @user.access_locked? = render Pajamas::BadgeComponent.new(s_('AdminUsers|Locked'), variant: :warning) - = render_if_exists 'admin/users/auditor_user_badge' - = render_if_exists 'admin/users/gma_user_badge' + - if can?(current_user, :admin_all_resources) + = render_if_exists 'admin/users/auditor_user_badge' + = render_if_exists 'admin/users/gma_user_badge' - .gl-my-3.gl-flex.gl-flex-wrap.-gl-my-2.-gl-mx-2 - - if @user != current_user - - if impersonation_enabled? - .gl-p-2 - %span.btn-group{ class: !@can_impersonate ? 'has-tooltip' : nil, title: @impersonation_error_text } - = render Pajamas::ButtonComponent.new(disabled: !@can_impersonate, method: :post, href: impersonate_admin_user_path(@user), button_options: { data: { testid: 'impersonate-user-link' } }) do - = _('Impersonate') - - if can_force_email_confirmation?(@user) - .gl-p-2 - = render Pajamas::ButtonComponent.new(variant: :default, button_options: { class: 'js-confirm-modal-button', data: confirm_user_data(@user) }) do - = _('Confirm user') - .gl-p-2 - = render Pajamas::ButtonComponent.new(variant: :confirm, href: new_admin_user_identity_path(@user)) do - = _('New identity') - .gl-p-2 - #js-admin-user-actions{ data: admin_user_actions_data_attributes(@user) } + - if can?(current_user, :admin_all_resources) + .gl-my-3.gl-flex.gl-flex-wrap.-gl-my-2.-gl-mx-2 + - if @user != current_user + - if impersonation_enabled? + .gl-p-2 + %span.btn-group{ class: !@can_impersonate ? 'has-tooltip' : nil, title: @impersonation_error_text } + = render Pajamas::ButtonComponent.new(disabled: !@can_impersonate, method: :post, href: impersonate_admin_user_path(@user), button_options: { data: { testid: 'impersonate-user-link' } }) do + = _('Impersonate') + - if can_force_email_confirmation?(@user) + .gl-p-2 + = render Pajamas::ButtonComponent.new(variant: :default, button_options: { class: 'js-confirm-modal-button', data: confirm_user_data(@user) }) do + = _('Confirm user') + .gl-p-2 + = render Pajamas::ButtonComponent.new(variant: :confirm, href: new_admin_user_identity_path(@user)) do + = _('New identity') + .gl-p-2 + #js-admin-user-actions{ data: admin_user_actions_data_attributes(@user) } = gl_tabs_nav do = gl_tab_link_to _("Account"), admin_user_path(@user) - = gl_tab_link_to _("Groups and projects"), projects_admin_user_path(@user) - = gl_tab_link_to _("SSH keys"), keys_admin_user_path(@user) - = gl_tab_link_to _("Identities"), admin_user_identities_path(@user) - - if impersonation_tokens_enabled? - = gl_tab_link_to _("Impersonation Tokens"), admin_user_impersonation_tokens_path(@user), data: { testid: 'impersonation-tokens-tab' } + - if can?(current_user, :admin_all_resources) + = gl_tab_link_to _("Groups and projects"), projects_admin_user_path(@user) + = gl_tab_link_to _("SSH keys"), keys_admin_user_path(@user) + = gl_tab_link_to _("Identities"), admin_user_identities_path(@user) + - if impersonation_tokens_enabled? + = gl_tab_link_to _("Impersonation Tokens"), admin_user_impersonation_tokens_path(@user), data: { testid: 'impersonation-tokens-tab' } .gl-mb-3 diff --git a/app/views/admin/users/_tabs.html.haml b/app/views/admin/users/_tabs.html.haml index 4810a453f011df..defb8b075f708f 100644 --- a/app/views/admin/users/_tabs.html.haml +++ b/app/views/admin/users/_tabs.html.haml @@ -3,5 +3,6 @@ = gl_tabs_nav({ class: 'gl-grow gl-border-0', data: { testid: 'admin-users-tabs' } }) do = gl_tab_link_to s_('AdminUsers|Users'), admin_users_path - = render_if_exists 'admin/users/role_promotion_requests_tab' - = gl_tab_link_to s_('AdminUsers|Cohorts'), admin_cohorts_path + - if can?(current_user, :admin_all_resources) + = render_if_exists 'admin/users/role_promotion_requests_tab' + = gl_tab_link_to s_('AdminUsers|Cohorts'), admin_cohorts_path diff --git a/app/views/admin/users/index.html.haml b/app/views/admin/users/index.html.haml index b2062af6393a74..a939707ec8123b 100644 --- a/app/views/admin/users/index.html.haml +++ b/app/views/admin/users/index.html.haml @@ -2,10 +2,11 @@ = render ::Layouts::PageHeadingComponent.new(_('Users'), options: { data: { event_tracking_load: 'true', event_tracking: 'view_admin_users_pageload' } }) do |c| - c.with_actions do - = render_if_exists 'admin/users/admin_email_users' - = render_if_exists 'admin/users/admin_export_user_permissions' - = render Pajamas::ButtonComponent.new(variant: :confirm, href: new_admin_user_path) do - = s_('AdminUsers|New user') + - if can?(current_user, :admin_all_resources) + = render_if_exists 'admin/users/admin_email_users' + = render_if_exists 'admin/users/admin_export_user_permissions' + = render Pajamas::ButtonComponent.new(variant: :confirm, href: new_admin_user_path) do + = s_('AdminUsers|New user') = render 'tabs' diff --git a/app/views/admin/users/show.html.haml b/app/views/admin/users/show.html.haml index 4d4c248bd9c4ef..a4856bb58ed1e7 100644 --- a/app/views/admin/users/show.html.haml +++ b/app/views/admin/users/show.html.haml @@ -58,7 +58,8 @@ %span.gl-text-subtle= _('Secondary email:') .gl-col-span-2 %strong= render partial: 'shared/email_with_badge', locals: { email: email.email, verified: email.confirmed? } - = link_button_to nil, remove_email_admin_user_path(@user, email), data: { confirm: _("Are you sure you want to remove %{email}?") % { email: email.email }, 'confirm-btn-variant': 'danger' }, method: :delete, class: 'gl-float-right has-tooltip', title: _('Remove secondary email'), id: "remove_email_#{email.id}", category: :tertiary, size: :small, icon: 'remove' + - if can?(current_user, :admin_all_resources) + = link_button_to nil, remove_email_admin_user_path(@user, email), data: { confirm: _("Are you sure you want to remove %{email}?") % { email: email.email }, 'confirm-btn-variant': 'danger' }, method: :delete, class: 'gl-float-right has-tooltip', title: _('Remove secondary email'), id: "remove_email_#{email.id}", category: :tertiary, size: :small, icon: 'remove' %li{ class: list_item_classes } %span.gl-text-subtle ID: .gl-col-span-2 diff --git a/doc/api/graphql/reference/index.md b/doc/api/graphql/reference/index.md index aee3983c204da7..fa96f0abbb2a70 100644 --- a/doc/api/graphql/reference/index.md +++ b/doc/api/graphql/reference/index.md @@ -41162,6 +41162,7 @@ Member role admin permission. | `READ_ADMIN_DASHBOARD` | Read-only access to admin dashboard. | | `READ_ADMIN_MONITORING` | Allows read access to system monitoring including system info, background migrations, health checks, audit logs, and gitaly in the Admin Area. | | `READ_ADMIN_SUBSCRIPTION` | Read subscription details in the Admin area. | +| `READ_ADMIN_USERS` | Allows read access to the user list and user details in the Admin area. | ### `MemberRolePermission` @@ -41192,6 +41193,7 @@ Member role permission. | `READ_ADMIN_DASHBOARD` | Read-only access to admin dashboard. | | `READ_ADMIN_MONITORING` | Allows read access to system monitoring including system info, background migrations, health checks, audit logs, and gitaly in the Admin Area. | | `READ_ADMIN_SUBSCRIPTION` | Read subscription details in the Admin area. | +| `READ_ADMIN_USERS` | Allows read access to the user list and user details in the Admin area. | | `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. | | `READ_CRM_CONTACT` | Read CRM contact. | diff --git a/ee/app/controllers/ee/admin/users_controller.rb b/ee/app/controllers/ee/admin/users_controller.rb index 7e22c659500a01..89d143fde6b1c5 100644 --- a/ee/app/controllers/ee/admin/users_controller.rb +++ b/ee/app/controllers/ee/admin/users_controller.rb @@ -4,8 +4,13 @@ module EE module Admin module UsersController + extend ::ActiveSupport::Concern extend ::Gitlab::Utils::Override + prepended do + authorize! :read_admin_users, only: [:index, :show] + end + def identity_verification_exemption if @user.add_identity_verification_exemption("set by #{current_user.username}") redirect_to [:admin, @user], notice: _('Identity verification exemption has been created.') diff --git a/ee/app/helpers/ee/admin/user_actions_helper.rb b/ee/app/helpers/ee/admin/user_actions_helper.rb new file mode 100644 index 00000000000000..a3e171f1f857e0 --- /dev/null +++ b/ee/app/helpers/ee/admin/user_actions_helper.rb @@ -0,0 +1,16 @@ +# frozen_string_literal: true + +module EE + module Admin + module UserActionsHelper + extend ::Gitlab::Utils::Override + + override :admin_actions + def admin_actions(user) + return super if can?(current_user, :admin_all_resources) + + [] + end + end + end +end diff --git a/ee/app/helpers/ee/sidebars_helper.rb b/ee/app/helpers/ee/sidebars_helper.rb index 975253d48b1721..50cd8138df7e52 100644 --- a/ee/app/helpers/ee/sidebars_helper.rb +++ b/ee/app/helpers/ee/sidebars_helper.rb @@ -101,6 +101,8 @@ def admin_area_link admin_runners_path elsif current_user.can?(:read_admin_subscription) admin_subscription_path + elsif current_user.can?(:read_admin_users) + admin_users_path else super end diff --git a/ee/app/policies/ee/global_policy.rb b/ee/app/policies/ee/global_policy.rb index f53c78a209542d..ab3ddf87f2ab9c 100644 --- a/ee/app/policies/ee/global_policy.rb +++ b/ee/app/policies/ee/global_policy.rb @@ -239,6 +239,8 @@ module GlobalPolicy enable :read_billable_member enable :read_licenses end + + rule { custom_role_enables_read_admin_users }.enable :read_admin_users end def duo_chat diff --git a/ee/config/custom_abilities/read_admin_users.yml b/ee/config/custom_abilities/read_admin_users.yml new file mode 100644 index 00000000000000..60c812fc06cda3 --- /dev/null +++ b/ee/config/custom_abilities/read_admin_users.yml @@ -0,0 +1,12 @@ +--- +title: View users +name: read_admin_users +description: Allows read access to the user list and user details in the Admin area +introduced_by_issue: https://gitlab.com/gitlab-org/gitlab/-/issues/508782 +introduced_by_mr: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/177514 +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_users.yml b/ee/config/feature_flags/wip/custom_ability_read_admin_users.yml new file mode 100644 index 00000000000000..2de0a5ab34039c --- /dev/null +++ b/ee/config/feature_flags/wip/custom_ability_read_admin_users.yml @@ -0,0 +1,9 @@ +--- +name: custom_ability_read_admin_users +feature_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/508782 +introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/177514 +rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/514376 +milestone: '17.9' +group: group::authorization +type: wip +default_enabled: false diff --git a/ee/lib/ee/sidebars/admin/menus/admin_overview_menu.rb b/ee/lib/ee/sidebars/admin/menus/admin_overview_menu.rb index 1b8357c0b7078c..8c6e618d458539 100644 --- a/ee/lib/ee/sidebars/admin/menus/admin_overview_menu.rb +++ b/ee/lib/ee/sidebars/admin/menus/admin_overview_menu.rb @@ -7,13 +7,37 @@ module Menus module AdminOverviewMenu extend ::Gitlab::Utils::Override + RESTRICTED_ADMIN_PERMISSIONS = { + read_admin_dashboard: :dashboard_menu_item, + read_admin_users: :users_menu_item + }.freeze + override :render? def render? - return super unless ::Feature.enabled?(:custom_ability_read_admin_dashboard, context.current_user) + super || restricted_administrator? + end + + override :configure_menu_items + def configure_menu_items + return super if administrator? + + RESTRICTED_ADMIN_PERMISSIONS.each_pair do |permission, name| + add_item(build_menu_item(name)) if can?(context.current_user, permission) + end + end - return true if context.current_user&.can?(:access_admin_area) + private + + def build_menu_item(name) + method(name).call + end + + def administrator? + can?(current_user, :admin_all_resources) + end - super + def restricted_administrator? + can_any?(current_user, RESTRICTED_ADMIN_PERMISSIONS.keys) end end end diff --git a/ee/spec/helpers/ee/admin/user_actions_helper_spec.rb b/ee/spec/helpers/ee/admin/user_actions_helper_spec.rb new file mode 100644 index 00000000000000..46bf7488d866df --- /dev/null +++ b/ee/spec/helpers/ee/admin/user_actions_helper_spec.rb @@ -0,0 +1,28 @@ +# frozen_string_literal: true + +require "spec_helper" + +RSpec.describe Admin::UserActionsHelper, feature_category: :user_management do + describe '#admin_actions', :enable_admin_mode do + let_it_be(:current_user) { build_stubbed(:user) } + let_it_be(:user) { build_stubbed(:user) } + + subject { helper.admin_actions(user) } + + before do + allow(helper).to receive(:current_user).and_return(current_user) + end + + context 'when the current user is a Restricted Administrator with the `read_admin_users` permission' do + let_it_be(:role) { build_stubbed(:member_role, :read_admin_users) } + let_it_be(:membership) { build_stubbed(:user_member_role, user: current_user, member_role: role) } + + before do + allow(helper).to receive(:can?).and_call_original + allow(helper).to receive(:can?).with(current_user, :read_admin_users).and_return(true) + end + + it { is_expected.to be_empty } + end + end +end diff --git a/ee/spec/helpers/sidebars_helper_spec.rb b/ee/spec/helpers/sidebars_helper_spec.rb index a75e669d3f0c72..7aff95fd1b8df7 100644 --- a/ee/spec/helpers/sidebars_helper_spec.rb +++ b/ee/spec/helpers/sidebars_helper_spec.rb @@ -279,14 +279,10 @@ let_it_be(:panel_type) { 'default' } let_it_be(:current_user_mode) { Gitlab::Auth::CurrentUserMode.new(user) } - let_it_be(:public_link) do - { title: s_('Navigation|Explore'), link: '/explore', icon: 'compass' } - end - let_it_be(:public_links_for_user) do [ { title: s_('Navigation|Your work'), link: '/', icon: 'work' }, - public_link, + { title: s_('Navigation|Explore'), link: '/explore', icon: 'compass' }, { title: s_('Navigation|Profile'), link: '/-/user_settings/profile', icon: 'profile' }, { title: s_('Navigation|Preferences'), link: '/-/profile/preferences', icon: 'preferences' } ] @@ -315,6 +311,7 @@ :read_admin_cicd | '/admin/runners' :read_admin_dashboard | '/admin' :read_admin_subscription | '/admin/subscription' + :read_admin_users | ref(:admin_users_path) end with_them do 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 69fd697262ae2f..dd0a8aeb06e165 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 @@ -3,40 +3,54 @@ require 'spec_helper' RSpec.describe Sidebars::Admin::Menus::AdminOverviewMenu, feature_category: :navigation do - let(:user) { build_stubbed(:user) } - let(:context) { Sidebars::Context.new(current_user: user, container: nil) } + let_it_be(:user, refind: true) { create(:user) } + + subject(:menu) { described_class.new(Sidebars::Context.new(current_user: user, container: nil)) } describe '#render' do - context 'with a regular user' do - subject(:admin_overview_menu) { described_class.new(context) } + it { is_expected.not_to be_render } + + [ + :read_admin_dashboard, + :read_admin_users + ].each do |permission| + context "when user has `#{permission}`" do + let_it_be(:role) { create(:member_role, permission) } + let_it_be(:membership) { create(:user_member_role, user: user, member_role: role) } - 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) - end - context 'when custom_ability_read_admin_dashboard FF is enabled' do - it 'renders' do - expect(admin_overview_menu.render?).to be(true) - end + stub_licensed_features(custom_roles: true) end - context 'when custom_ability_read_admin_dashboard FF is disabled' do - before do - stub_feature_flags(custom_ability_read_admin_dashboard: false) - end - - it 'does not render' do - expect(admin_overview_menu.render?).to be(false) - end - end + it { is_expected.to be_render } end + end + end - context 'when user can not access admin area' do - it 'does not render' do - expect(admin_overview_menu.render?).to be(false) + describe "#renderable_items" do + using RSpec::Parameterized::TableSyntax + + where(:permissions, :expected_menu_items) do + [:read_admin_dashboard] | [_('Dashboard')] + [:read_admin_users] | [_('Users')] + [:read_admin_dashboard, :read_admin_users] | [_('Dashboard'), _('Users')] + end + + with_them do + context "when user has `#{params[:permissions]}`" do + subject(:menu_items) { menu.renderable_items.map(&:title) } + + let!(:role) { create(:member_role, *permissions) } + let!(:membership) { create(:user_member_role, user: user, member_role: role) } + + before do + stub_licensed_features(custom_roles: true) end + + it { is_expected.to match_array(expected_menu_items) } end end end diff --git a/ee/spec/policies/global_policy_spec.rb b/ee/spec/policies/global_policy_spec.rb index fdb264a11bb1ca..a971172dd64b28 100644 --- a/ee/spec/policies/global_policy_spec.rb +++ b/ee/spec/policies/global_policy_spec.rb @@ -914,6 +914,7 @@ :read_admin_cicd | %i[read_admin_cicd] :read_admin_dashboard | %i[read_admin_dashboard access_admin_area] :read_admin_subscription | %i[read_admin_subscription read_billable_member read_licenses] + :read_admin_users | %i[read_admin_users] end with_them do diff --git a/ee/spec/requests/custom_roles/read_admin_users/request_spec.rb b/ee/spec/requests/custom_roles/read_admin_users/request_spec.rb new file mode 100644 index 00000000000000..f0bc8552628eea --- /dev/null +++ b/ee/spec/requests/custom_roles/read_admin_users/request_spec.rb @@ -0,0 +1,93 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe 'User with read_admin_users', feature_category: :user_management do + let_it_be(:current_user) { create(:user) } + let_it_be(:permission) { :read_admin_users } + let_it_be(:role) { create(:member_role, permission) } + let_it_be(:membership) { create(:user_member_role, user: current_user, member_role: role) } + + before do + stub_licensed_features(custom_roles: true) + sign_in(current_user) + end + + describe Admin::UsersController do + let_it_be(:other_user) { create(:user, :with_namespace) } + + it "GET #index" do + get admin_users_path + + expect(response).to have_gitlab_http_status(:ok) + expect(response).to render_template(:index) + end + + it "GET #show" do + get admin_user_path(other_user) + + expect(response).to have_gitlab_http_status(:ok) + expect(response).to render_template(:show) + end + + it "GET #projects" do + get projects_admin_user_path(other_user) + + expect(response).to have_gitlab_http_status(:not_found) + end + + it 'GET #new' do + get new_admin_user_path + + expect(response).to have_gitlab_http_status(:not_found) + end + + it 'GET #edit' do + get edit_admin_user_path(other_user) + + expect(response).to have_gitlab_http_status(:not_found) + end + + it 'POST #create' do + post admin_users_path, params: { user: attributes_for(:user) } + + expect(response).to have_gitlab_http_status(:not_found) + end + + it 'PATCH #update' do + patch admin_user_path(other_user), params: { user: attributes_for(:user) } + + expect(response).to have_gitlab_http_status(:not_found) + end + + it 'PUT #update' do + put admin_user_path(other_user), params: { user: attributes_for(:user) } + + expect(response).to have_gitlab_http_status(:not_found) + end + + it 'DELETE #destroy' do + delete admin_user_path(other_user) + + expect(response).to have_gitlab_http_status(:not_found) + end + + context "with `custom_ability_read_admin_users` feature flag disabled" do + before do + stub_feature_flags(custom_ability_read_admin_users: false) + end + + it "GET #index" do + get admin_users_path + + expect(response).to have_gitlab_http_status(:not_found) + end + + it "GET #show" do + get admin_user_path(other_user) + + expect(response).to have_gitlab_http_status(:not_found) + end + end + end +end diff --git a/ee/spec/views/admin/users/index.html.haml_spec.rb b/ee/spec/views/admin/users/index.html.haml_spec.rb index de241a803ced23..191b41c1658131 100644 --- a/ee/spec/views/admin/users/index.html.haml_spec.rb +++ b/ee/spec/views/admin/users/index.html.haml_spec.rb @@ -2,13 +2,14 @@ require 'spec_helper' -RSpec.describe 'admin/users/index' do +RSpec.describe 'admin/users/index', :enable_admin_mode, feature_category: :user_management do let(:should_check_namespace_plan) { false } + let(:admin) { build_stubbed(:user, :admin) } before do allow(Gitlab::CurrentSettings).to receive(:should_check_namespace_plan?) .and_return(should_check_namespace_plan) - allow(view).to receive(:container_class).and_return('ignored') + allow(view).to receive_messages(container_class: 'ignored', current_user: admin) create(:user) # to have at least one usser assign(:users, User.all.page(1)) assign(:cohorts, { months_included: 0, cohorts: [] }) diff --git a/spec/helpers/admin/user_actions_helper_spec.rb b/spec/helpers/admin/user_actions_helper_spec.rb index abfdabf3413df9..9187683853d5be 100644 --- a/spec/helpers/admin/user_actions_helper_spec.rb +++ b/spec/helpers/admin/user_actions_helper_spec.rb @@ -3,14 +3,13 @@ require "spec_helper" RSpec.describe Admin::UserActionsHelper, feature_category: :user_management do - describe '#admin_actions' do - let_it_be(:current_user) { build(:user) } + describe '#admin_actions', :enable_admin_mode do + let_it_be(:current_user) { build(:user, :admin) } subject { helper.admin_actions(user) } before do allow(helper).to receive(:current_user).and_return(current_user) - allow(helper).to receive(:can?).with(current_user, :destroy_user, user).and_return(true) end context 'the user is a bot' do @@ -20,8 +19,7 @@ end context 'the current user and user are the same' do - let_it_be(:user) { build(:user) } - let_it_be(:current_user) { user } + let_it_be(:user) { current_user } it { is_expected.to contain_exactly("edit") } end @@ -137,6 +135,7 @@ let_it_be(:user) { build(:user) } before do + allow(helper).to receive(:can?).and_call_original allow(helper).to receive(:can?).with(current_user, :destroy_user, user).and_return(false) end -- GitLab From 14ffa363e100daa7f5df4246951a7d6074a969ec Mon Sep 17 00:00:00 2001 From: mo khan Date: Mon, 3 Feb 2025 09:50:09 -0700 Subject: [PATCH 2/3] Move read_admin_users.yml to admin/ dir --- ee/config/custom_abilities/{ => admin}/read_admin_users.yml | 4 ---- 1 file changed, 4 deletions(-) rename ee/config/custom_abilities/{ => admin}/read_admin_users.yml (80%) diff --git a/ee/config/custom_abilities/read_admin_users.yml b/ee/config/custom_abilities/admin/read_admin_users.yml similarity index 80% rename from ee/config/custom_abilities/read_admin_users.yml rename to ee/config/custom_abilities/admin/read_admin_users.yml index 60c812fc06cda3..f225374f2c432d 100644 --- a/ee/config/custom_abilities/read_admin_users.yml +++ b/ee/config/custom_abilities/admin/read_admin_users.yml @@ -6,7 +6,3 @@ introduced_by_issue: https://gitlab.com/gitlab-org/gitlab/-/issues/508782 introduced_by_mr: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/177514 feature_category: admin milestone: '17.9' -group_ability: false -project_ability: false -admin_ability: true -requirements: [] -- GitLab From 349eca51e863ec133e317b467acc0dca07a682f9 Mon Sep 17 00:00:00 2001 From: mo khan Date: Mon, 3 Feb 2025 09:54:40 -0700 Subject: [PATCH 3/3] Enable admin mode in specs --- spec/lib/sidebars/admin/menus/admin_overview_menu_spec.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/spec/lib/sidebars/admin/menus/admin_overview_menu_spec.rb b/spec/lib/sidebars/admin/menus/admin_overview_menu_spec.rb index dd4ce041e17413..48fad412025a4c 100644 --- a/spec/lib/sidebars/admin/menus/admin_overview_menu_spec.rb +++ b/spec/lib/sidebars/admin/menus/admin_overview_menu_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe Sidebars::Admin::Menus::AdminOverviewMenu, feature_category: :navigation do +RSpec.describe Sidebars::Admin::Menus::AdminOverviewMenu, :enable_admin_mode, feature_category: :navigation do let(:user) { build_stubbed(:user, :admin) } let(:context) { Sidebars::Context.new(current_user: user, container: nil) } -- GitLab