diff options
author | Rémy Coutable <remy@gitlab.com> | 2016-09-28 12:42:33 +0300 |
---|---|---|
committer | Rémy Coutable <remy@rymai.me> | 2016-09-28 19:32:03 +0300 |
commit | 5af2bf0a44e44035dad5fa8be0dcf9a03f35fee5 (patch) | |
tree | 59f532a62296b496905eacbdb165ef742c60daa3 /spec | |
parent | f61cff79642221be7da7aa841b710ece8f8cfed1 (diff) |
Merge branch '18028-respect-fork-project' into 'security'
Enforce the fork_project permission in Projects::CreateService
Projects::ForkService delegates to this service almost entirely, but needed one small change so it would propagate create errors correctly.
CreateService#execute needs significant refactoring; it is now right at the complexity limit set by Rubocop. I avoided doing so in this commit to keep the diff as small as possible.
Closes https://gitlab.com/gitlab-org/gitlab-ce/issues/18028
See merge request !1996
Signed-off-by: Rémy Coutable <remy@rymai.me>
Diffstat (limited to 'spec')
-rw-r--r-- | spec/controllers/users_controller_spec.rb | 4 | ||||
-rw-r--r-- | spec/helpers/projects_helper_spec.rb | 2 | ||||
-rw-r--r-- | spec/models/forked_project_link_spec.rb | 1 | ||||
-rw-r--r-- | spec/requests/api/fork_spec.rb | 2 | ||||
-rw-r--r-- | spec/services/projects/fork_service_spec.rb | 25 | ||||
-rw-r--r-- | spec/services/system_note_service_spec.rb | 2 |
6 files changed, 29 insertions, 7 deletions
diff --git a/spec/controllers/users_controller_spec.rb b/spec/controllers/users_controller_spec.rb index c61ec174665..01fe2171fdc 100644 --- a/spec/controllers/users_controller_spec.rb +++ b/spec/controllers/users_controller_spec.rb @@ -74,8 +74,8 @@ describe UsersController do end context 'forked project' do - let!(:project) { create(:project) } - let!(:forked_project) { Projects::ForkService.new(project, user).execute } + let(:project) { create(:project) } + let(:forked_project) { Projects::ForkService.new(project, user).execute } before do sign_in(user) diff --git a/spec/helpers/projects_helper_spec.rb b/spec/helpers/projects_helper_spec.rb index 09e0bbfd00b..2ec049e09f7 100644 --- a/spec/helpers/projects_helper_spec.rb +++ b/spec/helpers/projects_helper_spec.rb @@ -11,7 +11,7 @@ describe ProjectsHelper do describe "can_change_visibility_level?" do let(:project) { create(:project) } - let(:user) { create(:user) } + let(:user) { create(:project_member, :reporter, user: create(:user), project: project).user } let(:fork_project) { Projects::ForkService.new(project, user).execute } it "returns false if there are no appropriate permissions" do diff --git a/spec/models/forked_project_link_spec.rb b/spec/models/forked_project_link_spec.rb index 3b817608ce0..9a63da786ff 100644 --- a/spec/models/forked_project_link_spec.rb +++ b/spec/models/forked_project_link_spec.rb @@ -6,6 +6,7 @@ describe ForkedProjectLink, "add link on fork" do let(:user) { create(:user, namespace: namespace) } before do + create(:project_member, :reporter, user: user, project: project_from) @project_to = fork_project(project_from, user) end diff --git a/spec/requests/api/fork_spec.rb b/spec/requests/api/fork_spec.rb index fa94e03ec32..b5d376889c2 100644 --- a/spec/requests/api/fork_spec.rb +++ b/spec/requests/api/fork_spec.rb @@ -12,7 +12,7 @@ describe API::API, api: true do end let(:project_user2) do - create(:project_member, :guest, user: user2, project: project) + create(:project_member, :reporter, user: user2, project: project) end describe 'POST /projects/fork/:id' do diff --git a/spec/services/projects/fork_service_spec.rb b/spec/services/projects/fork_service_spec.rb index 31bb7120d84..518bfc809e8 100644 --- a/spec/services/projects/fork_service_spec.rb +++ b/spec/services/projects/fork_service_spec.rb @@ -12,12 +12,26 @@ describe Projects::ForkService, services: true do description: 'wow such project') @to_namespace = create(:namespace) @to_user = create(:user, namespace: @to_namespace) + @from_project.add_user(@to_user, :developer) end context 'fork project' do + context 'when forker is a guest' do + before do + @guest = create(:user) + @from_project.add_user(@guest, :guest) + end + subject { fork_project(@from_project, @guest) } + + it { is_expected.not_to be_persisted } + it { expect(subject.errors[:forked_from_project_id]).to eq(['is forbidden']) } + end + describe "successfully creates project in the user namespace" do let(:to_project) { fork_project(@from_project, @to_user) } + it { expect(to_project).to be_persisted } + it { expect(to_project.errors).to be_empty } it { expect(to_project.owner).to eq(@to_user) } it { expect(to_project.namespace).to eq(@to_user.namespace) } it { expect(to_project.star_count).to be_zero } @@ -29,7 +43,9 @@ describe Projects::ForkService, services: true do it "should fail due to validation, not transaction failure" do @existing_project = create(:project, creator_id: @to_user.id, name: @from_project.name, namespace: @to_namespace) @to_project = fork_project(@from_project, @to_user) - expect(@existing_project.persisted?).to be_truthy + expect(@existing_project).to be_persisted + + expect(@to_project).not_to be_persisted expect(@to_project.errors[:name]).to eq(['has already been taken']) expect(@to_project.errors[:path]).to eq(['has already been taken']) end @@ -81,18 +97,23 @@ describe Projects::ForkService, services: true do @group = create(:group) @group.add_user(@group_owner, GroupMember::OWNER) @group.add_user(@developer, GroupMember::DEVELOPER) + @project.add_user(@developer, :developer) + @project.add_user(@group_owner, :developer) @opts = { namespace: @group } end context 'fork project for group' do it 'group owner successfully forks project into the group' do to_project = fork_project(@project, @group_owner, @opts) + + expect(to_project).to be_persisted + expect(to_project.errors).to be_empty expect(to_project.owner).to eq(@group) expect(to_project.namespace).to eq(@group) expect(to_project.name).to eq(@project.name) expect(to_project.path).to eq(@project.path) expect(to_project.description).to eq(@project.description) - expect(to_project.star_count).to be_zero + expect(to_project.star_count).to be_zero end end diff --git a/spec/services/system_note_service_spec.rb b/spec/services/system_note_service_spec.rb index 85dd30bf48c..2f69b54bf3c 100644 --- a/spec/services/system_note_service_spec.rb +++ b/spec/services/system_note_service_spec.rb @@ -445,7 +445,7 @@ describe SystemNoteService, services: true do end context 'commit with cross-reference from fork' do - let(:author2) { create(:user) } + let(:author2) { create(:project_member, :reporter, user: create(:user), project: project).user } let(:forked_project) { Projects::ForkService.new(project, author2).execute } let(:commit2) { forked_project.commit } |