Merge pull request #15708 from akshay-vishnoi/secure_password

SecurePassword - Validate password must be less than or equal to 72

2 files changed, 34 insertions, 0 deletions

diff --git a/activemodel/lib/active_model/secure_password.rb b/activemodel/lib/active_model/secure_password.rb
index 4033eb5808..879db59b34 100644
--- a/activemodel/lib/active_model/secure_password.rb
+++ b/activemodel/lib/active_model/secure_password.rb
@@ -2,6 +2,11 @@ module ActiveModel
   module SecurePassword
     extend ActiveSupport::Concern
 
+    # BCrypt hash function can handle maximum 72 characters, and if we pass
+    # password of length more than 72 characters it ignores extra characters.
+    # Hence need to put a restriction on password length.
+    MAX_PASSWORD_LENGTH_ALLOWED = 72
+
     class << self
       attr_accessor :min_cost # :nodoc:
     end
@@ -63,6 +68,7 @@ module ActiveModel
             record.errors.add(:password, :blank) unless record.password_digest.present?
           end
 
+          validates_length_of :password, maximum: ActiveModel::SecurePassword::MAX_PASSWORD_LENGTH_ALLOWED
           validates_confirmation_of :password, if: ->{ password.present? }
         end
 
diff --git a/activemodel/test/cases/secure_password_test.rb b/activemodel/test/cases/secure_password_test.rb
index bcd1e04a0f..f6dfdc5342 100644
--- a/activemodel/test/cases/secure_password_test.rb
+++ b/activemodel/test/cases/secure_password_test.rb
@@ -45,6 +45,20 @@ class SecurePasswordTest < ActiveModel::TestCase
     assert_equal ["can't be blank"], @user.errors[:password]
   end
 
+  test 'create a new user with validation and password length less than or equal to 72' do
+    @user.password = 'nakshay' * 10
+    @user.password_confirmation = @user.password
+    assert @user.valid?(:create), 'user should be valid'
+  end
+
+  test 'create a new user with validation and password length greater than 72' do
+    @user.password = 'nakshay' * 11
+    @user.password_confirmation = @user.password
+    assert !@user.valid?(:create), 'user should be invalid'
+    assert_equal 1, @user.errors.count
+    assert_equal ["is too long (maximum is 72 characters)"], @user.errors[:password]
+  end
+
   test "create a new user with validation and a blank password confirmation" do
     @user.password = 'password'
     @user.password_confirmation = ''
@@ -97,6 +111,20 @@ class SecurePasswordTest < ActiveModel::TestCase
     assert_equal ["can't be blank"], @existing_user.errors[:password]
   end
 
+  test 'updating an existing user with validation and password length less than or equal to 72' do
+    @existing_user.password = 'nakshay' * 10
+    @existing_user.password_confirmation = @existing_user.password
+    assert @existing_user.valid?(:update), 'user should be valid'
+  end
+
+  test 'updating an existing user with validation and password length greater than 72' do
+    @existing_user.password = 'nakshay' * 11
+    @existing_user.password_confirmation = @existing_user.password
+    assert !@existing_user.valid?(:update), 'user should be invalid'
+    assert_equal 1, @existing_user.errors.count
+    assert_equal ["is too long (maximum is 72 characters)"], @existing_user.errors[:password]
+  end
+
   test "updating an existing user with validation and a blank password confirmation" do
     @existing_user.password = 'password'
     @existing_user.password_confirmation = ''