From 8e87a2d80d5304e21d76147fb8e45cf1ae0d5932 Mon Sep 17 00:00:00 2001 From: Stefan Wrobel Date: Fri, 10 Jun 2011 01:37:43 -0700 Subject: [PATCH] Add strip_whitespace_keys which works like case_insensitive_keys but strips whitespace from emails --- lib/devise.rb | 5 ++++ lib/devise/models/authenticatable.rb | 4 ++- lib/devise/models/database_authenticatable.rb | 5 ++++ lib/generators/templates/devise.rb | 5 ++++ .../database_authenticatable_test.rb | 22 ++++++++++++++++ test/integration/recoverable_test.rb | 26 +++++++++++++++++++ test/models/database_authenticatable_test.rb | 10 +++++++ test/rails_app/config/initializers/devise.rb | 5 ++++ 8 files changed, 81 insertions(+), 1 deletion(-) diff --git a/lib/devise.rb b/lib/devise.rb index f5d9bcfb..0532ea84 100644 --- a/lib/devise.rb +++ b/lib/devise.rb @@ -79,6 +79,11 @@ module Devise # False by default for backwards compatibility. mattr_accessor :case_insensitive_keys @@case_insensitive_keys = false + + # Keys that should have whitespace stripped. + # False by default for backwards compatibility. + mattr_accessor :strip_whitespace_keys + @@strip_whitespace_keys = false # If http authentication is enabled by default. mattr_accessor :http_authenticatable diff --git a/lib/devise/models/authenticatable.rb b/lib/devise/models/authenticatable.rb index 0169b6f9..8a20f0b3 100644 --- a/lib/devise/models/authenticatable.rb +++ b/lib/devise/models/authenticatable.rb @@ -90,7 +90,7 @@ module Devise end module ClassMethods - Devise::Models.config(self, :authentication_keys, :request_keys, :case_insensitive_keys, :http_authenticatable, :params_authenticatable) + Devise::Models.config(self, :authentication_keys, :request_keys, :strip_whitespace_keys, :case_insensitive_keys, :http_authenticatable, :params_authenticatable) def params_authenticatable?(strategy) params_authenticatable.is_a?(Array) ? @@ -115,6 +115,7 @@ module Devise def find_for_authentication(conditions) conditions = filter_auth_params(conditions.dup) (case_insensitive_keys || []).each { |k| conditions[k].try(:downcase!) } + (strip_whitespace_keys || []).each { |k| conditions[k].try(:strip!) } to_adapter.find_first(conditions) end @@ -126,6 +127,7 @@ module Devise # Find an initialize a group of attributes based on a list of required attributes. def find_or_initialize_with_errors(required_attributes, attributes, error=:invalid) #:nodoc: (case_insensitive_keys || []).each { |k| attributes[k].try(:downcase!) } + (strip_whitespace_keys || []).each { |k| conditions[k].try(:strip!) } attributes = attributes.slice(*required_attributes) attributes.delete_if { |key, value| value.blank? } diff --git a/lib/devise/models/database_authenticatable.rb b/lib/devise/models/database_authenticatable.rb index ad1fdbb2..9763cec0 100644 --- a/lib/devise/models/database_authenticatable.rb +++ b/lib/devise/models/database_authenticatable.rb @@ -23,6 +23,7 @@ module Devise attr_reader :password, :current_password attr_accessor :password_confirmation before_validation :downcase_keys + before_validation :strip_whitespace end # Generates password encryption based on the given value. @@ -81,6 +82,10 @@ module Devise def downcase_keys (self.class.case_insensitive_keys || []).each { |k| self[k].try(:downcase!) } end + + def strip_whitespace + (self.class.strip_whitespace_keys || []).each { |k| self[k].try(:strip!) } + end # Digests the password using bcrypt. def password_digest(password) diff --git a/lib/generators/templates/devise.rb b/lib/generators/templates/devise.rb index 61d82fc9..92e19676 100644 --- a/lib/generators/templates/devise.rb +++ b/lib/generators/templates/devise.rb @@ -35,6 +35,11 @@ Devise.setup do |config| # These keys will be downcased upon creating or modifying a user and when used # to authenticate or find a user. Default is :email. config.case_insensitive_keys = [ :email ] + + # Configure which authentication keys should have whitespace stripped. + # These keys will have whitespace before and after removed upon creating or + # modifying a user and when used to authenticate or find a user. Default is :email. + config.strip_whitespace_keys = [ :email ] # Tell if authentication through request.params is enabled. True by default. # config.params_authenticatable = true diff --git a/test/integration/database_authenticatable_test.rb b/test/integration/database_authenticatable_test.rb index 1fe84c17..76d5a28e 100644 --- a/test/integration/database_authenticatable_test.rb +++ b/test/integration/database_authenticatable_test.rb @@ -22,6 +22,28 @@ class DatabaseAuthenticationTest < ActionController::IntegrationTest assert_not warden.authenticated?(:user) end end + + test 'sign in with email including extra spaces should succeed when email is in the list of strip whitespace keys' do + create_user(:email => ' foo@bar.com ') + + sign_in_as_user do + fill_in 'email', :with => 'foo@bar.com' + end + + assert warden.authenticated?(:user) + end + + test 'sign in with email including extra spaces should fail when email is NOT the list of strip whitespace keys' do + swap Devise, :case_insensitive_keys => [] do + create_user(:email => ' foo@bar.com ') + + sign_in_as_user do + fill_in 'email', :with => 'foo@bar.com' + end + + assert_not warden.authenticated?(:user) + end + end test 'sign in should not authenticate if not using proper authentication keys' do swap Devise, :authentication_keys => [:username] do diff --git a/test/integration/recoverable_test.rb b/test/integration/recoverable_test.rb index 27a3da3f..b240a50b 100644 --- a/test/integration/recoverable_test.rb +++ b/test/integration/recoverable_test.rb @@ -52,6 +52,32 @@ class PasswordTest < ActionController::IntegrationTest assert_contain 'not found' end end + + test 'reset password with email with extra whitespace should succeed when email is in the list of strip whitespace keys' do + create_user(:email => ' foo@bar.com ') + + request_forgot_password do + fill_in 'email', :with => 'foo@bar.com' + end + + assert_current_url '/users/sign_in' + assert_contain 'You will receive an email with instructions about how to reset your password in a few minutes.' + end + + test 'reset password with email with extra whitespace should fail when email is NOT the list of strip whitespace keys' do + swap Devise, :case_insensitive_keys => [] do + create_user(:email => ' foo@bar.com ') + + request_forgot_password do + fill_in 'email', :with => 'foo@bar.com' + end + + assert_response :success + assert_current_url '/users/password' + assert_have_selector "input[type=email][value='foo@bar.com']" + assert_contain 'not found' + end + end test 'authenticated user should not be able to visit forgot password page' do sign_in_as_user diff --git a/test/models/database_authenticatable_test.rb b/test/models/database_authenticatable_test.rb index 3beab90d..844f6db8 100644 --- a/test/models/database_authenticatable_test.rb +++ b/test/models/database_authenticatable_test.rb @@ -11,6 +11,16 @@ class DatabaseAuthenticatableTest < ActiveSupport::TestCase user.save! assert_equal email.downcase, user.email end + + test 'should remove whitespace from strip whitespace keys when saving' do + # strip_whitespace_keys is set to :email by default. + email = ' foo@bar.com ' + user = new_user(:email => email) + + assert_equal email, user.email + user.save! + assert_equal email.strip, user.email + end test 'find_for_authentication and filter_auth_params should not modify the conditions hash' do FilterAuthUser = Class.new(User) do diff --git a/test/rails_app/config/initializers/devise.rb b/test/rails_app/config/initializers/devise.rb index 9473db6a..fe1c7dff 100644 --- a/test/rails_app/config/initializers/devise.rb +++ b/test/rails_app/config/initializers/devise.rb @@ -35,6 +35,11 @@ Devise.setup do |config| # These keys will be downcased upon creating or modifying a user and when used # to authenticate or find a user. Default is :email. config.case_insensitive_keys = [ :email ] + + # Configure which authentication keys should have whitespace stripped. + # These keys will have whitespace before and after removed upon creating or + # modifying a user and when used to authenticate or find a user. Default is :email. + config.strip_whitespace_keys = [ :email ] # Tell if authentication through request.params is enabled. True by default. # config.params_authenticatable = true