From 0c57cd0f6b4bdc73c35e0fa960c9388621c2332b Mon Sep 17 00:00:00 2001 From: Frederico Camara Date: Thu, 14 Nov 2019 18:02:23 -0300 Subject: [PATCH 15/21] Implements permissions and restrictions to issue attachments --- app/controllers/issues_controller.rb | 13 ++++++++++--- app/models/issue.rb | 25 ++++++++++++++++++++++--- app/models/journal.rb | 2 ++ app/models/mailer.rb | 5 +++-- app/views/issues/_edit.html.erb | 6 ++++-- app/views/issues/index.api.rsb | 2 +- app/views/issues/new.html.erb | 4 ++-- app/views/issues/new.js.erb | 11 +++++++++++ app/views/issues/show.api.rsb | 2 +- app/views/issues/show.html.erb | 4 ++-- app/views/mailer/_issue.html.erb | 2 +- app/views/mailer/_issue.text.erb | 2 +- app/views/roles/_form.html.erb | 2 +- config/locales/en.yml | 4 ++++ config/locales/pt-BR.yml | 4 ++++ db/migrate/20161215142110_add_attachments_permissions.rb | 19 +++++++++++++++++++ lib/plugins/acts_as_searchable/lib/acts_as_searchable.rb | 1 + lib/redmine.rb | 15 ++++++++++----- lib/redmine/export/pdf/issues_pdf_helper.rb | 2 +- 19 files changed, 100 insertions(+), 25 deletions(-) create mode 100644 db/migrate/20161215142110_add_attachments_permissions.rb diff --git a/app/controllers/issues_controller.rb b/app/controllers/issues_controller.rb index 69a947b03..2493a8d22 100644 --- a/app/controllers/issues_controller.rb +++ b/app/controllers/issues_controller.rb @@ -88,6 +88,7 @@ class IssuesController < ApplicationController @journals = @issue.visible_journals_with_index @has_changesets = @issue.changesets.visible.preload(:repository, :user).exists? @relations = @issue.relations.select {|r| r.other_issue(@issue) && r.other_issue(@issue).visible? } + @attachments = @issue.attachments_visible?(User.current) ? @issue.attachments : [] @journals.reverse! if User.current.wants_comments_in_reverse_order? @@ -129,7 +130,9 @@ class IssuesController < ApplicationController 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.attachments_addable?(User.current) + @issue.save_attachments(params[:attachments] || (params[:issue] && params[:issue][:uploads])) + end if @issue.save call_hook(:controller_issues_new_after_save, { :params => params, :issue => @issue}) respond_to do |format| @@ -166,7 +169,9 @@ class IssuesController < ApplicationController def update return unless update_issue_from_params - @issue.save_attachments(params[:attachments] || (params[:issue] && params[:issue][:uploads])) + if @issue.attachments_addable?(User.current) + @issue.save_attachments(params[:attachments] || (params[:issue] && params[:issue][:uploads])) + end saved = false begin saved = save_issue_with_child_records @@ -282,7 +287,7 @@ class IssuesController < ApplicationController @versions = target_projects.map {|p| p.shared_versions.open}.reduce(:&) @categories = target_projects.map {|p| p.issue_categories}.reduce(:&) if @copy - @attachments_present = @issues.detect {|i| i.attachments.any?}.present? + @attachments_present = @issues.detect {|i| i.attachments.any? && i.attachments_visible?(User.current)}.present? @subtasks_present = @issues.detect {|i| !i.leaf?}.present? @watchers_present = User.current.allowed_to?(:add_issue_watchers, @projects) && Watcher.where(:watchable_type => 'Issue', @@ -348,6 +353,7 @@ class IssuesController < ApplicationController end journal = issue.init_journal(User.current, params[:notes]) issue.safe_attributes = attributes + issue.attachments = [] unless issue.attachments_addable?(User.current) if @copy call_hook(:controller_issues_bulk_edit_before_save, { :params => params, :issue => issue }) if issue.save saved_issues << issue @@ -568,6 +574,7 @@ class IssuesController < ApplicationController @priorities = IssuePriority.active @allowed_statuses = @issue.new_statuses_allowed_to(User.current) + @issue.attachments = [] unless @issue.attachments_addable?(User.current) end # Saves @issue and a time_entry from the parameters diff --git a/app/models/issue.rb b/app/models/issue.rb index 110457050..9e0c3ce3c 100644 --- a/app/models/issue.rb +++ b/app/models/issue.rb @@ -40,7 +40,10 @@ class Issue < ActiveRecord::Base has_many :relations_from, :class_name => 'IssueRelation', :foreign_key => 'issue_from_id', :dependent => :delete_all has_many :relations_to, :class_name => 'IssueRelation', :foreign_key => 'issue_to_id', :dependent => :delete_all - acts_as_attachable :after_add => :attachment_added, :after_remove => :attachment_removed + acts_as_attachable :after_add => :attachment_added, :after_remove => :attachment_removed, + :view_permission => :view_attachments, :edit_permission => :edit_attachments, + :delete_permission => :delete_attachments + acts_as_customizable acts_as_watchable acts_as_searchable :columns => ['subject', "#{table_name}.description"], @@ -186,9 +189,24 @@ class Issue < ActiveRecord::Base ) end + # Overrides Redmine::Acts::Attachable::InstanceMethods#attachments_editable? + def attachments_visible?(user=User.current) + user_tracker_permission?(user, :view_attachments) + end + + # Returns true if user or current user is allowed to add the attachment to the issue + def attachments_addable?(user=User.current) + user_tracker_permission?(user, :add_attachments) + end + # Overrides Redmine::Acts::Attachable::InstanceMethods#attachments_editable? def attachments_editable?(user=User.current) - attributes_editable?(user) + user_tracker_permission?(user, :edit_attachments) + end + + # Overrides Redmine::Acts::Attachable::InstanceMethods#attachments_editable? + def attachments_deletable?(user=User.current) + user_tracker_permission?(user, :delete_attachments) end # Returns true if user or current user is allowed to add notes to the issue @@ -278,7 +296,7 @@ class Issue < ActiveRecord::Base self.status = issue.status end self.author = User.current - unless options[:attachments] == false + if options[:attachments] == true && issue.attachments_visible?(user=User.current) self.attachments = issue.attachments.map do |attachement| attachement.copy(:container => self) end @@ -1639,6 +1657,7 @@ class Issue < ActiveRecord::Base copy.parent_issue_id = copied_issue_ids[child.parent_id] copy.fixed_version_id = nil unless child.fixed_version.present? && child.fixed_version.status == 'open' copy.assigned_to = nil unless child.assigned_to_id.present? && child.assigned_to.status == User::STATUS_ACTIVE + copy.attachments = [] unless copy.attachments_addable?(User.current) unless copy.save logger.error "Could not copy subtask ##{child.id} while copying ##{@copied_from.id} to ##{id} due to validation errors: #{copy.errors.full_messages.join(', ')}" if logger next diff --git a/app/models/journal.rb b/app/models/journal.rb index 7630e8654..9e7908edc 100644 --- a/app/models/journal.rb +++ b/app/models/journal.rb @@ -92,6 +92,8 @@ class Journal < ActiveRecord::Base detail.custom_field && detail.custom_field.visible_by?(project, user) elsif detail.property == 'relation' Issue.find_by_id(detail.value || detail.old_value).try(:visible?, user) + elsif detail.property == 'attachment' + self.issue.attachments_visible?(User.current) else true end diff --git a/app/models/mailer.rb b/app/models/mailer.rb index a6f1de6d1..ed9c54185 100644 --- a/app/models/mailer.rb +++ b/app/models/mailer.rb @@ -99,7 +99,7 @@ class Mailer < ActionMailer::Base end # Builds a mail for notifying user about an issue update - def issue_edit(user, journal) + def issue_edit(user, journal, att=false) issue = journal.journalized redmine_headers 'Project' => issue.project.identifier, 'Issue-Tracker' => issue.tracker.name, @@ -117,6 +117,7 @@ class Mailer < ActionMailer::Base @journal = journal @journal_details = journal.visible_details @issue_url = url_for(:controller => 'issues', :action => 'show', :id => issue, :anchor => "change-#{journal.id}") + @att = att mail :to => user, :subject => s @@ -132,7 +133,7 @@ class Mailer < ActionMailer::Base journal.notes? || journal.visible_details(user).any? end users.each do |user| - issue_edit(user, journal).deliver_later + issue_edit(user, journal, journal.issue.attachments_visible?(user)).deliver_later end end diff --git a/app/views/issues/_edit.html.erb b/app/views/issues/_edit.html.erb index 3afaee4ca..2b552465a 100644 --- a/app/views/issues/_edit.html.erb +++ b/app/views/issues/_edit.html.erb @@ -44,11 +44,12 @@ <%= call_hook(:view_issues_edit_notes_bottom, { :issue => @issue, :notes => @notes, :form => f }) %> + <% if @issue.attachments_addable?(User.current) %>
<%= l(:label_attachment_plural) %> - <% if @issue.attachments.any? && @issue.safe_attribute?('deleted_attachment_ids') %> + <% if @attachments.any? && @issue.safe_attribute?('deleted_attachment_ids') %>
<%= link_to l(:label_edit_attachments), '#', :onclick => "$('#existing-attachments').toggle(); return false;" %>
- <% @issue.attachments.each do |attachment| %> + <% @attachments.each do |attachment| %> <%= text_field_tag '', attachment.filename, :class => "icon icon-attachment filename", :disabled => true %>
+ <% end %> <% end %> diff --git a/app/views/issues/index.api.rsb b/app/views/issues/index.api.rsb index 4bba32549..c4970804c 100644 --- a/app/views/issues/index.api.rsb +++ b/app/views/issues/index.api.rsb @@ -29,7 +29,7 @@ api.array :issues, api_meta(:total_count => @issue_count, :offset => @offset, :l api.array :attachments do issue.attachments.each do |attachment| render_api_attachment(attachment, api) - end + end if issue.attachments_visible? end if include_in_api_response?('attachments') api.array :relations do diff --git a/app/views/issues/new.html.erb b/app/views/issues/new.html.erb index 22a174a11..2b6700b14 100644 --- a/app/views/issues/new.html.erb +++ b/app/views/issues/new.html.erb @@ -18,7 +18,7 @@

<% end %> <% if @copy_from && @copy_from.attachments.any? %> -

+

"> <%= check_box_tag 'copy_attachments', '1', @copy_attachments %>

@@ -30,7 +30,7 @@

<% end %> -

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

+

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

<%= render :partial => 'issues/watchers_form' %> diff --git a/app/views/issues/new.js.erb b/app/views/issues/new.js.erb index 702d922ab..c38135726 100644 --- a/app/views/issues/new.js.erb +++ b/app/views/issues/new.js.erb @@ -5,3 +5,14 @@ replaceIssueFormWith('<%= escape_javascript(render :partial => 'form') %>'); <% when "issue_category_id" %> $('#issue_assigned_to_id').find('option').first().html('<%= escape_javascript(@issue.category.try(:assigned_to).try(:name)).presence || ' '.html_safe %>'); <% end %> +<% if @issue.attachments_addable?(User.current) %> + <% if @copy_from.attachments_visible?(User.current) %> + $('#copy_attachments').parent().show(); + <% else %> + $('#copy_attachments').parent().hide(); + <% end %> + $('#attachments_form').show(); +<% else %> + $('#copy_attachments').parent().hide(); + $('#attachments_form').hide(); +<% end %> diff --git a/app/views/issues/show.api.rsb b/app/views/issues/show.api.rsb index f474ed9c6..dc056bbe6 100644 --- a/app/views/issues/show.api.rsb +++ b/app/views/issues/show.api.rsb @@ -32,7 +32,7 @@ api.issue do render_api_issue_children(@issue, api) if include_in_api_response?('children') api.array :attachments do - @issue.attachments.each do |attachment| + @attachments.each do |attachment| render_api_attachment(attachment, api) end end if include_in_api_response?('attachments') diff --git a/app/views/issues/show.html.erb b/app/views/issues/show.html.erb index a11a24df6..d8b7fe5fd 100644 --- a/app/views/issues/show.html.erb +++ b/app/views/issues/show.html.erb @@ -84,11 +84,11 @@ end %>

<%=l(:field_description)%>

- <%= textilizable @issue, :description, :attachments => @issue.attachments %> + <%= textilizable @issue, :description, :attachments => @attachments %>
<% end %> -<% if @issue.attachments.any? %> +<% if @attachments.any? %>

<%=l(:label_attachment_plural)%>

<%= link_to_attachments @issue, :thumbnails => true %> diff --git a/app/views/mailer/_issue.html.erb b/app/views/mailer/_issue.html.erb index 58287c658..7a5dd515a 100644 --- a/app/views/mailer/_issue.html.erb +++ b/app/views/mailer/_issue.html.erb @@ -4,7 +4,7 @@ <%= textilizable(issue, :description, :only_path => false) %> -<% if issue.attachments.any? %> +<% if issue.attachments.any? && @att %>
<%= l(:label_attachment_plural) %> <% issue.attachments.each do |attachment| %> <%= link_to_attachment attachment, :download => true, :only_path => false %> diff --git a/app/views/mailer/_issue.text.erb b/app/views/mailer/_issue.text.erb index 819aebad6..6899e0589 100644 --- a/app/views/mailer/_issue.text.erb +++ b/app/views/mailer/_issue.text.erb @@ -5,7 +5,7 @@ ---------------------------------------- <%= issue.description %> -<% if issue.attachments.any? -%> +<% if issue.attachments.any? && @att -%> ---<%= l(:label_attachment_plural).ljust(37, '-') %> <% issue.attachments.each do |attachment| -%> <%= attachment.filename %> (<%= number_to_human_size(attachment.filesize) %>) diff --git a/app/views/roles/_form.html.erb b/app/views/roles/_form.html.erb index 4a49d53de..e9f1442da 100644 --- a/app/views/roles/_form.html.erb +++ b/app/views/roles/_form.html.erb @@ -68,7 +68,7 @@

<%= l(:label_issue_tracking) %>

-<% permissions = [:view_issues, :add_issues, :edit_issues, :add_issue_notes, :delete_issues] & setable_permissions.collect(&:name) %> +<% permissions = [:view_issues, :add_issues, :edit_issues, :add_issue_notes, :delete_issues, :view_attachments, :add_attachments, :edit_attachments, :delete_attachments] & setable_permissions.collect(&:name) %>
diff --git a/config/locales/en.yml b/config/locales/en.yml index 6b434ff03..ef5eb9072 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -509,6 +509,10 @@ en: permission_view_private_notes: View private notes permission_set_notes_private: Set notes as private permission_delete_issues: Delete issues + permission_view_attachments: View attachments + permission_add_attachments: Add attachments + permission_edit_attachments: Edit attachments + permission_delete_attachments: Delete attachments permission_manage_public_queries: Manage public queries permission_save_queries: Save queries permission_view_gantt: View gantt chart diff --git a/config/locales/pt-BR.yml b/config/locales/pt-BR.yml index b99df40c5..7e086de8e 100644 --- a/config/locales/pt-BR.yml +++ b/config/locales/pt-BR.yml @@ -785,6 +785,10 @@ pt-BR: permission_manage_members: Gerenciar membros permission_edit_messages: Editar mensagens permission_delete_issues: Excluir tarefas + permission_view_attachments: Ver arquivos anexos + permission_add_attachments: Adicionar arquivos anexos + permission_edit_attachments: Editar arquivos anexos + permission_delete_attachments: Apagar arquivos anexos permission_view_issue_watchers: Ver lista de observadores permission_manage_repository: Gerenciar repositório permission_commit_access: Acesso do commit diff --git a/db/migrate/20161215142110_add_attachments_permissions.rb b/db/migrate/20161215142110_add_attachments_permissions.rb new file mode 100644 index 000000000..b67ec2ebb --- /dev/null +++ b/db/migrate/20161215142110_add_attachments_permissions.rb @@ -0,0 +1,19 @@ +class AddAttachmentsPermissions < ActiveRecord::Migration[4.2] + def self.up + Role.all.each do |r| + r.add_permission!(:view_attachments) if r.has_permission?(:view_issues) + r.add_permission!(:add_attachments) if r.has_permission?(:add_issues) + r.add_permission!(:edit_attachments) if r.has_permission?(:edit_issues) + r.add_permission!(:delete_attachments) if r.has_permission?(:delete_issues) + end + end + + def self.down + Role.all.each do |r| + r.remove_permission!(:view_attachments) + r.remove_permission!(:add_attachments) + r.remove_permission!(:edit_attachments) + r.remove_permission!(:delete_attachments) + end + end +end diff --git a/lib/plugins/acts_as_searchable/lib/acts_as_searchable.rb b/lib/plugins/acts_as_searchable/lib/acts_as_searchable.rb index e6b6b22fc..0770e824a 100644 --- a/lib/plugins/acts_as_searchable/lib/acts_as_searchable.rb +++ b/lib/plugins/acts_as_searchable/lib/acts_as_searchable.rb @@ -136,6 +136,7 @@ module Redmine r |= fetch_ranks_and_ids( search_scope(user, projects, options). joins(:attachments). + where("#{Project.allowed_to_condition(user, :view_attachments)}", false). where(search_tokens_condition(["#{Attachment.table_name}.filename", "#{Attachment.table_name}.description"], tokens, options[:all_words])), options[:limit] ) diff --git a/lib/redmine.rb b/lib/redmine.rb index f03b11db3..391fb6d88 100644 --- a/lib/redmine.rb +++ b/lib/redmine.rb @@ -102,20 +102,25 @@ Redmine::AccessControl.map do |map| :queries => :index, :reports => [:issue_report, :issue_report_details]}, :read => true - map.permission :add_issues, {:issues => [:new, :create], :attachments => :upload} - map.permission :edit_issues, {:issues => [:edit, :update, :bulk_edit, :bulk_update], :journals => [:new], :attachments => :upload} - map.permission :edit_own_issues, {:issues => [:edit, :update, :bulk_edit, :bulk_update], :journals => [:new], :attachments => :upload} - map.permission :copy_issues, {:issues => [:new, :create, :bulk_edit, :bulk_update], :attachments => :upload} + map.permission :add_issues, {:issues => [:new, :create]} + map.permission :edit_issues, {:issues => [:edit, :update, :bulk_edit, :bulk_update], :journals => [:new]} + map.permission :edit_own_issues, {:issues => [:edit, :update, :bulk_edit, :bulk_update], :journals => [:new]} + map.permission :copy_issues, {:issues => [:new, :create, :bulk_edit, :bulk_update]} map.permission :manage_issue_relations, {:issue_relations => [:index, :show, :create, :destroy]} map.permission :manage_subtasks, {} map.permission :set_issues_private, {} map.permission :set_own_issues_private, {}, :require => :loggedin - map.permission :add_issue_notes, {:issues => [:edit, :update], :journals => [:new], :attachments => :upload} + map.permission :add_issue_notes, {:issues => [:edit, :update], :journals => [:new]} map.permission :edit_issue_notes, {:journals => [:edit, :update]}, :require => :loggedin map.permission :edit_own_issue_notes, {:journals => [:edit, :update]}, :require => :loggedin map.permission :view_private_notes, {}, :read => true, :require => :member map.permission :set_notes_private, {}, :require => :member map.permission :delete_issues, {:issues => :destroy}, :require => :member + # Attachments + map.permission :add_attachments, {:attachments => :upload} + map.permission :view_attachments, {} + map.permission :edit_attachments, {} + map.permission :delete_attachments, {:attachments => :destroy}, :require => :member # Watchers map.permission :view_issue_watchers, {}, :read => true map.permission :add_issue_watchers, {:watchers => [:new, :create, :append, :autocomplete_for_user]} diff --git a/lib/redmine/export/pdf/issues_pdf_helper.rb b/lib/redmine/export/pdf/issues_pdf_helper.rb index 7e2c8a85f..c32eba48a 100644 --- a/lib/redmine/export/pdf/issues_pdf_helper.rb +++ b/lib/redmine/export/pdf/issues_pdf_helper.rb @@ -236,7 +236,7 @@ module Redmine end end - if issue.attachments.any? + if issue.attachments.any? && issue.attachments_visible?(User.current) pdf.SetFontStyle('B',9) pdf.RDMCell(190,5, l(:label_attachment_plural), "B") pdf.ln -- 2.17.1