From 1f9bbd6b42b377c9ab3906293c2d166b4e2fb138 Mon Sep 17 00:00:00 2001 From: Jean-Philippe Lang Date: Sun, 10 Jul 2016 10:58:00 +0000 Subject: [PATCH] Wrap journal attributes with a journal parameter and use safe_attributes (#22575). git-svn-id: http://svn.redmine.org/redmine/trunk@15621 e93f8b46-1217-0410-a6f0-8f06a7374b81 --- app/controllers/journals_controller.rb | 6 ++---- app/helpers/journals_helper.rb | 2 +- app/models/journal.rb | 7 +++++++ app/views/journals/_notes_form.html.erb | 8 +++++--- test/functional/journals_controller_test.rb | 15 ++++++++------- 5 files changed, 23 insertions(+), 15 deletions(-) diff --git a/app/controllers/journals_controller.rb b/app/controllers/journals_controller.rb index 6780916b7..00556c8e4 100644 --- a/app/controllers/journals_controller.rb +++ b/app/controllers/journals_controller.rb @@ -90,10 +90,8 @@ class JournalsController < ApplicationController def update (render_403; return false) unless @journal.editable_by?(User.current) - @journal.notes = params[:notes] if params[:notes] - @journal.private_notes = params[:private_notes].present? - (render_403; return false) if @journal.private_notes_changed? && User.current.allowed_to?(:set_notes_private, @journal.issue.project) == false - @journal.save if @journal.changed? + @journal.safe_attributes = params[:journal] + @journal.save @journal.destroy if @journal.details.empty? && @journal.notes.blank? call_hook(:controller_journals_edit_post, { :journal => @journal, :params => params}) respond_to do |format| diff --git a/app/helpers/journals_helper.rb b/app/helpers/journals_helper.rb index e31ad2544..0cf863648 100644 --- a/app/helpers/journals_helper.rb +++ b/app/helpers/journals_helper.rb @@ -45,7 +45,7 @@ module JournalsHelper :class => 'icon-only icon-edit' ) if editable links << link_to(l(:button_delete), - journal_path(journal, :notes => ""), + journal_path(journal, :journal => {:notes => ""}), :remote => true, :method => 'put', :data => {:confirm => l(:text_are_you_sure)}, :title => l(:button_delete), diff --git a/app/models/journal.rb b/app/models/journal.rb index 927f86f65..d65034c56 100644 --- a/app/models/journal.rb +++ b/app/models/journal.rb @@ -16,6 +16,8 @@ # Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. class Journal < ActiveRecord::Base + include Redmine::SafeAttributes + belongs_to :journalized, :polymorphic => true # added as a quick fix to allow eager loading of the polymorphic association # since always associated to an issue, for now @@ -50,6 +52,11 @@ class Journal < ActiveRecord::Base where("(#{Journal.table_name}.private_notes = ? OR (#{Project.allowed_to_condition(user, :view_private_notes, *args)}))", false) } + safe_attributes 'notes', + :if => lambda {|journal, user| journal.new_record? || journal.editable_by?(user)} + safe_attributes 'private_notes', + :if => lambda {|journal, user| user.allowed_to?(:set_notes_private, journal.project)} + def initialize(*args) super if journalized diff --git a/app/views/journals/_notes_form.html.erb b/app/views/journals/_notes_form.html.erb index e8e73bb18..3bd6a31fd 100644 --- a/app/views/journals/_notes_form.html.erb +++ b/app/views/journals/_notes_form.html.erb @@ -3,12 +3,14 @@ :method => 'put', :id => "journal-#{@journal.id}-form") do %> <%= label_tag "notes", l(:description_notes), :class => "hidden-for-sighted" %> - <%= text_area_tag :notes, @journal.notes, + <%= text_area_tag 'journal[notes]', @journal.notes, :id => "journal_#{@journal.id}_notes", :class => 'wiki-edit', :rows => (@journal.notes.blank? ? 10 : [[10, @journal.notes.length / 50].max, 100].min) %> - <% if @journal.issue.safe_attribute? 'private_notes' %> - <%= check_box_tag 'private_notes', '1', @journal.private_notes, :id => "journal_#{@journal.id}_private_notes" %> + <% if @journal.safe_attribute? 'private_notes' %> + <%= hidden_field_tag 'journal[private_notes]', '0' %> + <%= check_box_tag 'journal[private_notes]', '1', @journal.private_notes, :id => "journal_#{@journal.id}_private_notes" %> + <% end %> <%= call_hook(:view_journals_notes_form_after_notes, { :journal => @journal}) %>

<%= submit_tag l(:button_save) %> diff --git a/test/functional/journals_controller_test.rb b/test/functional/journals_controller_test.rb index 304f205f7..424a4aa8b 100644 --- a/test/functional/journals_controller_test.rb +++ b/test/functional/journals_controller_test.rb @@ -199,7 +199,7 @@ class JournalsControllerTest < ActionController::TestCase def test_update_xhr @request.session[:user_id] = 1 - xhr :post, :update, :id => 2, :notes => 'Updated notes' + xhr :post, :update, :id => 2, :journal => {:notes => 'Updated notes'} assert_response :success assert_template 'update' assert_equal 'text/javascript', response.content_type @@ -209,7 +209,7 @@ class JournalsControllerTest < ActionController::TestCase def test_update_xhr_with_private_notes_checked @request.session[:user_id] = 1 - xhr :post, :update, :id => 2, :private_notes => '1' + xhr :post, :update, :id => 2, :journal => {:private_notes => '1'} assert_response :success assert_template 'update' assert_equal 'text/javascript', response.content_type @@ -221,7 +221,7 @@ class JournalsControllerTest < ActionController::TestCase def test_update_xhr_with_private_notes_unchecked Journal.find(2).update_attributes(:private_notes => true) @request.session[:user_id] = 1 - xhr :post, :update, :id => 2 + xhr :post, :update, :id => 2, :journal => {:private_notes => '0'} assert_response :success assert_template 'update' assert_equal 'text/javascript', response.content_type @@ -230,20 +230,21 @@ class JournalsControllerTest < ActionController::TestCase assert_include 'journal-2-private_notes', response.body end - def test_update_xhr_with_private_notes_changes_and_without_set_private_notes_permission + def test_update_xhr_without_set_private_notes_permission_should_ignore_private_notes @request.session[:user_id] = 2 Role.find(1).add_permission! :edit_issue_notes Role.find(1).add_permission! :view_private_notes Role.find(1).remove_permission! :set_notes_private - xhr :post, :update, :id => 2, :private_notes => '1' - assert_response 403 + xhr :post, :update, :id => 2, :journal => {:private_notes => '1'} + assert_response :success + assert_equal false, Journal.find(2).private_notes end def test_update_xhr_with_empty_notes_should_delete_the_journal @request.session[:user_id] = 1 assert_difference 'Journal.count', -1 do - xhr :post, :update, :id => 2, :notes => '' + xhr :post, :update, :id => 2, :journal => {:notes => ''} assert_response :success assert_template 'update' assert_equal 'text/javascript', response.content_type -- 2.39.5