From 3511f8ac00fcff982c5287326648fe57b4b79c93 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?V=C3=ADctor=20Manuel=20Cruz=20Due=C3=B1as?= Date: Thu, 5 Jul 2012 19:10:34 +0200 Subject: [PATCH 1/4] Checking if unconfirmed_email has changed before to set update_needs_confirmation flash message. --- .../devise/registrations_controller.rb | 3 ++- test/integration/registerable_test.rb | 21 +++++++++++++++++++ 2 files changed, 23 insertions(+), 1 deletion(-) diff --git a/app/controllers/devise/registrations_controller.rb b/app/controllers/devise/registrations_controller.rb index b18e9f69..eae3c384 100644 --- a/app/controllers/devise/registrations_controller.rb +++ b/app/controllers/devise/registrations_controller.rb @@ -38,10 +38,11 @@ class Devise::RegistrationsController < DeviseController # the current user in place. def update 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 is_navigational_format? - if resource.respond_to?(:pending_reconfirmation?) && resource.pending_reconfirmation? + if resource.respond_to?(:pending_reconfirmation?) && resource.pending_reconfirmation? && (prev_unconfirmed_email != resource.unconfirmed_email) flash_key = :update_needs_confirmation end set_flash_message :notice, flash_key || :updated diff --git a/test/integration/registerable_test.rb b/test/integration/registerable_test.rb index 28953b45..3028b6bb 100644 --- a/test/integration/registerable_test.rb +++ b/test/integration/registerable_test.rb @@ -321,4 +321,25 @@ class ReconfirmableRegistrationTest < ActionController::IntegrationTest assert Admin.first.valid_password?('pas123') end + + test 'a signed in admin should not see a reconfirmation message if he did not change his email, despite having an unconfirmed email' do + sign_in_as_admin + + get edit_admin_registration_path + fill_in 'email', :with => 'admin.new@example.com' + fill_in 'current password', :with => '123456' + click_button 'Update' + + get edit_admin_registration_path + fill_in 'password', :with => 'pas123' + fill_in 'password confirmation', :with => 'pas123' + fill_in 'current password', :with => '123456' + click_button 'Update' + + assert_current_url '/admin_area/home' + assert_contain 'You updated your account successfully.' + + assert_equal "admin.new@example.com", Admin.first.unconfirmed_email + assert Admin.first.valid_password?('pas123') + end end \ No newline at end of file From d6d61fc5be9183a30d517350f6a8b0f4eb3e003c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Victor=20Cruz=20Due=C3=B1as?= Date: Sun, 5 May 2013 01:02:48 +0200 Subject: [PATCH 2/4] Adding tests for case_insensitive_keys and strip_whitespace_keys to param filter --- test/models/database_authenticatable_test.rb | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/test/models/database_authenticatable_test.rb b/test/models/database_authenticatable_test.rb index 1439a51f..01fded85 100644 --- a/test/models/database_authenticatable_test.rb +++ b/test/models/database_authenticatable_test.rb @@ -52,6 +52,18 @@ class DatabaseAuthenticatableTest < ActiveSupport::TestCase assert_equal( { "login" => "foo@bar.com", "bool1" => "true", "bool2" => "false", "fixnum" => "123", "will_be_converted" => "1..10" }, conditions) end + test 'param filter should filter case_insensitive_keys as insensitive' do + conditions = {'insensitive' => 'insensitive_VAL', 'sensitive' => 'sensitive_VAL'} + conditions = Devise::ParamFilter.new(['insensitive'], []).filter(conditions) + assert_equal( {'insensitive' => 'insensitive_val', 'sensitive' => 'sensitive_VAL'}, conditions ) + end + + test 'param filter should filter strip_whitespace_keys stripping whitespaces' do + conditions = {'strip_whitespace' => ' strip_whitespace_val ', 'do_not_strip_whitespace' => ' do_not_strip_whitespace_val '} + conditions = Devise::ParamFilter.new([], ['strip_whitespace']).filter(conditions) + assert_equal( {'strip_whitespace' => 'strip_whitespace_val', 'do_not_strip_whitespace' => ' do_not_strip_whitespace_val '}, conditions ) + end + test 'should respond to password and password confirmation' do user = new_user assert user.respond_to?(:password) From ae48fc8419e81fc6fdb65e45520137670e03d8e9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Victor=20Cruz=20Due=C3=B1as?= Date: Sun, 5 May 2013 01:19:37 +0200 Subject: [PATCH 3/4] Refactor to avoid duplication on param filter --- lib/devise/param_filter.rb | 20 +++++++++----------- 1 file changed, 9 insertions(+), 11 deletions(-) diff --git a/lib/devise/param_filter.rb b/lib/devise/param_filter.rb index 759518fa..29d083dc 100644 --- a/lib/devise/param_filter.rb +++ b/lib/devise/param_filter.rb @@ -8,21 +8,19 @@ module Devise def filter(conditions) conditions = stringify_params(conditions.dup) - @case_insensitive_keys.each do |k| - value = conditions[k] - next unless value.respond_to?(:downcase) - conditions[k] = value.downcase - end - - @strip_whitespace_keys.each do |k| - value = conditions[k] - next unless value.respond_to?(:strip) - conditions[k] = value.strip - end + apply_filter_method_to_condition_keys(conditions, :downcase, @case_insensitive_keys) + apply_filter_method_to_condition_keys(conditions, :strip, @strip_whitespace_keys) conditions end + def apply_filter_method_to_condition_keys(conditions, method, condition_keys) + condition_keys.each do |k| + value = conditions[k] + conditions[k] = value.send(method) if value.respond_to?(method) + end + end + # Force keys to be string to avoid injection on mongoid related database. def stringify_params(conditions) return conditions unless conditions.is_a?(Hash) From 75fdd2944d886bb8b4aa24173279d55d46a5f5be Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Victor=20Cruz=20Due=C3=B1as?= Date: Sun, 5 May 2013 10:12:57 +0200 Subject: [PATCH 4/4] Avoid hash mutation --- lib/devise/param_filter.rb | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/lib/devise/param_filter.rb b/lib/devise/param_filter.rb index 29d083dc..46b9ddd6 100644 --- a/lib/devise/param_filter.rb +++ b/lib/devise/param_filter.rb @@ -8,17 +8,19 @@ module Devise def filter(conditions) conditions = stringify_params(conditions.dup) - apply_filter_method_to_condition_keys(conditions, :downcase, @case_insensitive_keys) - apply_filter_method_to_condition_keys(conditions, :strip, @strip_whitespace_keys) + conditions.merge!(filtered_hash_by_method_for_given_keys(conditions.dup, :downcase, @case_insensitive_keys)) + conditions.merge!(filtered_hash_by_method_for_given_keys(conditions.dup, :strip, @strip_whitespace_keys)) conditions end - def apply_filter_method_to_condition_keys(conditions, method, condition_keys) + def filtered_hash_by_method_for_given_keys(conditions, method, condition_keys) condition_keys.each do |k| value = conditions[k] conditions[k] = value.send(method) if value.respond_to?(method) end + + conditions end # Force keys to be string to avoid injection on mongoid related database.