diff --git a/app/controllers/context_menus_controller.rb b/app/controllers/context_menus_controller.rb index 99b320d..4d22b0a 100644 --- a/app/controllers/context_menus_controller.rb +++ b/app/controllers/context_menus_controller.rb @@ -31,7 +31,7 @@ class ContextMenusController < ApplicationController @can = {:edit => User.current.allowed_to?(:edit_issues, @projects), :log_time => (@project && User.current.allowed_to?(:log_time, @project)), - :copy => User.current.allowed_to?(:add_issues, @projects), + :copy => User.current.allowed_to?(:copy_issues, @projects) && Issue.allowed_target_projects.any?, :delete => User.current.allowed_to?(:delete_issues, @projects) } if @project diff --git a/app/controllers/issues_controller.rb b/app/controllers/issues_controller.rb index de71554..b95856a 100644 --- a/app/controllers/issues_controller.rb +++ b/app/controllers/issues_controller.rb @@ -143,6 +143,9 @@ class IssuesController < ApplicationController end def create + unless User.current.allowed_to?(:add_issues, @issue.project) + raise ::Unauthorized + end call_hook(:controller_issues_new_before_save, { :params => params, :issue => @issue }) @issue.save_attachments(params[:attachments] || (params[:issue] && params[:issue][:uploads])) if @issue.save @@ -219,6 +222,12 @@ class IssuesController < ApplicationController @copy = params[:copy].present? @notes = params[:notes] + if @copy + unless User.current.allowed_to?(:copy_issues, @projects) + raise ::Unauthorized + end + end + @allowed_projects = Issue.allowed_target_projects if params[:issue] @target_project = @allowed_projects.detect {|p| p.id.to_s == params[:issue][:project_id].to_s} @@ -255,6 +264,19 @@ class IssuesController < ApplicationController @copy = params[:copy].present? attributes = parse_params_for_bulk_issue_attributes(params) + if @copy + unless User.current.allowed_to?(:copy_issues, @projects) + raise ::Unauthorized + end + target_projects = @projects + if attributes['project_id'].present? + target_projects = Project.where(:id => attributes['project_id']).to_a + end + unless User.current.allowed_to?(:add_issues, target_projects) + raise ::Unauthorized + end + end + unsaved_issues = [] saved_issues = [] @@ -407,6 +429,9 @@ class IssuesController < ApplicationController begin @issue.init_journal(User.current) @copy_from = Issue.visible.find(params[:copy_from]) + unless User.current.allowed_to?(:copy_issues, @copy_from.project) + raise ::Unauthorized + end @link_copy = link_copy?(params[:link_copy]) || request.get? @copy_attachments = params[:copy_attachments].present? || request.get? @copy_subtasks = params[:copy_subtasks].present? || request.get? diff --git a/app/helpers/application_helper.rb b/app/helpers/application_helper.rb index db10ed6..24d033a 100644 --- a/app/helpers/application_helper.rb +++ b/app/helpers/application_helper.rb @@ -343,8 +343,11 @@ module ApplicationHelper def project_tree_options_for_select(projects, options = {}) s = ''.html_safe - if options[:include_blank] - s << content_tag('option', ' '.html_safe, :value => '') + if blank_text = options[:include_blank] + if blank_text == true + blank_text = ' '.html_safe + end + s << content_tag('option', blank_text, :value => '') end project_tree(projects) do |project, level| name_prefix = (level > 0 ? ' ' * 2 * level + '» ' : '').html_safe diff --git a/app/models/issue.rb b/app/models/issue.rb index 5ea344a..dcf844b 100644 --- a/app/models/issue.rb +++ b/app/models/issue.rb @@ -424,6 +424,9 @@ class Issue < ActiveRecord::Base names = super names -= disabled_core_fields names -= read_only_attribute_names(user) + if new_record? && copy? + names |= %w(project_id) + end names end diff --git a/app/views/issues/_action_menu.html.erb b/app/views/issues/_action_menu.html.erb index 18c9e59..c330462 100644 --- a/app/views/issues/_action_menu.html.erb +++ b/app/views/issues/_action_menu.html.erb @@ -2,6 +2,6 @@ <%= link_to l(:button_edit), edit_issue_path(@issue), :onclick => 'showAndScrollTo("update", "issue_notes"); return false;', :class => 'icon icon-edit', :accesskey => accesskey(:edit) if @issue.editable? %> <%= link_to l(:button_log_time), new_issue_time_entry_path(@issue), :class => 'icon icon-time-add' if User.current.allowed_to?(:log_time, @project) %> <%= watcher_link(@issue, User.current) %> -<%= link_to l(:button_copy), project_copy_issue_path(@project, @issue), :class => 'icon icon-copy' if User.current.allowed_to?(:add_issues, @project) %> +<%= link_to l(:button_copy), project_copy_issue_path(@project, @issue), :class => 'icon icon-copy' if User.current.allowed_to?(:copy_issues, @project) && Issue.allowed_target_projects.any? %> <%= link_to l(:button_delete), issue_path(@issue), :data => {:confirm => issues_destroy_confirmation_message(@issue)}, :method => :delete, :class => 'icon icon-del' if User.current.allowed_to?(:delete_issues, @project) %> diff --git a/app/views/issues/bulk_edit.html.erb b/app/views/issues/bulk_edit.html.erb index bdaaf21..4cdc257 100644 --- a/app/views/issues/bulk_edit.html.erb +++ b/app/views/issues/bulk_edit.html.erb @@ -33,8 +33,9 @@

