[PATCH 4/4] redmine #3601: whitelist all attributes as needing to be explicitly set by default

Mo Morsi mmorsi at redhat.com
Tue Jul 31 20:03:37 UTC 2012


  Note this patch requires rails 3.2 to work (which will be in Fedora soon [1])

  This changes the application so that no mass assigments are permitted by default.

  I added the neccessary model changes so that things should be working again with the
  current mass assignments in place in the application (specs pass after this patch)

  Obviously this still doesn't solve the mass assignment issue, but the items that should
  be removed and explicitly set are marked with comments and thus we can tackle them one
  by one in the various areas that they need to be set explicitly in the application

[1] http://lists.fedoraproject.org/pipermail/ruby-sig/2012-July/001077.html
---
 src/app/controllers/application_controller.rb |    2 +-
 src/app/models/base_permission_object.rb      |    2 ++
 src/app/models/catalog.rb                     |    5 +++++
 src/app/models/catalog_entry.rb               |    5 +++++
 src/app/models/credential_definition.rb       |    6 ++++++
 src/app/models/deployable.rb                  |    5 +++++
 src/app/models/deployment.rb                  |    5 +++++
 src/app/models/derived_permission.rb          |    3 +++
 src/app/models/event.rb                       |    5 +++++
 src/app/models/frontend_realm.rb              |    2 ++
 src/app/models/hardware_profile.rb            |    2 ++
 src/app/models/hardware_profile_property.rb   |    2 ++
 src/app/models/hook.rb                        |    2 ++
 src/app/models/instance.rb                    |    6 ++++++
 src/app/models/instance_key.rb                |    5 +++++
 src/app/models/instance_match.rb              |    3 +++
 src/app/models/instance_task.rb               |    3 +++
 src/app/models/metadata_object.rb             |    1 +
 src/app/models/permission.rb                  |    3 +++
 src/app/models/pool.rb                        |    2 ++
 src/app/models/pool_family.rb                 |    2 ++
 src/app/models/privilege.rb                   |    2 ++
 src/app/models/property_enum_entry.rb         |    2 ++
 src/app/models/provider.rb                    |    2 ++
 src/app/models/provider_account.rb            |    2 ++
 src/app/models/provider_type.rb               |    2 ++
 src/app/models/quota.rb                       |    2 ++
 src/app/models/realm.rb                       |    2 ++
 src/app/models/realm_backend_target.rb        |    2 ++
 src/app/models/role.rb                        |    2 ++
 src/app/models/session_entity.rb              |    3 +++
 src/app/models/task.rb                        |    2 ++
 src/app/models/user.rb                        |    2 ++
 src/app/models/user_group.rb                  |    2 ++
 src/app/models/view_state.rb                  |    2 ++
 src/config/application.rb                     |    2 ++
 src/spec/models/instance_spec.rb              |    2 +-
 37 files changed, 102 insertions(+), 2 deletions(-)

diff --git a/src/app/controllers/application_controller.rb b/src/app/controllers/application_controller.rb
index b6381ff..daf6b01 100644
--- a/src/app/controllers/application_controller.rb
+++ b/src/app/controllers/application_controller.rb
@@ -172,7 +172,7 @@ class ApplicationController < ActionController::Base
     arr = Array.new
     instance_variables.each do |ivar|
       val = instance_variable_get(ivar)
