4
\$\begingroup\$

I have the following code here

It's a password generator that lets you choose a completely random password with a certain length of a custom password with different amounts of character types.

I want feedback of the general readability, style, and security (as it's a password generator), and if you would do anything differently.

The code:

# Imports libraries import secrets import string # Creates list of every possible character for the "general" option fulllist = string.ascii_lowercase + string.ascii_uppercase + string.digits + string.punctuation # Asks user what option they want generalorcustom = input("General (g) or custom (c) password: ") # Randomly grabs a character from the 'fulllist' as many times as specified def general(): return ''.join(secrets.choice(fulllist) for i in range(int(length))) def custom(): # Inputs how many of each character user wants lowernum = input("Lowwer case character count: ") uppernum = input("Upper case character count: ") digitnum = input("Digit count: ") specnum = input("Special character count: ") # Creates list, then randomly adds each characters of each type to list for specified amount temp_list = [] temp_list.extend(list(secrets.choice(string.ascii_lowercase) for i in range(int(lowernum)))) temp_list.extend(list(secrets.choice(string.ascii_uppercase) for i in range(int(uppernum)))) temp_list.extend(list(secrets.choice(string.digits) for i in range(int(digitnum)))) temp_list.extend(list(secrets.choice(string.punctuation) for i in range(int(specnum)))) # Creates final string, and randomly adds contents of temp_list to final string password = '' rangevar = list(range(len(temp_list))) while len(rangevar) > 0: picked_item = secrets.choice(rangevar) password += temp_list[picked_item] rangevar.remove(picked_item) return password # Checks and executes function as user requested if generalorcustom.lower() == "general" or generalorcustom.lower() == "g": length = input("Password length: ") print(general()) elif generalorcustom.lower() == "custom" or generalorcustom.lower() == "c": print(custom()) else: print('Error, invalid input') 
\$\endgroup\$
1
  • 3
    \$\begingroup\$Regarding security, longer passwords of only lowercase letters are stronger than shorter passwords containing mix. A 13-character lowercase-only password is harder to crack than a 10-character password that could have uppercase letters, lowercase letters, or digits. (26**13 > 62**10). Forcing users to use any specific mix of characters encourages shorter passwords, because they're easier to remember or type.\$\endgroup\$
    – chepner
    CommentedJan 20, 2023 at 21:40

1 Answer 1

3
\$\begingroup\$

A few minor inconsistencies and maybe two improvements.

PEP 8 styling or your own in other places uses lower_case_with_underscores but some variables like fulllist don't and that's alot of ls. For readability update variables accordingly (also generalorcustom).

Move .lower() to input so its only called once not 4 times.

In custom you get the password options then generate a list of indices to pick from. A more optimal solution would be to just shuffle temp_list (Fisher-Yates shuffle) as you have all the characters for the password you just want to scramble them now. After the shuffle you can use ''.join(temp_list)

Full shuffle:

for i in range(len(temp_list)-1, 1, -1): j=secrets.randbelow(i+1) temp = temp_list[i] temp_list[i] = temp_list[j] temp_list[j] = temp return ''.join(temp_list) 

You don't have any handlers for if the user enters invalid input for a number (like a letter or negative). I'd make a function that takes a prompt and returns a number if valid otherwise continues to prompt for a number, like so:

def get_number_input(prompt): num=0 while True: try: num = int(input(prompt)) if num < 0: print('Please enter a positive number') continue break except ValueError: print('Please enter a number') return num 

Instead of a single run, I would wrap the input with a while loop so a user could generate multiple passwords, with some sort of break/exit option added.

I would avoid the global length and instead pass length as a parameter to general

Everything together:

import secrets import string # Creates list of every possible character for the "general" option full_list = string.ascii_lowercase + string.ascii_uppercase + string.digits + string.punctuation def get_number_input(prompt): num = 0 while True: try: num = int(input(prompt)) if num < 0: print('Please enter a positive number') continue break except ValueError: print('Please enter a number') return num # Randomly grabs a character from the 'full_list' as many times as specified def general(length): return ''.join(secrets.choice(full_list) for i in range(length)) def custom(): # Inputs how many of each character user wants lower_num = get_number_input("Lower case character count: ") upper_num = get_number_input("Upper case character count: ") digit_num = get_number_input("Digit count: ") spec_num = get_number_input("Special character count: ") # Creates list, then randomly adds each characters of each type to list for specified amount temp_list = [] temp_list.extend(list(secrets.choice(string.ascii_lowercase) for i in range(lower_num))) temp_list.extend(list(secrets.choice(string.ascii_uppercase) for i in range(upper_num))) temp_list.extend(list(secrets.choice(string.digits) for i in range(digit_num))) temp_list.extend(list(secrets.choice(string.punctuation) for i in range(spec_num))) # shuffle choices for i in range(len(temp_list)-1,1,-1): j=secrets.randbelow(i+1) temp=temp_list[i] temp_list[i]=temp_list[j] temp_list[j]=temp return ''.join(temp_list) while True: # Asks user what option they want general_or_custom = input("General (g) or custom (c) password: ").lower() # Checks and executes function as user requested if general_or_custom == "general" or general_or_custom == "g": length = get_number_input("Password length: ") print(general(length)) elif general_or_custom == "custom" or general_or_custom == "c": print(custom()) else: print('Error, invalid input') break 
\$\endgroup\$

    Start asking to get answers

    Find the answer to your question by asking.

    Ask question

    Explore related questions

    See similar questions with these tags.