Introduce BaseSanitizer null sanitizer and controller-specific callbacks

This updates Devise's StrongParameter support to feature:

- A Null base sanitizer to support existing Rails 3.x installations that
  don't want to use StrongParameters yet
- A new, simpler API for ParameterSanitizer: #permit, #permit!, and #forbid
- Overrideable callbacks on a controller-basis, e.g. #create_sessions_params
  for passing the current scope's parameters through StrongParameters and
  a helper method, whitelisted_params, for rolling your own implementations
  of #create_x_params in your own controllers.
- Lots of tests!

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(resource_params)
+    self.resource = resource_class.send_confirmation_instructions(create_confirmation_params)
 
     if successfully_sent?(resource)
       respond_with({}, :location => after_resending_confirmation_instructions_path_for(resource_name))
@@ -39,4 +39,8 @@ class Devise::ConfirmationsController < DeviseController
     def after_confirmation_path_for(resource_name, resource)
       after_sign_in_path_for(resource)
     end
+
+    def create_confirmation_params
+      whitelisted_params(:confirmations)
+    end
 end

app/controllers/devise/registrations_controller.rb

@@ -10,7 +10,7 @@ class Devise::RegistrationsController < DeviseController
 
   # POST /resource
   def create
-    build_resource
+    build_resource(create_registration_params)
 
     if resource.save
       if resource.active_for_authentication?
@@ -40,7 +40,7 @@ class Devise::RegistrationsController < DeviseController
     self.resource = resource_class.to_adapter.get!(send(:"current_#{resource_name}").to_key)
     prev_unconfirmed_email = resource.unconfirmed_email if resource.respond_to?(:unconfirmed_email)
 
-    if resource.update_with_password(resource_params)
+    if resource.update_with_password(update_resource_params)
       if is_navigational_format?
         flash_key = update_needs_confirmation?(resource, prev_unconfirmed_email) ?
           :update_needs_confirmation : :updated
@@ -83,11 +83,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)
-    if request.get?
-      hash ||= {}
-    else
-      hash ||= resource_params || {}
-    end
+    hash ||= {}
     self.resource = resource_class.new_with_session(hash, session)
   end
 
@@ -120,4 +116,12 @@ class Devise::RegistrationsController < DeviseController
     send(:"authenticate_#{resource_name}!", :force => true)
     self.resource = send(:"current_#{resource_name}")
   end
+
+  def create_registration_params
+    whitelisted_params(:registrations)
+  end
+
+  def update_resource_params
+    whitelisted_params(:registrations)
+  end
 end

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(resource_params)
+    self.resource = resource_class.send_unlock_instructions(create_unlock_params)
 
     if successfully_sent?(resource)
       respond_with({}, :location => after_sending_unlock_instructions_path_for(resource))
@@ -40,4 +40,8 @@ class Devise::UnlocksController < DeviseController
     def after_unlock_path_for(resource)
       new_session_path(resource)
     end
+
+    def create_unlock_params
+      whitelisted_params(:unlocks)
+    end
 end

app/controllers/devise_controller.rb

@@ -95,7 +95,7 @@ MESSAGE
     # When building a resource, invoke strong_parameters require/permit
     # steps if the params hash includes the resource name.
     if params[resource_name]
-      hash ||= resource_params || {}
+      hash ||= whitelisted_params(controller_name) || {}
     else
       hash ||= {}
     end
@@ -187,17 +187,26 @@ MESSAGE
   # Setup a param sanitizer to filter parameters using strong_parameters. See
   # lib/devise/controllers/parameter_sanitizer.rb for more info. Override this
   # method in your application controller to use your own parameter sanitizer.
-  def devise_parameters_sanitizer
-    @devise_parameters_sanitizer ||= Devise::ParameterSanitizer.new
+  def devise_parameter_sanitizer
+    return super if defined?(super)
+    @devise_parameter_sanitizer ||= if defined?(ActionController::StrongParameters)
+      Devise::ParameterSanitizer.new(resource_name, params)
+    else
+      Devise::BaseSanitizer.new(resource_name, params)
+    end
   end
 
   # Return the params to be used for mass assignment passed through the
   # strong_parameters require/permit step. To customize the parameters
   # permitted for a specific controller, simply prepend a before_filter and
-  # call #permit_devise_param or #remove_permitted_devise_param on
-  # parameters_sanitizer to update the default allowed lists of permitted
-  # parameters.
+  # call #permit, #permit! or #forbid on devise_parameters_sanitizer to update
+  # the default allowed lists of permitted parameters for a specific
+  # controller/action combination.
+  def whitelisted_params(contr_name)
+    devise_parameter_sanitizer.sanitize_for(contr_name)
+  end
+
   def resource_params
-    params.require(resource_name).permit(devise_parameters_sanitizer.permitted_params_for(controller_name))
+    params.fetch(resource_name, {})
   end
 end

lib/devise.rb

@@ -10,6 +10,7 @@ module Devise
   autoload :FailureApp,         'devise/failure_app'
   autoload :OmniAuth,           'devise/omniauth'
   autoload :ParamFilter,        'devise/param_filter'