-      if val && val.respond_to?(:errors) && val.errors.size > 0
+      if val && val.respond_to?(:errors) && !val.errors.nil? && val.errors.size > 0
         hash[:object] = ivar[1, ivar.size]
         hash[:errors] ||= []
         val.errors.each {|key,msg|
diff --git a/src/app/models/base_permission_object.rb b/src/app/models/base_permission_object.rb
index c5d525a..e9cc23d 100644
--- a/src/app/models/base_permission_object.rb
+++ b/src/app/models/base_permission_object.rb
@@ -35,6 +35,8 @@ class BasePermissionObject < ActiveRecord::Base
            :include => [:role],
            :order => "derived_permissions.id ASC"
 
+  attr_accessible :name
+
   validates_presence_of :name
   validates_uniqueness_of :name
 
diff --git a/src/app/models/catalog.rb b/src/app/models/catalog.rb
index 4ea3df1..9fdc15f 100644
--- a/src/app/models/catalog.rb
+++ b/src/app/models/catalog.rb
@@ -46,6 +46,11 @@ class Catalog < ActiveRecord::Base
   before_create :set_pool_family
   after_update :update_deployable_permissions
 
+  attr_accessible :name
+
+  # TODO update app to set pool explicitly and remove this
+  attr_accessible :pool
+
   validates_presence_of :pool
   validates_presence_of :name
   validates_uniqueness_of :name
diff --git a/src/app/models/catalog_entry.rb b/src/app/models/catalog_entry.rb
index 03461d2..f532e41 100644
--- a/src/app/models/catalog_entry.rb
+++ b/src/app/models/catalog_entry.rb
@@ -30,6 +30,11 @@ class CatalogEntry < ActiveRecord::Base
   belongs_to :catalog
   belongs_to :deployable
 
+  attr_accessible :name, :description
+
+  # TODO update app to set owner and catalog explitly and remove this
+  attr_accessible :owner, :catalog
+
   validates_presence_of :catalog
   validates_presence_of :deployable
 
diff --git a/src/app/models/credential_definition.rb b/src/app/models/credential_definition.rb
index a920cb1..c87b0ca 100644
--- a/src/app/models/credential_definition.rb
+++ b/src/app/models/credential_definition.rb
@@ -31,6 +31,12 @@
 class CredentialDefinition < ActiveRecord::Base
   belongs_to :provider_type
   has_many :credentials
+
+  attr_accessible :name, :label, :input_type
+  
+  # TODO update add to set provider_type_id explicitly and remove this
+  attr_accessible :provider_type_id
+
   validates_presence_of :name
   validates_uniqueness_of :name, :scope => :provider_type_id
   validates_presence_of :label
diff --git a/src/app/models/deployable.rb b/src/app/models/deployable.rb
index e1d80d5..1c5e6bb 100644
--- a/src/app/models/deployable.rb
+++ b/src/app/models/deployable.rb
@@ -24,6 +24,11 @@ class Deployable < ActiveRecord::Base
 
   include PermissionedObject
 
+  attr_accessible :name, :description
+
+  # TODO update app to set owner explicitly and remove this
+  attr_accessible :owner, :assemblies, :xml
+
   validates_presence_of :name
   validates_uniqueness_of :name
   validates_length_of :name, :maximum => 1024
diff --git a/src/app/models/deployment.rb b/src/app/models/deployment.rb
index 277f9b3..d4b782d 100644
--- a/src/app/models/deployment.rb
+++ b/src/app/models/deployment.rb
@@ -69,6 +69,11 @@ class Deployment < ActiveRecord::Base
 
   scope :ascending_by_name, :order => 'deployments.name ASC'
 
+  attr_accessible :name
+
+  # TODO update app to set pool and owner explicitly and remove this
+  attr_accessible :pool, :owner, :pool_id
+
   before_validation :replace_special_characters_in_name
   validates_presence_of :pool_id
   validates_presence_of :name
diff --git a/src/app/models/derived_permission.rb b/src/app/models/derived_permission.rb
index 80d055d..bd9f730 100644
--- a/src/app/models/derived_permission.rb
+++ b/src/app/models/derived_permission.rb
@@ -46,6 +46,9 @@ class DerivedPermission < ActiveRecord::Base
   belongs_to :base_permission_object, :class_name => "BasePermissionObject",
                                       :foreign_key => "permission_object_id"
 
+  # TODO update app to set all these attributes explicitly and remove this
+  attr_accessible :permission, :permission_object, :role_id, :entity_id
+
   # role is copied from source permission
   belongs_to :role
   validates_presence_of :role_id
diff --git a/src/app/models/event.rb b/src/app/models/event.rb
index 8479c58..384e347 100644
--- a/src/app/models/event.rb
+++ b/src/app/models/event.rb
@@ -39,6 +39,11 @@ class Event < ActiveRecord::Base
   require "#{Rails.root}/lib/aeolus/event.rb"
   belongs_to :source, :polymorphic => true
 
+  attr_accessible :event_time, :summary, :status_code
+
+  # TODO update app to set these attributes explicitly and remove this
+  attr_accessible :source_type, :source_id, :source
+
   validates_presence_of :source_id
   validates_presence_of :source_type
 
diff --git a/src/app/models/frontend_realm.rb b/src/app/models/frontend_realm.rb
index 032d0fa..8e0e35c 100644
--- a/src/app/models/frontend_realm.rb
+++ b/src/app/models/frontend_realm.rb
@@ -36,6 +36,8 @@ class FrontendRealm < ActiveRecord::Base
   has_many :realm_backend_targets, :dependent => :destroy
   has_many :instances
 
+  attr_accessible :name
+
   # there is a problem with has_many through + polymophic in AR:
   # http://blog.hasmanythrough.com/2006/4/3/polymorphic-through
   # so we define explicitly backend_realms and backend_providers
diff --git a/src/app/models/hardware_profile.rb b/src/app/models/hardware_profile.rb
index 2099214..881a6e0 100644
--- a/src/app/models/hardware_profile.rb
+++ b/src/app/models/hardware_profile.rb
@@ -70,6 +70,8 @@ class HardwareProfile < ActiveRecord::Base
 
   accepts_nested_attributes_for :memory, :cpu, :storage, :architecture
 
+  attr_accessible :name, :external_key, :cpu, :memory, :storage, :architecture
+
   validates_presence_of :external_key, :if => Proc.new { |hwp| !hwp.provider.nil? }
   validates_uniqueness_of :external_key, :scope => [:provider_id], :if => Proc.new { |hwp| !hwp.provider.nil? }
 
diff --git a/src/app/models/hardware_profile_property.rb b/src/app/models/hardware_profile_property.rb
index 4f883eb..4af4fb2 100644
--- a/src/app/models/hardware_profile_property.rb
+++ b/src/app/models/hardware_profile_property.rb
@@ -52,6 +52,8 @@ class HardwareProfileProperty < ActiveRecord::Base
 
   has_many :property_enum_entries
 
+  attr_accessible :name, :kind, :unity, :value, :range_first, :range_last
+
   before_validation :is_form_empty?
   validates_presence_of :name
   validates_inclusion_of :name,
diff --git a/src/app/models/hook.rb b/src/app/models/hook.rb
index e95ece4..435b838 100644
--- a/src/app/models/hook.rb
+++ b/src/app/models/hook.rb
@@ -16,4 +16,6 @@
 class Hook < ActiveRecord::Base
   validates_presence_of :uri
   validates_presence_of :version
+
+  attr_accessible :uri, :version
 end
diff --git a/src/app/models/instance.rb b/src/app/models/instance.rb
index fad0d03..24f8384 100644
--- a/src/app/models/instance.rb
+++ b/src/app/models/instance.rb
@@ -93,6 +93,12 @@ class Instance < ActiveRecord::Base
   has_many :tasks, :as =>:task_target, :dependent => :destroy
   after_create "assign_owner_roles(owner)"
 
+  # TODO update app to set all these attributes explicitly and remove this
+  attr_accessible :name, :pool_id, :state, :external_key, :hardware_profile, :hardware_profile_id,
+                  :provider_account_id, :provider_account, :pool, :owner, :owner_id, :provider_account_id,
+                  :instance_hwp_id, :deployment_id, :provider_instance_id, :last_error, :frontend_realm, :image_uuid, :image_build_uuid,
+                  :assembly_xml
+
   validates_presence_of :pool_id
   validates_presence_of :hardware_profile_id
 
diff --git a/src/app/models/instance_key.rb b/src/app/models/instance_key.rb
index 457f5d6..19ebd39 100644
--- a/src/app/models/instance_key.rb
+++ b/src/app/models/instance_key.rb
@@ -34,6 +34,11 @@ class InstanceKey < ActiveRecord::Base
   belongs_to :instance
   before_destroy :destroy_instance_key
 
+  attr_accessible :name, :pem, :instance
+
+  # TODO update app to set instance explicitly and remove this
+  attr_accessible :instance
+
   def destroy_instance_key
     begin
       instance.provider_account.connect.key(self.name).destroy!
diff --git a/src/app/models/instance_match.rb b/src/app/models/instance_match.rb
index 8d797ca..9ef2100 100644
--- a/src/app/models/instance_match.rb
+++ b/src/app/models/instance_match.rb
@@ -5,6 +5,9 @@ class InstanceMatch < ActiveRecord::Base
   belongs_to :hardware_profile
   belongs_to :realm
 
+  # TODO update the app to set these attributes explicitly and remove this
+  attr_accessible :instance, :hardware_profile, :provider_account
+
   def equals?(other)
     return self.nil? && other.nil? if (self.nil? || other.nil?)
     self.pool_family_id == other.pool_family_id &&
diff --git a/src/app/models/instance_task.rb b/src/app/models/instance_task.rb
index d28e0e6..b71152d 100644
--- a/src/app/models/instance_task.rb
+++ b/src/app/models/instance_task.rb
@@ -50,7 +50,10 @@ class InstanceTask < Task
   # FIXME: do we need this for db-omatic?
   ACTION_UPDATE_STATE = "update_state"
 
+  attr_accessible :instance, :type, :state, :action, :failure_code
 
+  # TODO upate app to set task_target explicitly and remove this
+  attr_accessible :task_target_id, :task_target_type, :task_target
 
   def task_obj
     if self.instance
diff --git a/src/app/models/metadata_object.rb b/src/app/models/metadata_object.rb
index b344008..cd6f4b0 100644
--- a/src/app/models/metadata_object.rb
+++ b/src/app/models/metadata_object.rb
@@ -32,6 +32,7 @@
 # Likewise, all the methods added will be available for all controllers.
 
 class MetadataObject < ActiveRecord::Base
+  attr_accessible :key
 
   validates_presence_of :key
   validates_uniqueness_of :key
diff --git a/src/app/models/permission.rb b/src/app/models/permission.rb
index 9b4c0ac..db8f6de 100644
--- a/src/app/models/permission.rb
+++ b/src/app/models/permission.rb
@@ -36,6 +36,9 @@ class Permission < ActiveRecord::Base
   belongs_to :role
   belongs_to :entity
 
+  # TODO upate app to set these attributes explicitly and remove this
+  attr_accessible :role, :permission_object, :entity
+
   validates_presence_of :role_id
 
   validates_presence_of :entity_id
diff --git a/src/app/models/pool.rb b/src/app/models/pool.rb
index 13baebc..e3f025d 100644
--- a/src/app/models/pool.rb
+++ b/src/app/models/pool.rb
@@ -47,6 +47,8 @@ class Pool < ActiveRecord::Base
   #has_many :images,  :dependent => :destroy
   has_many :catalogs, :dependent => :destroy
 
+  attr_accessible :name, :quota, :pool_family, :pool_family_id, :enabled, :quota
+
   validates_presence_of :name
   validates_presence_of :quota
   validates_presence_of :pool_family
diff --git a/src/app/models/pool_family.rb b/src/app/models/pool_family.rb
index f464723..50c7d20 100644
--- a/src/app/models/pool_family.rb
+++ b/src/app/models/pool_family.rb
@@ -52,6 +52,8 @@ class PoolFamily < ActiveRecord::Base
   has_many :instances
   has_many :deployments
 
+  attr_accessible :name, :description, :quota, :quota_attributes
+
   validates_length_of :name, :maximum => 255
   validates_format_of :name, :with => /^[\w -]*$/n
 
diff --git a/src/app/models/privilege.rb b/src/app/models/privilege.rb
index 1d857db..fbaaab1 100644
--- a/src/app/models/privilege.rb
+++ b/src/app/models/privilege.rb
@@ -62,6 +62,8 @@ class Privilege < ActiveRecord::Base
               ProviderAccount => ACTIONS,
               User => [ CREATE, MODIFY, VIEW] }
 
