From 7ec4c1424d6a589cbf47fa9cd208bff33ef09e33 Mon Sep 17 00:00:00 2001 From: Adam Meehan Date: Tue, 15 May 2012 18:07:02 +1000 Subject: [PATCH] Add resource_params internal helper to param filtering In light of recent discussions around mass assignment security and the alternate solution of using the controller to filter params, not the model, a hook/helper is needed to be able to override how the params are filtered before they are used to build the resource. --- app/controllers/devise/confirmations_controller.rb | 2 +- app/controllers/devise/passwords_controller.rb | 4 ++-- app/controllers/devise/registrations_controller.rb | 4 ++-- app/controllers/devise/unlocks_controller.rb | 2 +- app/controllers/devise_controller.rb | 8 ++++++-- test/controllers/internal_helpers_test.rb | 7 +++++++ 6 files changed, 19 insertions(+), 8 deletions(-) diff --git a/app/controllers/devise/confirmations_controller.rb b/app/controllers/devise/confirmations_controller.rb index e23540f6..68014c92 100644 --- a/app/controllers/devise/confirmations_controller.rb +++ b/app/controllers/devise/confirmations_controller.rb @@ -6,7 +6,7 @@ class Devise::ConfirmationsController < DeviseController # POST /resource/confirmation def create - self.resource = resource_class.send_confirmation_instructions(params[resource_name]) + self.resource = resource_class.send_confirmation_instructions(resource_params) if successfully_sent?(resource) respond_with({}, :location => after_resending_confirmation_instructions_path_for(resource_name)) diff --git a/app/controllers/devise/passwords_controller.rb b/app/controllers/devise/passwords_controller.rb index 2e49bc59..ba960e87 100644 --- a/app/controllers/devise/passwords_controller.rb +++ b/app/controllers/devise/passwords_controller.rb @@ -8,7 +8,7 @@ class Devise::PasswordsController < DeviseController # POST /resource/password def create - self.resource = resource_class.send_reset_password_instructions(params[resource_name]) + self.resource = resource_class.send_reset_password_instructions(resource_params) if successfully_sent?(resource) respond_with({}, :location => after_sending_reset_password_instructions_path_for(resource_name)) @@ -25,7 +25,7 @@ class Devise::PasswordsController < DeviseController # PUT /resource/password def update - self.resource = resource_class.reset_password_by_token(params[resource_name]) + self.resource = resource_class.reset_password_by_token(resource_params) if resource.errors.empty? flash_message = resource.active_for_authentication? ? :updated : :updated_not_active diff --git a/app/controllers/devise/registrations_controller.rb b/app/controllers/devise/registrations_controller.rb index 7a3786b6..b18e9f69 100644 --- a/app/controllers/devise/registrations_controller.rb +++ b/app/controllers/devise/registrations_controller.rb @@ -39,7 +39,7 @@ class Devise::RegistrationsController < DeviseController def update self.resource = resource_class.to_adapter.get!(send(:"current_#{resource_name}").to_key) - if resource.update_with_password(params[resource_name]) + if resource.update_with_password(resource_params) if is_navigational_format? if resource.respond_to?(:pending_reconfirmation?) && resource.pending_reconfirmation? flash_key = :update_needs_confirmation @@ -77,7 +77,7 @@ class Devise::RegistrationsController < DeviseController # Build a devise resource passing in the session. Useful to move # temporary session data to the newly created user. def build_resource(hash=nil) - hash ||= params[resource_name] || {} + hash ||= resource_params || {} self.resource = resource_class.new_with_session(hash, session) end diff --git a/app/controllers/devise/unlocks_controller.rb b/app/controllers/devise/unlocks_controller.rb index 9c9ffdf7..45f6b2c1 100644 --- a/app/controllers/devise/unlocks_controller.rb +++ b/app/controllers/devise/unlocks_controller.rb @@ -8,7 +8,7 @@ class Devise::UnlocksController < DeviseController # POST /resource/unlock def create - self.resource = resource_class.send_unlock_instructions(params[resource_name]) + self.resource = resource_class.send_unlock_instructions(resource_params) if successfully_sent?(resource) respond_with({}, :location => after_sending_unlock_instructions_path_for(resource)) diff --git a/app/controllers/devise_controller.rb b/app/controllers/devise_controller.rb index 95de1b72..f62674be 100644 --- a/app/controllers/devise_controller.rb +++ b/app/controllers/devise_controller.rb @@ -5,7 +5,7 @@ class DeviseController < Devise.parent_controller.constantize helper DeviseHelper helpers = %w(resource scope_name resource_name signed_in_resource - resource_class devise_mapping) + resource_class resource_params devise_mapping) hide_action *helpers helper_method *helpers @@ -28,6 +28,10 @@ class DeviseController < Devise.parent_controller.constantize devise_mapping.to end + def resource_params + params[resource_name] + end + # Returns a signed in resource from session (if one exists) def signed_in_resource warden.authenticate(:scope => resource_name) @@ -81,7 +85,7 @@ MESSAGE # Build a devise resource. # Assignment bypasses attribute protection when :unsafe option is passed def build_resource(hash = nil, options = {}) - hash ||= params[resource_name] || {} + hash ||= resource_params || {} if options[:unsafe] self.resource = resource_class.new.tap do |resource| diff --git a/test/controllers/internal_helpers_test.rb b/test/controllers/internal_helpers_test.rb index 46e0c1ce..504aa90b 100644 --- a/test/controllers/internal_helpers_test.rb +++ b/test/controllers/internal_helpers_test.rb @@ -33,6 +33,13 @@ class HelpersTest < ActionController::TestCase assert_equal user, @controller.instance_variable_get(:@user) end + test 'get resource params from request params using resource name as key' do + user_params = {'name' => 'Shirley Templar'} + @controller.stubs(:params).returns(HashWithIndifferentAccess.new({'user' => user_params})) + + assert_equal user_params, @controller.resource_params + end + test 'resources methods are not controller actions' do assert @controller.class.action_methods.empty? end