fix: ensure passwords have correct characters when mix_case & special…

…_characters enabled (#2533)

doc/default/internet.md

@@ -32,10 +32,15 @@ Faker::Internet.username(specifier: 5..8)
 Faker::Internet.username(specifier: 8)
 
 # Keyword arguments: min_length, max_length, mix_case, special_characters
+# Default configuration is mix_case: true && special_characters: false
 Faker::Internet.password #=> "Vg5mSvY1UeRg7"
 Faker::Internet.password(min_length: 8) #=> "YfGjIk0hGzDqS0"
 Faker::Internet.password(min_length: 10, max_length: 20) #=> "EoC9ShWd1hWq4vBgFw"
+# min_length must be at least 1 if mix_case: false && special_characters: true
+Faker::Internet.password(min_length: 10, max_length: 20, mix_case: false, special_characters: true) #=> "$1109mw31h8359jm0!oo"
+# min_length must be at least 2 if mix_case: true && special_characters: false
 Faker::Internet.password(min_length: 10, max_length: 20, mix_case: true) #=> "3k5qS15aNmG"
+# min_length must be at least 3 if mix_case: true && special_characters: true
 Faker::Internet.password(min_length: 10, max_length: 20, mix_case: true, special_characters: true) #=> "*%NkOnJsH4"
 
 # Keyword arguments: subdomain, domain

lib/faker/default/internet.rb

@@ -147,37 +147,48 @@ def username(specifier: nil, separators: %w[. _])
       #
       # @faker.version 2.1.3
       def password(min_length: 8, max_length: 16, mix_case: true, special_characters: false)
-        raise ArgumentError, 'Password of length 1 can not have both mixed case and special characters' if min_length <= 1 && mix_case && special_characters
+        raise ArgumentError, 'max_length must be more than min_length' if max_length < min_length
 
-        min_alpha = mix_case && min_length > 1 ? 2 : 0
-        temp = Lorem.characters(number: min_length, min_alpha: min_alpha)
-        diff_length = max_length - min_length
-
-        if diff_length.positive?
-          diff_rand = rand(diff_length + 1)
-          temp += Lorem.characters(number: diff_rand)
-        end
+        character_types = []
+        required_min_length = 0
 
         if mix_case
-          alpha_count = 0
-          temp.chars.each_with_index do |char, index|
-            if char =~ /[[:alpha:]]/
-              temp[index] = char.upcase if alpha_count.even?
-              alpha_count += 1
-            end
-          end
+          character_types << :mix_case
+          required_min_length += 2
         end
 
         if special_characters
-          chars = %w[! @ # $ % ^ & *]
-          rand(1..min_length).times do |i|
-            temp[i] = chars[rand(chars.length)]
-          end
+          character_types << :special_characters
+          required_min_length += 1
+        end
+
+        raise ArgumentError, "min_length should be at least #{required_min_length} to enable #{character_types.join(', ')} configuration" if min_length < required_min_length
+
+        target_length = rand(min_length..max_length)
+
+        password = []
+        character_bag = []
+
+        # use lower_chars by default and add upper_chars if mix_case
+        lower_chars = ('a'..'z').to_a
+        password << lower_chars[rand(lower_chars.count - 1)]
+        character_bag += lower_chars
+
+        if character_types.include?(:mix_case)
+          upper_chars = ('A'..'Z').to_a
+          password << upper_chars[rand(upper_chars.count - 1)]
+          character_bag += upper_chars
+        end
+
+        if character_types.include?(:special_characters)
+          special_chars = %w[! @ # $ % ^ & *]
+          password << special_chars[rand(special_chars.count - 1)]
+          character_bag += special_chars
         end
 
-        temp[rand(temp.size - 1)] = Lorem.characters(number: 1, min_alpha: 1).upcase if mix_case && special_characters && !temp.match(/[A-z]+/)
+        password << character_bag[rand(character_bag.count - 1)] while password.length < target_length
 
-        temp
+        shuffle(password).join
       end
 
       ##

test/faker/default/test_faker_internet.rb

@@ -122,7 +122,9 @@ def test_password
 
   def test_password_with_integer_arg
     (1..32).each do |min_length|
-      assert @tester.password(min_length: min_length, mix_case: false).length >= min_length
+      max_length = min_length + 1
+
+      assert_includes (min_length..max_length), @tester.password(min_length: min_length, max_length: max_length, mix_case: false).length
     end
   end
 
@@ -157,9 +159,8 @@ def test_password_with_mixed_case
     assert downcase_count >= 1
   end
 
-  def test_password_with_min_length_eq_1
-    min_length = 1
-    password = @tester.password(min_length: min_length)
+  def test_password_with_min_length_eq_1_without_mix_case
+    password = @tester.password(min_length: 1, mix_case: false)
 
     assert_match(/\w+/, password)
   end
@@ -173,6 +174,12 @@ def test_password_with_min_length_and_max_length
     assert_includes (min_length..max_length), password.size, 'Password size is incorrect'
   end
 
+  def test_password_with_max_length_less_than_min_length
+    assert_raise 'max_length must be more than min_length' do
+      @tester.password(min_length: 8, max_length: 4)
+    end
+  end
+
   def test_password_without_mixed_case
     assert_match(/[^A-Z]+/, @tester.password(min_length: 8, max_length: 12, mix_case: false))
   end
@@ -194,25 +201,43 @@ def test_password_with_special_chars_and_mixed_case
     end
   end
 
-  def test_password_with_special_chars_and_mixed_case_on_2chars_password
+  def test_deterministic_password_with_special_chars_and_mixed_case
+    deterministically_verify -> { @tester.password(min_length: 4, max_length: 6, mix_case: true, special_characters: true) }, depth: 4 do |password|
+      assert_match(/[!@#$%\^&*]+/, password)
+      assert_match(/[A-z]+/, password)
+    end
+  end
+
+  def test_password_with_special_chars_and_mixed_case_on_3chars_password
     16.times do
-      password = @tester.password(min_length: 2, max_length: 6, mix_case: true, special_characters: true)
+      password = @tester.password(min_length: 3, max_length: 6, mix_case: true, special_characters: true)
 
       assert_match(/[!@#$%\^&*]+/, password)
       assert_match(/[A-z]+/, password)
     end
   end
 
-  def test_password_with_incompatible_min_length_and_requirements
-    assert_raise ArgumentError do
+  def test_password_with_invalid_min_length_for_mix_case_and_special_characters
+    assert_raise_message 'min_length should be at least 3 to enable mix_case, special_characters configuration' do
       @tester.password(min_length: 1, mix_case: true, special_characters: true)
     end
   end
 
   def test_password_with_compatible_min_length_and_requirements
     assert_nothing_raised do
       [false, true].each do |value|
-        @tester.password(min_length: 1, mix_case: value, special_characters: !value)
+        min_length = value ? 2 : 1
+        @tester.password(min_length: min_length, mix_case: value, special_characters: !value)
+      end
+    end
+  end
+
+  def test_deterministic_password_with_compatible_min_length_and_requirements
+    [false, true].each do |value|
+      min_length = value ? 2 : 1
+
+      deterministically_verify -> { @tester.password(min_length: min_length, mix_case: value, special_characters: !value) }, depth: 4 do |subject|
+        assert_nothing_raised { subject }
       end
     end
   end