From 3409333522a76ade39db41124df596b2b95eccc0 Mon Sep 17 00:00:00 2001 From: Jean-Philippe Lang Date: Sun, 12 Dec 2010 13:11:53 +0000 Subject: [PATCH] Makes issue safe_attributes extensible (#6000). git-svn-id: svn+ssh://rubyforge.org/var/svn/redmine/trunk@4491 e93f8b46-1217-0410-a6f0-8f06a7374b81 --- app/models/issue.rb | 57 ++++++------ lib/redmine/safe_attributes.rb | 75 ++++++++++++++++ test/unit/lib/redmine/safe_attributes_test.rb | 87 +++++++++++++++++++ 3 files changed, 188 insertions(+), 31 deletions(-) create mode 100644 lib/redmine/safe_attributes.rb create mode 100644 test/unit/lib/redmine/safe_attributes_test.rb diff --git a/app/models/issue.rb b/app/models/issue.rb index 1cd1b92f..abea0b83 100644 --- a/app/models/issue.rb +++ b/app/models/issue.rb @@ -16,6 +16,8 @@ # Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. class Issue < ActiveRecord::Base + include Redmine::SafeAttributes + belongs_to :project belongs_to :tracker belongs_to :status, :class_name => 'IssueStatus', :foreign_key => 'status_id' @@ -214,31 +216,29 @@ class Issue < ActiveRecord::Base write_attribute :estimated_hours, (h.is_a?(String) ? h.to_hours : h) end - SAFE_ATTRIBUTES = %w( - tracker_id - status_id - parent_issue_id - category_id - assigned_to_id - priority_id - fixed_version_id - subject - description - start_date - due_date - done_ratio - estimated_hours - custom_field_values - custom_fields - lock_version - ) unless const_defined?(:SAFE_ATTRIBUTES) + safe_attributes 'tracker_id', + 'status_id', + 'parent_issue_id', + 'category_id', + 'assigned_to_id', + 'priority_id', + 'fixed_version_id', + 'subject', + 'description', + 'start_date', + 'due_date', + 'done_ratio', + 'estimated_hours', + 'custom_field_values', + 'custom_fields', + 'lock_version', + :if => lambda {|issue, user| issue.new_record? || user.allowed_to?(:edit_issues, issue.project) } - SAFE_ATTRIBUTES_ON_TRANSITION = %w( - status_id - assigned_to_id - fixed_version_id - done_ratio - ) unless const_defined?(:SAFE_ATTRIBUTES_ON_TRANSITION) + safe_attributes 'status_id', + 'assigned_to_id', + 'fixed_version_id', + 'done_ratio', + :if => lambda {|issue, user| issue.new_statuses_allowed_to(user).any? } # Safely sets attributes # Should be called from controllers instead of #attributes= @@ -249,13 +249,8 @@ class Issue < ActiveRecord::Base return unless attrs.is_a?(Hash) # User can change issue attributes only if he has :edit permission or if a workflow transition is allowed - if new_record? || user.allowed_to?(:edit_issues, project) - attrs = attrs.reject {|k,v| !SAFE_ATTRIBUTES.include?(k)} - elsif new_statuses_allowed_to(user).any? - attrs = attrs.reject {|k,v| !SAFE_ATTRIBUTES_ON_TRANSITION.include?(k)} - else - return - end + attrs = delete_unsafe_attributes(attrs, user) + return if attrs.empty? # Tracker must be set before since new_statuses_allowed_to depends on it. if t = attrs.delete('tracker_id') diff --git a/lib/redmine/safe_attributes.rb b/lib/redmine/safe_attributes.rb new file mode 100644 index 00000000..6f87a233 --- /dev/null +++ b/lib/redmine/safe_attributes.rb @@ -0,0 +1,75 @@ +# Redmine - project management software +# Copyright (C) 2006-2010 Jean-Philippe Lang +# +# This program is free software; you can redistribute it and/or +# modify it under the terms of the GNU General Public License +# as published by the Free Software Foundation; either version 2 +# of the License, or (at your option) any later version. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with this program; if not, write to the Free Software +# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. + +module Redmine + module SafeAttributes + def self.included(base) + base.extend(ClassMethods) + end + + module ClassMethods + # Declares safe attributes + # An optional Proc can be given for conditional inclusion + # + # Example: + # safe_attributes 'title', 'pages' + # safe_attributes 'isbn', :if => {|book, user| book.author == user} + def safe_attributes(*args) + @safe_attributes ||= [] + if args.empty? + @safe_attributes + else + options = args.last.is_a?(Hash) ? args.pop : {} + @safe_attributes << [args, options] + end + end + end + + # Returns an array that can be safely set by user or current user + # + # Example: + # book.safe_attributes # => ['title', 'pages'] + # book.safe_attributes(book.author) # => ['title', 'pages', 'isbn'] + def safe_attribute_names(user=User.current) + names = [] + self.class.safe_attributes.collect do |attrs, options| + if options[:if].nil? || options[:if].call(self, user) + names += attrs.collect(&:to_s) + end + end + names.uniq + end + + # Returns a hash with unsafe attributes removed + # from the given attrs hash + # + # Example: + # book.delete_unsafe_attributes({'title' => 'My book', 'foo' => 'bar'}) + # # => {'title' => 'My book'} + def delete_unsafe_attributes(attrs, user=User.current) + safe = safe_attribute_names(user) + attrs.dup.delete_if {|k,v| !safe.include?(k)} + end + + # Sets attributes from attrs that are safe + # attrs is a Hash with string keys + def safe_attributes=(attrs, user=User.current) + return unless attrs.is_a?(Hash) + self.attributes = delete_unsafe_attributes(attrs, user) + end + end +end diff --git a/test/unit/lib/redmine/safe_attributes_test.rb b/test/unit/lib/redmine/safe_attributes_test.rb new file mode 100644 index 00000000..0821e795 --- /dev/null +++ b/test/unit/lib/redmine/safe_attributes_test.rb @@ -0,0 +1,87 @@ +# Redmine - project management software +# Copyright (C) 2006-2010 Jean-Philippe Lang +# +# This program is free software; you can redistribute it and/or +# modify it under the terms of the GNU General Public License +# as published by the Free Software Foundation; either version 2 +# of the License, or (at your option) any later version. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with this program; if not, write to the Free Software +# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. + +require File.dirname(__FILE__) + '/../../../test_helper' + +class Redmine::SafeAttributesTest < ActiveSupport::TestCase + + class Base + def attributes=(attrs) + attrs.each do |key, value| + send("#{key}=", value) + end + end + end + + class Person < Base + attr_accessor :firstname, :lastname, :login + include Redmine::SafeAttributes + safe_attributes :firstname, :lastname + safe_attributes :login, :if => lambda {|person, user| user.admin?} + end + + class Book < Base + attr_accessor :title + include Redmine::SafeAttributes + safe_attributes :title + end + + def test_safe_attribute_names + p = Person.new + assert_equal ['firstname', 'lastname'], p.safe_attribute_names(User.anonymous) + assert_equal ['firstname', 'lastname', 'login'], p.safe_attribute_names(User.find(1)) + end + + def test_safe_attribute_names_without_user + p = Person.new + User.current = nil + assert_equal ['firstname', 'lastname'], p.safe_attribute_names + User.current = User.find(1) + assert_equal ['firstname', 'lastname', 'login'], p.safe_attribute_names + end + + def test_set_safe_attributes + p = Person.new + p.send('safe_attributes=', {'firstname' => 'John', 'lastname' => 'Smith', 'login' => 'jsmith'}, User.anonymous) + assert_equal 'John', p.firstname + assert_equal 'Smith', p.lastname + assert_nil p.login + + p = Person.new + User.current = User.find(1) + p.send('safe_attributes=', {'firstname' => 'John', 'lastname' => 'Smith', 'login' => 'jsmith'}, User.find(1)) + assert_equal 'John', p.firstname + assert_equal 'Smith', p.lastname + assert_equal 'jsmith', p.login + end + + def test_set_safe_attributes_without_user + p = Person.new + User.current = nil + p.safe_attributes = {'firstname' => 'John', 'lastname' => 'Smith', 'login' => 'jsmith'} + assert_equal 'John', p.firstname + assert_equal 'Smith', p.lastname + assert_nil p.login + + p = Person.new + User.current = User.find(1) + p.safe_attributes = {'firstname' => 'John', 'lastname' => 'Smith', 'login' => 'jsmith'} + assert_equal 'John', p.firstname + assert_equal 'Smith', p.lastname + assert_equal 'jsmith', p.login + end +end