Do not pass unsanitized params to issue move service

3 files changed, 11 insertions, 9 deletions

diff --git a/app/controllers/projects/issues_controller.rb b/app/controllers/projects/issues_controller.rb
index e3486f576c0..9ec342a7b2a 100644
--- a/app/controllers/projects/issues_controller.rb
+++ b/app/controllers/projects/issues_controller.rb
@@ -88,7 +88,8 @@ class Projects::IssuesController < Projects::ApplicationController
 
   def update
     @issue = Issues::UpdateService.new(project, current_user, issue_params).execute(issue)
-    move_service = Issues::MoveService.new(project, current_user, params.require(:issue).permit!, @issue)
+    move_service = Issues::MoveService.new(project, current_user, issue_params,
+                                           @issue, params['move_to_project_id'])
 
     if move_service.move?
       @issue = move_service.execute
diff --git a/app/services/issues/move_service.rb b/app/services/issues/move_service.rb
index bba972382d9..55239d566f1 100644
--- a/app/services/issues/move_service.rb
+++ b/app/services/issues/move_service.rb
@@ -1,15 +1,12 @@
 module Issues
   class MoveService < Issues::BaseService
-    def initialize(project, current_user, params, issue)
+    def initialize(project, current_user, params, issue, new_project_id)
       super(project, current_user, params)
 
       @issue_old = issue
       @issue_new = @issue_old.dup
       @project_old = @project
-
-      if params['move_to_project_id']
-        @project_new = Project.find(params['move_to_project_id'])
-      end
+      @project_new = Project.find(new_project_id) if new_project_id
     end
 
     def execute
diff --git a/spec/services/issues/move_service_spec.rb b/spec/services/issues/move_service_spec.rb
index 8d9cc09ffc7..931ba06f6a1 100644
--- a/spec/services/issues/move_service_spec.rb
+++ b/spec/services/issues/move_service_spec.rb
@@ -7,10 +7,14 @@ describe Issues::MoveService, services: true do
   let(:old_project) { create(:project) }
   let(:old_issue) { create(:issue, title: title, description: description, project: old_project) }
   let(:new_project) { create(:project) }
-  let(:move_service) { described_class.new(old_project, user, move_params, old_issue) }
+  let(:issue_params) { old_issue.serializable_hash }
+
+  let(:move_service) do
+    described_class.new(old_project, user, issue_params, old_issue, new_project_id)
+  end
 
   shared_context 'issue move requested' do
-    let(:move_params) { { 'move_to_project_id' => new_project.id } }
+    let(:new_project_id) { new_project.id }
   end
 
   shared_context 'user can move issue' do
@@ -108,7 +112,7 @@ describe Issues::MoveService, services: true do
   end
 
   context 'issue move not requested' do
-    let(:move_params) { {} }
+    let(:new_project_id) { nil }
 
     describe '#move?' do
       subject { move_service.move? }