0

I'm generating a random password with a desired length. I want it to have at least 2 uppercase letters, 2 lowercase letters, 2 digits and 2 special characters. I've tried multiple things, but every time I get this recursion depth error. Can anybody tell me what I've done wrong?

list_lower =['a','b','c','d','e','f','g','h','i','j','k','l','m','n','o','p','q','r','s','t','u','v','w','x','y','z']
list_upper = ['A','B','C','D','E','F','G','H','I','J','K','L','M','N', 'O','P','Q','R','S','T','U','V','W','X','Y','Z'] 
list_digit = [1,2,3,4,5,6,7,8,9,0]
def generatePassword(desiredLength: int) -> str:
    x = 0
    password = ""
    for x in range (desiredLength):
        password = password + chr(random.randint(33,126))
        list(password)
        list_password = list(password)
        times_lower = 0
        times_upper = 0
        times_digit = 0
        times_special = 0
        for character in list_password:
            if character in list_lower:
                times_lower += 1
            elif character in list_upper:
                times_upper += 1
            elif character in list_digit:
                times_digit += 1
            else:
                times_special +=1
        if times_lower >= 2 and times_upper >= 2 and times_digit >= 2 and times_special >= 2:
            return password
        else:
            return generatePassword(desiredLength)

generatePassword(7)

I get the error in line 30 which makes the function recursive.

Aran-Fey
  • 39,665
  • 11
  • 104
  • 149
  • 3
    Why do it recursively at all? This doesn't seem like a natural use-case. Also -- please don't use things like "line 30". Your editor's line numbering is not preserved in your code. – John Coleman Sep 28 '18 at 15:52
  • Your breaking condition is based on randomness. No matter what you do, there will always be a non-zero chance of recurring too many times. – mypetlion Sep 28 '18 at 15:53
  • First, your function's base case is incorrect: when `desiredLength` is 0, the function returns `None` instead of an empty string. Second, the function calls itself with the same value of `desiredLength` (must be `desiredLength-1`). – DYZ Sep 28 '18 at 15:54
  • 1
    shouldn't the `if -else` branch that makes the recursive call be outside the inner for-loop? Otherwise, it will never possibly have those properties you are hoping (because `len(password) == 1`) and thus, it will always make the recursive call. Anyway, just use a `while` loop. No need for recursion. – juanpa.arrivillaga Sep 28 '18 at 15:55
  • @DYZ If I'm reading it correctly, I don't think this is a case of `desiredLength` going towards zero. I think what OP is going for is generating a random string of desired length, and if it's an invalid string, try again. Notice that the `password` variable is not passed to the recurred function call. – mypetlion Sep 28 '18 at 15:56
  • 1
    Also, even with a while-loop, there's a better approach. Choose, randomly, 2 characters from every category you require. Then randomly choose a category and a character until you fill up the desired length. Then shuffle. – juanpa.arrivillaga Sep 28 '18 at 15:58
  • @juanpa.arrivillaga that seems like the obvious solution to me too – Dan Sep 28 '18 at 16:02
  • @juanpa.arrivillaga That is a natural choice, although it does introduce a slight bias towards passwords with more than the requisite number of special characters. – John Coleman Sep 28 '18 at 16:06
  • @JohnColeman yeaah I was thinking about that. I suppose, you could sample from a distribution which "corrects" for the choices you've already hard-coded. Using the new [`random.choices`](https://docs.python.org/3/library/random.html#random.choices) might be the way to go. – juanpa.arrivillaga Sep 28 '18 at 17:10

3 Answers3

1

Calling generatePassword(7) will never generate a password with 2 of each of 4 distinct categories.

You don't need recursion at all.

def generatePassword(desiredLength: int) -> str:
    while True:
        password = ""
        for x in range (desiredLength):
            password = password + chr(random.randint(33,126))
        times_lower = 0
        times_upper = 0
        times_digit = 0
        times_special = 0
        for character in password:
            if character in list_lower:
                times_lower += 1
            elif character in list_upper:
                times_upper += 1
            elif character in list_digit:
                times_digit += 1
            else:
                times_special +=1
        if times_lower >= 2 and times_upper >= 2 and times_digit >= 2 and times_special >= 2:
            return password
        else
            print ("Rejecting ", password)

That will loop forever if asked to generate a password of length 7 or less. We can improve that by checking the desired length first

if desiredLength < 8:
    raise ArgumentError("Cannot generate passwords shorter than 8 characters")
Caleth
  • 52,200
  • 2
  • 44
  • 75
0

times_digit will never be >= 2 because it tests stings (e.g. "2" against the integers in your list, (e.g. 2) change your list_digit to

list_digit = ["1","2","3","4","5","6","7","8","9","0"]

and try again.

By the way this could be done much simpler and doensn't need a recursive function.

bened
  • 11
  • 3
0

If you are generating passwords, it's important that you generate ones that actually have enough randomness to not be predictable.

Random string generation with upper case letters and digits in Python

Has a great breakdown of how to generate a password that's truly random:

''.join(random.SystemRandom().choice(string.ascii_uppercase + string.digits) for _ in range(N))

(adding "special characters" and "lowercase characters" omitted to preserve the existing code)

I know that this is a somewhat oblique answer (i.e. it does not answer the question directly), so here's a potential solution if you still need the "it must contain these types of characters" (even though that would actually reduce security):

import random
import string
from collections import Counter

def gen(N):
    return ''.join(random.SystemRandom().choice(string.ascii_letters + string.digits + string.punctuation) for _ in range(N))

while True:
    pw = gen(8)
    counts = Counter(pw)
    upper = lower = digit = special = 0
    for (letter, count) in counts.items():
        if (letter in string.ascii_lowercase):
            lower += 1
        elif (letter in string.ascii_uppercase):
            upper += 1
        elif (letter in string.digits):
            digit += 1
        else:
            special += 1
            pass
    if (lower > 1 and upper > 1 and digit > 1 and special > 1):
        print("password is {}".format(pw))
        break
    print("failed password: {}".format(pw))