From 57eb39548879109dff3813129fca7acbcca23f71 Mon Sep 17 00:00:00 2001 From: Grzegorz Bizon Date: Wed, 24 Feb 2016 11:52:02 +0100 Subject: [PATCH] Do not pass unsanitized params to issue move service --- app/controllers/projects/issues_controller.rb | 3 ++- app/services/issues/move_service.rb | 7 ++----- spec/services/issues/move_service_spec.rb | 10 +++++++--- 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? } -- GitLab