diff options
author | GitLab Release Tools Bot <robert+release-tools@gitlab.com> | 2019-10-24 18:53:44 +0000 |
---|---|---|
committer | GitLab Release Tools Bot <robert+release-tools@gitlab.com> | 2019-10-24 18:53:44 +0000 |
commit | c620b8f91759e38b429eb931eb3f75202321fb96 (patch) | |
tree | 74b888943a5a81c0953f776c043ea964b0e8a51b | |
parent | 19c5bdd7cc3908f40c284c51c75e5b6f69048c15 (diff) | |
parent | 4aa9bb16e679273cf516922e1f7b995ae36a72d0 (diff) | |
download | gitlab-ce-c620b8f91759e38b429eb931eb3f75202321fb96.tar.gz |
Merge branch 'security-developer-transfer-project-12-2' into '12-2-stable'
Require Maintainer permission on group where project is transferred to
See merge request gitlab/gitlabhq!3473
-rw-r--r-- | app/policies/group_policy.rb | 2 | ||||
-rw-r--r-- | app/policies/namespace_policy.rb | 2 | ||||
-rw-r--r-- | app/services/projects/transfer_service.rb | 2 | ||||
-rw-r--r-- | changelogs/unreleased/security-developer-transfer-project.yml | 5 | ||||
-rw-r--r-- | spec/policies/group_policy_spec.rb | 82 | ||||
-rw-r--r-- | spec/policies/namespace_policy_spec.rb | 3 | ||||
-rw-r--r-- | spec/requests/api/projects_spec.rb | 16 | ||||
-rw-r--r-- | spec/services/projects/transfer_service_spec.rb | 18 |
8 files changed, 128 insertions, 2 deletions
diff --git a/app/policies/group_policy.rb b/app/policies/group_policy.rb index c686e7763bb..987f252546d 100644 --- a/app/policies/group_policy.rb +++ b/app/policies/group_policy.rb @@ -124,6 +124,8 @@ class GroupPolicy < BasePolicy rule { developer & developer_maintainer_access }.enable :create_projects rule { create_projects_disabled }.prevent :create_projects + rule { maintainer & can?(:create_projects) }.enable :transfer_projects + def access_level return GroupMember::NO_ACCESS if @user.nil? diff --git a/app/policies/namespace_policy.rb b/app/policies/namespace_policy.rb index 2babcb0a2d9..926a8b7264d 100644 --- a/app/policies/namespace_policy.rb +++ b/app/policies/namespace_policy.rb @@ -14,4 +14,6 @@ class NamespacePolicy < BasePolicy end rule { personal_project & ~can_create_personal_project }.prevent :create_projects + + rule { (owner | admin) & can?(:create_projects) }.enable :transfer_projects end diff --git a/app/services/projects/transfer_service.rb b/app/services/projects/transfer_service.rb index 233dcf37e35..b94d618dc43 100644 --- a/app/services/projects/transfer_service.rb +++ b/app/services/projects/transfer_service.rb @@ -95,7 +95,7 @@ module Projects @new_namespace && can?(current_user, :change_namespace, project) && @new_namespace.id != project.namespace_id && - current_user.can?(:create_projects, @new_namespace) + current_user.can?(:transfer_projects, @new_namespace) end def update_namespace_and_visibility(to_namespace) diff --git a/changelogs/unreleased/security-developer-transfer-project.yml b/changelogs/unreleased/security-developer-transfer-project.yml new file mode 100644 index 00000000000..fe533fc099a --- /dev/null +++ b/changelogs/unreleased/security-developer-transfer-project.yml @@ -0,0 +1,5 @@ +--- +title: Require Maintainer permission on group where project is transferred to +merge_request: +author: +type: security diff --git a/spec/policies/group_policy_spec.rb b/spec/policies/group_policy_spec.rb index be55d94daec..ede07a5f6a2 100644 --- a/spec/policies/group_policy_spec.rb +++ b/spec/policies/group_policy_spec.rb @@ -335,6 +335,88 @@ describe GroupPolicy do end end + context 'transfer_projects' do + shared_examples_for 'allowed to transfer projects' do + before do + group.update(project_creation_level: project_creation_level) + end + + it { is_expected.to be_allowed(:transfer_projects) } + end + + shared_examples_for 'not allowed to transfer projects' do + before do + group.update(project_creation_level: project_creation_level) + end + + it { is_expected.to be_disallowed(:transfer_projects) } + end + + context 'reporter' do + let(:current_user) { reporter } + + it_behaves_like 'not allowed to transfer projects' do + let(:project_creation_level) { ::Gitlab::Access::NO_ONE_PROJECT_ACCESS } + end + + it_behaves_like 'not allowed to transfer projects' do + let(:project_creation_level) { ::Gitlab::Access::MAINTAINER_PROJECT_ACCESS } + end + + it_behaves_like 'not allowed to transfer projects' do + let(:project_creation_level) { ::Gitlab::Access::DEVELOPER_MAINTAINER_PROJECT_ACCESS } + end + end + + context 'developer' do + let(:current_user) { developer } + + it_behaves_like 'not allowed to transfer projects' do + let(:project_creation_level) { ::Gitlab::Access::NO_ONE_PROJECT_ACCESS } + end + + it_behaves_like 'not allowed to transfer projects' do + let(:project_creation_level) { ::Gitlab::Access::MAINTAINER_PROJECT_ACCESS } + end + + it_behaves_like 'not allowed to transfer projects' do + let(:project_creation_level) { ::Gitlab::Access::DEVELOPER_MAINTAINER_PROJECT_ACCESS } + end + end + + context 'maintainer' do + let(:current_user) { maintainer } + + it_behaves_like 'not allowed to transfer projects' do + let(:project_creation_level) { ::Gitlab::Access::NO_ONE_PROJECT_ACCESS } + end + + it_behaves_like 'allowed to transfer projects' do + let(:project_creation_level) { ::Gitlab::Access::MAINTAINER_PROJECT_ACCESS } + end + + it_behaves_like 'allowed to transfer projects' do + let(:project_creation_level) { ::Gitlab::Access::DEVELOPER_MAINTAINER_PROJECT_ACCESS } + end + end + + context 'owner' do + let(:current_user) { owner } + + it_behaves_like 'not allowed to transfer projects' do + let(:project_creation_level) { ::Gitlab::Access::NO_ONE_PROJECT_ACCESS } + end + + it_behaves_like 'allowed to transfer projects' do + let(:project_creation_level) { ::Gitlab::Access::MAINTAINER_PROJECT_ACCESS } + end + + it_behaves_like 'allowed to transfer projects' do + let(:project_creation_level) { ::Gitlab::Access::DEVELOPER_MAINTAINER_PROJECT_ACCESS } + end + end + end + context "create_projects" do context 'when group has no project creation level set' do let(:group) { create(:group, project_creation_level: nil) } diff --git a/spec/policies/namespace_policy_spec.rb b/spec/policies/namespace_policy_spec.rb index 99fa8b1fe44..b48185838e0 100644 --- a/spec/policies/namespace_policy_spec.rb +++ b/spec/policies/namespace_policy_spec.rb @@ -6,7 +6,7 @@ describe NamespacePolicy do let(:admin) { create(:admin) } let(:namespace) { create(:namespace, owner: owner) } - let(:owner_permissions) { [:create_projects, :admin_namespace, :read_namespace] } + let(:owner_permissions) { [:create_projects, :admin_namespace, :read_namespace, :transfer_projects] } subject { described_class.new(current_user, namespace) } @@ -31,6 +31,7 @@ describe NamespacePolicy do let(:owner) { create(:user, projects_limit: 0) } it { is_expected.to be_disallowed(:create_projects) } + it { is_expected.to be_disallowed(:transfer_projects) } end end diff --git a/spec/requests/api/projects_spec.rb b/spec/requests/api/projects_spec.rb index 1d7ca85cdd2..74fe683f053 100644 --- a/spec/requests/api/projects_spec.rb +++ b/spec/requests/api/projects_spec.rb @@ -2609,6 +2609,22 @@ describe API::Projects do expect(response).to have_gitlab_http_status(400) end end + + context 'when authenticated as developer' do + before do + group.add_developer(user) + end + + context 'target namespace allows developers to create projects' do + let(:group) { create(:group, project_creation_level: ::Gitlab::Access::DEVELOPER_MAINTAINER_PROJECT_ACCESS) } + + it 'fails transferring the project to the target namespace' do + put api("/projects/#{project.id}/transfer", user), params: { namespace: group.id } + + expect(response).to have_gitlab_http_status(400) + end + end + end end it_behaves_like 'custom attributes endpoints', 'projects' do diff --git a/spec/services/projects/transfer_service_spec.rb b/spec/services/projects/transfer_service_spec.rb index a47c10d991a..cb784de0b1c 100644 --- a/spec/services/projects/transfer_service_spec.rb +++ b/spec/services/projects/transfer_service_spec.rb @@ -222,6 +222,24 @@ describe Projects::TransferService do it { expect(project.errors[:new_namespace]).to include('Project with same name or path in target namespace already exists') } end + context 'target namespace allows developers to create projects' do + let(:group) { create(:group, project_creation_level: ::Gitlab::Access::DEVELOPER_MAINTAINER_PROJECT_ACCESS) } + + context 'the user is a member of the target namespace with developer permissions' do + subject(:transfer_project_result) { transfer_project(project, user, group) } + + before do + group.add_developer(user) + end + + it 'does not allow project transfer to the target namespace' do + expect(transfer_project_result).to eq false + expect(project.namespace).to eq(user.namespace) + expect(project.errors[:new_namespace]).to include('Transfer failed, please contact an admin.') + end + end + end + def transfer_project(project, user, new_namespace) service = Projects::TransferService.new(project, user) |