+  autoload :BaseSanitizer,      'devise/parameter_sanitizer'
   autoload :ParameterSanitizer, 'devise/parameter_sanitizer'
   autoload :TestHelpers,        'devise/test_helpers'
   autoload :TimeInflector,      'devise/time_inflector'

lib/devise/parameter_sanitizer.rb

@@ -1,23 +1,42 @@
 module Devise
-  class ParameterSanitizer
-    attr_reader :allowed_params
+  class BaseSanitizer
+    attr_reader :params, :resource_name, :allowed_params
 
-    # Return a list of parameter names permitted to be mass-assigned for the
-    # passed controller.
-    def permitted_params_for(controller_name)
-      allowed_params.fetch(key_for_controller_name(controller_name), [])
+    def initialize(resource_name, params)
+      @resource_name, @params = resource_name, params
+      @allowed_params = {}
+    end
+
+    def default_params
+      params.fetch(resource_name, {})
+    end
+
+    def sanitize_for(controller)
+      default_params
+    end
+  end
+
+  class ParameterSanitizer < BaseSanitizer
+    # Return the allowed parameters passed through the StrongParametesr
+    # require/permit step according to the allowed_params setup via
+    # #permit, #permit!, #forbid, and any defaults.
+    def sanitize_for(controller)
+      permitted_params = allowed_params.fetch(param_key(controller), []).to_a
+
+      params.require(resource_name).permit(permitted_params)
     end
 
     # Set up a new parameter sanitizer with a set of allowed parameters. This
     # gets initialized on each request so that parameters may be augmented or
     # changed as needed via before_filter.
-    def initialize
+    def initialize(resource_name, params)
+      super
       @allowed_params = {
-        :confirmations_controller => [:email],
-        :passwords_controller => authentication_keys + [:password, :password_confirmation, :reset_password_token],
-        :registrations_controller => authentication_keys + [:password, :password_confirmation, :current_password],
-        :sessions_controller => authentication_keys + [:password],
-        :unlocks_controller => [:email]
+        :confirmations => [:email],
+        :passwords     => auth_keys | [:password, :password_confirmation, :reset_password_token],
+        :registrations => auth_keys | [:password, :password_confirmation, :current_password],
+        :sessions      => auth_keys | [:password],
+        :unlocks       => [:email]
       }
     end
 
@@ -27,38 +46,41 @@ module Devise
     # that when adding a new controller, use the full controller name
     # (:confirmations_controller) and not the short names
     # (:confirmation/:confirmations).
-    def permit_devise_param(controller_name, param_name)
-      @allowed_params[key_for_controller_name(controller_name)] << param_name
+    def permit(controller_name, *param_names)
+      @allowed_params[param_key(controller_name)] |= param_names
+      true
+    end
+
+    def permit!(controller_name, *param_names)
+      @allowed_params[param_key(controller_name)] = param_names
       true
     end
 
     # Remove specific allowed parameter for a Devise controller. If the
     # controller_name doesn't exist in allowed_params, it will be added to it
     # as an empty array.
-    def remove_permitted_devise_param(controller_name, param_name)
-      @allowed_params[key_for_controller_name(controller_name)].delete(param_name)
+    def forbid(controller_name, *param_names)
+      @allowed_params[param_key(controller_name)] -= param_names
       true
     end
 
     protected
 
-    def authentication_keys
+    def auth_keys
       Array(::Devise.authentication_keys)
     end
 
     # Flexibly allow access to permitting/denying/checking parameters by
     # controller name in the following key formats: :confirmations_controller,
     # :confirmations, :confirmation
-    def key_for_controller_name(name)
-      if allowed_params.has_key?(name.to_sym)
-        name.to_sym
-      elsif allowed_params.has_key?(:"#{name}s_controller")
-        :"#{name}s_controller"
-      elsif allowed_params.has_key?(:"#{name}_controller")
-        :"#{name}_controller"
+    def param_key(controller_name)
+      k = controller_name.to_sym
+
+      if allowed_params.has_key?(k)
+        k
       else
-        @allowed_params[name.to_sym] = []
-        name.to_sym
+        @allowed_params[k] = []
+        k
       end
     end
   end

test/parameter_sanitizer_test.rb

@@ -1,52 +1,112 @@
 require 'test_helper'
+require 'devise/parameter_sanitizer'
 
-class ParameterSanitizerTest < ActiveSupport::TestCase
+class BaseSanitizerTest < ActiveSupport::TestCase
   def sanitizer
-    Devise::ParameterSanitizer.new
+    @sanitizer ||= Devise::BaseSanitizer.new(:user, {})
   end
 
