From 1583c35f9f9401bb2c98ff474ada89943f6448bd Mon Sep 17 00:00:00 2001 From: Mark Yoon Date: Fri, 20 Apr 2012 16:42:34 -0500 Subject: [PATCH] Add attr_accessible to all models (for Rails 3.2 compatibility) and protect Survey#acesss_code, #active_at, #inactive_at, ResponseSet#access_code, #started_at, #completed_at, as well as (on all models that have them) #api_id, #created_at, #modified_at. See http://weblog.rubyonrails.org/2012/3/30/ann-rails-3-2-3-has-been-released/ Closes #302. Changes #263. --- lib/surveyor/models/answer_methods.rb | 3 +++ .../models/dependency_condition_methods.rb | 5 ++++- lib/surveyor/models/dependency_methods.rb | 3 +++ lib/surveyor/models/question_group_methods.rb | 3 +++ lib/surveyor/models/question_methods.rb | 3 +++ lib/surveyor/models/response_methods.rb | 3 +++ lib/surveyor/models/response_set_methods.rb | 3 +++ lib/surveyor/models/survey_methods.rb | 5 ++++- lib/surveyor/models/survey_section_methods.rb | 3 +++ .../models/validation_condition_methods.rb | 6 ++++-- lib/surveyor/models/validation_methods.rb | 3 +++ lib/surveyor/parser.rb | 6 +++++- lib/surveyor/redcap_parser.rb | 2 ++ spec/models/answer_spec.rb | 7 +++++++ spec/models/dependency_condition_spec.rb | 7 +++++++ spec/models/dependency_spec.rb | 5 +++++ spec/models/question_group_spec.rb | 7 +++++++ spec/models/question_spec.rb | 7 +++++++ spec/models/response_set_spec.rb | 14 +++++++++++++- spec/models/response_spec.rb | 8 ++++++++ spec/models/survey_section_spec.rb | 5 +++++ spec/models/survey_spec.rb | 11 ++++++++++- spec/models/validation_condition_spec.rb | 6 +++++- spec/models/validation_spec.rb | 5 +++++ 24 files changed, 122 insertions(+), 8 deletions(-) diff --git a/lib/surveyor/models/answer_methods.rb b/lib/surveyor/models/answer_methods.rb index 8fbeec6c..5e10c1bd 100644 --- a/lib/surveyor/models/answer_methods.rb +++ b/lib/surveyor/models/answer_methods.rb @@ -20,6 +20,9 @@ def self.included(base) # base.send :validates_numericality_of, :question_id, :allow_nil => false, :only_integer => true @@validations_already_included = true end + + # Whitelisting attributes + base.send :attr_accessible, :question, :question_id, :text, :short_text, :help_text, :weight, :response_class, :reference_identifier, :data_export_identifier, :common_namespace, :common_identifier, :display_order, :is_exclusive, :display_length, :custom_class, :custom_renderer, :default_value, :display_type end include RenderText diff --git a/lib/surveyor/models/dependency_condition_methods.rb b/lib/surveyor/models/dependency_condition_methods.rb index 5380fa80..d044a671 100644 --- a/lib/surveyor/models/dependency_condition_methods.rb +++ b/lib/surveyor/models/dependency_condition_methods.rb @@ -21,7 +21,10 @@ def self.included(base) end base.send :include, Surveyor::ActsAsResponse # includes "as" instance method - + + # Whitelisting attributes + base.send :attr_accessible, :dependency, :question, :answer, :dependency_id, :rule_key, :question_id, :operator, :answer_id, :datetime_value, :integer_value, :float_value, :unit, :text_value, :string_value, :response_other + # Class methods base.instance_eval do def operators diff --git a/lib/surveyor/models/dependency_methods.rb b/lib/surveyor/models/dependency_methods.rb index dbe1092f..60b6c95b 100644 --- a/lib/surveyor/models/dependency_methods.rb +++ b/lib/surveyor/models/dependency_methods.rb @@ -18,6 +18,9 @@ def self.included(base) @@validations_already_included = true end + # Whitelisting attributes + base.send :attr_accessible, :question, :question_group, :question_id, :question_group_id, :rule + # Attribute aliases base.send :alias_attribute, :dependent_question_id, :question_id end diff --git a/lib/surveyor/models/question_group_methods.rb b/lib/surveyor/models/question_group_methods.rb index 2d38f8b6..127d60ea 100644 --- a/lib/surveyor/models/question_group_methods.rb +++ b/lib/surveyor/models/question_group_methods.rb @@ -7,6 +7,9 @@ def self.included(base) # Associations base.send :has_many, :questions base.send :has_one, :dependency + + # Whitelisting attributes + base.send :attr_accessible, :text, :help_text, :reference_identifier, :data_export_identifier, :common_namespace, :common_identifier, :display_type, :custom_class, :custom_renderer end include RenderText diff --git a/lib/surveyor/models/question_methods.rb b/lib/surveyor/models/question_methods.rb index c43d3619..2265cda7 100644 --- a/lib/surveyor/models/question_methods.rb +++ b/lib/surveyor/models/question_methods.rb @@ -25,6 +25,9 @@ def self.included(base) @@validations_already_included = true end + + # Whitelisting attributes + base.send :attr_accessible, :survey_section, :question_group, :survey_section_id, :question_group_id, :text, :short_text, :help_text, :pick, :reference_identifier, :data_export_identifier, :common_namespace, :common_identifier, :display_order, :display_type, :is_mandatory, :display_width, :custom_class, :custom_renderer, :correct_answer_id end include RenderText diff --git a/lib/surveyor/models/response_methods.rb b/lib/surveyor/models/response_methods.rb index 787e7168..9858060f 100644 --- a/lib/surveyor/models/response_methods.rb +++ b/lib/surveyor/models/response_methods.rb @@ -19,6 +19,9 @@ def self.included(base) end base.send :include, Surveyor::ActsAsResponse # includes "as" instance method + # Whitelisting attributes + base.send :attr_accessible, :response_set, :question, :answer, :date_value, :time_value, :response_set_id, :question_id, :answer_id, :datetime_value, :integer_value, :float_value, :unit, :text_value, :string_value, :response_other, :response_group, :survey_section_id + # Class methods base.instance_eval do def applicable_attributes(attrs) diff --git a/lib/surveyor/models/response_set_methods.rb b/lib/surveyor/models/response_set_methods.rb index a8c0b992..e2bea249 100644 --- a/lib/surveyor/models/response_set_methods.rb +++ b/lib/surveyor/models/response_set_methods.rb @@ -24,6 +24,9 @@ def self.included(base) # Attributes base.send :attr_protected, :completed_at + + # Whitelisting attributes + base.send :attr_accessible, :survey, :responses_attributes, :user_id, :survey_id # Class methods base.instance_eval do diff --git a/lib/surveyor/models/survey_methods.rb b/lib/surveyor/models/survey_methods.rb index a42b3edb..91a2cbd3 100644 --- a/lib/surveyor/models/survey_methods.rb +++ b/lib/surveyor/models/survey_methods.rb @@ -20,7 +20,10 @@ def self.included(base) base.send :validates_uniqueness_of, :access_code @@validations_already_included = true - end + end + + # Whitelisting attributes + base.send :attr_accessible, :title, :description, :reference_identifier, :data_export_identifier, :common_namespace, :common_identifier, :css_url, :custom_class, :display_order # Class methods base.instance_eval do diff --git a/lib/surveyor/models/survey_section_methods.rb b/lib/surveyor/models/survey_section_methods.rb index d4615e12..8ab33bd7 100644 --- a/lib/surveyor/models/survey_section_methods.rb +++ b/lib/surveyor/models/survey_section_methods.rb @@ -19,6 +19,9 @@ def self.included(base) @@validations_already_included = true end + + # Whitelisting attributes + base.send :attr_accessible, :survey, :survey_id, :title, :description, :reference_identifier, :data_export_identifier, :common_namespace, :common_identifier, :display_order, :custom_class end # Instance Methods diff --git a/lib/surveyor/models/validation_condition_methods.rb b/lib/surveyor/models/validation_condition_methods.rb index d33f48f7..36754d53 100644 --- a/lib/surveyor/models/validation_condition_methods.rb +++ b/lib/surveyor/models/validation_condition_methods.rb @@ -6,7 +6,6 @@ def self.included(base) base.send :belongs_to, :validation # Scopes - @@validations_already_included ||= nil unless @@validations_already_included # Validations @@ -20,7 +19,10 @@ def self.included(base) end base.send :include, Surveyor::ActsAsResponse # includes "as" instance method - + + # Whitelisting attributes + base.send :attr_accessible, :validation, :validation_id, :rule_key, :operator, :question_id, :answer_id, :datetime_value, :integer_value, :float_value, :unit, :text_value, :string_value, :response_other, :regexp + # Class methods base.instance_eval do def operators diff --git a/lib/surveyor/models/validation_methods.rb b/lib/surveyor/models/validation_methods.rb index 59520392..3a86ac37 100644 --- a/lib/surveyor/models/validation_methods.rb +++ b/lib/surveyor/models/validation_methods.rb @@ -18,6 +18,9 @@ def self.included(base) @@validations_already_included = true end + + # Whitelisting attributes + base.send :attr_accessible, :answer, :answer_id, :rule, :message end # Instance Methods diff --git a/lib/surveyor/parser.rb b/lib/surveyor/parser.rb index e6042dc5..5d46003a 100644 --- a/lib/surveyor/parser.rb +++ b/lib/surveyor/parser.rb @@ -133,6 +133,8 @@ class Question < ActiveRecord::Base # attributes attr_accessor :correct, :context_reference before_save :resolve_correct_answers + + attr_accessible :correct, :context_reference def self.parse_and_build(context, args, original_method, reference_identifier) # clear context @@ -154,7 +156,7 @@ def self.parse_and_build(context, args, original_method, reference_identifier) # add grid answers if context[:question_group] && context[:question_group].display_type == "grid" (context[:grid_answers] || []).each do |grid_answer| - a = context[:question].answers.build(grid_answer.attributes) + a = context[:question].answers.build(grid_answer.attributes.reject{|k,v| %w(id api_id created_at updated_at).include?(k)}) context[:answer_references][reference_identifier] ||= {} unless reference_identifier.blank? context[:answer_references][reference_identifier][grid_answer.reference_identifier] = a unless reference_identifier.blank? or grid_answer.reference_identifier.blank? end @@ -190,6 +192,8 @@ class DependencyCondition < ActiveRecord::Base attr_accessor :question_reference, :answer_reference, :context_reference before_save :resolve_references + attr_accessible :question_reference, :answer_reference, :context_reference + def self.parse_and_build(context, args, original_method, reference_identifier) # clear context context.delete_if{|k,v| k == :dependency_condition} diff --git a/lib/surveyor/redcap_parser.rb b/lib/surveyor/redcap_parser.rb index 3fbd8078..753c2922 100644 --- a/lib/surveyor/redcap_parser.rb +++ b/lib/surveyor/redcap_parser.rb @@ -161,6 +161,8 @@ def self.decompose_rule(str) class DependencyCondition < ActiveRecord::Base attr_accessor :question_reference, :answer_reference, :lookup_reference before_save :resolve_references + attr_accessible :question_reference, :answer_reference, :lookup_reference + def resolve_references return unless lookup_reference print "resolve(#{question_reference},#{answer_reference})" diff --git a/spec/models/answer_spec.rb b/spec/models/answer_spec.rb index dc9772f8..2fbcf84d 100644 --- a/spec/models/answer_spec.rb +++ b/spec/models/answer_spec.rb @@ -42,6 +42,13 @@ @answer.api_id.length.should == 36 end + it "should protect api_id, timestamps" do + saved_attrs = @answer.attributes + lambda {@answer.update_attributes(:created_at => 3.days.ago, :modified_at => 3.hours.ago)}.should raise_error(ActiveModel::MassAssignmentSecurity::Error) + lambda {@answer.update_attributes(:api_id => "NEW")}.should raise_error(ActiveModel::MassAssignmentSecurity::Error) + @answer.attributes.should == saved_attrs + end + require 'mustache' class FakeMustacheContext < ::Mustache def site diff --git a/spec/models/dependency_condition_spec.rb b/spec/models/dependency_condition_spec.rb index 6a826470..6f762c85 100644 --- a/spec/models/dependency_condition_spec.rb +++ b/spec/models/dependency_condition_spec.rb @@ -78,6 +78,13 @@ @dependency_condition.stub!(:is_met?).and_return(true) @dependency_condition.to_hash(@rs) end + + it "should protect timestamps" do + saved_attrs = @dependency_condition.attributes + lambda {@dependency_condition.update_attributes(:created_at => 3.days.ago, :modified_at => 3.hours.ago)}.should raise_error(ActiveModel::MassAssignmentSecurity::Error) + @dependency_condition.attributes.should == saved_attrs + end + end describe "to_hash" do diff --git a/spec/models/dependency_spec.rb b/spec/models/dependency_spec.rb index d7fe5dc9..b4ea42b3 100644 --- a/spec/models/dependency_spec.rb +++ b/spec/models/dependency_spec.rb @@ -43,6 +43,11 @@ @dependency.rule = "a and b" @dependency.should have(1).error_on(:rule) end + it "should protect timestamps" do + saved_attrs = @dependency.attributes + lambda {@dependency.update_attributes(:created_at => 3.days.ago, :modified_at => 3.hours.ago)}.should raise_error(ActiveModel::MassAssignmentSecurity::Error) + @dependency.attributes.should == saved_attrs + end end diff --git a/spec/models/question_group_spec.rb b/spec/models/question_group_spec.rb index 54ade643..cc88d774 100644 --- a/spec/models/question_group_spec.rb +++ b/spec/models/question_group_spec.rb @@ -32,4 +32,11 @@ @dependency.should_receive(:is_met?).and_return(false) @question_group.css_class(Factory(:response_set)).should == "g_dependent g_hidden foo bar" end + it "should protect api_id, timestamps" do + saved_attrs = @question_group.attributes + lambda {@question_group.update_attributes(:created_at => 3.days.ago, :modified_at => 3.hours.ago)}.should raise_error(ActiveModel::MassAssignmentSecurity::Error) + lambda {@question_group.update_attributes(:api_id => "NEW")}.should raise_error(ActiveModel::MassAssignmentSecurity::Error) + @question_group.attributes.should == saved_attrs + end + end diff --git a/spec/models/question_spec.rb b/spec/models/question_spec.rb index 9e1be717..a67d4c3e 100644 --- a/spec/models/question_spec.rb +++ b/spec/models/question_spec.rb @@ -42,6 +42,13 @@ it "should have an api_id" do @question.api_id.length.should == 36 end + + it "should protect api_id, timestamps" do + saved_attrs = @question.attributes + lambda {@question.update_attributes(:created_at => 3.days.ago, :modified_at => 3.hours.ago)}.should raise_error(ActiveModel::MassAssignmentSecurity::Error) + lambda {@question.update_attributes(:api_id => "NEW")}.should raise_error(ActiveModel::MassAssignmentSecurity::Error) + @question.attributes.should == saved_attrs + end end describe Question, "that has answers" do diff --git a/spec/models/response_set_spec.rb b/spec/models/response_set_spec.rb index 7c2680f5..b5500a50 100644 --- a/spec/models/response_set_spec.rb +++ b/spec/models/response_set_spec.rb @@ -12,6 +12,16 @@ @response_set.access_code.should_not be_nil @response_set.access_code.length.should == 10 end + + it "should protect api_id, timestamps" do + saved_attrs = @response_set.attributes + lambda {@response_set.update_attributes(:created_at => 3.days.ago, :modified_at => 3.hours.ago)}.should raise_error(ActiveModel::MassAssignmentSecurity::Error) + lambda {@response_set.update_attributes(:api_id => "NEW")}.should raise_error(ActiveModel::MassAssignmentSecurity::Error) + lambda {@response_set.update_attributes(:access_code => "AND")}.should raise_error(ActiveModel::MassAssignmentSecurity::Error) + lambda {@response_set.update_attributes(:started_at => 10.days.ago)}.should raise_error(ActiveModel::MassAssignmentSecurity::Error) + lambda {@response_set.update_attributes(:completed_at => 2.hours.ago)}.should raise_error(ActiveModel::MassAssignmentSecurity::Error) + @response_set.attributes.should == saved_attrs + end describe '#access_code' do let!(:rs1) { Factory(:response_set).tap { |rs| rs.update_attribute(:access_code, 'one') } } @@ -19,7 +29,9 @@ # Regression test for #263 it 'accepts an access code in the constructor' do - ResponseSet.new(:access_code => 'eleven').access_code.should == 'eleven' + rs = ResponseSet.new + rs.access_code = 'eleven' + rs.access_code.should == 'eleven' end # Regression test for #263 diff --git a/spec/models/response_spec.rb b/spec/models/response_spec.rb index d1b41970..b9dcfcd6 100644 --- a/spec/models/response_spec.rb +++ b/spec/models/response_spec.rb @@ -39,6 +39,14 @@ response2 = Factory(:response, :question => Factory(:question), :answer => Factory(:answer), :response_set => @response.response_set, :created_at => (@response.created_at + 1)) Response.all.should == [@response, response2] end + + it "should protect api_id, timestamps" do + saved_attrs = @response.attributes + lambda {@response.update_attributes(:created_at => 3.days.ago, :modified_at => 3.hours.ago)}.should raise_error(ActiveModel::MassAssignmentSecurity::Error) + lambda {@response.update_attributes(:api_id => "NEW")}.should raise_error(ActiveModel::MassAssignmentSecurity::Error) + @response.attributes.should == saved_attrs + end + describe "returns the response as the type requested" do it "returns 'string'" do diff --git a/spec/models/survey_section_spec.rb b/spec/models/survey_section_spec.rb index bf18ebb6..271ed9e6 100644 --- a/spec/models/survey_section_spec.rb +++ b/spec/models/survey_section_spec.rb @@ -16,6 +16,11 @@ # @survey_section.survey_id = nil # @survey_section.should have(1).error_on(:survey) end + it "should protect timestamps" do + saved_attrs = @survey_section.attributes + lambda {@survey_section.update_attributes(:created_at => 3.days.ago, :modified_at => 3.hours.ago)}.should raise_error(ActiveModel::MassAssignmentSecurity::Error) + @survey_section.attributes.should == saved_attrs + end end describe SurveySection, "with questions" do diff --git a/spec/models/survey_spec.rb b/spec/models/survey_spec.rb index d7bf2265..585ee4f3 100644 --- a/spec/models/survey_spec.rb +++ b/spec/models/survey_spec.rb @@ -111,5 +111,14 @@ @survey.active?.should be_false @survey.active_at.should be_nil end - + + it "should protect access_code, api_id, active_at, inactive_at, timestamps" do + saved_attrs = @survey.attributes + lambda {@survey.update_attributes(:access_code => "NEW")}.should raise_error(ActiveModel::MassAssignmentSecurity::Error) + lambda {@survey.update_attributes(:api_id => "AND")}.should raise_error(ActiveModel::MassAssignmentSecurity::Error) + lambda {@survey.update_attributes(:active_at => 2.days.ago)}.should raise_error(ActiveModel::MassAssignmentSecurity::Error) + lambda {@survey.update_attributes(:inactive_at => 3.days.from_now)}.should raise_error(ActiveModel::MassAssignmentSecurity::Error) + lambda {@survey.update_attributes(:created_at => 3.days.ago, :modified_at => 3.hours.ago)}.should raise_error(ActiveModel::MassAssignmentSecurity::Error) + @survey.attributes.should == saved_attrs + end end diff --git a/spec/models/validation_condition_spec.rb b/spec/models/validation_condition_spec.rb index 8ba53f20..f8abed49 100644 --- a/spec/models/validation_condition_spec.rb +++ b/spec/models/validation_condition_spec.rb @@ -49,7 +49,11 @@ @validation_condition.operator = "#" @validation_condition.should have(1).error_on(:operator) end - + it "should protect timestamps" do + saved_attrs = @validation_condition.attributes + lambda {@validation_condition.update_attributes(:created_at => 3.days.ago, :modified_at => 3.hours.ago)}.should raise_error(ActiveModel::MassAssignmentSecurity::Error) + @validation_condition.attributes.should == saved_attrs + end end describe ValidationCondition, "validating responses" do diff --git a/spec/models/validation_spec.rb b/spec/models/validation_spec.rb index df970676..5cd7552b 100644 --- a/spec/models/validation_spec.rb +++ b/spec/models/validation_spec.rb @@ -30,6 +30,11 @@ @validation.rule = "a and b" @validation.should have(1).error_on(:rule) end + it "should protect timestamps" do + saved_attrs = @validation.attributes + lambda {@validation.update_attributes(:created_at => 3.days.ago, :modified_at => 3.hours.ago)}.should raise_error(ActiveModel::MassAssignmentSecurity::Error) + @validation.attributes.should == saved_attrs + end end describe Validation, "reporting its status" do def test_var(vhash, vchashes, ahash, rhash)