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) %>
+ <% 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} %>
<% 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 %>