Script to generate random password

Question

I made this random password script. It works, but i think it is kind of bloated. The part where random characters are being added is the area where I think it needs work. I don't want to declare if and else statement for every combination of lowercase list, uppercase list and digits list(I am taking input from user whether they want uppercase,lowercase,digits or any possible combination of these three). Is there any possible way in which I can just check which of these three values are yes and then just use only those.

Also what is more efficient using lower() to turn the list called using string module into lower case or just calling the lowercase list itself.

import random
from string import ascii_letters ,digits
while True:
 userchoice_size = int(input("Enter the length of the password : ")) # Asks for length of password
 userchoice_syntax_lower = input("Do you want lower case letter?(y or n) ")
 userchoice_syntax_upper = input("Do you want upper case letter?(y or n) ")
 userchoice_syntax_number = input("Do you want numbers?(y or n) ")
 counter=0
 password='' # String which stores the password
 choice_list=[1,2,3]
 while counter <= userchoice_size:
 choice_number = random.choice(choice_list)
 if choice_number == 1:
 if userchoice_syntax_upper == 'y': # Uppercase list
 password = password + str(random.choice(ascii_letters.upper())) 
 counter += 1
 if choice_number == 2:
 if userchoice_syntax_number=='y': # Digits list
 password = password + str(random.choice(digits))
 counter += 1
 if choice_number == 3:
 if userchoice_syntax_lower == 'y': # Lower case list
 password= password + str(random.choice(ascii_letters.lower())) # Converts all letters into lowercase then randomly select and store in list
 counter += 1
 print (password)

Answer 1

About your code

Style

Your style is mostly fine, except lacking a bit of white space, especially after the imports, and having mostly useless comments that break the 80 character barrier.

Input validation

You don't check if the user input can be parsed as an integer in the first input. The int() methods validates its input, however a bad input will crash your program. It would be preferable to reject the input, and ask again.

You don't validate the input for the yes/no question either, and it is much worse. Typing anything other than a lowercase y will be treated as a no. At the very least, add a .lower() to handle inputs like Y. It would be high priority to check for n/N, instead of silently treating anything else than y as meaning no.

Since you ask 3 yes/no questions (and possibly more if you'd like to add stuff like special characters), you should delegate the input and its validation to a separate method.

Bug

If the user says "no" (more precisely: not "y") to all 3 questions, the program will hang indefinitely, as counter will never increase and the inner while loop will never end.

Security

random is not a cryptographically secure random number generator. Use secrets instead, which is very similar to use.

Also, ideally you'd want the probability for any character to be chosen to be the same. By separating the character categories as you did, digits are over-represented if they appear alongside letters.

Outer while loop

You put the entire code in an infinite loop. How many passwords do you think a user would want to generate?

My take on the problem

def generate_password():
 password_length = ask_for_int('Enter the length of the password')
 
 choices = ''
 if ask_for_bool('Use lowercase letters? [y/n]'):
 choices += string.ascii_lowercase
 if ask_for_bool('Use uppercase letters? [y/n]'):
 choices += string.ascii_uppercase
 if ask_for_bool('Use digits? [y/n]'):
 choices += string.digits
 
 password = ''
 for _ in range(password_length):
 password += secrets.choice(choices)
 
 return password
def ask_for_int(msg):
 while True:
 print(msg)
 try:
 return int(input('> '))
 except ValueError:
 print('Unable to parse number. Please try again.')
def ask_for_bool(msg):
 while True:
 print(msg)
 user_in = input('> ').lower()
 if user_in =='y':
 return True
 if user_in =='n':
 return False
 print('Unable to parse answer. Please try again.')

• Reinderien: password += should be replaced with ''.join - secrets even shows an example doing so.
• gazoh: @Reinderien Is there a reason - besides performance - for doing so? I don't like list comprehensions, as they are rather hard to read. I assume that arsh is quite new to python, and I feel that code readability is more valuable than a clever one-liner, especially at a beginner level.
• Reinderien: The use of ''.join in this case is already well-understood, and is more "standard" than "clever". Terseness and performance are both factors, and for some it's more legible than a loop. For the scale of password generation performance will not matter, but in other applications your loop will be O(n²) where join is O(n).
• Reinderien: More generally: I try to give submissions on this site the best code possible and not necessarily code that they're guaranteed to understand, since presumably they came here to learn. People that come here are aspiring.

Answer 2

Character distribution

You randomly choose between uppercase, lowercase and digit characters with an equal probability, but there are more letters than digit characters. That means characters will be under-represented in your password's probability distribution. Easier and more accurate is to make one string of all eligible characters and choose from it randomly.

Redundant str

No need for str here:

str(random.choice(ascii_letters.upper())) 

Move your increment

counter += 1

should not be repeated three times in if statements. You can do it once at the end of the loop. Better yet,

for _ in range(userchoice_size):

Choice list

This:

choice_list=[1,2,3]
choice_number = random.choice(choice_list)

should not use a list, and should be replaced with a call to randrange.

Secure entropy

For password generation it's crucially important to use a secure source of entropy. Read this quote carefully:

Python uses the Mersenne Twister as the core generator. It produces 53-bit precision floats and has a period of 2**19937-1. The underlying implementation in C is both fast and threadsafe. The Mersenne Twister is one of the most extensively tested random number generators in existence. However, being completely deterministic, it is not suitable for all purposes, and is completely unsuitable for cryptographic purposes.

Emphasis mine. A better built-in option is secrets.

• Roland Illig: There's the reference for the quote: docs.python.org/3/library/random.html