From 74513278170d262aca7e1fb3cea129437b6c3c97 Mon Sep 17 00:00:00 2001
From: Frederico Camara <frederico.camara@serpro.gov.br>
Date: Sat, 7 Dec 2024 17:44:24 -0300
Subject: [PATCH] 
 Implements-permissions-and-restrictions-to-issue-att-redmine6.0-stable

---
 app/controllers/issues_controller.rb          | 10 ++++++++--
 app/models/issue.rb                           | 20 ++++++++++++++-----
 app/models/journal.rb                         |  2 ++
 app/models/mailer.rb                          |  5 +++--
 app/views/issues/_edit.html.erb               |  8 +++-----
 app/views/issues/edit.js.erb                  |  8 +++++++-
 app/views/issues/index.api.rsb                | 12 ++++++-----
 app/views/issues/new.html.erb                 |  5 +++--
 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 ++++
 ...41207140210_add_attachments_permissions.rb | 19 ++++++++++++++++++
 .../lib/acts_as_searchable.rb                 |  1 +
 lib/redmine/export/pdf/issues_pdf_helper.rb   |  2 +-
 lib/redmine/preparation.rb                    | 15 +++++++++-----
 20 files changed, 104 insertions(+), 34 deletions(-)
 create mode 100644 db/migrate/20241207140210_add_attachments_permissions.rb

diff --git a/app/controllers/issues_controller.rb b/app/controllers/issues_controller.rb
index c5af8658f..b30f9c3f6 100644
--- a/app/controllers/issues_controller.rb
+++ b/app/controllers/issues_controller.rb
@@ -100,6 +100,7 @@ class IssuesController < ApplicationController
     if !api_request? || include_in_api_response?('relations')
       @relations = @issue.relations.select {|r| r.other_issue(@issue)&.visible?}
     end
+    @attachments = @issue.attachments_visible? ? @issue.attachments : []
     if !api_request? || include_in_api_response?('allowed_statuses')
       @allowed_statuses = @issue.new_statuses_allowed_to(User.current)
     end
@@ -149,7 +150,9 @@ class IssuesController < ApplicationController
     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?
+      @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|
@@ -184,6 +187,7 @@ class IssuesController < ApplicationController
   def edit
     return unless update_issue_from_params
 
+    @attachments = @issue.attachments_visible?(User.current) ? @issue.attachments : []
     respond_to do |format|
       format.html {}
       format.js
@@ -328,7 +332,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?}.present? &&
                                (Setting.copy_attachments_on_issue_copy == 'ask')
       @subtasks_present = @issues.detect {|i| !i.leaf?}.present?
       @watchers_present = User.current.allowed_to?(:add_issue_watchers, @projects) &&
@@ -396,6 +400,7 @@ class IssuesController < ApplicationController
       end
       journal = issue.init_journal(User.current, params[:notes])
       issue.safe_attributes = attributes
+      issue.attachments = [] unless issue.attachments_addable? if @copy
       call_hook(:controller_issues_bulk_edit_before_save, {:params => params, :issue => issue})
       if issue.save
         saved_issues << issue
@@ -647,6 +652,7 @@ class IssuesController < ApplicationController
 
     @priorities = IssuePriority.active
     @allowed_statuses = @issue.new_statuses_allowed_to(User.current)
+    @issue.attachments = [] unless @issue.attachments_addable?
   end
 
   # Saves @issue and a time_entry from the parameters
diff --git a/app/models/issue.rb b/app/models/issue.rb
index 980bd56f0..49e56c85f 100644
--- a/app/models/issue.rb
+++ b/app/models/issue.rb
@@ -42,7 +42,10 @@ class Issue < ApplicationRecord
   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"],
@@ -200,13 +203,19 @@ class Issue < ApplicationRecord
     )
   end
 
+  # Overrides Redmine::Acts::Attachable::InstanceMethods#attachments_visible?
+  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)
-    attributes_editable?(user) || notes_addable?(user)
+    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
 
   # Returns true if user or current user is allowed to add notes to the issue
@@ -221,7 +230,7 @@ class Issue < ApplicationRecord
 
   # Overrides Redmine::Acts::Attachable::InstanceMethods#attachments_deletable?
   def attachments_deletable?(user=User.current)
-    attributes_editable?(user)
+    user_tracker_permission?(user, :delete_attachments)
   end
 
   def initialize(attributes=nil, *args)
@@ -309,7 +318,7 @@ class Issue < ApplicationRecord
       self.status = issue.status
     end
     self.author = User.current
-    unless options[:attachments] == false
+    if options[:attachments] == true && issue.attachments_visible?
       self.attachments = issue.attachments.map do |attachement|
         attachement.copy(:container => self)
       end
@@ -1801,6 +1810,7 @@ class Issue < ApplicationRecord
                  child.assigned_to.status == User::STATUS_ACTIVE
           copy.assigned_to = nil
         end
