This is my first project using Python. I made a simple password generator that checks user input. How can I improve it?
import random
def password_generator():
password = []
letters = ["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"]
password_length = 0
password_numbers = []
password_letters = []
# Input the length of the password
while True:
password_length_input = input("Choose the length of your password with numbers between 6 and 15:\n")
if not password_length_input.isnumeric():
print(f"{password_length_input} is not a number, try again:")
continue
else:
password_length = int(password_length_input)
print(f"Password length: {password_length}")
if 6 <= password_length <= 15:
break
else:
print("The password must be between 6 and 15 characters, try again:")
continue
# Input the amount of numbers in password
while True:
password_numbers_input = \
input(f"Choose the amount of numbers you want in your password, max {password_length}\n")
if not password_numbers_input.isnumeric():
print(f"{password_numbers_input} is not a number try again")
continue
elif int(password_numbers_input) > password_length:
password_numbers = 0
print(f"The value is too high, choose maximum {password_length} numbers")
continue
else:
password_numbers = int(password_numbers_input)
print(f"Password numbers: {password_numbers}")
for number in range(0,password_numbers):
password.append(random.randrange(0,9))
break
# Check for numbers and letters in password
while True:
if password_numbers == password_length:
print(f"The password will be only {password_numbers} numbers, no letters.")
break
else:
password_letters = password_length - password_numbers
print(f"""Your password will be {password_length} characters with {password_numbers} numbers and {password_letters} letters.""")
for letter in range(0,password_letters):
password.append(random.choice(letters))
break
random.shuffle(password)
password_string = ''.join([str(item) for item in password])
print(f"Your password is:\n{password_string}")
password_generator()
Usage example:
Choose the length of your password with numbers between 6 and 15:
Password length: 8
Choose the amount of numbers you want in your password, max 8
Password numbers: 2
Your password will be 8 characters with 2 numbers and 6 letters.
Your password is:
pzc11bmf
5 Answers 5
letters = ["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"]
This method of writing the alphabet is very error prone. I would import string
and use string.ascii_lowercase
in place of letters
. If you want to generate your own range of letters for whatever reason, I would write
letters = [chr(n) for n in range(ord('a'), ord('z') + 1)]
since there's then no danger of omitting or duplicating a letter.
password_length = 0 password_numbers = [] password_letters = []
These default values are never used. The defaults for password_numbers
and password_letters
don't make sense since those variables hold numbers. I would delete all three lines.
if not password_length_input.isnumeric(): print(f"{password_length_input} is not a number, try again:") continue else: password_length = int(password_length_input) print(f"Password length: {password_length}")
I would instead write
try:
password_length = int(password_length_input)
except ValueError:
print(f"{password_length_input} is not a number, try again:")
continue
print(f"Password length: {password_length}")
while True: if password_numbers == password_length: ... break else: ... break
There is no sense in having a while
loop here since you always break out of it on the first iteration.
range(0,password_numbers)
You can just write range(password_numbers)
.
password.append(random.randrange(0,9))
This will append a digit from 0 to 8 inclusive, never 9. If you want all ten digits you should write random.randrange(10)
. Or, perhaps better, use random.choice(string.digits)
.
password_string = ''.join([str(item) for item in password])
If you use string.digits
then every element of password
will be a character so you can simplify this to password_string = ''.join(password)
.
A more straightforward way of generating a random string:
import random
import string
def get_random_string(length):
letters = string.ascii_lowercase
result_str = ''.join(random.choice(letters) for i in range(length))
print("Random string of length", length, "is:", result_str)
get_random_string(8)
get_random_string(8)
get_random_string(6)
borrowed from here, and there are more examples.
Now if you have specific requirements like a minimum number of digits, you can either tweak the formula, or generate two lists and merge them while shuffling the values.
There is an example in the link I quoted above: "Generate a random alphanumeric string with a fixed count of letters and digits" => merging two list comprehensions.
The way you are doing it is procedural but not Pythonic. It is kinda reinventing the wheel.
At the very least, your list of allowed characters should look like this:
letters = "abcdefghijklmnopqrstuvwxyz"
Then you pick out a random letter eg letters[3]
will return 'd' since the list is 0-based and Python treats strings as sequences of characters. Using shuffle like you are already doing, you can write more concise code.
Your password generator is good to start with, here are a few ways you can improve.
- Use a more secure method of generating random numbers and letters. The
random
library that you are currently using is not suitable for generating cryptographic quality random numbers and characters. Instead, you should use thesecrets
module which is specifically designed for generating secure random numbers. - You could also include special characters like '#', '@', '$' and so on. To include them, you can add them to your list of characters, along with upper-case letters as well, this way it would make the password even more secure.
- Another thing to consider is the minimum length of the password, as per security standard, it should be minimum of 8 characters. Also the number of digits, special characters and upper case letters required could also be made a requirement.
- Your current code only checks for numeric input for the password length and number of digits but no validation for input of the number of special characters and upper-case letters, this could be added as well.
- Finally, you could create a function that checks for the strength of the generated password, and prompt the user to generate a new password if the generated password doesn't meet certain strength criteria.
You can find more information on how to generate secure random numbers in the Python documentation: https://docs.python.org/3/library/secrets.html
Also, you may find it helpful to read the OWASP password storage verifier, it has a lot of information on how to create strong passwords and how to store them: https://owasp.org/www-project-cheat-sheets/cheatsheets/Password_Storage_Cheat_Sheet/
First
I suggest creating separate methods for each while loops.
Second
"while True" loop is not a good practice, I think. Instead of that, use condition.
Third
I suggest creating PasswordGenerator class which will contain your code. It will help you to expand your code in the future.
Base structure for your project
class PasswordGenerator():
check_declared_password_length():
...
check_amount_of_password_numbers():
...
*
*
*
For the end remember to create functions with one responsibility. After that, you can write unit tests for each of that and it will be easier.
I definitely don't recommend using random
for generating secure passwords. The reason being random
is predictable and anyone can guess the next set of passwords that it is going to generate. So it is better to replace random
with secrets
and use secrets.SystemRandom()
Also, note one more flaw as you are using random.choice()
method it can repeat the characters while generating. So if you don’t want to repeat the characters and still want to use random
, then use random.sample()
method.
If you are looking for a secure and robust password, Python has a module called as secrets, and you could utilize this to generate a random secured password.
The algorithm used by secrets is less predictable when compared to the random string module generation.
import secrets
import string
def generateRandomCryptoPassword(length):
# Generate random password using cryptographic way
letters = string.ascii_lowercase + string.digits + string.punctuation
result_str = ''.join(secrets.choice(letters) for i in range(length))
print("Strong Password using Crpyto of length", length, "is:", result_str)
generateRandomCryptoPassword(12)
Source : Generate a random string of a given length in Python
-
1\$\begingroup\$ Your assertion that all of
random
is predictable is incorrect. If you read secrets' source then you'll seesecrets.SystemRandom
israndom.SystemRandom
. \$\endgroup\$2021年10月03日 23:41:59 +00:00Commented Oct 3, 2021 at 23:41 -
1\$\begingroup\$ what you are saying is partially true, it internally uses random.SystemRandom. but t the same time if you look at the secrets.py they do retrieve a random strings in hex first and they convert into base64 which is more secure. (base64.urlsafe_b64encode(tok).rstrip(b'=').decode('ascii') \$\endgroup\$Srinivas Ramakrishna– Srinivas Ramakrishna2021年10月03日 23:50:01 +00:00Commented Oct 3, 2021 at 23:50
Explore related questions
See similar questions with these tags.
python create-password 8 2
\$\endgroup\$random
should not be used for security purposes, but you can usesecrets
instead; you probably should replacerandom
withsecrets.SystemRandom()
if generating real passwords or any other cryptographically secure value. \$\endgroup\$