Commit a6d268b6 authored by Ash McKenzie's avatar Ash McKenzie

Merge branch 'fix-group-transfer-to-subgroup' into 'master'

Fix for group transfer service to deny transferring a group to its subgroup

See merge request gitlab-org/gitlab!31495
parents 0d72e1b0 5284132c
...@@ -45,6 +45,7 @@ module Groups ...@@ -45,6 +45,7 @@ module Groups
raise_transfer_error(:invalid_policies) unless valid_policies? raise_transfer_error(:invalid_policies) unless valid_policies?
raise_transfer_error(:namespace_with_same_path) if namespace_with_same_path? raise_transfer_error(:namespace_with_same_path) if namespace_with_same_path?
raise_transfer_error(:group_contains_images) if group_projects_contain_registry_images? raise_transfer_error(:group_contains_images) if group_projects_contain_registry_images?
raise_transfer_error(:cannot_transfer_to_subgroup) if transfer_to_subgroup?
end end
def group_is_already_root? def group_is_already_root?
...@@ -55,6 +56,11 @@ module Groups ...@@ -55,6 +56,11 @@ module Groups
@new_parent_group && @new_parent_group.id == @group.parent_id @new_parent_group && @new_parent_group.id == @group.parent_id
end end
def transfer_to_subgroup?
@new_parent_group && \
@group.self_and_descendants.pluck_primary_key.include?(@new_parent_group.id)
end
def valid_policies? def valid_policies?
return false unless can?(current_user, :admin_group, @group) return false unless can?(current_user, :admin_group, @group)
...@@ -125,7 +131,8 @@ module Groups ...@@ -125,7 +131,8 @@ module Groups
group_is_already_root: s_('TransferGroup|Group is already a root group.'), group_is_already_root: s_('TransferGroup|Group is already a root group.'),
same_parent_as_current: s_('TransferGroup|Group is already associated to the parent group.'), same_parent_as_current: s_('TransferGroup|Group is already associated to the parent group.'),
invalid_policies: s_("TransferGroup|You don't have enough permissions."), invalid_policies: s_("TransferGroup|You don't have enough permissions."),
group_contains_images: s_('TransferGroup|Cannot update the path because there are projects under this group that contain Docker images in their Container Registry. Please remove the images from your projects first and try again.') group_contains_images: s_('TransferGroup|Cannot update the path because there are projects under this group that contain Docker images in their Container Registry. Please remove the images from your projects first and try again.'),
cannot_transfer_to_subgroup: s_('TransferGroup|Cannot transfer group to one of its subgroup.')
}.freeze }.freeze
end end
end end
......
---
title: Fix group transfer service to deny moving group to its subgroup
merge_request: 31495
author: Abhisek Datta
type: fixed
...@@ -22915,6 +22915,9 @@ msgstr "" ...@@ -22915,6 +22915,9 @@ msgstr ""
msgid "Transfer project" msgid "Transfer project"
msgstr "" msgstr ""
msgid "TransferGroup|Cannot transfer group to one of its subgroup."
msgstr ""
msgid "TransferGroup|Cannot update the path because there are projects under this group that contain Docker images in their Container Registry. Please remove the images from your projects first and try again." msgid "TransferGroup|Cannot update the path because there are projects under this group that contain Docker images in their Container Registry. Please remove the images from your projects first and try again."
msgstr "" msgstr ""
......
...@@ -216,6 +216,15 @@ describe Groups::TransferService do ...@@ -216,6 +216,15 @@ describe Groups::TransferService do
end end
end end
context 'when a group is transferred to its subgroup' do
let(:new_parent_group) { create(:group, parent: group) }
it 'does not execute the transfer' do
expect(transfer_service.execute(new_parent_group)).to be_falsy
expect(transfer_service.error).to match(/Cannot transfer group to one of its subgroup/)
end
end
context 'when transferring a group with group descendants' do context 'when transferring a group with group descendants' do
let!(:subgroup1) { create(:group, :private, parent: group) } let!(:subgroup1) { create(:group, :private, parent: group) }
let!(:subgroup2) { create(:group, :internal, parent: group) } let!(:subgroup2) { create(:group, :internal, parent: group) }
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment