Here's a password generator I created.
### THIS PROGRAM WAS CREATED FOR LEARNING PURPOSES ###
### AND SHOULD NOT BE USED TO CREATE PASSWORDS! ###
import string
from enum import Enum
from random import randint, shuffle, choice
class PasswordError(Exception):
__module__ = Exception.__module__
# Types of characters possible
class Types(Enum):
CAP = 'CAP' # Capital
SMA = 'SMA' # Small
DIG = 'DIG' # Digits
SPE = 'SPE' # Special
# Characters for each type of possible characters
type_chars = {
Types.CAP: string.ascii_uppercase,
Types.SMA: string.ascii_lowercase,
Types.DIG: string.digits,
Types.SPE: '!()-.?[]_`~;:@#$%^&*='
}
def password_generator(min_length=6, max_length=20, caps=1, small=1, digits=1, special=1):
types = {
Types.CAP: caps,
Types.SMA: small,
Types.DIG: digits,
Types.SPE: special,
}
num_chars = sum(list(types.values())) # Number of mandatory characters
min_length = max(num_chars, min_length) # In case 'num_chars' is greater
# Number of characters required for each possible type of character
# Is greater than maximum possible length
if min_length > max_length:
raise PasswordError(f'No password with the given criteria')
length = randint(min_length, max_length)
# List that stores the "types" of possible character
char_list = []
# Mandatory requirements
for typ, val in zip(types.keys(), types.values()):
char_list.extend([typ] * val)
# The remaining values to fill
for rem in range(length - num_chars):
char_list.append(choice(list(types.keys())))
shuffle(char_list)
password = ''
for typ in char_list:
password += choice(type_chars[typ])
return password
if __name__ == '__main__':
min_length = int(input('Minimum number of characters required: '))
max_length = int(input('Maximum number of characters possible: '))
print()
caps = int(input('Number of capital letters required: '))
small = int(input('Number of small letters required: '))
digits = int(input('Number of digits required: '))
special = int(input('Number of special characters required: '))
print()
number = int(input('Number of passwords required: '))
print('\n' + '-' * 50 + '\n')
print(' Here are your passwords: \n')
for i in range(number):
print(password_generator(min_length, max_length, caps, small, digits, special))
You can also modify the mandatory types of characters (Like 3 caps, 2 digits, etc). Also, minimum length and maximum length can be specified.
I think this code looks a bit ugly. How do I improve this?
4 Answers 4
Refactoring in steps
Enumeration class
Types
is too generic name for the enum representing char types. Renamed to CharTypes
.
Instead CAP
, SMA
, DIG
, SPE
as enumeration members are better replaced with a more common/familiar and comprehensive abbreviations/associations:UPPER
, LOWER
, DIGIT
and SPECIAL
.
Since string.ascii_uppercase
and other string.*
are essentially just string constants - they can be easily set as enumeration values:
class CharTypes(Enum):
UPPER = string.ascii_uppercase # Capital
LOWER = string.ascii_lowercase # Small
DIGIT = string.digits # Digits
SPECIAL = '!()-.?[]_`~;:@#$%^&*=' # Special
thus, making all intermediate re-mappings like type_chars
and types
(in password_generator
function) redundant and unnecessary.
password_generator
function
The function signature is slightly changed in arguments names to conform with CharTypes
members:
def password_generator(min_length=6, max_length=20, upper=1, lower=1, digits=1, special=1)
types
mapping is eliminated as redundant.
char counts passed as arguments are gathered and summed at once:
char_counts = (upper, lower, digits, special)
num_chars = sum(char_counts)
Avoid overwriting/assigning to function argument like min_length = max(num_chars, min_length)
as min_length
might be potentially referenced as "original" argument value (and relied on) in other places in the function's body.
A safer way is assigning it to a separate variable:
min_len = max(num_chars, min_length)
length
variable is renamed to target_length
(to emphasize the final size).
char_list
is renamed to char_types
as it's aimed to accumulate CharTypes
enum members
Two for
loops which performed char_list.extend
and char_list.append
are efficiently replaced with 2 generators which further joined/merged by itertools.chain
function:
char_types = list(chain(*([c_type] * num for c_type, num in zip(CharTypes, char_counts)),
(choice(char_types_enums) for _ in range(target_length - num_chars))))
Furthermore, itertools.chain
is smart enough to skip empty generators (if let's say there's no remaining values to fill).
The last for
loop (accumulating password from random chars) is simply replaced with str.join
call on generator expression:
password = ''.join(choice(char_type.value) for char_type in char_types)
The whole crucial functionality is now shortened to the following:
import string
from enum import Enum
from random import randint, shuffle, choice
from itertools import chain
class PasswordError(Exception):
pass
class CharTypes(Enum):
UPPER = string.ascii_uppercase # Capital
LOWER = string.ascii_lowercase # Small
DIGIT = string.digits # Digits
SPECIAL = '!()-.?[]_`~;:@#$%^&*=' # Special
def password_generator(min_length=6, max_length=20, upper=1, lower=1, digits=1, special=1):
char_counts = (upper, lower, digits, special)
num_chars = sum(char_counts) # Number of mandatory characters
min_len = max(num_chars, min_length) # In case 'num_chars' is greater
# If number of characters required for each possible char type
# is greater than maximum possible length
if min_len > max_length:
raise PasswordError(f'No password with the given criteria')
target_length = randint(min_len, max_length)
char_types_enums = list(CharTypes) # get list of enums to pass `random.choice` call
# List of char "types" comprised of: mandatory requirements + remaining values to fill
char_types = list(chain(*([c_type] * num for c_type, num in zip(CharTypes, char_counts)),
(choice(char_types_enums) for _ in range(target_length - num_chars))))
shuffle(char_types)
password = ''.join(choice(char_type.value) for char_type in char_types)
return password
if __name__ == '__main__':
....
Sample usage:
Minimum number of characters required: >? 10
Maximum number of characters possible: >? 30
Number of capital letters required: >? 5
Number of small letters required: >? 4
Number of digits required: >? 6
Number of special characters required: >? 5
Number of passwords required: >? 4
--------------------------------------------------
Here are your passwords:
32S%km3A^v04h9pwR-T7O;=0O
mh8a:38Q-pGS3PtGs)e0P1g)$(#0U1
z@a0r;b7v.~K!8S@R343J7L
Mie:8Ec0C=3Cz93HPHDFm_84#;6@
-
\$\begingroup\$ Yes, treating incoming args as const is often considered good style especially in a function of non-trivial size (and if nothing else makes debugging easier). But the choice of names is problematic here:
min_len
vs.min_length
are visually very similar. A reader looking at some part of the function could easily miss that it was reading a different var name, not the arg. Naming is hard, especially when you need multiple variables with similar semantic meaning/purpose but holding different values. \$\endgroup\$Peter Cordes– Peter Cordes2019年11月26日 07:19:21 +00:00Commented Nov 26, 2019 at 7:19 -
\$\begingroup\$ -1 for not using
secrets
\$\endgroup\$Greg Schmit– Greg Schmit2019年11月26日 16:12:09 +00:00Commented Nov 26, 2019 at 16:12 -
\$\begingroup\$ @GregSchmit-ReinstateMonica, You would need to figure out one simple thing - I didn't review a cryptography but the codebase. But at least you've downvoted openly, although not very convincingly, in my opinion \$\endgroup\$RomanPerekhrest– RomanPerekhrest2019年11月26日 16:26:53 +00:00Commented Nov 26, 2019 at 16:26
-
1\$\begingroup\$ AlexV already wrote an answer using
secrets
(why it has more upvotes), but yours is the accepted answer so it would be good to usesecrets
on yours as well. I'm sorry you're offended that you didn't know to use that module when generating secrets. \$\endgroup\$Greg Schmit– Greg Schmit2019年11月26日 17:07:38 +00:00Commented Nov 26, 2019 at 17:07 -
3\$\begingroup\$ @GregSchmit-ReinstateMonica while recommending
secrets
would improve the quality, I don't see in this answer whererandom
is specifically recommended. Answers need not cover every issue in every line of the code, and if an answer skips over certain components, I don't think that invalidates the rest of the answer. Issecrets
the way to go for the cryptography? Absolutely. Is that relevant for this specific answer? I don't think so, especially since AlexV covered that bit perfectly in their answer \$\endgroup\$C.Nivs– C.Nivs2019年11月26日 19:51:40 +00:00Commented Nov 26, 2019 at 19:51
The other answers have great hints that you should definitely follow.
However, since we are talking about passwords here, a little bit of extra security might not hurt. Especially if it's readily available in the secrets
module in Python 3. secrets
uses a cryptograhically strong random number generator (in contrast to the pseudo random number generator in the normal random
module). There is a special warning in that regard in the docs of random. If you want to stick to random
, at least use an instance of random.SystemRandom()
, which is basically what secrets
does. An example:
RNG = random.SystemRandom()
def password_generator(...):
...
# The remaining values to fill
type_ = list(types.keys())
for rem in range(length - num_chars):
char_list.append(RNG.choice(type_))
# alternatively
char_list.append(secrets.choice(type_))
...
random.choice
uses what is called a pseudo-random number generator, i.e. an algorithm that generates a deterministic "randomly looking" sequence of bytes starting from a given seed. secrets.choice
uses a randomness source implemented in the OS, which likely takes electrical noise and other things into consideration to generate non-deterministic random data. random.org has a comprehensive article on the differences at https://www.random.org/randomness/. And of course, there is also the obligatory Wikipedia page about Randomness.
-
\$\begingroup\$ Great answer! But can I ask why
secrets.choice
is cryptographically stronger thanrandom.choice
? The random library page doesn't quite explain it. \$\endgroup\$Sriv– Sriv2019年11月25日 09:16:11 +00:00Commented Nov 25, 2019 at 9:16 -
2\$\begingroup\$ Sorry this wasn't the accepted answer. This really was a great answer and a great point, but it mostly improves the output cryptographically, but does not improve the code as a whole which @RomanPerekhrest's answer does. \$\endgroup\$Sriv– Sriv2019年11月25日 16:39:36 +00:00Commented Nov 25, 2019 at 16:39
There are a few things I'd suggest to clean up the code.
First, you should be able to write the following
for typ, val in zip(types.keys(), types.values()):
char_list.extend([typ] * val)
without using zip
by doing as follows
for typ, val in types.items():
char_list.extend([typ] * val)
Comprehensions
Comprehensions are a great way to clean up.
The first case would be
for rem in range(length - num_chars):
char_list.append(choice(list(types.keys())))
as
char_list.extend([choice(list(types.keys())) for _ in range(length - num_chars)])
And a second time
password = ''
for typ in char_list:
password += choice(type_chars[typ])
return password
as
return "".join([choice(type_chars[typ]) for typ in char_list])
Functions
I'd probably put the following piece of code as a separate function to make it more modular and manageable
# List that stores the "types" of possible character
char_list = []
# Mandatory requirements
for typ, val in zip(types.keys(), types.values()):
char_list.extend([typ] * val)
# The remaining values to fill
for rem in range(length - num_chars):
char_list.append(choice(list(types.keys())))
shuffle(char_list)
Likewise, with the suggested list comprehension that makes the password.
def make_password(char_list)
return "".join([choice(type_chars[typ]) for typ in char_list])
Optionals (fancy)
If you want to be very fancy, you can use dataclasses
or attrs
to package the options to the main function. This would allow you to make some validation of the input, namely, that everything you get are numbers (particularly ints
or a string that can be parsed as such). Such a dataclass can be thought of as the communication layer between the front end (whatever is in the __main__
part of the program) and the backend part in generate_password
.
I say this because your program will fail if you don't give numbers. For example in the first logic line num_chars = sum(list(types.values()))
.
You can generate a password simply with the random
module.
import string
import random
ls = string.ascii_letters + string.digits + string.punctuation
length = random.randrange(6, 20)
pwd = random.choices(ls, k=length)
print('new password: ', ''.join(pwd))
Output:
new password: xsC+.vrsZ<$\
-
1\$\begingroup\$ Great idea! I should have thought of that! In the case of really needing to use the generator, I'll use this method. But right now, I'm trying to improve my programming skills! :-D \$\endgroup\$Sriv– Sriv2019年11月25日 13:52:17 +00:00Commented Nov 25, 2019 at 13:52
Explore related questions
See similar questions with these tags.
password
intoPassword123!
instead, which add exactly zero security. \$\endgroup\$password
and 'medium' forPassword123!
and 'strong' forP@ssword123ドル!
\$\endgroup\$secrets
module for cryptographic-quality random numbers. \$\endgroup\$