diff options
author | Sean McGivern <sean@gitlab.com> | 2019-07-22 15:17:23 +0000 |
---|---|---|
committer | Sean McGivern <sean@gitlab.com> | 2019-07-22 15:17:23 +0000 |
commit | e48851de62086b65c75a3dd802743e722d5d7be8 (patch) | |
tree | e41184b94bd2945a8d8fbaf6004cc8247fdd8ea4 /spec | |
parent | ffb39001616f0c9457785e44bcabfc7342e9f919 (diff) | |
parent | 6c4ddc409fe95215502b6427452715edd31b41ab (diff) | |
download | gitlab-ce-e48851de62086b65c75a3dd802743e722d5d7be8.tar.gz |
Merge branch 'maintainers-can-create-subgroup' into 'master'
Add a group setting to allow Maintainers to create sub-groups
See merge request gitlab-org/gitlab-ce!29718
Diffstat (limited to 'spec')
-rw-r--r-- | spec/controllers/admin/groups_controller_spec.rb | 8 | ||||
-rw-r--r-- | spec/factories/groups.rb | 4 | ||||
-rw-r--r-- | spec/features/admin/admin_groups_spec.rb | 8 | ||||
-rw-r--r-- | spec/features/groups/group_settings_spec.rb | 8 | ||||
-rw-r--r-- | spec/features/groups/show_spec.rb | 99 | ||||
-rw-r--r-- | spec/models/group_spec.rb | 7 | ||||
-rw-r--r-- | spec/policies/group_policy_spec.rb | 127 | ||||
-rw-r--r-- | spec/requests/api/groups_spec.rb | 4 | ||||
-rw-r--r-- | spec/services/groups/create_service_spec.rb | 8 | ||||
-rw-r--r-- | spec/support/shared_contexts/policies/group_policy_shared_context.rb | 2 |
10 files changed, 249 insertions, 26 deletions
diff --git a/spec/controllers/admin/groups_controller_spec.rb b/spec/controllers/admin/groups_controller_spec.rb index 509d8944e3a..1123563c1e3 100644 --- a/spec/controllers/admin/groups_controller_spec.rb +++ b/spec/controllers/admin/groups_controller_spec.rb @@ -68,5 +68,13 @@ describe Admin::GroupsController do post :update, params: { id: group.to_param, group: { project_creation_level: ::Gitlab::Access::NO_ONE_PROJECT_ACCESS } } end.to change { group.reload.project_creation_level }.to(::Gitlab::Access::NO_ONE_PROJECT_ACCESS) end + + it 'updates the subgroup_creation_level successfully' do + expect do + post :update, + params: { id: group.to_param, + group: { subgroup_creation_level: ::Gitlab::Access::OWNER_SUBGROUP_ACCESS } } + end.to change { group.reload.subgroup_creation_level }.to(::Gitlab::Access::OWNER_SUBGROUP_ACCESS) + end end end diff --git a/spec/factories/groups.rb b/spec/factories/groups.rb index 18a0c2ec731..b67ab955ffc 100644 --- a/spec/factories/groups.rb +++ b/spec/factories/groups.rb @@ -45,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/admin/admin_groups_spec.rb b/spec/features/admin/admin_groups_spec.rb index bac2972b0fb..a08902c30be 100644 --- a/spec/features/admin/admin_groups_spec.rb +++ b/spec/features/admin/admin_groups_spec.rb @@ -103,6 +103,14 @@ describe 'Admin Groups' do expect_selected_visibility(group.visibility_level) end + it 'shows the subgroup creation level dropdown populated with the group subgroup_creation_level value' do + group = create(:group, :private, :owner_subgroup_creation_only) + + visit admin_group_edit_path(group) + + expect(page).to have_content('Allowed to create subgroups') + end + it 'edit group path does not change group name', :js do group = create(:group, :private) diff --git a/spec/features/groups/group_settings_spec.rb b/spec/features/groups/group_settings_spec.rb index 5cef5f0521f..676769c25fe 100644 --- a/spec/features/groups/group_settings_spec.rb +++ b/spec/features/groups/group_settings_spec.rb @@ -85,6 +85,14 @@ describe 'Edit group settings' do end end + describe 'subgroup creation level menu' do + it 'shows the selection menu' do + visit edit_group_path(group) + + expect(page).to have_content('Allowed to create subgroups') + end + end + describe 'edit group avatar' do before do visit edit_group_path(group) diff --git a/spec/features/groups/show_spec.rb b/spec/features/groups/show_spec.rb index 9671a4d8c49..bed998a0859 100644 --- a/spec/features/groups/show_spec.rb +++ b/spec/features/groups/show_spec.rb @@ -56,32 +56,103 @@ describe 'Group show page' do end context 'subgroup support' do - let(:user) { create(:user) } + let(:restricted_group) do + create(:group, subgroup_creation_level: ::Gitlab::Access::OWNER_SUBGROUP_ACCESS) + end - before do - group.add_owner(user) - sign_in(user) + let(:relaxed_group) do + create(:group, subgroup_creation_level: ::Gitlab::Access::MAINTAINER_SUBGROUP_ACCESS) end - context 'when subgroups are supported', :js, :nested_groups do + let(:owner) { create(:user) } + let(:maintainer) { create(:user) } + + context 'for owners' do + let(:path) { group_path(restricted_group) } + before do - allow(Group).to receive(:supports_nested_objects?) { true } - visit path + restricted_group.add_owner(owner) + sign_in(owner) end - it 'allows creating subgroups' do - expect(page).to have_css("li[data-text='New subgroup']", visible: false) + context 'when subgroups are supported', :nested_groups do + before do + allow(Group).to receive(:supports_nested_objects?) { true } + end + + it 'allows creating subgroups' do + visit path + + expect(page) + .to have_css("li[data-text='New subgroup']", visible: false) + end + end + + context 'when subgroups are not supported' do + before do + allow(Group).to receive(:supports_nested_objects?) { false } + end + + it 'does not allow creating subgroups' do + visit path + + expect(page) + .not_to have_selector("li[data-text='New subgroup']", visible: false) + end end end - context 'when subgroups are not supported' do + context 'for maintainers' do before do - allow(Group).to receive(:supports_nested_objects?) { false } - visit path + sign_in(maintainer) end - it 'allows creating subgroups' do - expect(page).not_to have_selector("li[data-text='New subgroup']", visible: false) + context 'when subgroups are supported', :nested_groups do + before do + allow(Group).to receive(:supports_nested_objects?) { true } + end + + context 'when subgroup_creation_level is set to maintainers' do + before do + relaxed_group.add_maintainer(maintainer) + end + + it 'allows creating subgroups' do + path = group_path(relaxed_group) + visit path + + expect(page) + .to have_css("li[data-text='New subgroup']", visible: false) + end + end + + context 'when subgroup_creation_level is set to owners' do + before do + restricted_group.add_maintainer(maintainer) + end + + it 'does not allow creating subgroups' do + path = group_path(restricted_group) + visit path + + expect(page) + .not_to have_selector("li[data-text='New subgroup']", + visible: false) + end + end + end + + context 'when subgroups are not supported' do + before do + allow(Group).to receive(:supports_nested_objects?) { false } + end + + it 'does not allow creating subgroups' do + visit path + + expect(page) + .not_to have_selector("li[data-text='New subgroup']", visible: false) + end end end end diff --git a/spec/models/group_spec.rb b/spec/models/group_spec.rb index 470ce65707d..c7fb0f51075 100644 --- a/spec/models/group_spec.rb +++ b/spec/models/group_spec.rb @@ -994,4 +994,11 @@ describe Group do expect(group.project_creation_level).to eq(Gitlab::CurrentSettings.default_project_creation) end end + + describe 'subgroup_creation_level' do + it 'defaults to maintainers' do + 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 59f3a961d50..dc3675a7b9e 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) do + create(:group, :private, subgroup_creation_level: ::Gitlab::Access::MAINTAINER_SUBGROUP_ACCESS) + end + + it 'allows every maintainer permission plus creating subgroups' 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 'allows every maintainer permission' 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 @@ -145,7 +171,8 @@ describe GroupPolicy do it 'allows every owner permission except creating subgroups' do create_subgroup_permission = [:create_subgroup] - updated_owner_permissions = owner_permissions - create_subgroup_permission + updated_owner_permissions = + owner_permissions - create_subgroup_permission expect_disallowed(*create_subgroup_permission) expect_allowed(*updated_owner_permissions) @@ -157,16 +184,32 @@ describe GroupPolicy do it 'allows every owner permission except creating subgroups' do create_subgroup_permission = [:create_subgroup] - updated_owner_permissions = owner_permissions - create_subgroup_permission + updated_owner_permissions = + owner_permissions - create_subgroup_permission expect_disallowed(*create_subgroup_permission) expect_allowed(*updated_owner_permissions) end end + + context 'maintainer' do + let(:current_user) { maintainer } + + it 'allows every maintainer permission except creating subgroups' do + create_subgroup_permission = [:create_subgroup] + updated_maintainer_permissions = + maintainer_permissions - create_subgroup_permission + + expect_disallowed(*create_subgroup_permission) + expect_allowed(*updated_maintainer_permissions) + end + end 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) do + create(:group, :private, :owner_subgroup_creation_only, parent: group) + end before do nested_group.add_guest(guest) @@ -461,6 +504,72 @@ describe GroupPolicy do end end + context "create_subgroup" do + context 'when group has subgroup creation level set to owner' do + let(:group) do + create( + :group, + subgroup_creation_level: ::Gitlab::Access::OWNER_SUBGROUP_ACCESS) + end + + context 'reporter' do + let(:current_user) { reporter } + + it { is_expected.to be_disallowed(:create_subgroup) } + end + + context 'developer' do + let(:current_user) { developer } + + it { is_expected.to be_disallowed(:create_subgroup) } + end + + context 'maintainer' do + let(:current_user) { maintainer } + + it { is_expected.to be_disallowed(:create_subgroup) } + end + + context 'owner' do + let(:current_user) { owner } + + it { is_expected.to be_allowed(:create_subgroup) } + end + end + + context 'when group has subgroup creation level set to maintainer' do + let(:group) do + create( + :group, + subgroup_creation_level: ::Gitlab::Access::MAINTAINER_SUBGROUP_ACCESS) + end + + context 'reporter' do + let(:current_user) { reporter } + + it { is_expected.to be_disallowed(:create_subgroup) } + end + + context 'developer' do + let(:current_user) { developer } + + it { is_expected.to be_disallowed(:create_subgroup) } + end + + context 'maintainer' do + let(:current_user) { maintainer } + + it { is_expected.to be_allowed(:create_subgroup) } + end + + context 'owner' do + let(:current_user) { owner } + + it { is_expected.to be_allowed(:create_subgroup) } + end + end + end + it_behaves_like 'clusterable policies' do let(:clusterable) { create(:group) } let(:cluster) do 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/services/groups/create_service_spec.rb b/spec/services/groups/create_service_spec.rb index c5ff6cdbacd..a7c95428485 100644 --- a/spec/services/groups/create_service_spec.rb +++ b/spec/services/groups/create_service_spec.rb @@ -87,6 +87,14 @@ describe Groups::CreateService, '#execute' do it { is_expected.to be_persisted } end + + context 'as maintainer' do + before do + group.add_maintainer(user) + end + + it { is_expected.to be_persisted } + 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[ |