Merge pull request rails#47708 from rails/fix-am-secure-password-length-validation

Improve Password Length Validation for BCrypt Compatibility

Author: guilleiguaran

activemodel/CHANGELOG.md

@@ -1,3 +1,22 @@
+*   Improve password length validation in ActiveModel::SecurePassword to consider byte size for BCrypt compatibility.
+
+    The previous password length validation only considered the character count, which may not
+    accurately reflect the 72-byte size limit imposed by BCrypt. This change updates the validation
+    to consider both character count and byte size while keeping the character length validation in place.
+
+    ```ruby
+      user = User.new(password: "a" * 73)  # 73 characters
+      user.valid? # => false
+      user.errors[:password] # => ["is too long (maximum is 72 characters)"]
+
+
+      user = User.new(password: "あ" * 25)  # 25 characters, 75 bytes
+      user.valid? # => false
+      user.errors[:password] # => ["is too long (maximum is 72 bytes)"]
+    ```
+
+    *ChatGPT*, *Guillermo Iguaran*
+
 *   `has_secure_password` now generates an `#{attribute}_salt` method that returns the salt
     used to compute the password digest. The salt will change whenever the password is changed,
     so it can be used to create single-use password reset tokens with `generates_token_for`:

activemodel/lib/active_model/locale/en.yml

@@ -18,6 +18,7 @@ en:
       too_long:
         one: "is too long (maximum is 1 character)"
         other: "is too long (maximum is %{count} characters)"
+      password_too_long: "is too long"
       too_short:
         one: "is too short (minimum is 1 character)"
         other: "is too short (minimum is %{count} characters)"

activemodel/lib/active_model/secure_password.rb

@@ -132,7 +132,14 @@ def has_secure_password(attribute = :password, validations: true)
             end
           end
 
-          validates_length_of attribute, maximum: ActiveModel::SecurePassword::MAX_PASSWORD_LENGTH_ALLOWED
+          # Validates that the password does not exceed the maximum allowed bytes for BCrypt (72 bytes).
+          validate do |record|
+            password_value = record.public_send(attribute)
+            if password_value.present? && password_value.bytesize > ActiveModel::SecurePassword::MAX_PASSWORD_LENGTH_ALLOWED
+              record.errors.add(attribute, :password_too_long)
+            end
+          end
+
           validates_confirmation_of attribute, allow_blank: true
         end
       end

activemodel/test/cases/secure_password_test.rb

@@ -62,12 +62,22 @@ class SecurePasswordTest < ActiveModel::TestCase
     assert_equal ["can’t be blank"], @user.errors[:password]
   end
 
-  test "create a new user with validation and password length greater than 72" do
+  test "create a new user with validation and password length greater than 72 characters" do
     @user.password = "a" * 73
     @user.password_confirmation = "a" * 73
     assert_not @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]
+    assert_equal ["is too long"], @user.errors[:password]
+  end
+
+  test "create a new user with validation and password byte size greater than 72 bytes" do
+    # Create a password with 73 bytes by using a 3-byte Unicode character (e.g., "あ") 24 times, followed by a 1-byte character "a".
+    # This will result in a password length of 25 characters, but with a byte size of 73.
+    @user.password = "あ" * 24 + "a"
+    @user.password_confirmation = "あ" * 24 + "a"
+    assert_not @user.valid?(:create), "user should be invalid"
+    assert_equal 1, @user.errors.count
+    assert_equal ["is too long"], @user.errors[:password]
   end
 
   test "create a new user with validation and a blank password confirmation" do
@@ -142,7 +152,7 @@ class SecurePasswordTest < ActiveModel::TestCase
     @existing_user.password_confirmation = "a" * 73
     assert_not @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]
+    assert_equal ["is too long"], @existing_user.errors[:password]
   end
 
   test "updating an existing user with validation and a blank password confirmation" do