+  attr_accessible :role, :target_type, :action
+
   belongs_to :role
   validates_presence_of :role_id
   validates_presence_of :target_type
diff --git a/src/app/models/property_enum_entry.rb b/src/app/models/property_enum_entry.rb
index 7032536..b61e3da 100644
--- a/src/app/models/property_enum_entry.rb
+++ b/src/app/models/property_enum_entry.rb
@@ -34,6 +34,8 @@ class PropertyEnumEntry < ActiveRecord::Base
 
   belongs_to :hardware_profile_property
 
+  attr_accessible :value, :hardware_profile_property
+
   validates_presence_of :value
   validates_presence_of :hardware_profile_property
   validates_numericality_of :value, :greater_than => 0,
diff --git a/src/app/models/provider.rb b/src/app/models/provider.rb
index cf2a7e7..2be719d 100644
--- a/src/app/models/provider.rb
+++ b/src/app/models/provider.rb
@@ -44,6 +44,8 @@ class Provider < ActiveRecord::Base
   has_many :frontend_realms, :through => :realm_backend_targets
   belongs_to :provider_type
 
+  attr_accessible :name, :provider_type, :url, :hardware_profiles, :realms
+
   validates_presence_of :name
   validates_uniqueness_of :name
   validates_presence_of :provider_type_id