+        copy.attachments = [] unless copy.attachments_addable?
         unless copy.save
           if logger
             logger.error(
diff --git a/app/models/journal.rb b/app/models/journal.rb
index 446eac7a6..6c2a718e5 100644
--- a/app/models/journal.rb
+++ b/app/models/journal.rb
@@ -111,6 +111,8 @@ class Journal < ApplicationRecord
         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?
       else
         true
       end
diff --git a/app/models/mailer.rb b/app/models/mailer.rb
index b64e94991..8f5010a6c 100644
--- a/app/models/mailer.rb
+++ b/app/models/mailer.rb
@@ -110,7 +110,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,
@@ -129,6 +129,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
@@ -144,7 +145,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 f00b65750..8c4400b62 100644
--- a/app/views/issues/_edit.html.erb
+++ b/app/views/issues/_edit.html.erb
@@ -47,12 +47,11 @@
       <%= update_data_sources_for_auto_complete({users: watchers_autocomplete_for_mention_path(project_id: @issue.project, q: '', object_type: 'issue',
                                                                                                object_id: @issue.id)}) %>
     <% end %>
-    <% if @issue.attachments_addable? %>
-      <fieldset id="add_attachments"><legend><%= l(:label_attachment_plural) %></legend>
-        <% if @issue.attachments.any? && @issue.safe_attribute?('deleted_attachment_ids') %>
+      <fieldset id="add_attachments" style="<%= "display: none;" unless @issue.attachments_addable?(User.current) %>"><legend><%= l(:label_attachment_plural) %></legend>
+        <% if @attachments.any? && @issue.safe_attribute?('deleted_attachment_ids') %>
         <div class="contextual"><%= link_to l(:label_edit_attachments), '#', :onclick => "$('#existing-attachments').toggle(); return false;" %></div>
         <div id="existing-attachments" style="<%= @issue.deleted_attachment_ids.blank? ? 'display:none;' : '' %>">
-          <% @issue.attachments.each do |attachment| %>
+          <% @attachments.each do |attachment| %>
           <span class="existing-attachment">
             <%= sprite_icon('attachment', size: 12) %>
             <%= text_field_tag '', attachment.filename, :class => "icon icon-attachment filename", :disabled => true %>
@@ -72,7 +71,6 @@
           <%= render :partial => 'attachments/form', :locals => {:container => @issue} %>
         </div>
       </fieldset>
-    <% end %>
     </div>
 
     <%= f.hidden_field :lock_version %>
diff --git a/app/views/issues/edit.js.erb b/app/views/issues/edit.js.erb
index e95336824..2fa284475 100644
--- a/app/views/issues/edit.js.erb
+++ b/app/views/issues/edit.js.erb
@@ -7,8 +7,14 @@ replaceIssueFormWith('<%= escape_javascript(render :partial => 'form') %>');
 
 <% if @issue.notes_addable? %>
   $('#add_notes').show();
-  $('#add_attachments').show();
 <% else %>
   $('#add_notes').hide();
+<% end %>
+
+<% if @issue.attachments_addable? %>
+  $('#add_attachments').show();
+  $('#attachments_form').show();
+<% else %>
   $('#add_attachments').hide();
+  $('#attachments_form').hide();
 <% end %>
\ No newline at end of file
diff --git a/app/views/issues/index.api.rsb b/app/views/issues/index.api.rsb
index f1627cdc4..798c6293e 100644
--- a/app/views/issues/index.api.rsb
+++ b/app/views/issues/index.api.rsb
@@ -31,11 +31,13 @@ api.array :issues, api_meta(:total_count => @issue_count, :offset => @offset, :l
       api.updated_on issue.updated_on
       api.closed_on  issue.closed_on
 
-      api.array :attachments do
-        issue.attachments.each do |attachment|
-          render_api_attachment(attachment, api)
-        end
-      end if include_in_api_response?('attachments')
+      if issue.attachments_visible?
+        api.array :attachments do
+          issue.attachments.each do |attachment|
+            render_api_attachment(attachment, api)
+          end
+        end if include_in_api_response?('attachments')
+      end
 
       api.array :relations do
         issue.relations.each do |relation|
diff --git a/app/views/issues/new.html.erb b/app/views/issues/new.html.erb
index 40089215c..df953f5ae 100644
--- a/app/views/issues/new.html.erb
+++ b/app/views/issues/new.html.erb
@@ -17,7 +17,8 @@
       <%= check_box_tag 'link_copy', '1', @link_copy %>
     </p>
     <% end %>
-    <% if @copy_from && Setting.copy_attachments_on_issue_copy == 'ask' && @copy_from.attachments.any? %>
+    <% if @copy_from && Setting.copy_attachments_on_issue_copy == 'ask' && @copy_from.attachments.any? &&
+          @copy_from.attachments_visible? && @issue.attachments_addable? %>
     <p>
       <label for="copy_attachments"><%= l(:label_copy_attachments) %></label>
       <%= check_box_tag 'copy_attachments', '1', @copy_attachments %>
@@ -30,7 +31,7 @@
     </p>
     <% end %>
 
-    <p id="attachments_form"><label><%= l(:label_attachment_plural) %></label><%= render :partial => 'attachments/form', :locals => {:container => @issue} %></p>
+    <p id="attachments_form" style="<%= "display: none;" unless @issue.attachments_addable?(User.current) %>"><label><%= l(:label_attachment_plural) %></label><%= render :partial => 'attachments/form', :locals => {:container => @issue} %></p>
 
     <div id="watchers_form_container">
       <%= render :partial => 'issues/watchers_form' %>
diff --git a/app/views/issues/new.js.erb b/app/views/issues/new.js.erb
index a7e8cf88b..e589586be 100644
--- a/app/views/issues/new.js.erb
+++ b/app/views/issues/new.js.erb
@@ -8,3 +8,14 @@ replaceIssueFormWith('<%= escape_javascript(render :partial => 'form') %>');
     '<%= escape_javascript(
            @issue.category.try(:assigned_to).try(:name)).presence || '&nbsp;'.html_safe %>');
 <% end %>
+<% if @issue.attachments_addable? %>
+  <% if @copy_from && @copy_from.attachments_visible? %>
+    $('#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 2c341163a..97e992ba7 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 8f732032a..6203be320 100644
--- a/app/views/issues/show.html.erb
+++ b/app/views/issues/show.html.erb
@@ -93,11 +93,11 @@ end %>
 
   <p><strong><%=l(:field_description)%></strong></p>
   <div id="issue_description_wiki" class="wiki">
-  <%= textilizable @issue, :description, :attachments => @issue.attachments %>
+  <%= textilizable @issue, :description, :attachments => @attachments %>
   </div>
 </div>
 <% end %>
-<% if @issue.attachments.any? %>
+<% if @attachments.any? %>
   <hr />
   <p><strong><%=l(:label_attachment_plural)%></strong></p>
   <%= link_to_attachments @issue, :thumbnails => true %>
diff --git a/app/views/mailer/_issue.html.erb b/app/views/mailer/_issue.html.erb
index b46218445..d7b4ec426 100644
--- a/app/views/mailer/_issue.html.erb
+++ b/app/views/mailer/_issue.html.erb
@@ -7,7 +7,7 @@
 
 <%= textilizable(issue, :description, :only_path => false) %>
 
-<% if issue.attachments.any? %>
+<% if issue.attachments.any? && @att %>
   <fieldset class="attachments"><legend><%= l(:label_attachment_plural) %></legend>
   <% 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 44bc54c75..9a9d66e11 100644
--- a/app/views/roles/_form.html.erb
+++ b/app/views/roles/_form.html.erb
@@ -72,7 +72,7 @@
 
 <div id="role-permissions-trackers" class="view_issues_shown">
 <h3><%= l(:label_issue_tracking) %></h3>
-<% 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) %>
 
 <div class="autoscroll">
 <table class="list">
diff --git a/config/locales/en.yml b/config/locales/en.yml
index cea117d09..43741e0d5 100644
--- a/config/locales/en.yml
+++ b/config/locales/en.yml
@@ -548,6 +548,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 e35dfcbf3..b6efafd71 100644
--- a/config/locales/pt-BR.yml
+++ b/config/locales/pt-BR.yml
@@ -777,6 +777,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/20241207140210_add_attachments_permissions.rb b/db/migrate/20241207140210_add_attachments_permissions.rb
new file mode 100644
index 000000000..b67ec2ebb
--- /dev/null
+++ b/db/migrate/20241207140210_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 d393285b0..31813e41b 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/export/pdf/issues_pdf_helper.rb b/lib/redmine/export/pdf/issues_pdf_helper.rb
index b74531b38..4aef624d1 100644
--- a/lib/redmine/export/pdf/issues_pdf_helper.rb
+++ b/lib/redmine/export/pdf/issues_pdf_helper.rb
@@ -234,7 +234,7 @@ module Redmine
             end
           end
 
-          if issue.attachments.any?
+          if issue.attachments.any? && issue.attachments_visible?
             pdf.SetFontStyle('B', 9)
             pdf.RDMCell(190, 5, l(:label_attachment_plural), "B")
             pdf.ln
diff --git a/lib/redmine/preparation.rb b/lib/redmine/preparation.rb
index 1aeb81e46..ddc1e4dde 100644
--- a/lib/redmine/preparation.rb
+++ b/lib/redmine/preparation.rb
@@ -59,20 +59,25 @@ module Redmine
                                         :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, :autocomplete_for_mention]}
-- 
2.43.0

