2
\$\begingroup\$

A few days ago I posted my password generator project to help me learn and become more comfortable. I got a lot of great replies from that and I've sense updated and would love another look at the program.

I've made it so that I can import it and use it to generator a password. I've also added support for completely custom subsets of characters.

Throw any suggests or comments you have! Anything is welcome.

import string
from string import ascii_lowercase
from string import ascii_uppercase
from string import digits as numeric
from string import punctuation
import secrets
import argparse
from argparse import HelpFormatter

def generate_characters(character_set, character_amount):
    for _ in range(0, character_amount):
        yield secrets.choice(character_set)

def shuffle(input_str):
    output = ""
    for _ in range(0, len(input_str)):
        index = secrets.randbelow(len(input_str))
        output += "".join(input_str[index])
        input_str = "".join([input_str[:index], input_str[index + 1 :]])
    return output

def generate_password(password_length,
                      subset_lowercase=ascii_lowercase, subset_uppercase=ascii_uppercase,
                      subset_numeric=numeric, subset_special="!@#$%^&*",
                      min_lowercase=1, min_uppercase=1,
                      min_numeric=1, min_special=1):
    superset = "".join([subset_lowercase, subset_uppercase, subset_numeric, subset_special])
    password = "".join(generate_characters(subset_lowercase, min_lowercase))
    password += "".join(generate_characters(subset_uppercase, min_uppercase))
    password += "".join(generate_characters(subset_numeric, min_numeric))
    password += "".join(generate_characters(subset_special, min_special))
    password += "".join(generate_characters(superset, password_length-len(password)))
    return shuffle(password)

if __name__ == "__main__":
    parser = argparse.ArgumentParser(
        formatter_class=HelpFormatter,
        description="Generates a password",
        usage="")

    parser.add_argument(
        "-len",
        "--length",
        type=int,
        default=24,
        dest="password_length",
        help="Length of the generated password")
    parser.add_argument(
        "-lc",
        "--lower",
        type=int,
        default=1,
        dest="min_lowercase",
        help="Minimum number of lowercase alpha characters")
    parser.add_argument(
        "-uc",
        "--upper",
        type=int,
        default=1,
        dest="min_uppercase",
        help="Minimum number of uppercase alpha characters")
    parser.add_argument(
        "-num",
        "--numeric",
        type=int,
        default=1,
        dest="min_numeric",
        help="Minimum number of numeric characters")
    parser.add_argument(
        "-sp",
        "--special",
        type=int,
        default=1,
        dest="min_special",
        help="Minimum number of special characters")
    parser.add_argument(
        "-ext",
        "--extended",
        action="store_const",
        default=False,
        const=True,
        dest="special_extended",
        help="Toggles the extended special character subset. Passwords may not be accepted by all services")
    parser.add_argument(
        "-sl",
        "--subset_lower",
        type=str,
        default=ascii_lowercase,
        dest="subset_lower",
        help="Allows for a custom subset of lowercase characters")
    parser.add_argument(
        "-su",
        "--subset_upper",
        type=str,
        default=ascii_uppercase,
        dest="subset_upper",
        help="Allows for a custom subset of uppercase characters")
    parser.add_argument(
        "-sn",
        "--subset_numeric",
        type=str,
        default=numeric,
        dest="subset_numeric",
        help="Allows for a custom subset of numeric characters")
    parser.add_argument(
        "-ss",
        "--subset_special",
        default="",
        type=str,
        dest="subset_special",
        help="Allows for a custom subset of special characters")

    args = parser.parse_args()

    if args.subset_special:
        special = args.subset_special
    elif args.special_extended:
        special = punctuation
    else:
        special = "!@#$%^&*"

    generated_password = generate_password(
        args.password_length,
        args.subset_lower,
        args.subset_upper,
        args.subset_numeric,
        special,
        args.min_lowercase,
        args.min_uppercase,
        args.min_numeric,
        args.min_special,
    )

    print("Password:", generated_password)

\$\endgroup\$
2
  • \$\begingroup\$ talking about the code it is pretty well-written and formatted, good job!. \$\endgroup\$
    – LIL LOFIE
    Jul 7 '20 at 4:50
  • \$\begingroup\$ Can you link your older code here? I want to understand what secrets is doing. \$\endgroup\$ Jul 7 '20 at 8:36
1
\$\begingroup\$
  • You have a lot of inconsistantcies.

    • import string from string import ... But then only using import secrets.

      I would only use import string.

    • You do [:index] but also [index + 1 :].

    • You do index + 1 but you also do password_length-len(password).

    • You start generate_password using a one argument per line style, and then don't for the rest of the arguments.

  • You should move "!@#$%^&*" into a constant, as you've duplicated it.

  • You can use random.SystemRandom.choices rather than generate_characters. SystemRandom uses os.urandom which is "suitable for cryptographic use."

    import random
    
    srandom = random.SystemRandom()
    
    
    def generate_characters(character_set, character_amount):
        return srandom.choices(character_set, k=character_amount)
    
  • You can use random.SystemRandom.sample to replace shuffle.

  • Your current method is really inefficent it runs in \$O(n^2)\$ time. As you're building a new list every iteration.

    "".join([input_str[:index], input_str[index + 1 :]])
    

    Instead change input_str to a list and use a similar algorithm by swapping the current index with the selected. Also known as the Fisher–Yates shuffle.

    def shuffle(input_str):
        output = list(input_str)
        for i in range(len(input_str)):
            index = srandom.randrange(i, len(input_str))
            output[i], output[index] = output[index], output[i]
        return "".join(output)
    
  • I'm not a fan of passing so many keyword arguments to generate_password. I would instead make it take tuples of (subset, amount) and build the password that way.

    You can loop over these arguments so that the code is simple too.

    def generate_password(password_length, *subsets):
        password = "".join(
            generate_characters(subset, minimum)
            for subset, minimum in subsets
        )
        superset = "".join(subset for subset, _ in subsets)
        password += "".join(generate_characters(superset, password_length - len(password)))
        return shuffle(password)
    
import string
import random
import argparse

srandom = random.SystemRandom()


def generate_password(password_length, *subsets):
    password = "".join(
        "".join(srandom.choices(subset, k=minimum))
        for subset, minimum in subsets
    )
    superset = "".join(subset for subset, _ in subsets)
    password += "".join(srandom.choices(superset, k=password_length - len(password)))
    return "".join(srandom.sample(password, len(password)))


if __name__ == "__main__":
    ...

    generated_password = generate_password(
        args.password_length,
        (args.subset_lower, args.min_lowercase),
        (args.subset_upper, args.min_uppercase),
        (args.subset_numeric, args.min_numeric),
        (special, args.min_special),
    )
    print("Password:", generated_password)
\$\endgroup\$
3
  • \$\begingroup\$ Is srandom.choices different from random.choices? \$\endgroup\$ Jul 7 '20 at 9:08
  • \$\begingroup\$ @VisheshMangla Yes, please read the description accompanying the code as it will explain how it is and why it is fine. \$\endgroup\$
    – user226435
    Jul 7 '20 at 9:10
  • \$\begingroup\$ ok, thanks for the info. \$\endgroup\$ Jul 7 '20 at 9:14

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.