diff --git a/src/app/models/provider_account.rb b/src/app/models/provider_account.rb
index 067f5cf..6ad9591 100644
--- a/src/app/models/provider_account.rb
+++ b/src/app/models/provider_account.rb
@@ -61,6 +61,8 @@ class ProviderAccount < ActiveRecord::Base
   # Helpers
   attr_accessor :x509_cert_priv_file, :x509_cert_pub_file
 
+  attr_accessible :provider, :quota, :label
+
   # Validations
   validates_presence_of :provider
   validates_presence_of :label
diff --git a/src/app/models/provider_type.rb b/src/app/models/provider_type.rb
index 205752d..19c5e5d 100644
--- a/src/app/models/provider_type.rb
+++ b/src/app/models/provider_type.rb
@@ -33,6 +33,8 @@ class ProviderType < ActiveRecord::Base
   has_many :providers
   has_many :credential_definitions, :dependent => :destroy
 
+  attr_accessible :id, :name, :deltacloud_driver
+
   validates_presence_of :name
   validates_uniqueness_of :name
   validates_presence_of :deltacloud_driver
diff --git a/src/app/models/quota.rb b/src/app/models/quota.rb
index 2c8dcee..6aa6672 100644
--- a/src/app/models/quota.rb
+++ b/src/app/models/quota.rb
@@ -41,6 +41,8 @@ class Quota < ActiveRecord::Base
   has_one :provider_account
   has_one :user
 