<%= select_tag('issue[project_id]', - content_tag('option', l(:label_no_change_option), :value => '') + - project_tree_options_for_select(@allowed_projects, :selected => @target_project), + project_tree_options_for_select(@allowed_projects, + :include_blank => ((!@copy || (@projects & @allowed_projects == @projects)) ? l(:label_no_change_option) : false), + :selected => @target_project), :onchange => "updateBulkEditFrom('#{escape_javascript url_for(:action => 'bulk_edit', :format => 'js')}')") %>

<% end %> diff --git a/config/locales/en.yml b/config/locales/en.yml index 43936dc..f1a04a0 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -429,6 +429,7 @@ en: permission_view_issues: View Issues permission_add_issues: Add issues permission_edit_issues: Edit issues + permission_copy_issues: Copy issues permission_manage_issue_relations: Manage issue relations permission_set_issues_private: Set issues public or private permission_set_own_issues_private: Set own issues public or private diff --git a/config/locales/fr.yml b/config/locales/fr.yml index e077e92..17b9a4c 100644 --- a/config/locales/fr.yml +++ b/config/locales/fr.yml @@ -449,6 +449,7 @@ fr: permission_view_issues: Voir les demandes permission_add_issues: Créer des demandes permission_edit_issues: Modifier les demandes + permission_copy_issues: Copier les demandes permission_manage_issue_relations: Gérer les relations permission_set_issues_private: Rendre les demandes publiques ou privées permission_set_own_issues_private: Rendre ses propres demandes publiques ou privées diff --git a/lib/redmine.rb b/lib/redmine.rb index 23db652..fc05a14 100644 --- a/lib/redmine.rb +++ b/lib/redmine.rb @@ -100,6 +100,7 @@ Redmine::AccessControl.map do |map| :read => true map.permission :add_issues, {:issues => [:new, :create, :update_form], :attachments => :upload} map.permission :edit_issues, {:issues => [:edit, :update, :bulk_edit, :bulk_update, :update_form], :journals => [:new], :attachments => :upload} + map.permission :copy_issues, {:issues => [:new, :create, :bulk_edit, :bulk_update, :update_form], :attachments => :upload} map.permission :manage_issue_relations, {:issue_relations => [:index, :show, :create, :destroy]} map.permission :manage_subtasks, {} map.permission :set_issues_private, {} diff --git a/test/fixtures/roles.yml b/test/fixtures/roles.yml index 000173b..674c6d4 100644 --- a/test/fixtures/roles.yml +++ b/test/fixtures/roles.yml @@ -17,6 +17,7 @@ roles_001: - :view_issues - :add_issues - :edit_issues + - :copy_issues - :manage_issue_relations - :manage_subtasks - :add_issue_notes @@ -77,6 +78,7 @@ roles_002: - :view_issues - :add_issues - :edit_issues + - :copy_issues - :manage_issue_relations - :manage_subtasks - :add_issue_notes diff --git a/test/functional/issues_controller_test.rb b/test/functional/issues_controller_test.rb index 1794c74..7a58d5e 100644 --- a/test/functional/issues_controller_test.rb +++ b/test/functional/issues_controller_test.rb @@ -2473,6 +2473,20 @@ class IssuesControllerTest < ActionController::TestCase assert_select '#main-menu a.new-issue[href="/projects/ecookbook/issues/new"]' end + def test_new_as_copy_without_add_issues_permission_should_not_propose_current_project_as_target + user = setup_user_with_copy_but_not_add_permission + + @request.session[:user_id] = user.id + get :new, :project_id => 1, :copy_from => 1 + + assert_response :success + assert_template 'new' + assert_select 'select[name=?]', 'issue[project_id]' do + assert_select 'option[value="1"]', 0 + assert_select 'option[value="2"]', :text => 'OnlineStore' + end + end + def test_new_as_copy_with_attachments_should_show_copy_attachments_checkbox @request.session[:user_id] = 2 issue = Issue.find(3) @@ -3770,9 +3784,26 @@ class IssuesControllerTest < ActionController::TestCase assert_not_nil issues assert_equal [1, 2, 3], issues.map(&:id).sort + assert_select 'select[name=?]', 'issue[project_id]' do + assert_select 'option[value=""]' + end assert_select 'input[name=copy_attachments]' end + def test_get_bulk_copy_without_add_issues_permission_should_not_propose_current_project_as_target + user = setup_user_with_copy_but_not_add_permission + @request.session[:user_id] = user.id + + get :bulk_edit, :ids => [1, 2, 3], :copy => '1' + assert_response :success + assert_template 'bulk_edit' + + assert_select 'select[name=?]', 'issue[project_id]' do + assert_select 'option[value=""]', 0 + assert_select 'option[value="2"]' + end + end + def test_bulk_copy_to_another_project @request.session[:user_id] = 2 assert_difference 'Issue.count', 2 do @@ -3788,6 +3819,32 @@ class IssuesControllerTest < ActionController::TestCase end end + def test_bulk_copy_without_add_issues_permission_should_be_allowed_on_project_with_permission + user = setup_user_with_copy_but_not_add_permission + @request.session[:user_id] = user.id + + assert_difference 'Issue.count', 3 do + post :bulk_update, :ids => [1, 2, 3], :issue => {:project_id => '2'}, :copy => '1' + assert_response 302 + end + end + + def test_bulk_copy_on_same_project_without_add_issues_permission_should_be_denied + user = setup_user_with_copy_but_not_add_permission + @request.session[:user_id] = user.id + + post :bulk_update, :ids => [1, 2, 3], :issue => {:project_id => ''}, :copy => '1' + assert_response 403 + end + + def test_bulk_copy_on_different_project_without_add_issues_permission_should_be_denied + user = setup_user_with_copy_but_not_add_permission + @request.session[:user_id] = user.id + + post :bulk_update, :ids => [1, 2, 3], :issue => {:project_id => '1'}, :copy => '1' + assert_response 403 + end + def test_bulk_copy_should_allow_not_changing_the_issue_attributes @request.session[:user_id] = 2 issues = [ @@ -4079,4 +4136,13 @@ class IssuesControllerTest < ActionController::TestCase assert_select 'input[name=issues][value="1"][type=hidden]' end end + + def setup_user_with_copy_but_not_add_permission + Role.all.each {|r| r.remove_permission! :add_issues} + Role.find_by_name('Manager').add_permission! :add_issues + user = User.generate! + User.add_to_project(user, Project.find(1), Role.find_by_name('Developer')) + User.add_to_project(user, Project.find(2), Role.find_by_name('Manager')) + user + end end