-  test '#permitted_params_for allows querying of allowed parameters by controller' do
-    assert_equal [:email], sanitizer.permitted_params_for(:confirmations_controller)
-    assert_equal [:email, :password, :password_confirmation, :reset_password_token], sanitizer.permitted_params_for(:password)
-    assert_equal [:email], sanitizer.permitted_params_for(:unlocks)
-  end
-
-  test '#permitted_params_for returns an empty array for a bad key' do
-    assert_equal [], sanitizer.permitted_params_for(:bad_key)
-  end
-
-  test '#permit_devise_param allows adding an allowed param for a specific controller' do
-    subject = sanitizer
-
-    subject.permit_devise_param(:confirmations_controller, :other)
-
-    assert_equal [:email, :other], subject.permitted_params_for(:confirmations_controller)
-  end
-
-  test '#remove_permitted_devise_param allows disallowing a param for a specific controller' do
-    subject = sanitizer
-
-    subject.remove_permitted_devise_param(:confirmations_controller, :email)
-
-    assert_equal [], subject.permitted_params_for(:confirmations_controller)
-  end
-
-  test '#permit_devise_param allows adding additional devise controllers' do
-    subject = sanitizer
-
-    subject.permit_devise_param(:invitations_controller, :email)
-
-    assert_equal [:email], subject.permitted_params_for(:invitations)
-  end
-
-  test '#remove_permitted_devise_param fails gracefully when removing a missing param' do
-    subject = sanitizer
-
-    # perform twice, just to be sure it handles it gracefully
-    subject.remove_permitted_devise_param(:invitations_controller, :email)
-    subject.remove_permitted_devise_param(:invitations_controller, :email)
-
-    assert_equal [], subject.permitted_params_for(:invitations)
+  test '#default_params returns the params passed in' do
+    assert_equal({}, sanitizer.default_params)
+  end
+end
+
+if defined?(ActionController::StrongParameters)
+
+  require 'active_model/forbidden_attributes_protection'
+
+  class ParameterSanitizerTest < ActiveSupport::TestCase
+    def sanitizer(p={})
+      @sanitizer ||= Devise::ParameterSanitizer.new(:user, p)
+    end
+
+    test '#permit allows adding an allowed param for a specific controller' do
+      sanitizer.permit(:confirmations, :other)
+
+      assert_equal [:email, :other], sanitizer.allowed_params[:confirmations]
+    end
+
+    test '#permit allows adding multiple allowed params for a specific controller' do
+      sanitizer.permit(:confirmations, :other, :testing)
+
+      assert_equal [:email, :other, :testing], sanitizer.allowed_params[:confirmations]
+    end
+
+    test '#permit! overrides allowed params for a specific controller' do
+      sanitizer.permit!(:confirmations, :other, :testing)
+
+      assert_equal [:other, :testing], sanitizer.allowed_params[:confirmations]
+    end
+
+    test '#forbid allows disallowing a param for a specific controller' do
+      sanitizer.forbid(:confirmations, :email)
+
+      assert_equal [], sanitizer.allowed_params[:confirmations]
+    end
+
+    test '#forbid allows disallowing multiple params for a specific controller' do
+      sanitizer.forbid(:sessions, :email, :password)
+
+      assert_equal [], sanitizer.allowed_params[:sessions]
+    end
+
+    test '#permit allows adding additional devise controllers' do
+      sanitizer.permit(:invitations, :email)
+
+      assert_equal [:email], sanitizer.allowed_params[:invitations]
+    end
+
+    test '#permit allows adding additional devise controllers with multiple params' do
+      sanitizer.permit(:invitations, :email, :pin)
+
+      assert_includes sanitizer.allowed_params[:invitations], :pin
+      assert_includes sanitizer.allowed_params[:invitations], :email
+    end
+
+    test '#forbid fails gracefully when removing a missing param' do
+      # perform twice, just to be sure it handles it gracefully
+      sanitizer.forbid(:invitations, :email)
+      sanitizer.forbid(:invitations, :email)
+
+      assert_equal [], sanitizer.allowed_params[:invitations]
+    end
+
+    test '#forbid fails gracefully when removing multiple missing params' do
+      # perform twice, just to be sure it handles it gracefully
+      sanitizer.forbid(:invitations, :email, :badkey)
+      sanitizer.forbid(:invitations, :email, :badkey)
+
+      assert_equal [], sanitizer.allowed_params[:invitations]
+    end
+
+    test '#sanitize_for tries to require the resource name on params' do
+      params = ActionController::Parameters.new({:admin => {}})
+
+      assert_raises ActionController::ParameterMissing do
+        sanitizer(params).sanitize_for(:sessions)
+      end
+    end
+
+    test '#sanitize_for performs the permit step of strong_parameters, restricting passed attributes' do
+      params = ActionController::Parameters.new({:user => {:admin => true}})
+
+      # removes the admin flag
+      assert_equal({}, sanitizer(params).sanitize_for(:sessions))
+    end
+
+    test '#sanitize_for respects any updates to allowed_params' do
+      params = ActionController::Parameters.new({:user => {:admin => true}})
+      sanitizer(params).permit(:sessions, :admin)
+
+      assert_equal({'admin' => true}, sanitizer(params).sanitize_for(:sessions))
+    end
+
+    test '#sanitize_for works with newly added controllers' do
+      params = ActionController::Parameters.new({:user => {:email => 'abc@example.com', :pin => '1234'}})
+      sanitizer(params).permit(:invitations, :email, :pin)
+
+      assert_equal({'email' => 'abc@example.com', 'pin' => '1234'}, sanitizer(params).sanitize_for(:invitations))
+    end
   end
 end