Merge branch 'fix/permissions-when-importing-members' into 'master'

Check permissions when importing project members

Closes #14899

See merge request !3535

3 files changed, 60 insertions, 3 deletions

diff --git a/CHANGELOG b/CHANGELOG
index a4bb4589f3a..513d8589c68 100644
--- a/CHANGELOG
+++ b/CHANGELOG
@@ -22,6 +22,9 @@ v 8.7.0 (unreleased)
   - Improved UX of the navigation sidebar
   - Build status notifications
 
+v 8.6.5 (unreleased)
+  - Check permissions when user attempts to import members from another project
+
 v 8.6.4
   - Don't attempt to fetch any tags from a forked repo (Stan Hu)
 
diff --git a/app/controllers/projects/project_members_controller.rb b/app/controllers/projects/project_members_controller.rb
index e7bddc4a6f1..e457db2f0b7 100644
--- a/app/controllers/projects/project_members_controller.rb
+++ b/app/controllers/projects/project_members_controller.rb
@@ -94,9 +94,14 @@ class Projects::ProjectMembersController < Projects::ApplicationController
   end
 
   def apply_import
-    giver = Project.find(params[:source_project_id])
-    status = @project.team.import(giver, current_user)
-    notice = status ? "Successfully imported" : "Import failed"
+    source_project = Project.find(params[:source_project_id])
+
+    if can?(current_user, :read_project_member, source_project)
+      status = @project.team.import(source_project, current_user)
+      notice = status ? "Successfully imported" : "Import failed"
+    else
+      return render_404
+    end
 
     redirect_to(namespace_project_project_members_path(project.namespace, project),
                 notice: notice)
diff --git a/spec/controllers/projects/project_members_controller_spec.rb b/spec/controllers/projects/project_members_controller_spec.rb
new file mode 100644
index 00000000000..d47e4ab9a4f
--- /dev/null
+++ b/spec/controllers/projects/project_members_controller_spec.rb
@@ -0,0 +1,49 @@
+require('spec_helper')
+
+describe Projects::ProjectMembersController do
+  let(:project) { create(:project) }
+  let(:another_project) { create(:project, :private) }
+  let(:user) { create(:user) }
+  let(:member) { create(:user) }
+
+  before do
+    project.team << [user, :master]
+    another_project.team << [member, :guest]
+    sign_in(user)
+  end
+
+  describe '#apply_import' do
+    shared_context 'import applied' do
+      before do
+        post(:apply_import, namespace_id: project.namespace.to_param,
+                            project_id: project.to_param,
+                            source_project_id: another_project.id)
+      end
+    end
+
+    context 'when user can access source project members' do
+      before { another_project.team << [user, :guest] }
+      include_context 'import applied'
+
+      it 'imports source project members' do
+        expect(project.team_members).to include member
+        expect(response).to set_flash.to 'Successfully imported'
+        expect(response).to redirect_to(
+          namespace_project_project_members_path(project.namespace, project)
+        )
+      end
+    end
+
+    context 'when user is not member of a source project' do
+      include_context 'import applied'
+
+      it 'does not import team members' do
+        expect(project.team_members).to_not include member
+      end
+
+      it 'responds with not found' do
+        expect(response.status).to eq 404
+      end
+    end
+  end
+end