Defect #18855

User with only Move Issue rights in the project can still create issues using mass copy!

Added by Scott Cunningham almost 4 years ago. Updated over 3 years ago.

Status:ClosedStart date:
Priority:NormalDue date:
Assignee:Jean-Philippe Lang% Done:

0%

Category:Issues permissions
Target version:3.0.0
Resolution:Fixed Affected version:2.5.2

Description

I found this bug when I was trying to use a project with a list of issues as a template for other projects (process flow). I assigned members to custom role "Copy" which only allows viewing and moving issues. If, however, the user does not change the project (i.e. copy into other project), new issues will be created within the existing project where they do not have rights!

I am running 2.5.2 on a Bitnami stack. I do not have the chance to try 2.6.x at the moment.

Note - we use task instead of issue in our language file.

Custom Role Copy settings:

View and Move rights only

User does not have issue edit rights (correct)

Cannot edit task

User can copy multiple issues at once (correct)

Bulk copy possible

Copy screenshot

Copy screenshot

Issues were added to existing project without regard to no Add Issue rights (not correct)

Issues were added (wrong)

01-role-rights.png - View and Move rights only (20.7 KB) Scott Cunningham, 2015-01-15 19:49

02-no-edit-rights.png - Cannot edit task (7.15 KB) Scott Cunningham, 2015-01-15 19:49

03-multiple-copy-possible.png - Bulk copy possible (7.84 KB) Scott Cunningham, 2015-01-15 19:50

04-copy-screen.png - Copy screenshot (24.6 KB) Scott Cunningham, 2015-01-15 19:50

05-issues-added-without-rights.png - Issues were added (wrong) (15.1 KB) Scott Cunningham, 2015-01-15 19:50


Related issues

Related to Redmine - Patch #28311: Remove unused i18n key "permission_move_issues" Closed

Associated revisions

Revision 13981
Added by Jean-Philippe Lang over 3 years ago

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.

Revision 13985
Added by Jean-Philippe Lang over 3 years ago

Adds a :copy_issues permission (#18855).

When copy is allowed, target projects are those on which the user has the :add_issues permission.

Revision 13986
Added by Jean-Philippe Lang over 3 years ago

Adds :permission_copy_issues string to locales (#18855).

Revision 13990
Added by Jean-Philippe Lang over 3 years ago

Fixed the migration for SQLServer (#18855).

History

#1 Updated by Scott Cunningham almost 4 years ago

I believe I have tracked down the problem.

Context menu Copy calls the bulk_edit function in issues_controller.rb:
  1. checks if user has move issue rights
  2. builds an allowed projects list by calling allowed_target_projects_on_move in issue.rb:
  3. which checks projects for move rights, not add rights...

So for copy, the program checks for move-out and move-in rights. But move-in rights is really add rights.

I think instead, move rights should be checked at source project and then add rights at destination project. This should block a user from copying issues into a project where they do not have add issue rights.

issues_controller.rb snippet
  # Bulk edit/copy a set of issues
  def bulk_edit
    @issues.sort!
    @copy = params[:copy].present?
    @notes = params[:notes]

    if User.current.allowed_to?(:move_issues, @projects)   # <----------- this is correct: can user move/copy in the first place
      @allowed_projects = Issue.allowed_target_projects_on_move   # <-------- i think this is wrong: target projects should only be add rights
      if params[:issue]
        @target_project = @allowed_projects.detect {|p| p.id.to_s == params[:issue][:project_id].to_s}
        if @target_project
          target_projects = [@target_project]
        end
      end
    end
    target_projects ||= @projects

#2 Updated by Scott Cunningham almost 4 years ago

I made a small patch and destination projects are now only ones with Add issue rights.

Unresolved: If the user does not change the project pull down from (No change), then new issues will still be created even when the permissions should not allow it. This is past my knowledge point now.

  1. Modify models\issue.rb file:
      # Returns a scope of projects that user can move issues to
      def self.allowed_target_projects_on_move(user=User.current)
        Project.where(Project.allowed_to_condition(user, :move_issues))
      end
    
      # Returns a scope of projects that user can add issues to          # <--- new
      def self.allowed_target_projects_on_copy(user=User.current)        # <--- new
        Project.where(Project.allowed_to_condition(user, :add_issues))   # <--- new
      end                                                                # <--- new
    
  2. Modify controllers\issues_controller.rb file:
      # Bulk edit/copy a set of issues
      def bulk_edit
        @issues.sort!
        @copy = params[:copy].present?
        @notes = params[:notes]
    
        if User.current.allowed_to?(:move_issues, @projects)
          #@allowed_projects = Issue.allowed_target_projects_on_move    # <---- comment out
          @allowed_projects = Issue.allowed_target_projects_on_copy     # <---- new line
          if params[:issue]
            @target_project = @allowed_projects.detect {|p| p.id.to_s == params[:issue][:project_id].to_s}
            if @target_project
              target_projects = [@target_project]
            end
          end
        end
        target_projects ||= @projects
    

#3 Updated by Jean-Philippe Lang almost 4 years ago

  • Target version set to Candidate for next major release

#4 Updated by Jean-Philippe Lang over 3 years ago

  • Status changed from New to Closed
  • Assignee set to Jean-Philippe Lang
  • Target version changed from Candidate for next major release to 3.0.0
  • Resolution set to Fixed

This is now fixed. The :move_issues permission is removed (r13981) and replaced with a :copy_issues permissionn (r13985). When allowed to copy issues, use can copy them to projects on which he has the :add_issues permission.

#5 Updated by Go MAEDA 7 months ago

  • Related to Patch #28311: Remove unused i18n key "permission_move_issues" added

Also available in: Atom PDF