Merge pull request #2452 from latortuga/bad-strong-params-on-failed-login

Add failing test case for bad parameter sanitizing when failing to login

lib/devise/parameter_sanitizer.rb

@@ -40,8 +40,12 @@ module Devise
       end
     end
 
+    # These are the params used to sign in a user so we don't need to
+    # mass-assign the password param in order to authenticate. Excluding it
+    # here allows us to construct a new user without sensitive information if
+    # authentication fails.
     def sign_in
-      default_params.permit(*auth_keys)
+      default_params.permit(*auth_keys + [:password])
     end
 
     def sign_up
@@ -53,7 +57,7 @@ module Devise
     end
 
     def auth_keys
-      resource_class.authentication_keys
+      resource_class.authentication_keys.respond_to?(:keys) ? resource_class.authentication_keys.keys : resource_class.authentication_keys
     end
   end
 end

test/controllers/sessions_controller_test.rb

@@ -4,6 +4,20 @@ class SessionsControllerTest < ActionController::TestCase
   tests Devise::SessionsController
   include Devise::TestHelpers
 
+  test "#create doesn't raise unpermitted params when sign in fails" do
+    ActiveSupport::Notifications.subscribe /unpermitted_parameters/ do |name, start, finish, id, payload|
+      flunk "Unpermitted params: #{payload}"
+    end
+    request.env["devise.mapping"] = Devise.mappings[:user]
+    request.session["user_return_to"] = 'foo.bar'
+    user = create_user
+    post :create, :user => {
+      :email => "wrong@email.com",
+      :password => "wrongpassword"
+    }
+    assert_equal 200, @response.status
+  end
+
   test "#create works even with scoped views" do
     swap Devise, :scoped_views => true do
       request.env["devise.mapping"] = Devise.mappings[:user]

test/parameter_sanitizer_test.rb

@@ -22,7 +22,14 @@ if defined?(ActionController::StrongParameters)
 
     test 'filters some parameters on sign in by default' do
       sanitizer = sanitizer(user: { "email" => "jose", "password" => "invalid" })
-      assert_equal({ "email" => "jose" }, sanitizer.for(:sign_in))
+      assert_equal({ "email" => "jose", "password" => "invalid" }, sanitizer.for(:sign_in))
+    end
+
+    test 'handles auth keys as a hash' do
+      swap Devise, :authentication_keys => {:email => true} do
+        sanitizer = sanitizer(user: { "email" => "jose", "password" => "invalid" })
+        assert_equal({ "email" => "jose", "password" => "invalid" }, sanitizer.for(:sign_in))
+      end
     end
 
     test 'filters some parameters on sign up by default' do