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

mzatko at redhat.com mzatko at redhat.com
Wed Oct 10 18:06:31 UTC 2012


From: Mo Morsi <mmorsi at redhat.com>

  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

Signed-off-by: Maros Zatko <mzatko at redhat.com>
---
 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                      | 3 +++
 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 +-
 36 files changed, 102 insertions(+), 1 deletion(-)

diff --git a/src/app/models/base_permission_object.rb b/src/app/models/base_permission_object.rb
index faf5e53..7e23ea8 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 811a513..9d92044 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 9062c04..920f466 100644
--- a/src/app/models/deployment.rb
+++ b/src/app/models/deployment.rb
@@ -71,6 +71,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 2eb2c78..9407857 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 9819b66..c3e5077 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 cecc6be..622920a 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 8e02002..0c7b8b6 100644
--- a/src/app/models/instance.rb
+++ b/src/app/models/instance.rb
@@ -95,6 +95,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 d5c8f64..b555a57 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 f9afd85..bf0478a 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 04736e2..03b06fc 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 3b767f7..fad6844 100644
--- a/src/app/models/pool.rb
+++ b/src/app/models/pool.rb
@@ -50,6 +50,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 a644651..5d64a87 100644
--- a/src/app/models/pool_family.rb
+++ b/src/app/models/pool_family.rb
@@ -56,6 +56,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 1c36cbf..b0ebbba 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 acc7d64..e341666 100644
--- a/src/app/models/provider_account.rb
+++ b/src/app/models/provider_account.rb
@@ -69,6 +69,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 7052253..76cf5c6 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 83e630a..e566b92 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 2989042..df85356 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 98ac974..9a2d23a 100644
--- a/src/app/models/session_entity.rb
+++ b/src/app/models/session_entity.rb
@@ -19,6 +19,9 @@ class SessionEntity < ActiveRecord::Base
   belongs_to :entity
   belongs_to :permission_session
 
+  attr_accessible :user, :session, :entity,
+                  :user_id, :session_id, :entity_id
+
   validates_presence_of :user_id
   validates_presence_of :permission_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 5db8e95..6258d0b 100644
--- a/src/app/models/user.rb
+++ b/src/app/models/user.rb
@@ -79,6 +79,9 @@ class User < ActiveRecord::Base
 
   validate :validate_ldap_changes, :if => Proc.new {|user|
     !user.new_record? && SETTINGS_CONFIG[:auth][:strategy] == "ldap"}
+
+  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 9aae611..8412dfd 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 bc1f0fe..bfa2171 100644
--- a/src/app/models/view_state.rb
+++ b/src/app/models/view_state.rb
@@ -33,6 +33,8 @@ class ViewState < ActiveRecord::Base
   self.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 7a314f1..74bcba7 100644
--- a/src/spec/models/instance_spec.rb
+++ b/src/spec/models/instance_spec.rb
@@ -172,7 +172,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.11.4




More information about the aeolus-devel mailing list