+  attr_accessible :running_instances, :total_instances, :maximum_running_instances, :maximum_total_instances
+
   validates_numericality_of :maximum_total_instances,
                             :greater_than_or_equal_to => 0,
                             :less_than_or_equal_to => 2147483647,
diff --git a/src/app/models/realm.rb b/src/app/models/realm.rb
index 962cd70..1e8e110 100644
--- a/src/app/models/realm.rb
+++ b/src/app/models/realm.rb
@@ -41,6 +41,8 @@ class Realm < ActiveRecord::Base
   has_many :realm_backend_targets, :as => :realm_or_provider, :dependent => :destroy
   has_many :frontend_realms, :through => :realm_backend_targets
 
+  attr_accessible :name, :external_key, :provider
+
   validates_presence_of :external_key
   validates_uniqueness_of :external_key, :scope => :provider_id
 
diff --git a/src/app/models/realm_backend_target.rb b/src/app/models/realm_backend_target.rb
index c3801b1..875fe39 100644
--- a/src/app/models/realm_backend_target.rb
+++ b/src/app/models/realm_backend_target.rb
@@ -31,6 +31,8 @@ class RealmBackendTarget < ActiveRecord::Base
   belongs_to :realm,  :class_name => 'Realm', :foreign_key => 'realm_or_provider_id'
   belongs_to :provider,  :class_name => 'Provider', :foreign_key => 'realm_or_provider_id'
 
+  attr_accessible :frontend_realm, :realm_or_provider
+
   validates_uniqueness_of :frontend_realm_id, :scope => [:realm_or_provider_id, :realm_or_provider_type]
   validates_presence_of :realm_or_provider
 
diff --git a/src/app/models/role.rb b/src/app/models/role.rb
index dcf3178..ddff768 100644
--- a/src/app/models/role.rb
+++ b/src/app/models/role.rb
@@ -33,6 +33,8 @@ class Role < ActiveRecord::Base
   has_many :derived_permissions, :dependent => :destroy
   has_many :privileges, :dependent => :destroy
 
+  attr_accessible :name, :scope, :assign_to_owner
+
   validates_presence_of :scope
   validates_presence_of :name
   validates_uniqueness_of :name
