Project

General

Profile

« Previous | Next » 

Revision 13981

Removed :move_issues permission (#18855).

This permission was wrongly used to allow bulk issue copy. To prevent user from moving an issue to another project, the project field should now be set to read-only in the workflow permissions. A migration does this automatically for roles that have the edit_issues permission without having the move_issues permission.

View differences:

trunk/app/controllers/context_menus_controller.rb
31 31

  
32 32
    @can = {:edit => User.current.allowed_to?(:edit_issues, @projects),
33 33
            :log_time => (@project && User.current.allowed_to?(:log_time, @project)),
34
            :move => (@project && User.current.allowed_to?(:move_issues, @project)),
35
            :copy => (@issue && @project.trackers.include?(@issue.tracker) && User.current.allowed_to?(:add_issues, @project)),
34
            :copy => User.current.allowed_to?(:add_issues, @projects),
36 35
            :delete => User.current.allowed_to?(:delete_issues, @projects)
37 36
            }
38 37
    if @project
trunk/app/controllers/issues_controller.rb
219 219
    @copy = params[:copy].present?
220 220
    @notes = params[:notes]
221 221

  
222
    if User.current.allowed_to?(:move_issues, @projects)
223
      @allowed_projects = Issue.allowed_target_projects_on_move
224
      if params[:issue]
225
        @target_project = @allowed_projects.detect {|p| p.id.to_s == params[:issue][:project_id].to_s}
226
        if @target_project
227
          target_projects = [@target_project]
228
        end
222
    @allowed_projects = Issue.allowed_target_projects
223
    if params[:issue]
224
      @target_project = @allowed_projects.detect {|p| p.id.to_s == params[:issue][:project_id].to_s}
225
      if @target_project
226
        target_projects = [@target_project]
229 227
      end
230 228
    end
231 229
    target_projects ||= @projects
trunk/app/models/issue.rb
378 378
    :if => lambda {|issue, user|
379 379
      if issue.new_record?
380 380
        issue.copy?
381
      elsif user.allowed_to?(:move_issues, issue.project)
382
        Issue.allowed_target_projects_on_move.count > 1
381
      else
382
        user.allowed_to?(:edit_issues, issue.project)
383 383
      end
384 384
    }
385 385

  
......
1282 1282

  
1283 1283
  # Returns a scope of projects that user can assign the issue to
1284 1284
  def allowed_target_projects(user=User.current)
1285
    if new_record?
1286
      Project.where(Project.allowed_to_condition(user, :add_issues))
1287
    else
1288
      self.class.allowed_target_projects_on_move(user)
1289
    end
1285
    current_project = new_record? ? nil : project
1286
    self.class.allowed_target_projects(user, current_project)
1290 1287
  end
1291 1288

  
1292
  # Returns a scope of projects that user can move issues to
1293
  def self.allowed_target_projects_on_move(user=User.current)
1294
    Project.where(Project.allowed_to_condition(user, :move_issues))
1289
  # Returns a scope of projects that user can assign issues to
1290
  # If current_project is given, it will be included in the scope
1291
  def self.allowed_target_projects(user=User.current, current_project=nil)
1292
    condition = Project.allowed_to_condition(user, :add_issues)
1293
    if current_project
1294
      condition = ["(#{condition}) OR #{Project.table_name}.id = ?", current_project.id]
1295
    end
1296
    Project.where(condition)
1295 1297
  end
1296 1298

  
1297 1299
  private
trunk/app/views/context_menus/issues.html.erb
130 130
          :class => 'icon-copy', :disabled => !@can[:copy] %></li>
131 131
<% else %>
132 132
  <li><%= context_menu_link l(:button_copy), bulk_edit_issues_path(:ids => @issue_ids, :copy => '1'),
133
                          :class => 'icon-copy', :disabled => !@can[:move] %></li>
133
                          :class => 'icon-copy', :disabled => !@can[:copy] %></li>
134 134
<% end %>
135 135
  <li><%= context_menu_link l(:button_delete), issues_path(:ids => @issue_ids, :back_url => @back),
136 136
                            :method => :delete, :data => {:confirm => issues_destroy_confirmation_message(@issues)}, :class => 'icon-del', :disabled => !@can[:delete] %></li>
trunk/db/migrate/20150208105930_replace_move_issues_permission.rb
1
class ReplaceMoveIssuesPermission < ActiveRecord::Migration
2
  def self.up
3
    Role.all.each do |role|
4
      if role.has_permission?(:edit_issues) && !role.has_permission?(:move_issues)
5
        # inserts one ligne per trakcer and status
6
        WorkflowPermission.connection.insert_sql(
7
          "INSERT INTO #{WorkflowPermission.table_name} (tracker_id, old_status_id, role_id, type, field_name, rule)" +
8
          " SELECT t.id, s.id, #{role.id}, 'WorkflowPermission', 'project_id', 'readonly'" +
9
          " FROM #{Tracker.table_name} t, #{IssueStatus.table_name} s"
10
        )
11
      end
12
    end
13
  end
14

  
15
  def self.down
16
    raise IrreversibleMigration
17
  end
18
end
0 19

  
trunk/lib/redmine.rb
109 109
    map.permission :edit_own_issue_notes, {:journals => :edit}, :require => :loggedin
110 110
    map.permission :view_private_notes, {}, :read => true, :require => :member
111 111
    map.permission :set_notes_private, {}, :require => :member
112
    map.permission :move_issues, {:issues => [:bulk_edit, :bulk_update]}, :require => :loggedin
113 112
    map.permission :delete_issues, {:issues => :destroy}, :require => :member
114 113
    # Queries
115 114
    map.permission :manage_public_queries, {:queries => [:new, :create, :edit, :update, :destroy]}, :require => :member
trunk/test/fixtures/roles.yml
20 20
    - :manage_issue_relations
21 21
    - :manage_subtasks
22 22
    - :add_issue_notes
23
    - :move_issues
24 23
    - :delete_issues
25 24
    - :view_issue_watchers
26 25
    - :add_issue_watchers
......
81 80
    - :manage_issue_relations
82 81
    - :manage_subtasks
83 82
    - :add_issue_notes
84
    - :move_issues
85 83
    - :delete_issues
86 84
    - :view_issue_watchers
87 85
    - :save_queries
......
128 126
    - :edit_issues
129 127
    - :manage_issue_relations
130 128
    - :add_issue_notes
131
    - :move_issues
132 129
    - :view_issue_watchers
133 130
    - :save_queries
134 131
    - :view_gantt
trunk/test/unit/issue_test.rb
1211 1211
    assert issue.save
1212 1212
  end
1213 1213

  
1214
  def test_allowed_target_projects_on_move_should_include_projects_with_issue_tracking_enabled
1215
    assert_include Project.find(2), Issue.allowed_target_projects_on_move(User.find(2))
1214
  def test_allowed_target_projects_should_include_projects_with_issue_tracking_enabled
1215
    assert_include Project.find(2), Issue.allowed_target_projects(User.find(2))
1216 1216
  end
1217 1217

  
1218
  def test_allowed_target_projects_on_move_should_not_include_projects_with_issue_tracking_disabled
1218
  def test_allowed_target_projects_should_not_include_projects_with_issue_tracking_disabled
1219 1219
    Project.find(2).disable_module! :issue_tracking
1220
    assert_not_include Project.find(2), Issue.allowed_target_projects_on_move(User.find(2))
1220
    assert_not_include Project.find(2), Issue.allowed_target_projects(User.find(2))
1221 1221
  end
1222 1222

  
1223 1223
  def test_move_to_another_project_with_same_category

Also available in: Unified diff