diff --git a/db/migrate/20190626175626_add_group_creation_level_to_namespaces.rb b/db/migrate/20190626175626_add_group_creation_level_to_namespaces.rb index eed0ba25f27..85ac89af46e 100644 --- a/db/migrate/20190626175626_add_group_creation_level_to_namespaces.rb +++ b/db/migrate/20190626175626_add_group_creation_level_to_namespaces.rb @@ -12,6 +12,7 @@ class AddGroupCreationLevelToNamespaces < ActiveRecord::Migration[5.1] :subgroup_creation_level, :integer, default: 0) + change_column_default(:namespaces, :subgroup_creation_level, 1) end end diff --git a/spec/controllers/admin/groups_controller_spec.rb b/spec/controllers/admin/groups_controller_spec.rb index 72f389513f8..398f587bafe 100644 --- a/spec/controllers/admin/groups_controller_spec.rb +++ b/spec/controllers/admin/groups_controller_spec.rb @@ -70,14 +70,13 @@ describe Admin::GroupsController do end it 'updates the subgroup_creation_level successfully' do - MAINTAINER = ::Gitlab::Access::MAINTAINER_SUBGROUP_ACCESS + OWNER = ::Gitlab::Access::OWNER_SUBGROUP_ACCESS expect do post :update, params: { id: group.to_param, - group: { subgroup_creation_level: MAINTAINER } } - end.to change { group.reload.subgroup_creation_level } - .to(MAINTAINER) + group: { subgroup_creation_level: OWNER } } + end.to change { group.reload.subgroup_creation_level }.to(OWNER) end end end diff --git a/spec/factories/groups.rb b/spec/factories/groups.rb index 2f50fbfe2fa..b67ab955ffc 100644 --- a/spec/factories/groups.rb +++ b/spec/factories/groups.rb @@ -5,7 +5,6 @@ FactoryBot.define do type 'Group' owner nil project_creation_level ::Gitlab::Access::MAINTAINER_PROJECT_ACCESS - subgroup_creation_level ::Gitlab::Access::OWNER_SUBGROUP_ACCESS after(:create) do |group| if group.owner @@ -46,5 +45,9 @@ FactoryBot.define do trait :auto_devops_disabled do auto_devops_enabled false end + + trait :owner_subgroup_creation_only do + subgroup_creation_level ::Gitlab::Access::OWNER_SUBGROUP_ACCESS + end end end diff --git a/spec/features/groups/show_spec.rb b/spec/features/groups/show_spec.rb index 163906010fa..48ba9064327 100644 --- a/spec/features/groups/show_spec.rb +++ b/spec/features/groups/show_spec.rb @@ -72,10 +72,11 @@ describe 'Group show page' do context 'when subgroups are supported', :js, :nested_groups do before do allow(Group).to receive(:supports_nested_objects?) { true } - visit path end it 'allows creating subgroups' do + visit path + expect(page) .to have_css("li[data-text='New subgroup']", visible: false) end @@ -84,10 +85,11 @@ describe 'Group show page' do context 'when subgroups are not supported' do before do allow(Group).to receive(:supports_nested_objects?) { false } - visit path end it 'does not allow creating subgroups' do + visit path + expect(page) .not_to have_selector("li[data-text='New subgroup']", visible: false) end @@ -102,10 +104,9 @@ describe 'Group show page' do context 'when subgroups are supported', :js, :nested_groups do before do allow(Group).to receive(:supports_nested_objects?) { true } - visit path end - context 'when subgroup_creation_level is set to maintainer' do + context 'when subgroup_creation_level is set to maintainers' do let(:group) do create(:group, subgroup_creation_level: ::Gitlab::Access::MAINTAINER_SUBGROUP_ACCESS) @@ -113,7 +114,9 @@ describe 'Group show page' do it 'allows creating subgroups' do visit path - expect(page).to have_css("li[data-text='New subgroup']", visible: false) + + expect(page) + .to have_css("li[data-text='New subgroup']", visible: false) end end @@ -125,6 +128,7 @@ describe 'Group show page' do it 'does not allow creating subgroups' do visit path + expect(page) .not_to have_css("li[data-text='New subgroup']", visible: false) end @@ -134,10 +138,11 @@ describe 'Group show page' do context 'when subgroups are not supported' do before do allow(Group).to receive(:supports_nested_objects?) { false } - visit path end it 'does not allow creating subgroups' do + visit path + expect(page) .not_to have_selector("li[data-text='New subgroup']", visible: false) end diff --git a/spec/models/group_spec.rb b/spec/models/group_spec.rb index 9ae18d7bab7..c7fb0f51075 100644 --- a/spec/models/group_spec.rb +++ b/spec/models/group_spec.rb @@ -997,9 +997,8 @@ describe Group do describe 'subgroup_creation_level' do it 'defaults to maintainers' do - group = create (:group) - - expect(group.subgroup_creation_level).to eq(Gitlab::Access::MAINTAINER_SUBGROUP_ACCESS) + expect(group.subgroup_creation_level) + .to eq(Gitlab::Access::MAINTAINER_SUBGROUP_ACCESS) end end end diff --git a/spec/policies/group_policy_spec.rb b/spec/policies/group_policy_spec.rb index da186f63eca..7fba62d2aa8 100644 --- a/spec/policies/group_policy_spec.rb +++ b/spec/policies/group_policy_spec.rb @@ -98,12 +98,38 @@ describe GroupPolicy do context 'maintainer' do let(:current_user) { maintainer } - it do - expect_allowed(*guest_permissions) - expect_allowed(*reporter_permissions) - expect_allowed(*developer_permissions) - expect_allowed(*maintainer_permissions) - expect_disallowed(*owner_permissions) + context 'with subgroup_creation level set to maintainer' do + let(:group) { create(:group, + :private, + subgroup_creation_level: ::Gitlab::Access::MAINTAINER_SUBGROUP_ACCESS) } + + it do + allow(Group).to receive(:supports_nested_objects?).and_return(true) + + create_subgroup_permission = [:create_subgroup] + updated_maintainer_permissions = + maintainer_permissions + create_subgroup_permission + updated_owner_permissions = + owner_permissions - create_subgroup_permission + + expect_allowed(*guest_permissions) + expect_allowed(*reporter_permissions) + expect_allowed(*developer_permissions) + expect_allowed(*updated_maintainer_permissions) + expect_disallowed(*updated_owner_permissions) + end + end + + context 'with subgroup_creation_level set to owner' do + it do + allow(Group).to receive(:supports_nested_objects?).and_return(true) + + expect_allowed(*guest_permissions) + expect_allowed(*reporter_permissions) + expect_allowed(*developer_permissions) + expect_allowed(*maintainer_permissions) + expect_disallowed(*owner_permissions) + end end end @@ -181,7 +207,10 @@ describe GroupPolicy do end describe 'private nested group use the highest access level from the group and inherited permissions', :nested_groups do - let(:nested_group) { create(:group, :private, parent: group) } + let(:nested_group) { create(:group, + :private, + :owner_subgroup_creation_only, + parent: group) } before do nested_group.add_guest(guest) diff --git a/spec/requests/api/groups_spec.rb b/spec/requests/api/groups_spec.rb index c41408fba65..52d926d5484 100644 --- a/spec/requests/api/groups_spec.rb +++ b/spec/requests/api/groups_spec.rb @@ -803,10 +803,10 @@ describe API::Groups do group2.add_maintainer(user1) end - it 'cannot create subgroups' do + it 'can create subgroups' do post api("/groups", user1), params: { parent_id: group2.id, name: 'foo', path: 'foo' } - expect(response).to have_gitlab_http_status(403) + expect(response).to have_gitlab_http_status(201) end end end 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 b4808ac0068..74389c4d82b 100644 --- a/spec/support/shared_contexts/policies/group_policy_shared_context.rb +++ b/spec/support/shared_contexts/policies/group_policy_shared_context.rb @@ -7,7 +7,7 @@ RSpec.shared_context 'GroupPolicy context' do let(:maintainer) { create(:user) } let(:owner) { create(:user) } let(:admin) { create(:admin) } - let(:group) { create(:group, :private) } + let(:group) { create(:group, :private, :owner_subgroup_creation_only) } let(:guest_permissions) do %i[