diff --git a/src/app/models/session_entity.rb b/src/app/models/session_entity.rb
index ea3f547..2db1bce 100644
--- a/src/app/models/session_entity.rb
+++ b/src/app/models/session_entity.rb
@@ -18,6 +18,9 @@ class SessionEntity < ActiveRecord::Base
   belongs_to :user
   belongs_to :entity
 
+  attr_accessible :user, :session, :entity,
+                  :user_id, :session_id, :entity_id
+
   validates_presence_of :user_id
   validates_presence_of :session_id
   validates_presence_of :entity_id
diff --git a/src/app/models/task.rb b/src/app/models/task.rb
index 6d65fcc..2f1bfe7 100644
--- a/src/app/models/task.rb
+++ b/src/app/models/task.rb
@@ -67,6 +67,8 @@ class Task < ActiveRecord::Base
 
   FAILURE_CODES = [FAILURE_PROVIDER_NOT_FOUND, FAILURE_PROVIDER_CONTACT_FAILED, FAILURE_PROVIDER_RETURNED_FAILED, FAILURE_OVER_POOL_QUOTA]
 
+  attr_accessible :created_at, :time_started, :time_ended, :state
+
   validates_inclusion_of :failure_code,
     :in => FAILURE_CODES + [nil]
 
diff --git a/src/app/models/user.rb b/src/app/models/user.rb
index 73579e5..54a0197 100644
--- a/src/app/models/user.rb
+++ b/src/app/models/user.rb
@@ -74,6 +74,8 @@ class User < ActiveRecord::Base
   before_validation :strip_whitespace
   after_save :update_entity
 
+  attr_accessible :login, :password, :password_confirmation, :first_name, :last_name, :quota, :email, :ignore_password
+
   validates_presence_of :quota
   validates_length_of :first_name, :maximum => 255, :allow_blank => true
   validates_length_of :last_name,  :maximum => 255, :allow_blank => true
diff --git a/src/app/models/user_group.rb b/src/app/models/user_group.rb
index 3d4513d..c79eed2 100644
--- a/src/app/models/user_group.rb
+++ b/src/app/models/user_group.rb
@@ -31,6 +31,8 @@ class UserGroup < ActiveRecord::Base
   MEMBERSHIP_SOURCE_LOCAL = "local"
   MEMBERSHIP_SOURCES = [MEMBERSHIP_SOURCE_LOCAL, MEMBERSHIP_SOURCE_LDAP]
 
+  attr_accessible :name, :membership_source
+
   validates_presence_of :name
   # scope name by membership_source to prevent errors if users are later added
   # to external ldap groups that have the same name as local groups
diff --git a/src/app/models/view_state.rb b/src/app/models/view_state.rb
index 3d763b6..8b643eb 100644
--- a/src/app/models/view_state.rb
+++ b/src/app/models/view_state.rb
@@ -33,6 +33,8 @@ class ViewState < ActiveRecord::Base
   set_primary_key :uuid
   belongs_to :user
 
+  attr_accessible :name, :controller, :action, :station, :association
+
   validates_presence_of :name
   validates_uniqueness_of :name
   validates_presence_of :user_id
diff --git a/src/config/application.rb b/src/config/application.rb
index 88d6d1d..21a4cd8 100644
--- a/src/config/application.rb
+++ b/src/config/application.rb
@@ -80,5 +80,7 @@ module Conductor
     #config.middleware.swap Rack::MethodOverride, Rack::RestfulSubmit
     config.middleware.insert_before(Rack::MethodOverride, Rack::RestfulSubmit)
     ActiveRecord::Base.include_root_in_json = false
+
+    config.active_record.whitelist_attributes = true
   end
 end
diff --git a/src/spec/models/instance_spec.rb b/src/spec/models/instance_spec.rb
index 87dd77d..81dd75b 100644
--- a/src/spec/models/instance_spec.rb
+++ b/src/spec/models/instance_spec.rb
@@ -171,7 +171,7 @@ describe Instance do
   end
 
   it "should return empty list of instance actions when connect to provider fails" do
-    provider = Factory.build(:mock_provider2)
+    provider = Factory.create(:mock_provider2)
     cloud_account = Factory.build(:provider_account, :provider => provider)
     cloud_account.stub!(:connect).and_return(nil)
     cloud_account.stub!(:valid_credentials?).and_return(true)
-- 
1.7.10.2




More information about the aeolus-devel mailing list