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

@@ -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(
+  safe_attributes 'tracker_id',
-    tracker_id
+    'status_id',
-    status_id
+    'parent_issue_id',
-    parent_issue_id
+    'category_id',
-    category_id
+    'assigned_to_id',
-    assigned_to_id
+    'priority_id',
-    priority_id
+    'fixed_version_id',
-    fixed_version_id
+    'subject',
-    subject
+    'description',
-    description
+    'start_date',
-    start_date
+    'due_date',
-    due_date
+    'done_ratio',
-    done_ratio
+    'estimated_hours',
-    estimated_hours
+    'custom_field_values',
-    custom_field_values
+    'custom_fields',
-    custom_fields
+    'lock_version',
-    lock_version
+    :if => lambda {|issue, user| issue.new_record? || user.allowed_to?(:edit_issues, issue.project) }
-  ) unless const_defined?(:SAFE_ATTRIBUTES)
   
-  SAFE_ATTRIBUTES_ON_TRANSITION = %w(
+  safe_attributes 'status_id',
-    status_id
+    'assigned_to_id',
-    assigned_to_id
+    'fixed_version_id',
-    fixed_version_id
+    'done_ratio',
-    done_ratio
+    :if => lambda {|issue, user| issue.new_statuses_allowed_to(user).any? }
-  ) unless const_defined?(:SAFE_ATTRIBUTES_ON_TRANSITION)
 
   # 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 = delete_unsafe_attributes(attrs, user)
-      attrs = attrs.reject {|k,v| !SAFE_ATTRIBUTES.include?(k)}
+    return if attrs.empty? 
-    elsif new_statuses_allowed_to(user).any?
-      attrs = attrs.reject {|k,v| !SAFE_ATTRIBUTES_ON_TRANSITION.include?(k)}
-    else
-      return
-    end
     
     # Tracker must be set before since new_statuses_allowed_to depends on it.
     if t = attrs.delete('tracker_id')

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

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