From: Jean-Philippe Lang Date: Mon, 6 Jun 2016 09:41:50 +0000 (+0000) Subject: Merged 15430, 15464 to 15469, 15475, 15476 (#285, #7839). X-Git-Tag: 3.3.0~39 X-Git-Url: https://source.dussan.org/?a=commitdiff_plain;h=6e68d008c4dc50f46585e30aabaa5bc9859bee0f;p=redmine.git Merged 15430, 15464 to 15469, 15475, 15476 (#285, #7839). git-svn-id: http://svn.redmine.org/redmine/branches/3.3-stable@15478 e93f8b46-1217-0410-a6f0-8f06a7374b81 --- diff --git a/app/controllers/context_menus_controller.rb b/app/controllers/context_menus_controller.rb index 1e5652d09..66ec35085 100644 --- a/app/controllers/context_menus_controller.rb +++ b/app/controllers/context_menus_controller.rb @@ -29,11 +29,11 @@ class ContextMenusController < ApplicationController @allowed_statuses = @issues.map(&:new_statuses_allowed_to).reduce(:&) - @can = {:edit => User.current.allowed_to?(:edit_issues, @projects), + @can = {:edit => @issues.all?(&:attributes_editable?), :log_time => (@project && User.current.allowed_to?(:log_time, @project)), :copy => User.current.allowed_to?(:copy_issues, @projects) && Issue.allowed_target_projects.any?, :add_watchers => User.current.allowed_to?(:add_issue_watchers, @projects), - :delete => User.current.allowed_to?(:delete_issues, @projects) + :delete => @issues.all?(&:deletable?) } if @project if @issue @@ -41,12 +41,11 @@ class ContextMenusController < ApplicationController else @assignables = @project.assignable_users end - @trackers = @project.trackers else #when multiple projects, we only keep the intersection of each set @assignables = @projects.map(&:assignable_users).reduce(:&) - @trackers = @projects.map(&:trackers).reduce(:&) end + @trackers = @projects.map {|p| Issue.allowed_target_trackers(p) }.reduce(:&) @versions = @projects.map {|p| p.shared_versions.open}.reduce(:&) @priorities = IssuePriority.active.reverse diff --git a/app/controllers/issues_controller.rb b/app/controllers/issues_controller.rb index cf402de22..67956667a 100644 --- a/app/controllers/issues_controller.rb +++ b/app/controllers/issues_controller.rb @@ -211,6 +211,10 @@ class IssuesController < ApplicationController unless User.current.allowed_to?(:copy_issues, @projects) raise ::Unauthorized end + else + unless @issues.all?(&:attributes_editable?) + raise ::Unauthorized + end end @allowed_projects = Issue.allowed_target_projects @@ -230,7 +234,7 @@ class IssuesController < ApplicationController end @custom_fields = @issues.map{|i|i.editable_custom_fields}.reduce(:&) @assignables = target_projects.map(&:assignable_users).reduce(:&) - @trackers = target_projects.map(&:trackers).reduce(:&) + @trackers = target_projects.map {|p| Issue.allowed_target_trackers(p) }.reduce(:&) @versions = target_projects.map {|p| p.shared_versions.open}.reduce(:&) @categories = target_projects.map {|p| p.issue_categories}.reduce(:&) if @copy @@ -263,6 +267,10 @@ class IssuesController < ApplicationController unless User.current.allowed_to?(:add_issues, target_projects) raise ::Unauthorized end + else + unless @issues.all?(&:attributes_editable?) + raise ::Unauthorized + end end unsaved_issues = [] @@ -316,6 +324,7 @@ class IssuesController < ApplicationController end def destroy + raise Unauthorized unless @issues.all?(&:deletable?) @hours = TimeEntry.where(:issue_id => @issues.map(&:id)).sum(:hours).to_f if @hours > 0 case params[:todo] @@ -465,9 +474,15 @@ class IssuesController < ApplicationController @issue.safe_attributes = attrs if @issue.project - @issue.tracker ||= @issue.project.trackers.first + @issue.tracker ||= @issue.allowed_target_trackers.first if @issue.tracker.nil? - render_error l(:error_no_tracker_in_project) + if @issue.project.trackers.any? + # None of the project trackers is allowed to the user + render_error :message => l(:error_no_tracker_allowed_for_new_issue_in_project), :status => 403 + else + # Project has no trackers + render_error l(:error_no_tracker_in_project) + end return false end if @issue.status.nil? diff --git a/app/helpers/issues_helper.rb b/app/helpers/issues_helper.rb index b505765d8..ce6d9f8f2 100644 --- a/app/helpers/issues_helper.rb +++ b/app/helpers/issues_helper.rb @@ -168,6 +168,16 @@ module IssuesHelper link_to(l(:button_add), new_project_issue_path(issue.project, :issue => attrs)) end + def trackers_options_for_select(issue) + trackers = issue.allowed_target_trackers + if issue.new_record? && issue.parent_issue_id.present? + trackers = trackers.reject do |tracker| + issue.tracker_id != tracker.id && tracker.disabled_core_fields.include?('parent_issue_id') + end + end + trackers.collect {|t| [t.name, t.id]} + end + class IssueFieldsRows include ActionView::Helpers::TagHelper diff --git a/app/models/issue.rb b/app/models/issue.rb index 0ee8ff3d3..e0b99e4e6 100644 --- a/app/models/issue.rb +++ b/app/models/issue.rb @@ -120,10 +120,10 @@ class Issue < ActiveRecord::Base # Returns a SQL conditions string used to find all issues visible by the specified user def self.visible_condition(user, options={}) Project.allowed_to_condition(user, :view_issues, options) do |role, user| - if user.id && user.logged? + sql = if user.id && user.logged? case role.issues_visibility when 'all' - nil + '1=1' when 'default' user_ids = [user.id] + user.groups.map(&:id).compact "(#{table_name}.is_private = #{connection.quoted_false} OR #{table_name}.author_id = #{user.id} OR #{table_name}.assigned_to_id IN (#{user_ids.join(',')}))" @@ -136,13 +136,22 @@ class Issue < ActiveRecord::Base else "(#{table_name}.is_private = #{connection.quoted_false})" end + unless role.permissions_all_trackers?(:view_issues) + tracker_ids = role.permissions_tracker_ids(:view_issues) + if tracker_ids.any? + sql = "(#{sql} AND #{table_name}.tracker_id IN (#{tracker_ids.join(',')}))" + else + sql = '1=0' + end + end + sql end end # Returns true if usr or current user is allowed to view the issue def visible?(usr=nil) (usr || User.current).allowed_to?(:view_issues, self.project) do |role, user| - if user.logged? + visible = if user.logged? case role.issues_visibility when 'all' true @@ -156,17 +165,36 @@ class Issue < ActiveRecord::Base else !self.is_private? end + unless role.permissions_all_trackers?(:view_issues) + visible &&= role.permissions_tracker_ids?(:view_issues, tracker_id) + end + visible end end - # Returns true if user or current user is allowed to edit or add a note to the issue + # Returns true if user or current user is allowed to edit or add notes to the issue def editable?(user=User.current) - attributes_editable?(user) || user.allowed_to?(:add_issue_notes, project) + attributes_editable?(user) || notes_addable?(user) end # Returns true if user or current user is allowed to edit the issue def attributes_editable?(user=User.current) - user.allowed_to?(:edit_issues, project) + user_tracker_permission?(user, :edit_issues) + end + + # Overrides Redmine::Acts::Attachable::InstanceMethods#attachments_editable? + def attachments_editable?(user=User.current) + attributes_editable?(user) + end + + # Returns true if user or current user is allowed to add notes to the issue + def notes_addable?(user=User.current) + user_tracker_permission?(user, :add_issue_notes) + end + + # Returns true if user or current user is allowed to delete the issue + def deletable?(user=User.current) + user_tracker_permission?(user, :delete_issues) end def initialize(attributes=nil, *args) @@ -416,10 +444,10 @@ class Issue < ActiveRecord::Base 'custom_fields', 'lock_version', 'notes', - :if => lambda {|issue, user| issue.new_record? || user.allowed_to?(:edit_issues, issue.project) } + :if => lambda {|issue, user| issue.new_record? || issue.attributes_editable?(user) } safe_attributes 'notes', - :if => lambda {|issue, user| user.allowed_to?(:add_issue_notes, issue.project)} + :if => lambda {|issue, user| issue.notes_addable?(user)} safe_attributes 'private_notes', :if => lambda {|issue, user| !issue.new_record? && user.allowed_to?(:set_notes_private, issue.project)} @@ -434,7 +462,7 @@ class Issue < ActiveRecord::Base } safe_attributes 'parent_issue_id', - :if => lambda {|issue, user| (issue.new_record? || user.allowed_to?(:edit_issues, issue.project)) && + :if => lambda {|issue, user| (issue.new_record? || issue.attributes_editable?(user)) && user.allowed_to?(:manage_subtasks, issue.project)} def safe_attribute_names(user=nil) @@ -479,12 +507,14 @@ class Issue < ActiveRecord::Base end if (t = attrs.delete('tracker_id')) && safe_attribute?('tracker_id') - self.tracker_id = t + if allowed_target_trackers(user).where(:id => t.to_i).exists? + self.tracker_id = t + end end if project - # Set the default tracker to accept custom field values + # Set a default tracker to accept custom field values # even if tracker is not specified - self.tracker ||= project.trackers.first + self.tracker ||= allowed_target_trackers(user).first end statuses_allowed = new_statuses_allowed_to(user) @@ -822,16 +852,6 @@ class Issue < ActiveRecord::Base !leaf? end - def assignable_trackers - trackers = project.trackers - if new_record? && parent_issue_id.present? - trackers = trackers.reject do |tracker| - tracker_id != tracker.id && tracker.disabled_core_fields.include?('parent_issue_id') - end - end - trackers - end - # Users the issue can be assigned to def assignable_users users = project.assignable_users.to_a @@ -1373,9 +1393,43 @@ class Issue < ActiveRecord::Base end Project.where(condition).having_trackers end + + # Returns a scope of trackers that user can assign the issue to + def allowed_target_trackers(user=User.current) + self.class.allowed_target_trackers(project, user, tracker_id_was) + end + + # Returns a scope of trackers that user can assign project issues to + def self.allowed_target_trackers(project, user=User.current, current_tracker=nil) + if project + scope = project.trackers.sorted + unless user.admin? + roles = user.roles_for_project(project).select {|r| r.has_permission?(:add_issues)} + unless roles.any? {|r| r.permissions_all_trackers?(:add_issues)} + tracker_ids = roles.map {|r| r.permissions_tracker_ids(:add_issues)}.flatten.uniq + if current_tracker + tracker_ids << current_tracker + end + scope = scope.where(:id => tracker_ids) + end + end + scope + else + Tracker.none + end + end private + def user_tracker_permission?(user, permission) + if user.admin? + true + else + roles = user.roles_for_project(project).select {|r| r.has_permission?(permission)} + roles.any? {|r| r.permissions_all_trackers?(permission) || r.permissions_tracker_ids?(permission, tracker_id)} + end + end + def after_project_change # Update project_id on related time entries TimeEntry.where({:issue_id => id}).update_all(["project_id = ?", project_id]) diff --git a/app/models/issue_import.rb b/app/models/issue_import.rb index b6b20a1b1..5b19ac966 100644 --- a/app/models/issue_import.rb +++ b/app/models/issue_import.rb @@ -37,7 +37,7 @@ class IssueImport < Import # Returns a scope of trackers that user is allowed to # import issue to def allowed_target_trackers - project.trackers + Issue.allowed_target_trackers(project, user) end def tracker diff --git a/app/models/mail_handler.rb b/app/models/mail_handler.rb index 40bd02a07..a619f115d 100644 --- a/app/models/mail_handler.rb +++ b/app/models/mail_handler.rb @@ -199,7 +199,14 @@ class MailHandler < ActionMailer::Base end issue = Issue.new(:author => user, :project => project) - issue.safe_attributes = issue_attributes_from_keywords(issue) + attributes = issue_attributes_from_keywords(issue) + if handler_options[:no_permission_check] + issue.tracker_id = attributes['tracker_id'] + if project + issue.tracker_id ||= project.trackers.first.try(:id) + end + end + issue.safe_attributes = attributes issue.safe_attributes = {'custom_field_values' => custom_field_values_from_keywords(issue)} issue.subject = cleaned_up_subject if issue.subject.blank? @@ -420,10 +427,6 @@ class MailHandler < ActionMailer::Base 'done_ratio' => get_keyword(:done_ratio, :format => '(\d|10)?0') }.delete_if {|k, v| v.blank? } - if issue.new_record? && attrs['tracker_id'].nil? - attrs['tracker_id'] = issue.project.trackers.first.try(:id) - end - attrs end diff --git a/app/models/role.rb b/app/models/role.rb index defbc311d..89538aa4d 100644 --- a/app/models/role.rb +++ b/app/models/role.rb @@ -73,6 +73,7 @@ class Role < ActiveRecord::Base acts_as_positioned :scope => :builtin serialize :permissions, ::Role::PermissionsAttributeCoder + store :settings, :accessors => [:permissions_all_trackers, :permissions_tracker_ids] attr_protected :builtin validates_presence_of :name @@ -188,6 +189,56 @@ class Role < ActiveRecord::Base setable_permissions end + def permissions_tracker_ids(*args) + if args.any? + Array(permissions_tracker_ids[args.first.to_s]).map(&:to_i) + else + super || {} + end + end + + def permissions_tracker_ids=(arg) + h = arg.to_hash + h.values.each {|v| v.reject!(&:blank?)} + super(h) + end + + # Returns true if tracker_id belongs to the list of + # trackers for which permission is given + def permissions_tracker_ids?(permission, tracker_id) + permissions_tracker_ids(permission).include?(tracker_id) + end + + def permissions_all_trackers + super || {} + end + + def permissions_all_trackers=(arg) + super(arg.to_hash) + end + + # Returns true if permission is given for all trackers + def permissions_all_trackers?(permission) + permissions_all_trackers[permission.to_s].to_s != '0' + end + + # Sets the trackers that are allowed for a permission. + # tracker_ids can be an array of tracker ids or :all for + # no restrictions. + # + # Examples: + # role.set_permission_trackers :add_issues, [1, 3] + # role.set_permission_trackers :add_issues, :all + def set_permission_trackers(permission, tracker_ids) + h = {permission.to_s => (tracker_ids == :all ? '1' : '0')} + self.permissions_all_trackers = permissions_all_trackers.merge(h) + + h = {permission.to_s => (tracker_ids == :all ? [] : tracker_ids)} + self.permissions_tracker_ids = permissions_tracker_ids.merge(h) + + self + end + # Find all the roles that can be given to a project member def self.find_all_givable Role.givable.to_a diff --git a/app/views/issues/_action_menu.html.erb b/app/views/issues/_action_menu.html.erb index c3304626d..b535faec9 100644 --- a/app/views/issues/_action_menu.html.erb +++ b/app/views/issues/_action_menu.html.erb @@ -3,5 +3,5 @@ <%= 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?(: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) %> +<%= link_to l(:button_delete), issue_path(@issue), :data => {:confirm => issues_destroy_confirmation_message(@issue)}, :method => :delete, :class => 'icon icon-del' if @issue.deletable? %> diff --git a/app/views/issues/_edit.html.erb b/app/views/issues/_edit.html.erb index 24a01cad0..67e33246d 100644 --- a/app/views/issues/_edit.html.erb +++ b/app/views/issues/_edit.html.erb @@ -27,21 +27,22 @@ <% end %> <% end %> - -
<%= l(:field_notes) %> - <%= f.text_area :notes, :cols => 60, :rows => 10, :class => 'wiki-edit', :no_label => true %> - <%= wikitoolbar_for 'issue_notes' %> - - <% if @issue.safe_attribute? 'private_notes' %> - <%= f.check_box :private_notes, :no_label => true %> + <% if @issue.notes_addable? %> +
<%= l(:field_notes) %> + <%= f.text_area :notes, :cols => 60, :rows => 10, :class => 'wiki-edit', :no_label => true %> + <%= wikitoolbar_for 'issue_notes' %> + + <% if @issue.safe_attribute? 'private_notes' %> + <%= f.check_box :private_notes, :no_label => true %> + <% end %> + + <%= call_hook(:view_issues_edit_notes_bottom, { :issue => @issue, :notes => @notes, :form => f }) %> +
+ +
<%= l(:label_attachment_plural) %> +

<%= render :partial => 'attachments/form', :locals => {:container => @issue} %>

+
<% end %> - - <%= call_hook(:view_issues_edit_notes_bottom, { :issue => @issue, :notes => @notes, :form => f }) %> -
- -
<%= l(:label_attachment_plural) %> -

<%= render :partial => 'attachments/form', :locals => {:container => @issue} %>

-
<%= f.hidden_field :lock_version %> diff --git a/app/views/issues/_form.html.erb b/app/views/issues/_form.html.erb index 788a2f38e..1e82b6f84 100644 --- a/app/views/issues/_form.html.erb +++ b/app/views/issues/_form.html.erb @@ -14,7 +14,7 @@ <% end %> <% if @issue.safe_attribute? 'tracker_id' %> -

<%= f.select :tracker_id, @issue.assignable_trackers.collect {|t| [t.name, t.id]}, {:required => true}, +

<%= f.select :tracker_id, trackers_options_for_select(@issue), {:required => true}, :onchange => "updateIssueFrom('#{escape_javascript update_issue_form_path(@project, @issue)}', this)" %>

<% end %> diff --git a/app/views/issues/_history.html.erb b/app/views/issues/_history.html.erb index 993dfee3c..307ac909b 100644 --- a/app/views/issues/_history.html.erb +++ b/app/views/issues/_history.html.erb @@ -1,4 +1,4 @@ -<% reply_links = authorize_for('issues', 'edit') -%> +<% reply_links = issue.notes_addable? -%> <% for journal in journals %>
diff --git a/app/views/issues/index.html.erb b/app/views/issues/index.html.erb index 7dcba2629..fd762023f 100644 --- a/app/views/issues/index.html.erb +++ b/app/views/issues/index.html.erb @@ -1,5 +1,5 @@
- <% if User.current.allowed_to?(:add_issues, @project, :global => true) && (@project.nil? || @project.trackers.any?) %> + <% if User.current.allowed_to?(:add_issues, @project, :global => true) && (@project.nil? || Issue.allowed_target_trackers(@project).any?) %> <%= link_to l(:label_issue_new), _new_project_issue_path(@project), :class => 'icon icon-add new-issue' %> <% end %>
diff --git a/app/views/issues/show.html.erb b/app/views/issues/show.html.erb index 70a7fe165..2cbff32e5 100644 --- a/app/views/issues/show.html.erb +++ b/app/views/issues/show.html.erb @@ -77,7 +77,7 @@ end %> <% if @issue.description? %>
- <%= link_to l(:button_quote), quoted_issue_path(@issue), :remote => true, :method => 'post', :class => 'icon icon-comment' if authorize_for('issues', 'edit') %> + <%= link_to l(:button_quote), quoted_issue_path(@issue), :remote => true, :method => 'post', :class => 'icon icon-comment' if @issue.notes_addable? %>

<%=l(:field_description)%>

diff --git a/app/views/roles/_form.html.erb b/app/views/roles/_form.html.erb index 84d5de185..39acc34a4 100644 --- a/app/views/roles/_form.html.erb +++ b/app/views/roles/_form.html.erb @@ -7,17 +7,17 @@ <% end %> <% unless @role.anonymous? %> -

<%= f.select :issues_visibility, Role::ISSUES_VISIBILITY_OPTIONS.collect {|v| [l(v.last), v.first]} %>

+

<%= f.select :issues_visibility, Role::ISSUES_VISIBILITY_OPTIONS.collect {|v| [l(v.last), v.first]} %>

<% end %> <% unless @role.anonymous? %> -

<%= f.select :time_entries_visibility, Role::TIME_ENTRIES_VISIBILITY_OPTIONS.collect {|v| [l(v.last), v.first]} %>

+

<%= f.select :time_entries_visibility, Role::TIME_ENTRIES_VISIBILITY_OPTIONS.collect {|v| [l(v.last), v.first]} %>

<% end %>

<%= f.select :users_visibility, Role::USERS_VISIBILITY_OPTIONS.collect {|v| [l(v.last), v.first]} %>

<% unless @role.builtin? %> -

+

-<%= javascript_tag do %> -$(document).ready(function(){ - $("#role_permissions_manage_members").change(function(){ - $("#manage_members_options").toggle($(this).is(":checked")); - }).change(); -}); +
+

<%= l(:label_issue_tracking) %>

+<% permissions = %w(view_issues add_issues edit_issues add_issue_notes delete_issues) %> + +
+ + + + + <% permissions.each do |permission| %> + + <% end %> + + + + + <% permissions.each do |permission| %> + + <% end %> + + <% Tracker.sorted.all.each do |tracker| %> + "> + + <% permissions.each do |permission| %> + + <% end %> + + <% end %> + +
<%= l(:label_tracker) %>"><%= l("permission_#{permission}") %>
<%= l(:label_tracker_all) %>"> + <%= hidden_field_tag "role[permissions_all_trackers][#{permission}]", '0', :id => nil %> + <%= check_box_tag "role[permissions_all_trackers][#{permission}]", + '1', + @role.permissions_all_trackers?(permission), + :class => "#{permission}_shown", + :data => {:disables => ".#{permission}_tracker"} %> +
<%= tracker.name %>"><%= check_box_tag "role[permissions_tracker_ids][#{permission}][]", + tracker.id, + @role.permissions_tracker_ids?(permission, tracker.id), + :class => "#{permission}_tracker", + :id => "role_permissions_tracker_ids_add_issues_#{tracker.id}" %>
+
+ +<% permissions.each do |permission| %> + <%= hidden_field_tag "role[permissions_tracker_ids][#{permission}][]", '' %> <% end %> +
diff --git a/config/locales/en.yml b/config/locales/en.yml index 9dd03560c..522cb5227 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -213,6 +213,7 @@ en: error_can_not_read_import_file: "An error occurred while reading the file to import" error_attachment_extension_not_allowed: "Attachment extension %{extension} is not allowed" error_ldap_bind_credentials: "Invalid LDAP Account/Password" + error_no_tracker_allowed_for_new_issue_in_project: "The project doesn't have any trackers for which you can create an issue" mail_subject_lost_password: "Your %{value} password" mail_body_lost_password: 'To change your password, click on the following link:' @@ -561,6 +562,7 @@ en: label_member_plural: Members label_tracker: Tracker label_tracker_plural: Trackers + label_tracker_all: All trackers label_tracker_new: New tracker label_workflow: Workflow label_issue_status: Issue status diff --git a/config/locales/fr.yml b/config/locales/fr.yml index 6a72024f6..760c84c95 100644 --- a/config/locales/fr.yml +++ b/config/locales/fr.yml @@ -233,6 +233,7 @@ fr: error_can_not_read_import_file: "Une erreur est survenue lors de la lecture du fichier à importer" error_attachment_extension_not_allowed: "L'extension %{extension} n'est pas autorisée" error_ldap_bind_credentials: "Identifiant ou mot de passe LDAP incorrect" + error_no_tracker_allowed_for_new_issue_in_project: "Le projet ne dispose d'aucun tracker sur lequel vous pouvez créer une demande" mail_subject_lost_password: "Votre mot de passe %{value}" mail_body_lost_password: 'Pour changer votre mot de passe, cliquez sur le lien suivant :' @@ -573,6 +574,7 @@ fr: label_member_plural: Membres label_tracker: Tracker label_tracker_plural: Trackers + label_tracker_all: Tous les trackers label_tracker_new: Nouveau tracker label_workflow: Workflow label_issue_status: Statut de demandes diff --git a/db/migrate/20160529063352_add_roles_settings.rb b/db/migrate/20160529063352_add_roles_settings.rb new file mode 100644 index 000000000..921187ec1 --- /dev/null +++ b/db/migrate/20160529063352_add_roles_settings.rb @@ -0,0 +1,5 @@ +class AddRolesSettings < ActiveRecord::Migration + def change + add_column :roles, :settings, :text + end +end \ No newline at end of file diff --git a/lib/redmine.rb b/lib/redmine.rb index 1d37a523b..6dfc880ff 100644 --- a/lib/redmine.rb +++ b/lib/redmine.rb @@ -88,8 +88,6 @@ Redmine::AccessControl.map do |map| map.permission :add_subprojects, {:projects => [:new, :create]}, :require => :member map.project_module :issue_tracking do |map| - # Issue categories - map.permission :manage_categories, {:projects => :settings, :issue_categories => [:index, :show, :new, :create, :edit, :update, :destroy]}, :require => :member # Issues map.permission :view_issues, {:issues => [:index, :show], :auto_complete => [:issues], @@ -120,62 +118,64 @@ Redmine::AccessControl.map do |map| map.permission :add_issue_watchers, {:watchers => [:new, :create, :append, :autocomplete_for_user]} map.permission :delete_issue_watchers, {:watchers => :destroy} map.permission :import_issues, {:imports => [:new, :create, :settings, :mapping, :run, :show]} + # Issue categories + map.permission :manage_categories, {:projects => :settings, :issue_categories => [:index, :show, :new, :create, :edit, :update, :destroy]}, :require => :member end map.project_module :time_tracking do |map| - map.permission :log_time, {:timelog => [:new, :create]}, :require => :loggedin map.permission :view_time_entries, {:timelog => [:index, :report, :show]}, :read => true + map.permission :log_time, {:timelog => [:new, :create]}, :require => :loggedin map.permission :edit_time_entries, {:timelog => [:edit, :update, :destroy, :bulk_edit, :bulk_update]}, :require => :member map.permission :edit_own_time_entries, {:timelog => [:edit, :update, :destroy,:bulk_edit, :bulk_update]}, :require => :loggedin map.permission :manage_project_activities, {:project_enumerations => [:update, :destroy]}, :require => :member end map.project_module :news do |map| - map.permission :manage_news, {:news => [:new, :create, :edit, :update, :destroy], :comments => [:destroy], :attachments => :upload}, :require => :member map.permission :view_news, {:news => [:index, :show]}, :public => true, :read => true + map.permission :manage_news, {:news => [:new, :create, :edit, :update, :destroy], :comments => [:destroy], :attachments => :upload}, :require => :member map.permission :comment_news, {:comments => :create} end map.project_module :documents do |map| + map.permission :view_documents, {:documents => [:index, :show, :download]}, :read => true map.permission :add_documents, {:documents => [:new, :create, :add_attachment], :attachments => :upload}, :require => :loggedin map.permission :edit_documents, {:documents => [:edit, :update, :add_attachment], :attachments => :upload}, :require => :loggedin map.permission :delete_documents, {:documents => [:destroy]}, :require => :loggedin - map.permission :view_documents, {:documents => [:index, :show, :download]}, :read => true end map.project_module :files do |map| - map.permission :manage_files, {:files => [:new, :create], :attachments => :upload}, :require => :loggedin map.permission :view_files, {:files => :index, :versions => :download}, :read => true + map.permission :manage_files, {:files => [:new, :create], :attachments => :upload}, :require => :loggedin end map.project_module :wiki do |map| - map.permission :manage_wiki, {:wikis => [:edit, :destroy]}, :require => :member - map.permission :rename_wiki_pages, {:wiki => :rename}, :require => :member - map.permission :delete_wiki_pages, {:wiki => [:destroy, :destroy_version]}, :require => :member map.permission :view_wiki_pages, {:wiki => [:index, :show, :special, :date_index]}, :read => true - map.permission :export_wiki_pages, {:wiki => [:export]}, :read => true map.permission :view_wiki_edits, {:wiki => [:history, :diff, :annotate]}, :read => true + map.permission :export_wiki_pages, {:wiki => [:export]}, :read => true map.permission :edit_wiki_pages, :wiki => [:new, :edit, :update, :preview, :add_attachment], :attachments => :upload + map.permission :rename_wiki_pages, {:wiki => :rename}, :require => :member + map.permission :delete_wiki_pages, {:wiki => [:destroy, :destroy_version]}, :require => :member map.permission :delete_wiki_pages_attachments, {} map.permission :protect_wiki_pages, {:wiki => :protect}, :require => :member + map.permission :manage_wiki, {:wikis => [:edit, :destroy]}, :require => :member end map.project_module :repository do |map| - map.permission :manage_repository, {:repositories => [:new, :create, :edit, :update, :committers, :destroy]}, :require => :member - map.permission :browse_repository, {:repositories => [:show, :browse, :entry, :raw, :annotate, :changes, :diff, :stats, :graph]}, :read => true map.permission :view_changesets, {:repositories => [:show, :revisions, :revision]}, :read => true + map.permission :browse_repository, {:repositories => [:show, :browse, :entry, :raw, :annotate, :changes, :diff, :stats, :graph]}, :read => true map.permission :commit_access, {} map.permission :manage_related_issues, {:repositories => [:add_related_issue, :remove_related_issue]} + map.permission :manage_repository, {:repositories => [:new, :create, :edit, :update, :committers, :destroy]}, :require => :member end map.project_module :boards do |map| - map.permission :manage_boards, {:boards => [:new, :create, :edit, :update, :destroy]}, :require => :member map.permission :view_messages, {:boards => [:index, :show], :messages => [:show]}, :public => true, :read => true map.permission :add_messages, {:messages => [:new, :reply, :quote], :attachments => :upload} map.permission :edit_messages, {:messages => :edit, :attachments => :upload}, :require => :member map.permission :edit_own_messages, {:messages => :edit, :attachments => :upload}, :require => :loggedin map.permission :delete_messages, {:messages => :destroy}, :require => :member map.permission :delete_own_messages, {:messages => :destroy}, :require => :loggedin + map.permission :manage_boards, {:boards => [:new, :create, :edit, :update, :destroy]}, :require => :member end map.project_module :calendar do |map| @@ -233,7 +233,7 @@ Redmine::MenuManager.map :project_menu do |menu| menu.push :issues, { :controller => 'issues', :action => 'index' }, :param => :project_id, :caption => :label_issue_plural menu.push :new_issue, { :controller => 'issues', :action => 'new', :copy_from => nil }, :param => :project_id, :caption => :label_issue_new, :html => { :accesskey => Redmine::AccessKeys.key_for(:new_issue) }, - :if => Proc.new { |p| Setting.new_project_issue_tab_enabled? && p.trackers.any? }, + :if => Proc.new { |p| Setting.new_project_issue_tab_enabled? && Issue.allowed_target_trackers(p).any? }, :permission => :add_issues menu.push :gantt, { :controller => 'gantts', :action => 'show' }, :param => :project_id, :caption => :label_gantt menu.push :calendar, { :controller => 'calendars', :action => 'show' }, :param => :project_id, :caption => :label_calendar diff --git a/public/javascripts/application.js b/public/javascripts/application.js index 743b14ff6..347611bb2 100644 --- a/public/javascripts/application.js +++ b/public/javascripts/application.js @@ -719,9 +719,10 @@ function toggleDisabledOnChange() { var checked = $(this).is(':checked'); $($(this).data('disables')).attr('disabled', checked); $($(this).data('enables')).attr('disabled', !checked); + $($(this).data('shows')).toggle(checked); } function toggleDisabledInit() { - $('input[data-disables], input[data-enables]').each(toggleDisabledOnChange); + $('input[data-disables], input[data-enables], input[data-shows]').each(toggleDisabledOnChange); } (function ( $ ) { @@ -751,7 +752,7 @@ function toggleDisabledInit() { }( jQuery )); $(document).ready(function(){ - $('#content').on('change', 'input[data-disables], input[data-enables]', toggleDisabledOnChange); + $('#content').on('change', 'input[data-disables], input[data-enables], input[data-shows]', toggleDisabledOnChange); toggleDisabledInit(); }); diff --git a/public/stylesheets/application.css b/public/stylesheets/application.css index 83060eaa2..c276ebd27 100644 --- a/public/stylesheets/application.css +++ b/public/stylesheets/application.css @@ -152,6 +152,7 @@ table.list td.buttons img, div.buttons img {vertical-align:middle;} table.list td.reorder {width:15%; white-space:nowrap; text-align:center; } table.list table.progress td {padding-right:0px;} table.list caption { text-align: left; padding: 0.5em 0.5em 0.5em 0; } +#role-permissions-trackers table.list th {white-space:normal;} .table-list-cell {display: table-cell; vertical-align: top; padding:2px; } diff --git a/test/functional/issues_controller_test.rb b/test/functional/issues_controller_test.rb index 32f9d8f11..dc50d1331 100644 --- a/test/functional/issues_controller_test.rb +++ b/test/functional/issues_controller_test.rb @@ -1864,6 +1864,31 @@ class IssuesControllerTest < ActionController::TestCase end end + def test_new_should_propose_allowed_trackers + role = Role.find(1) + role.set_permission_trackers 'add_issues', [1, 3] + role.save! + @request.session[:user_id] = 2 + + get :new, :project_id => 1 + assert_response :success + assert_select 'select[name=?]', 'issue[tracker_id]' do + assert_select 'option', 2 + assert_select 'option[value="1"]' + assert_select 'option[value="3"]' + end + end + + def test_new_without_allowed_trackers_should_respond_with_403 + role = Role.find(1) + role.set_permission_trackers 'add_issues', [] + role.save! + @request.session[:user_id] = 2 + + get :new, :project_id => 1 + assert_response 403 + end + def test_new_should_preselect_default_version version = Version.generate!(:project_id => 1) Project.find(1).update_attribute :default_version_id, version.id @@ -2432,6 +2457,23 @@ class IssuesControllerTest < ActionController::TestCase assert_nil issue.custom_field_value(cf2) end + def test_create_should_ignore_unallowed_trackers + role = Role.find(1) + role.set_permission_trackers :add_issues, [3] + role.save! + @request.session[:user_id] = 2 + + issue = new_record(Issue) do + post :create, :project_id => 1, :issue => { + :tracker_id => 1, + :status_id => 1, + :subject => 'Test' + } + assert_response 302 + end + assert_equal 3, issue.tracker_id + end + def test_post_create_with_watchers @request.session[:user_id] = 2 ActionMailer::Base.deliveries.clear @@ -3830,6 +3872,30 @@ class IssuesControllerTest < ActionController::TestCase assert_redirected_to '/issues/11?issue_count=3&issue_position=2&next_issue_id=12&prev_issue_id=8' end + def test_update_with_permission_on_tracker_should_be_allowed + role = Role.find(1) + role.set_permission_trackers :edit_issues, [1] + role.save! + issue = Issue.generate!(:project_id => 1, :tracker_id => 1, :subject => 'Original subject') + + @request.session[:user_id] = 2 + put :update, :id => issue.id, :issue => {:subject => 'Changed subject'} + assert_response 302 + assert_equal 'Changed subject', issue.reload.subject + end + + def test_update_without_permission_on_tracker_should_be_denied + role = Role.find(1) + role.set_permission_trackers :edit_issues, [1] + role.save! + issue = Issue.generate!(:project_id => 1, :tracker_id => 2, :subject => 'Original subject') + + @request.session[:user_id] = 2 + put :update, :id => issue.id, :issue => {:subject => 'Changed subject'} + assert_response 302 + assert_equal 'Original subject', issue.reload.subject + end + def test_get_bulk_edit @request.session[:user_id] = 2 get :bulk_edit, :ids => [1, 3] @@ -4660,6 +4726,32 @@ class IssuesControllerTest < ActionController::TestCase assert_response 404 end + def test_destroy_with_permission_on_tracker_should_be_allowed + role = Role.find(1) + role.set_permission_trackers :delete_issues, [1] + role.save! + issue = Issue.generate!(:project_id => 1, :tracker_id => 1) + + @request.session[:user_id] = 2 + assert_difference 'Issue.count', -1 do + delete :destroy, :id => issue.id + end + assert_response 302 + end + + def test_destroy_without_permission_on_tracker_should_be_denied + role = Role.find(1) + role.set_permission_trackers :delete_issues, [2] + role.save! + issue = Issue.generate!(:project_id => 1, :tracker_id => 1) + + @request.session[:user_id] = 2 + assert_no_difference 'Issue.count' do + delete :destroy, :id => issue.id + end + assert_response 403 + end + def test_default_search_scope get :index diff --git a/test/functional/roles_controller_test.rb b/test/functional/roles_controller_test.rb index 8ce469395..915b658c9 100644 --- a/test/functional/roles_controller_test.rb +++ b/test/functional/roles_controller_test.rb @@ -132,6 +132,22 @@ class RolesControllerTest < ActionController::TestCase assert_equal [:edit_project], role.permissions end + def test_update_trackers_permissions + put :update, :id => 1, :role => { + :permissions_all_trackers => {'add_issues' => '0'}, + :permissions_tracker_ids => {'add_issues' => ['1', '3', '']} + } + + assert_redirected_to '/roles' + role = Role.find(1) + + assert_equal({'add_issues' => '0'}, role.permissions_all_trackers) + assert_equal({'add_issues' => ['1', '3']}, role.permissions_tracker_ids) + + assert_equal false, role.permissions_all_trackers?(:add_issues) + assert_equal [1, 3], role.permissions_tracker_ids(:add_issues).sort + end + def test_update_with_failure put :update, :id => 1, :role => {:name => ''} assert_response :success diff --git a/test/unit/issue_test.rb b/test/unit/issue_test.rb index d7c7d231b..3b7391a3c 100644 --- a/test/unit/issue_test.rb +++ b/test/unit/issue_test.rb @@ -342,6 +342,69 @@ class IssueTest < ActiveSupport::TestCase assert_include issue, issues end + def test_visible_scope_for_member_with_limited_tracker_ids + role = Role.find(1) + role.set_permission_trackers :view_issues, [2] + role.save! + user = User.find(2) + + issues = Issue.where(:project_id => 1).visible(user).to_a + assert issues.any? + assert_equal [2], issues.map(&:tracker_id).uniq + + assert Issue.where(:project_id => 1).all? {|issue| issue.visible?(user) ^ issue.tracker_id != 2} + end + + def test_visible_scope_should_consider_tracker_ids_on_each_project + user = User.generate! + + project1 = Project.generate! + role1 = Role.generate! + role1.add_permission! :view_issues + role1.set_permission_trackers :view_issues, :all + role1.save! + User.add_to_project(user, project1, role1) + + project2 = Project.generate! + role2 = Role.generate! + role2.add_permission! :view_issues + role2.set_permission_trackers :view_issues, [2] + role2.save! + User.add_to_project(user, project2, role2) + + visible_issues = [ + Issue.generate!(:project => project1, :tracker_id => 1), + Issue.generate!(:project => project1, :tracker_id => 2), + Issue.generate!(:project => project2, :tracker_id => 2) + ] + hidden_issue = Issue.generate!(:project => project2, :tracker_id => 1) + + issues = Issue.where(:project_id => [project1.id, project2.id]).visible(user) + assert_equal visible_issues.map(&:id), issues.ids.sort + + assert visible_issues.all? {|issue| issue.visible?(user)} + assert !hidden_issue.visible?(user) + end + + def test_visible_scope_should_not_consider_roles_without_view_issues_permission + user = User.generate! + role1 = Role.generate! + role1.remove_permission! :view_issues + role1.set_permission_trackers :view_issues, :all + role1.save! + role2 = Role.generate! + role2.add_permission! :view_issues + role2.set_permission_trackers :view_issues, [2] + role2.save! + User.add_to_project(user, Project.find(1), [role1, role2]) + + issues = Issue.where(:project_id => 1).visible(user).to_a + assert issues.any? + assert_equal [2], issues.map(&:tracker_id).uniq + + assert Issue.where(:project_id => 1).all? {|issue| issue.visible?(user) ^ issue.tracker_id != 2} + end + def test_visible_scope_for_admin user = User.find(1) user.members.each(&:destroy) @@ -737,9 +800,10 @@ class IssueTest < ActiveSupport::TestCase target = Tracker.find(2) target.core_fields = %w(assigned_to_id due_date) target.save! + user = User.find(2) - issue = Issue.new(:tracker => source) - issue.safe_attributes = {'tracker_id' => 2, 'due_date' => '2012-07-14'} + issue = Issue.new(:project => Project.find(1), :tracker => source) + issue.send :safe_attributes=, {'tracker_id' => 2, 'due_date' => '2012-07-14'}, user assert_equal target, issue.tracker assert_equal Date.parse('2012-07-14'), issue.due_date end @@ -1437,6 +1501,91 @@ class IssueTest < ActiveSupport::TestCase assert_not_include project, Issue.allowed_target_projects(User.find(1)) end + def test_allowed_target_trackers_with_one_role_allowed_on_all_trackers + user = User.generate! + role = Role.generate! + role.add_permission! :add_issues + role.set_permission_trackers :add_issues, :all + role.save! + User.add_to_project(user, Project.find(1), role) + + assert_equal [1, 2, 3], Issue.new(:project => Project.find(1)).allowed_target_trackers(user).ids.sort + end + + def test_allowed_target_trackers_with_one_role_allowed_on_some_trackers + user = User.generate! + role = Role.generate! + role.add_permission! :add_issues + role.set_permission_trackers :add_issues, [1, 3] + role.save! + User.add_to_project(user, Project.find(1), role) + + assert_equal [1, 3], Issue.new(:project => Project.find(1)).allowed_target_trackers(user).ids.sort + end + + def test_allowed_target_trackers_with_two_roles_allowed_on_some_trackers + user = User.generate! + role1 = Role.generate! + role1.add_permission! :add_issues + role1.set_permission_trackers :add_issues, [1] + role1.save! + role2 = Role.generate! + role2.add_permission! :add_issues + role2.set_permission_trackers :add_issues, [3] + role2.save! + User.add_to_project(user, Project.find(1), [role1, role2]) + + assert_equal [1, 3], Issue.new(:project => Project.find(1)).allowed_target_trackers(user).ids.sort + end + + def test_allowed_target_trackers_with_two_roles_allowed_on_all_trackers_and_some_trackers + user = User.generate! + role1 = Role.generate! + role1.add_permission! :add_issues + role1.set_permission_trackers :add_issues, :all + role1.save! + role2 = Role.generate! + role2.add_permission! :add_issues + role2.set_permission_trackers :add_issues, [1, 3] + role2.save! + User.add_to_project(user, Project.find(1), [role1, role2]) + + assert_equal [1, 2, 3], Issue.new(:project => Project.find(1)).allowed_target_trackers(user).ids.sort + end + + def test_allowed_target_trackers_should_not_consider_roles_without_add_issues_permission + user = User.generate! + role1 = Role.generate! + role1.remove_permission! :add_issues + role1.set_permission_trackers :add_issues, :all + role1.save! + role2 = Role.generate! + role2.add_permission! :add_issues + role2.set_permission_trackers :add_issues, [1, 3] + role2.save! + User.add_to_project(user, Project.find(1), [role1, role2]) + + assert_equal [1, 3], Issue.new(:project => Project.find(1)).allowed_target_trackers(user).ids.sort + end + + def test_allowed_target_trackers_without_project_should_be_empty + issue = Issue.new + assert_nil issue.project + assert_equal [], issue.allowed_target_trackers(User.find(2)).ids + end + + def test_allowed_target_trackers_should_include_current_tracker + user = User.generate! + role = Role.generate! + role.add_permission! :add_issues + role.set_permission_trackers :add_issues, [3] + role.save! + User.add_to_project(user, Project.find(1), role) + + issue = Issue.generate!(:project => Project.find(1), :tracker => Tracker.find(1)) + assert_equal [1, 3], issue.allowed_target_trackers(user).ids.sort + end + def test_move_to_another_project_with_same_category issue = Issue.find(1) issue.project = Project.find(2)