From e029ad7b0cc68ecb36a752a1027cbeff3312e80d Mon Sep 17 00:00:00 2001 From: Drew Ulmer Date: Tue, 25 Jun 2013 13:44:39 -0500 Subject: [PATCH] Fix improper login param sanitization permit This includes a failing test case that hooks into ActiveSupport Notifications to catch the param permit error. --- lib/devise/parameter_sanitizer.rb | 8 ++++++-- test/controllers/sessions_controller_test.rb | 14 ++++++++++++++ test/parameter_sanitizer_test.rb | 9 ++++++++- 3 files changed, 28 insertions(+), 3 deletions(-) diff --git a/lib/devise/parameter_sanitizer.rb b/lib/devise/parameter_sanitizer.rb index a05a49c1..aa78b21f 100644 --- a/lib/devise/parameter_sanitizer.rb +++ b/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 diff --git a/test/controllers/sessions_controller_test.rb b/test/controllers/sessions_controller_test.rb index 7af0a686..0bfc8be8 100644 --- a/test/controllers/sessions_controller_test.rb +++ b/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] diff --git a/test/parameter_sanitizer_test.rb b/test/parameter_sanitizer_test.rb index 5043dd06..34f58392 100644 --- a/test/parameter_sanitizer_test.rb +++ b/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