diff --git a/.gitignore b/.gitignore index b07b413..2318685 100644 --- a/.gitignore +++ b/.gitignore @@ -1,3 +1,4 @@ .DS_Store html/* pkg +*.swp diff --git a/lib/couchrest/mixins/attribute_protection.rb b/lib/couchrest/mixins/attribute_protection.rb new file mode 100644 index 0000000..4eb04a0 --- /dev/null +++ b/lib/couchrest/mixins/attribute_protection.rb @@ -0,0 +1,74 @@ +module CouchRest + module Mixins + module AttributeProtection + # Attribute protection from mass assignment to CouchRest properties + # + # Protected methods will be removed from + # * new + # * update_attributes + # * upate_attributes_without_saving + # * attributes= + # + # There are two modes of protection + # 1) Declare accessible poperties, assume all the rest are protected + # property :name, :accessible => true + # property :admin # this will be automatically protected + # + # 2) Declare protected properties, assume all the rest are accessible + # property :name # this will not be protected + # property :admin, :protected => true + # + # Note: you cannot set both flags in a single class + + def self.included(base) + base.extend(ClassMethods) + end + + module ClassMethods + def accessible_properties + properties.select { |prop| prop.options[:accessible] } + end + + def protected_properties + properties.select { |prop| prop.options[:protected] } + end + end + + def accessible_properties + self.class.accessible_properties + end + + def protected_properties + self.class.protected_properties + end + + def remove_protected_attributes(attributes) + protected_names = properties_to_remove_from_mass_assignment.map { |prop| prop.name } + return attributes if protected_names.empty? + + attributes.reject! do |key, value| + protected_names.include?(key.to_s) + end + + attributes || {} + end + + private + + def properties_to_remove_from_mass_assignment + has_protected = !protected_properties.empty? + has_accessible = !accessible_properties.empty? + + if !has_protected && !has_accessible + [] + elsif has_protected && !has_accessible + protected_properties + elsif has_accessible && !has_protected + properties.reject { |prop| prop.options[:accessible] } + else + raise "Set either :accessible or :protected for #{self.class}, but not both" + end + end + end + end +end diff --git a/lib/couchrest/mixins/extended_document_mixins.rb b/lib/couchrest/mixins/extended_document_mixins.rb index 89b25d6..89d650a 100644 --- a/lib/couchrest/mixins/extended_document_mixins.rb +++ b/lib/couchrest/mixins/extended_document_mixins.rb @@ -6,3 +6,4 @@ require File.join(File.dirname(__FILE__), 'validation') require File.join(File.dirname(__FILE__), 'extended_attachments') require File.join(File.dirname(__FILE__), 'class_proxy') require File.join(File.dirname(__FILE__), 'collection') +require File.join(File.dirname(__FILE__), 'attribute_protection') diff --git a/lib/couchrest/more/extended_document.rb b/lib/couchrest/more/extended_document.rb index f33481d..ddb0c0c 100644 --- a/lib/couchrest/more/extended_document.rb +++ b/lib/couchrest/more/extended_document.rb @@ -14,6 +14,7 @@ module CouchRest include CouchRest::Mixins::ExtendedAttachments include CouchRest::Mixins::ClassProxy include CouchRest::Mixins::Collection + include CouchRest::Mixins::AttributeProtection def self.subclasses @subclasses ||= [] @@ -40,11 +41,7 @@ module CouchRest def initialize(passed_keys={}) apply_defaults # defined in CouchRest::Mixins::Properties - passed_keys.each do |k,v| - if self.respond_to?("#{k}=") - self.send("#{k}=", passed_keys.delete(k)) - end - end if passed_keys + set_attributes(passed_keys) super cast_keys # defined in CouchRest::Mixins::Properties unless self['_id'] && self['_rev'] @@ -150,12 +147,8 @@ module CouchRest # make a copy, we don't want to change arguments attrs = hash.dup %w[_id _rev created_at updated_at].each {|attr| attrs.delete(attr)} - attrs.each do |k, v| - raise NoMethodError, "#{k}= method not available, use property :#{k}" unless self.respond_to?("#{k}=") - end - attrs.each do |k, v| - self.send("#{k}=",v) - end + check_properties_exist(attrs) + set_attributes(attrs) end alias :attributes= :update_attributes_without_saving @@ -280,6 +273,22 @@ module CouchRest end end end - + + private + + def check_properties_exist(attrs) + attrs.each do |k, v| + raise NoMethodError, "#{k}= method not available, use property :#{k}" unless self.respond_to?("#{k}=") + end + end + + def set_attributes(hash) + attrs = remove_protected_attributes(hash) + attrs.each do |k,v| + if self.respond_to?("#{k}=") + self.send("#{k}=", attrs.delete(k)) + end + end + end end end diff --git a/spec/couchrest/more/attribute_protection_spec.rb b/spec/couchrest/more/attribute_protection_spec.rb new file mode 100644 index 0000000..3063531 --- /dev/null +++ b/spec/couchrest/more/attribute_protection_spec.rb @@ -0,0 +1,94 @@ +require File.expand_path("../../../spec_helper", __FILE__) + +describe "ExtendedDocument", "no declarations" do + class NoProtection < CouchRest::ExtendedDocument + use_database TEST_SERVER.default_database + property :name + property :phone + end + + it "should not protect anything through new" do + user = NoProtection.new(:name => "will", :phone => "555-5555") + + user.name.should == "will" + user.phone.should == "555-5555" + end + + it "should not protect anything through attributes=" do + user = NoProtection.new + user.attributes = {:name => "will", :phone => "555-5555"} + + user.name.should == "will" + user.phone.should == "555-5555" + end +end + +describe "ExtendedDocument", "accessible flag" do + class WithAccessible < CouchRest::ExtendedDocument + use_database TEST_SERVER.default_database + property :name, :accessible => true + property :admin, :default => false + end + + it "should recognize accessible properties" do + props = WithAccessible.accessible_properties.map { |prop| prop.name} + props.should include("name") + props.should_not include("admin") + end + + it "should protect non-accessible properties set through new" do + user = WithAccessible.new(:name => "will", :admin => true) + + user.name.should == "will" + user.admin.should == false + end + + it "should protect non-accessible properties set through attributes=" do + user = WithAccessible.new + user.attributes = {:name => "will", :admin => true} + + user.name.should == "will" + user.admin.should == false + end +end + +describe "ExtendedDocument", "protected flag" do + class WithProtected < CouchRest::ExtendedDocument + use_database TEST_SERVER.default_database + property :name + property :admin, :default => false, :protected => true + end + + it "should recognize protected properties" do + props = WithProtected.protected_properties.map { |prop| prop.name} + props.should_not include("name") + props.should include("admin") + end + + it "should protect non-accessible properties set through new" do + user = WithProtected.new(:name => "will", :admin => true) + + user.name.should == "will" + user.admin.should == false + end + + it "should protect non-accessible properties set through attributes=" do + user = WithProtected.new + user.attributes = {:name => "will", :admin => true} + + user.name.should == "will" + user.admin.should == false + end +end + +describe "ExtendedDocument", "protected flag" do + class WithBoth < CouchRest::ExtendedDocument + use_database TEST_SERVER.default_database + property :name, :accessible => true + property :admin, :default => false, :protected => true + end + + it "should raise an error when both are set" do + lambda { WithBoth.new }.should raise_error + end +end