Adds attribute protection to properties
Public Facing * through either :protected or :accessible8 flags * prevents protected attributes from being set in mass assignment Developer Facing * refactors #initialize and #update_attribute_without_saving to use same private methods to set attributes on ExtendedDocument * adds new mixin to do protection Signed-off-by: Tapajós <tapajos@gmail.com>
This commit is contained in:
parent
58d621d399
commit
b5d09afef5
1
.gitignore
vendored
1
.gitignore
vendored
|
@ -1,3 +1,4 @@
|
||||||
.DS_Store
|
.DS_Store
|
||||||
html/*
|
html/*
|
||||||
pkg
|
pkg
|
||||||
|
*.swp
|
||||||
|
|
74
lib/couchrest/mixins/attribute_protection.rb
Normal file
74
lib/couchrest/mixins/attribute_protection.rb
Normal file
|
@ -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
|
|
@ -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__), 'extended_attachments')
|
||||||
require File.join(File.dirname(__FILE__), 'class_proxy')
|
require File.join(File.dirname(__FILE__), 'class_proxy')
|
||||||
require File.join(File.dirname(__FILE__), 'collection')
|
require File.join(File.dirname(__FILE__), 'collection')
|
||||||
|
require File.join(File.dirname(__FILE__), 'attribute_protection')
|
||||||
|
|
|
@ -14,6 +14,7 @@ module CouchRest
|
||||||
include CouchRest::Mixins::ExtendedAttachments
|
include CouchRest::Mixins::ExtendedAttachments
|
||||||
include CouchRest::Mixins::ClassProxy
|
include CouchRest::Mixins::ClassProxy
|
||||||
include CouchRest::Mixins::Collection
|
include CouchRest::Mixins::Collection
|
||||||
|
include CouchRest::Mixins::AttributeProtection
|
||||||
|
|
||||||
def self.subclasses
|
def self.subclasses
|
||||||
@subclasses ||= []
|
@subclasses ||= []
|
||||||
|
@ -40,11 +41,7 @@ module CouchRest
|
||||||
|
|
||||||
def initialize(passed_keys={})
|
def initialize(passed_keys={})
|
||||||
apply_defaults # defined in CouchRest::Mixins::Properties
|
apply_defaults # defined in CouchRest::Mixins::Properties
|
||||||
passed_keys.each do |k,v|
|
set_attributes(passed_keys)
|
||||||
if self.respond_to?("#{k}=")
|
|
||||||
self.send("#{k}=", passed_keys.delete(k))
|
|
||||||
end
|
|
||||||
end if passed_keys
|
|
||||||
super
|
super
|
||||||
cast_keys # defined in CouchRest::Mixins::Properties
|
cast_keys # defined in CouchRest::Mixins::Properties
|
||||||
unless self['_id'] && self['_rev']
|
unless self['_id'] && self['_rev']
|
||||||
|
@ -150,12 +147,8 @@ module CouchRest
|
||||||
# make a copy, we don't want to change arguments
|
# make a copy, we don't want to change arguments
|
||||||
attrs = hash.dup
|
attrs = hash.dup
|
||||||
%w[_id _rev created_at updated_at].each {|attr| attrs.delete(attr)}
|
%w[_id _rev created_at updated_at].each {|attr| attrs.delete(attr)}
|
||||||
attrs.each do |k, v|
|
check_properties_exist(attrs)
|
||||||
raise NoMethodError, "#{k}= method not available, use property :#{k}" unless self.respond_to?("#{k}=")
|
set_attributes(attrs)
|
||||||
end
|
|
||||||
attrs.each do |k, v|
|
|
||||||
self.send("#{k}=",v)
|
|
||||||
end
|
|
||||||
end
|
end
|
||||||
alias :attributes= :update_attributes_without_saving
|
alias :attributes= :update_attributes_without_saving
|
||||||
|
|
||||||
|
@ -281,5 +274,21 @@ module CouchRest
|
||||||
end
|
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
|
||||||
end
|
end
|
||||||
|
|
94
spec/couchrest/more/attribute_protection_spec.rb
Normal file
94
spec/couchrest/more/attribute_protection_spec.rb
Normal file
|
@ -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
|
Loading…
Reference in a new issue