diff --git a/app/controllers/groups_controller.rb b/app/controllers/groups_controller.rb
index 2ca7c7e368a3edc9b028f3db4efc0f84a296dd56..a049c7de974be755636bf8e3c8d10c3837f508b8 100644
--- a/app/controllers/groups_controller.rb
+++ b/app/controllers/groups_controller.rb
@@ -21,7 +21,9 @@ class GroupsController < Groups::ApplicationController
before_action :group, except: [:index, :new, :create]
# Authorize
- before_action :authorize_admin_group!, only: [:edit, :update, :destroy, :projects, :transfer, :export, :download_export]
+ before_action :authorize_admin_group!, only: [:update, :projects, :transfer, :export, :download_export]
+ before_action :authorize_view_edit_page!, only: :edit
+ before_action :authorize_remove_group!, only: :destroy
before_action :authorize_create_group!, only: [:new]
before_action :load_recaptcha, only: [:new], if: -> { captcha_required? }
diff --git a/app/policies/group_policy.rb b/app/policies/group_policy.rb
index adbc399866b74f212b6d87c3efba0ced0638d17d..8c2664d638a3e8c61ce7494aff4e16191c072fb0 100644
--- a/app/policies/group_policy.rb
+++ b/app/policies/group_policy.rb
@@ -250,6 +250,8 @@ class GroupPolicy < Namespaces::GroupProjectNamespaceSharedPolicy
enable :read_billing
enable :edit_billing
+
+ enable :remove_group
end
rule { can?(:read_nested_project_resources) }.policy do
@@ -372,6 +374,8 @@ class GroupPolicy < Namespaces::GroupProjectNamespaceSharedPolicy
rule { maintainer & ~raise_admin_package_to_owner_enabled }.enable :admin_package
rule { owner & raise_admin_package_to_owner_enabled }.enable :admin_package
+ rule { can?(:remove_group) }.enable :view_edit_page
+
def access_level(for_any_session: false)
return GroupMember::NO_ACCESS if @user.nil?
return GroupMember::NO_ACCESS unless user_is_user?
diff --git a/app/views/groups/edit.html.haml b/app/views/groups/edit.html.haml
index 5c09fdcd021cc5935271661831aa000ad13a9a2e..1c1388013afe5005cbef04bc94aa3b0ae2ff615c 100644
--- a/app/views/groups/edit.html.haml
+++ b/app/views/groups/edit.html.haml
@@ -5,57 +5,62 @@
= render 'shared/namespaces/cascading_settings/lock_popovers'
-%section.settings.gs-general.no-animate.expanded#js-general-settings
- .settings-header
- %h4.settings-title.js-settings-toggle.js-settings-toggle-trigger-only{ role: 'button' }
- = _('Naming, visibility')
- = render Pajamas::ButtonComponent.new(button_options: { class: 'js-settings-toggle' }) do
- = _('Collapse')
- %p.gl-text-secondary
- = _('Update your group name, description, avatar, and visibility.')
- = link_to _('Learn more about groups.'), help_page_path('user/group/index')
- .settings-content
- = render 'groups/settings/general'
+- if can?(current_user, :admin_group, @group)
+ %section.settings.gs-general.no-animate.expanded#js-general-settings
+ .settings-header
+ %h4.settings-title.js-settings-toggle.js-settings-toggle-trigger-only{ role: 'button' }
+ = _('Naming, visibility')
+ = render Pajamas::ButtonComponent.new(button_options: { class: 'js-settings-toggle' }) do
+ = _('Collapse')
+ %p.gl-text-secondary
+ = _('Update your group name, description, avatar, and visibility.')
+ = link_to _('Learn more about groups.'), help_page_path('user/group/index')
+ .settings-content
+ = render 'groups/settings/general'
-%section.settings.gs-permissions.no-animate#js-permissions-settings{ class: ('expanded' if expanded), data: { testid: 'permissions-settings' } }
- .settings-header
- %h4.settings-title.js-settings-toggle.js-settings-toggle-trigger-only{ role: 'button' }
- = _('Permissions and group features')
- = render Pajamas::ButtonComponent.new(button_options: { class: 'js-settings-toggle' }) do
- = expanded ? _('Collapse') : _('Expand')
- %p.gl-text-secondary
- = _('Configure advanced permissions, Large File Storage, two-factor authentication, and customer relations settings.')
- .settings-content
- = render 'groups/settings/permissions'
+ %section.settings.gs-permissions.no-animate#js-permissions-settings{ class: ('expanded' if expanded), data: { testid: 'permissions-settings' } }
+ .settings-header
+ %h4.settings-title.js-settings-toggle.js-settings-toggle-trigger-only{ role: 'button' }
+ = _('Permissions and group features')
+ = render Pajamas::ButtonComponent.new(button_options: { class: 'js-settings-toggle' }) do
+ = expanded ? _('Collapse') : _('Expand')
+ %p.gl-text-secondary
+ = _('Configure advanced permissions, Large File Storage, two-factor authentication, and customer relations settings.')
+ .settings-content
+ = render 'groups/settings/permissions'
-= render_if_exists 'groups/settings/merge_requests/merge_requests', expanded: expanded, group: @group
-= render_if_exists 'groups/settings/merge_requests/merge_request_approval_settings', expanded: expanded, group: @group, user: current_user
-= render_if_exists 'groups/analytics', expanded: expanded
+ = render_if_exists 'groups/settings/merge_requests/merge_requests', expanded: expanded, group: @group
+ = render_if_exists 'groups/settings/merge_requests/merge_request_approval_settings', expanded: expanded, group: @group, user: current_user
+ = render_if_exists 'groups/analytics', expanded: expanded
-%section.settings.no-animate#js-badge-settings{ class: ('expanded' if expanded) }
- .settings-header
- %h4.settings-title.js-settings-toggle.js-settings-toggle-trigger-only{ role: 'button' }
- = s_('GroupSettings|Badges')
- = render Pajamas::ButtonComponent.new(button_options: { class: 'js-settings-toggle' }) do
- = expanded ? _('Collapse') : _('Expand')
- %p.gl-text-secondary
- = s_('GroupSettings|Customize this group\'s badges.')
- = link_to s_('GroupSettings|What are badges?'), help_page_path('user/project/badges')
- .settings-content
- = render 'shared/badges/badge_settings'
+ %section.settings.no-animate#js-badge-settings{ class: ('expanded' if expanded) }
+ .settings-header
+ %h4.settings-title.js-settings-toggle.js-settings-toggle-trigger-only{ role: 'button' }
+ = s_('GroupSettings|Badges')
+ = render Pajamas::ButtonComponent.new(button_options: { class: 'js-settings-toggle' }) do
+ = expanded ? _('Collapse') : _('Expand')
+ %p.gl-text-secondary
+ = s_('GroupSettings|Customize this group\'s badges.')
+ = link_to s_('GroupSettings|What are badges?'), help_page_path('user/project/badges')
+ .settings-content
+ = render 'shared/badges/badge_settings'
-= render_if_exists 'groups/compliance_frameworks', expanded: expanded
-= render_if_exists 'groups/custom_project_templates_setting'
-= render_if_exists 'groups/templates_setting', expanded: expanded
-= render_if_exists 'shared/groups/max_pages_size_setting'
+ = render_if_exists 'groups/compliance_frameworks', expanded: expanded
+ = render_if_exists 'groups/custom_project_templates_setting'
+ = render_if_exists 'groups/templates_setting', expanded: expanded
+ = render_if_exists 'shared/groups/max_pages_size_setting'
-%section.settings.gs-advanced.no-animate#js-advanced-settings{ class: ('expanded' if expanded), data: { testid: 'advanced-settings-content' } }
- .settings-header
- %h4.settings-title.js-settings-toggle.js-settings-toggle-trigger-only{ role: 'button' }
- = _('Advanced')
- = render Pajamas::ButtonComponent.new(button_options: { class: 'js-settings-toggle' }) do
- = expanded ? _('Collapse') : _('Expand')
- %p.gl-text-secondary
- = _('Perform advanced options such as changing path, transferring, exporting, or removing the group.')
- .settings-content
- = render 'groups/settings/advanced'
+ %section.settings.gs-advanced.no-animate#js-advanced-settings{ class: ('expanded' if expanded), data: { testid: 'advanced-settings-content' } }
+ .settings-header
+ %h4.settings-title.js-settings-toggle.js-settings-toggle-trigger-only{ role: 'button' }
+ = _('Advanced')
+ = render Pajamas::ButtonComponent.new(button_options: { class: 'js-settings-toggle' }) do
+ = expanded ? _('Collapse') : _('Expand')
+ %p.gl-text-secondary
+ = _('Perform advanced options such as changing path, transferring, exporting, or removing the group.')
+ .settings-content
+ = render 'groups/settings/advanced'
+- elsif can?(current_user, :remove_group, @group)
+ = render 'groups/settings/remove', group: @group, remove_form_id: 'js-remove-group-form'
+ = render_if_exists 'groups/settings/restore', group: @group
+ = render_if_exists 'groups/settings/immediately_remove', group: @group, remove_form_id: 'js-remove-group-form'
diff --git a/db/migrate/20240219143045_add_remove_group_to_member_roles.rb b/db/migrate/20240219143045_add_remove_group_to_member_roles.rb
new file mode 100644
index 0000000000000000000000000000000000000000..22047afd163b96a2466e5d2f23c16b0afbbf74a2
--- /dev/null
+++ b/db/migrate/20240219143045_add_remove_group_to_member_roles.rb
@@ -0,0 +1,15 @@
+# frozen_string_literal: true
+
+class AddRemoveGroupToMemberRoles < Gitlab::Database::Migration[2.2]
+ milestone '16.10'
+
+ enable_lock_retries!
+
+ def up
+ add_column :member_roles, :remove_group, :boolean, default: false, null: false
+ end
+
+ def down
+ remove_column :member_roles, :remove_group
+ end
+end
diff --git a/db/schema_migrations/20240219143045 b/db/schema_migrations/20240219143045
new file mode 100644
index 0000000000000000000000000000000000000000..5f98c2db602700f4ecc0c0d3dc47840b3869d001
--- /dev/null
+++ b/db/schema_migrations/20240219143045
@@ -0,0 +1 @@
+12e8704b5f9d37baadb7e125d08096c7a1df93f667d64ff1f9496e59e046d0a4
\ No newline at end of file
diff --git a/db/structure.sql b/db/structure.sql
index 241efc083256c336ee905ab6c4010f84de1c2171..a97440285cf1ff740592048f54e13114aeeeeb05 100644
--- a/db/structure.sql
+++ b/db/structure.sql
@@ -4106,7 +4106,6 @@ CREATE TABLE application_settings (
lock_toggle_security_policy_custom_ci boolean DEFAULT false NOT NULL,
toggle_security_policies_policy_scope boolean DEFAULT false NOT NULL,
lock_toggle_security_policies_policy_scope boolean DEFAULT false NOT NULL,
- include_optional_metrics_in_service_ping boolean DEFAULT true NOT NULL,
rate_limits jsonb DEFAULT '{}'::jsonb NOT NULL,
elasticsearch_max_code_indexing_concurrency integer DEFAULT 30 NOT NULL,
enable_member_promotion_management boolean DEFAULT false NOT NULL,
@@ -4120,6 +4119,7 @@ CREATE TABLE application_settings (
lock_duo_features_enabled boolean DEFAULT false NOT NULL,
asciidoc_max_includes smallint DEFAULT 32 NOT NULL,
clickhouse jsonb DEFAULT '{}'::jsonb NOT NULL,
+ include_optional_metrics_in_service_ping boolean DEFAULT true NOT NULL,
CONSTRAINT app_settings_container_reg_cleanup_tags_max_list_size_positive CHECK ((container_registry_cleanup_tags_service_max_list_size >= 0)),
CONSTRAINT app_settings_container_registry_pre_import_tags_rate_positive CHECK ((container_registry_pre_import_tags_rate >= (0)::numeric)),
CONSTRAINT app_settings_dep_proxy_ttl_policies_worker_capacity_positive CHECK ((dependency_proxy_ttl_group_policy_worker_capacity >= 0)),
@@ -10670,6 +10670,7 @@ CREATE TABLE member_roles (
remove_project boolean DEFAULT false NOT NULL,
admin_terraform_state boolean DEFAULT false NOT NULL,
admin_cicd_variables boolean DEFAULT false NOT NULL,
+ remove_group boolean DEFAULT false NOT NULL,
occupies_seat boolean DEFAULT false NOT NULL,
CONSTRAINT check_4364846f58 CHECK ((char_length(description) <= 255)),
CONSTRAINT check_9907916995 CHECK ((char_length(name) <= 255))
diff --git a/doc/api/graphql/reference/index.md b/doc/api/graphql/reference/index.md
index df2e53bda31b0b9d30e0005c72bfe8e0d6c450ac..67967940b64e3549ef8153e5d79d7136c007971e 100644
--- a/doc/api/graphql/reference/index.md
+++ b/doc/api/graphql/reference/index.md
@@ -32100,6 +32100,7 @@ Member role permission.
| `READ_CODE` | Allows read-only access to the source code. |
| `READ_DEPENDENCY` | Allows read-only access to the dependencies and licenses. |
| `READ_VULNERABILITY` | Read vulnerability reports and security dashboards. |
+| `REMOVE_GROUP` | Ability to delete or restore a group. This ability does not allow deleting top level groups. Review the Retention period settings to prevent accidental deletion. |
| `REMOVE_PROJECT` | Allows deletion of projects. |
### `MemberRolesOrderBy`
diff --git a/doc/user/custom_roles/abilities.md b/doc/user/custom_roles/abilities.md
index 013ca1252fe357c4010923cdc2a744a862708962..38698cefaf55af7a8ef16bd36d743eeac2c82a04 100644
--- a/doc/user/custom_roles/abilities.md
+++ b/doc/user/custom_roles/abilities.md
@@ -41,6 +41,7 @@ These requirements are documented in the `Required permission` column in the fol
| Name | Required permission | Description | Introduced in | Feature flag | Enabled in |
|:-----|:------------|:------------------|:---------|:--------------|:---------|
| [`archive_project`](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/134998) | | Allows archiving of projects. | GitLab [16.6](https://gitlab.com/gitlab-org/gitlab/-/issues/425957) | `archive_project` | GitLab [16.7](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/139260) |
+| [`remove_group`](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/145166) | | Ability to delete or restore a group. This ability does not allow deleting top level groups. Review the Retention period settings to prevent accidental deletion. | GitLab [16.10](https://gitlab.com/gitlab-org/gitlab/-/issues/425962) | | |
| [`remove_project`](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/139696) | | Allows deletion of projects. | GitLab [16.8](https://gitlab.com/gitlab-org/gitlab/-/issues/425959) | | |
## Infrastructure as code
diff --git a/ee/app/assets/javascripts/groups/settings/permissions/index.js b/ee/app/assets/javascripts/groups/settings/permissions/index.js
index 9a1539fe4c246f1040df40b5cd1c51cd5e878602..21f2c723d123c8232a4f58fc00d8ba3928e51648 100644
--- a/ee/app/assets/javascripts/groups/settings/permissions/index.js
+++ b/ee/app/assets/javascripts/groups/settings/permissions/index.js
@@ -83,6 +83,9 @@ const onGroupPermissionsFormSubmit = (event) => {
export const initGroupPermissionsFormSubmit = () => {
const groupPermissionsForm = document.querySelector('.js-general-permissions-form');
+ if (!groupPermissionsForm) {
+ return;
+ }
const confirmModalWrapper = document.createElement('div');
confirmModalWrapper.className = confirmModalWrapperClassName;
diff --git a/ee/app/controllers/ee/groups_controller.rb b/ee/app/controllers/ee/groups_controller.rb
index c94d27a9c0af547608d05ae4dba26c8e2629fae8..70723df85ebfd27ac4d951ef578f0de9ac7748f4 100644
--- a/ee/app/controllers/ee/groups_controller.rb
+++ b/ee/app/controllers/ee/groups_controller.rb
@@ -12,9 +12,7 @@ module GroupsController
include GeoInstrumentation
include GitlabSubscriptions::SeatCountAlert
- alias_method :ee_authorize_admin_group!, :authorize_admin_group!
-
- before_action :ee_authorize_admin_group!, only: [:restore]
+ before_action :authorize_remove_group!, only: [:destroy, :restore]
before_action :check_subscription!, only: [:destroy]
before_action do
diff --git a/ee/app/policies/ee/group_policy.rb b/ee/app/policies/ee/group_policy.rb
index 010ec03a4d4e73b942aa32ec68fa71947724c36f..a917cf579186fabd23471ae5f12f5375f9b0a4f3 100644
--- a/ee/app/policies/ee/group_policy.rb
+++ b/ee/app/policies/ee/group_policy.rb
@@ -252,6 +252,15 @@ module GroupPolicy
).has_ability?
end
+ desc "Custom role on group that enables removing groups"
+ condition(:role_enables_remove_group) do
+ ::Auth::MemberRoleAbilityLoader.new(
+ user: @user,
+ resource: @subject,
+ ability: :remove_group
+ ).has_ability?
+ end
+
rule { owner & unique_project_download_limit_enabled }.policy do
enable :ban_group_member
end
@@ -561,6 +570,10 @@ module GroupPolicy
enable :admin_cicd_variables
end
+ rule { custom_roles_allowed & role_enables_remove_group & has_parent }.policy do
+ enable :remove_group
+ end
+
rule { can?(:read_group_security_dashboard) }.policy do
enable :create_vulnerability_export
enable :read_security_resource
diff --git a/ee/app/services/groups/mark_for_deletion_service.rb b/ee/app/services/groups/mark_for_deletion_service.rb
index e55d86d31f94b0abc912b52b8af05f0116934fea..e311d02c820a03e04c85e286554ddd6cd1d78e12 100644
--- a/ee/app/services/groups/mark_for_deletion_service.rb
+++ b/ee/app/services/groups/mark_for_deletion_service.rb
@@ -3,7 +3,7 @@
module Groups
class MarkForDeletionService < Groups::BaseService
def execute
- return error(_('You are not authorized to perform this action')) unless can?(current_user, :admin_group, group)
+ return error(_('You are not authorized to perform this action')) unless can?(current_user, :remove_group, group)
return error(_('Group has been already marked for deletion')) if group.marked_for_deletion?
result = create_deletion_schedule
diff --git a/ee/app/services/groups/restore_service.rb b/ee/app/services/groups/restore_service.rb
index 19592a6b455e48efe042fe5b84b954522fe2ba55..7676f56d68ea3d180652c94c6cedba3c2e904408 100644
--- a/ee/app/services/groups/restore_service.rb
+++ b/ee/app/services/groups/restore_service.rb
@@ -3,7 +3,7 @@
module Groups
class RestoreService < Groups::BaseService
def execute
- return error(_('You are not authorized to perform this action')) unless can?(current_user, :admin_group, group)
+ return error(_('You are not authorized to perform this action')) unless can?(current_user, :remove_group, group)
return error(_('Group has not been marked for deletion')) unless group.marked_for_deletion?
result = remove_deletion_schedule
diff --git a/ee/config/custom_abilities/remove_group.yml b/ee/config/custom_abilities/remove_group.yml
new file mode 100644
index 0000000000000000000000000000000000000000..e3f2c33b13232ba4db97644a6763a412715f7ecf
--- /dev/null
+++ b/ee/config/custom_abilities/remove_group.yml
@@ -0,0 +1,11 @@
+---
+name: remove_group
+description: Ability to delete or restore a group. This ability does not allow deleting top level groups. Review the Retention period settings to prevent accidental deletion.
+introduced_by_issue: https://gitlab.com/gitlab-org/gitlab/-/issues/425962
+introduced_by_mr: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/145166
+feature_category: groups_and_projects
+milestone: "16.10"
+group_ability: true
+project_ability: false
+requirements: []
+available_from_access_level: 50
diff --git a/ee/lib/ee/api/groups.rb b/ee/lib/ee/api/groups.rb
index 57b6b2380d56b2e70b714bc40e36e94d09f17400..bfbd8388a2b49a44b722b3d1ee226a82ddf14977 100644
--- a/ee/lib/ee/api/groups.rb
+++ b/ee/lib/ee/api/groups.rb
@@ -219,7 +219,7 @@ def delete_group(group)
desc 'Restore a group.'
post ':id/restore', feature_category: :groups_and_projects do
- authorize! :admin_group, user_group
+ authorize! :remove_group, user_group
break not_found! unless user_group.licensed_feature_available?(:adjourned_deletion_for_projects_and_groups)
result = ::Groups::RestoreService.new(user_group, current_user).execute
diff --git a/ee/lib/ee/sidebars/groups/menus/settings_menu.rb b/ee/lib/ee/sidebars/groups/menus/settings_menu.rb
index 096b238c32b728d2e77a23965a2a79ab3abae6b2..7021ce16fc6cbf95cfa91095bb24bf09db672851 100644
--- a/ee/lib/ee/sidebars/groups/menus/settings_menu.rb
+++ b/ee/lib/ee/sidebars/groups/menus/settings_menu.rb
@@ -21,24 +21,20 @@ def configure_menu_items
add_item(billing_menu_item)
add_item(reporting_menu_item)
else
- if can?(context.current_user, :read_resource_access_tokens, context.group)
- # Managing group acccess tokens is a custom ability independent of the access level.
- add_item(access_tokens_menu_item)
- end
-
- # Push Rules are the only group setting that can also be edited by maintainers.
- # They only get the Repository settings which only show the Push Rules section for maintainers.
- add_item(repository_menu_item) if can?(context.current_user, :change_push_rules, context.group)
-
- # Managing CI/CD settings is a custom ability independent of the access level.
- add_item(ci_cd_menu_item) if can?(context.current_user, :admin_cicd_variables, context.group)
-
- add_item(billing_menu_item) if can?(context.current_user, :read_billing, context.group)
+ add_menu_item_for_ability(general_menu_item, :remove_group)
+ add_menu_item_for_ability(access_tokens_menu_item, :read_resource_access_tokens)
+ add_menu_item_for_ability(repository_menu_item, :change_push_rules)
+ add_menu_item_for_ability(ci_cd_menu_item, :admin_cicd_variables)
+ add_menu_item_for_ability(billing_menu_item, :read_billing)
end
end
private
+ def add_menu_item_for_ability(menu_item, ability)
+ add_item(menu_item) if can?(context.current_user, ability, context.group)
+ end
+
def roles_and_permissions_menu_item
return ::Sidebars::NilMenuItem.new(item_id: :roles_and_permissions) unless custom_roles_enabled?
diff --git a/ee/spec/lib/ee/sidebars/groups/menus/settings_menu_spec.rb b/ee/spec/lib/ee/sidebars/groups/menus/settings_menu_spec.rb
index 9f543d941dfa75dc96128719e286e177729a14a6..26beea708ba68952654229f100267cfe93b1dede 100644
--- a/ee/spec/lib/ee/sidebars/groups/menus/settings_menu_spec.rb
+++ b/ee/spec/lib/ee/sidebars/groups/menus/settings_menu_spec.rb
@@ -316,5 +316,27 @@
end
end
end
+
+ context 'when the user is not an owner but has `remove_group` custom ability', feature_category: :permissions do
+ let_it_be(:user) { create(:user) }
+
+ subject { menu.renderable_items.find { |e| e.item_id == item_id } }
+
+ before do
+ allow(Ability).to receive(:allowed?).and_call_original
+ allow(Ability).to receive(:allowed?).with(user, :admin_group, group).and_return(false)
+ allow(Ability).to receive(:allowed?).with(user, :remove_group, group).and_return(true)
+ end
+
+ describe 'General menu item' do
+ let(:item_id) { :general }
+
+ it { is_expected.to be_present }
+
+ it 'does not show any other menu items' do
+ expect(menu.renderable_items.length).to equal(1)
+ end
+ end
+ end
end
end
diff --git a/ee/spec/models/ee/user_spec.rb b/ee/spec/models/ee/user_spec.rb
index 242c2277c670bcbd18b3100728cf7c74c9114861..b92844e5ef06716eb0f44480b8555218f32e0c0c 100644
--- a/ee/spec/models/ee/user_spec.rb
+++ b/ee/spec/models/ee/user_spec.rb
@@ -1272,6 +1272,7 @@
OR manage_project_access_tokens = true
OR read_dependency = true
OR read_vulnerability = true
+ OR remove_group = true
OR remove_project = true\)\)\)\)'.squish # allow_cross_joins_across_databases
end
diff --git a/ee/spec/policies/group_policy_spec.rb b/ee/spec/policies/group_policy_spec.rb
index a13ed1bfc2b2a8d7fc4cfbf51c2be8f92256f924..cf280393cfa42c0192283d1f3b9110c0a020b112 100644
--- a/ee/spec/policies/group_policy_spec.rb
+++ b/ee/spec/policies/group_policy_spec.rb
@@ -3284,6 +3284,24 @@ def create_member_role(member, abilities = member_role_abilities)
it_behaves_like 'custom roles abilities'
end
+
+ context 'for a custom role with the `remove_group` ability' do
+ let(:member_role_abilities) { { remove_group: true } }
+ let(:allowed_abilities) { [:remove_group, :view_edit_page] }
+
+ it_behaves_like 'custom roles abilities'
+
+ context 'when the group is a top level group' do
+ before do
+ create_member_role(parent_group_member_guest)
+ stub_licensed_features(custom_roles: true)
+ end
+
+ subject { described_class.new(current_user, parent_group) }
+
+ it { is_expected.to be_disallowed(*allowed_abilities) }
+ end
+ end
end
context 'for :read_limit_alert' do
diff --git a/ee/spec/requests/api/member_roles_spec.rb b/ee/spec/requests/api/member_roles_spec.rb
index b93047ff302dd7ecb07110fee072ad2ecc8cd94d..b556783444a455fc1c30948f718dc9d2cb7ae9e9 100644
--- a/ee/spec/requests/api/member_roles_spec.rb
+++ b/ee/spec/requests/api/member_roles_spec.rb
@@ -109,6 +109,7 @@
"manage_group_access_tokens" => false,
"manage_project_access_tokens" => false,
"archive_project" => false,
+ "remove_group" => false,
"remove_project" => false,
"admin_cicd_variables" => false,
"group_id" => group_id
@@ -128,6 +129,7 @@
"manage_group_access_tokens" => false,
"manage_project_access_tokens" => false,
"archive_project" => false,
+ "remove_group" => false,
"remove_project" => false,
"admin_cicd_variables" => false,
"group_id" => group_id
diff --git a/ee/spec/requests/custom_roles/remove_group/request_spec.rb b/ee/spec/requests/custom_roles/remove_group/request_spec.rb
new file mode 100644
index 0000000000000000000000000000000000000000..f42e9878e2fa34ec38951911f1cde0b49536e62a
--- /dev/null
+++ b/ee/spec/requests/custom_roles/remove_group/request_spec.rb
@@ -0,0 +1,78 @@
+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe 'User with the `remove_group` custom ability', feature_category: :groups_and_projects do
+ let_it_be(:user) { create(:user) }
+ let_it_be(:group) { create(:group) }
+ let_it_be(:role) { create(:member_role, :guest, namespace: group, remove_group: true) }
+ let_it_be(:member) { create(:group_member, :guest, member_role: role, user: user, group: group) }
+ let_it_be(:subgroup) { create(:group, parent: group) }
+
+ before do
+ stub_licensed_features(custom_roles: true, adjourned_deletion_for_projects_and_groups: true)
+ end
+
+ describe GroupsController do
+ before do
+ sign_in(user)
+ end
+
+ describe '#edit' do
+ it 'user can view the edit page of the subgroup' do
+ get edit_group_path(subgroup)
+
+ expect(response).to have_gitlab_http_status(:ok)
+ expect(response.body).to include(_('Remove group'))
+ end
+ end
+
+ describe '#destroy' do
+ it 'user can delete the subgroup' do
+ delete group_path(subgroup)
+
+ expect(response).to have_gitlab_http_status(:found)
+ expect(subgroup.reload).to be_marked_for_deletion
+ end
+ end
+
+ describe '#restore' do
+ before do
+ create(:group_deletion_schedule, group: subgroup)
+ end
+
+ it 'user can restore the subgroup' do
+ post group_restore_path(subgroup)
+
+ expect(response).to have_gitlab_http_status(:found)
+ expect(subgroup.reload).not_to be_marked_for_deletion
+ end
+ end
+ end
+
+ describe API::Groups do
+ include ApiHelpers
+
+ describe '#delete' do
+ it 'user can delete the subgroup' do
+ delete api("/groups/#{subgroup.id}", user)
+
+ expect(response).to have_gitlab_http_status(:accepted)
+ expect(subgroup.reload).to be_marked_for_deletion
+ end
+ end
+
+ describe '#restore' do
+ before do
+ create(:group_deletion_schedule, group: subgroup)
+ end
+
+ it 'user can restore the subgroup' do
+ post api("/groups/#{subgroup.id}/restore", user)
+
+ expect(response).to have_gitlab_http_status(:created)
+ expect(subgroup.reload).not_to be_marked_for_deletion
+ end
+ end
+ end
+end
diff --git a/ee/spec/support/shared_examples/views/registration_features_prompt_settings_link_shared_examples.rb b/ee/spec/support/shared_examples/views/registration_features_prompt_settings_link_shared_examples.rb
index 230ff310bf68512d1e23a2a234bbe87edb88e380..448ba6435dffb689c5eba636bc3c7dc70dbdc6c7 100644
--- a/ee/spec/support/shared_examples/views/registration_features_prompt_settings_link_shared_examples.rb
+++ b/ee/spec/support/shared_examples/views/registration_features_prompt_settings_link_shared_examples.rb
@@ -7,6 +7,7 @@
context 'as regular user' do
before do
allow(view).to receive(:current_user) { user }
+ allow(view).to receive(:can?).with(user, :admin_group, anything).and_return(false)
end
it 'does not render settings link', :aggregate_failures do
@@ -17,6 +18,7 @@
context 'as admin' do
before do
allow(view).to receive(:current_user) { admin }
+ allow(view).to receive(:can?).with(admin, :admin_group, anything).and_return(true)
end
it 'renders settings link', :aggregate_failures do
diff --git a/ee/spec/views/groups/edit.html.haml_spec.rb b/ee/spec/views/groups/edit.html.haml_spec.rb
index 4b77219e83c7327b248d5c1a52e7163897e0e841..d74f5cb80df4accbe4c383e6cb20d08dce1bcc80 100644
--- a/ee/spec/views/groups/edit.html.haml_spec.rb
+++ b/ee/spec/views/groups/edit.html.haml_spec.rb
@@ -170,4 +170,22 @@
it_behaves_like 'does not render allowed_email_domain setting'
end
end
+
+ context 'when rendering for a user that is not an owner', feature_category: :permissions do
+ before do
+ group.add_guest(user)
+ end
+
+ subject { render }
+
+ it { is_expected.not_to have_text(_('Remove group')) }
+
+ context 'when the user can remove groups' do
+ before do
+ allow(view).to receive(:can?).with(user, :remove_group, group).and_return(true)
+ end
+
+ it { is_expected.to have_text(_('Remove group')) }
+ end
+ end
end
diff --git a/lib/api/groups.rb b/lib/api/groups.rb
index 7b755a76f29c3abbe4fba74d915f835489828759..0132ab9c2829f4d3048e81ead0fa4fba700a5413 100644
--- a/lib/api/groups.rb
+++ b/lib/api/groups.rb
@@ -291,7 +291,7 @@ def check_subscription!(group)
end
delete ":id", feature_category: :groups_and_projects, urgency: :low do
group = find_group!(params[:id])
- authorize! :admin_group, group
+ authorize! :remove_group, group
check_subscription! group
delete_group(group)
diff --git a/spec/support/shared_contexts/policies/group_policy_shared_context.rb b/spec/support/shared_contexts/policies/group_policy_shared_context.rb
index 509bc413b8d4c161f3661f7241da8565a143fe18..565de038f51c24c5f70a63cd552ddc31ec837e0b 100644
--- a/spec/support/shared_contexts/policies/group_policy_shared_context.rb
+++ b/spec/support/shared_contexts/policies/group_policy_shared_context.rb
@@ -80,6 +80,8 @@
destroy_issue
admin_member_access_request
update_git_access_protocol
+ remove_group
+ view_edit_page
]
end