Repository: joke2k/faker

Issue:
Param switches on faker.password() don't guarantee valid password
The format switches on `faker.password()` (`special_chars, digits, upper_case, lower_case`) don't always return passwords matching those rules.

This is problematic as when using generated passwords in unit tests, where passwords must conform to validity rules (e.g. "must contain numbers"), tests can randomly fail.

I expected that these switches would guarantee the function returns a conforming password. e.g. `faker.password(digits=True)` always returns a password containing digits, but this is not the case.


Discussion:

Patch:
diff --git a/faker/providers/misc/__init__.py b/faker/providers/misc/__init__.py
index 32531a60..9b318691 100644
--- a/faker/providers/misc/__init__.py
+++ b/faker/providers/misc/__init__.py
@@ -88,13 +88,33 @@ class Provider(BaseProvider):
         @param lower_case: Boolean. Whether to use lower letters
         @return: String. Random password
         """
-        chars = ""
+        choices = ""
+        required_tokens = []
         if special_chars:
-            chars += "!@#$%^&*()_+"
+            required_tokens.append(random.choice("!@#$%^&*()_+"))
+            choices += "!@#$%^&*()_+"
         if digits:
-            chars += string.digits
+            required_tokens.append(random.choice(string.digits))
+            choices += string.digits
         if upper_case:
-            chars += string.ascii_uppercase
+            required_tokens.append(random.choice(string.ascii_uppercase))
+            choices += string.ascii_uppercase
         if lower_case:
-            chars += string.ascii_lowercase
-        return ''.join(random.choice(chars) for x in range(length))
+            required_tokens.append(random.choice(string.ascii_lowercase))
+            choices += string.ascii_lowercase
+
+        assert len(required_tokens) <= length, "Required length is shorter than required characters"
+
+        # Generate a first version of the password
+        chars = [random.choice(choices) for x in range(length)]
+
+        # Pick some unique locations
+        random_indexes = set()
+        while len(random_indexes) < len(required_tokens):
+            random_indexes.add(random.randint(0, len(chars) - 1))
+
+        # Replace them with the required characters
+        for i, index in enumerate(random_indexes):
+            chars[index] = required_tokens[i]
+
+        return ''.join(chars)


Test patch:
diff --git a/faker/tests/__init__.py b/faker/tests/__init__.py
index 4eeaa3c7..6502a448 100644
--- a/faker/tests/__init__.py
+++ b/faker/tests/__init__.py
@@ -9,6 +9,7 @@ import json
 import os
 import time
 import unittest
+import string
 import sys
 
 try:
@@ -458,6 +459,22 @@ class FactoryTestCase(unittest.TestCase):
             datetime.datetime.now(utc).replace(second=0, microsecond=0)
         )
 
+    def test_password(self):
+        from faker.providers.misc import Provider
+
+        def in_string(char, _str):
+            return char in _str
+
+        for _ in range(999):
+            password = Provider.password()
+
+            self.assertTrue(any([in_string(char, password) for char in "!@#$%^&*()_+"]))
+            self.assertTrue(any([in_string(char, password) for char in string.digits]))
+            self.assertTrue(any([in_string(char, password) for char in string.ascii_uppercase]))
+            self.assertTrue(any([in_string(char, password) for char in string.ascii_lowercase]))
+
+        self.assertRaises(AssertionError, Provider.password, length=2)
+
     def test_prefix_suffix_always_string(self):
         # Locales known to contain `*_male` and `*_female`.
         for locale in ("bg_BG", "dk_DK", "en", "ru_RU", "tr_TR"):