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\$
3

1 Answer 1

Reset to default
1
\$\begingroup\$

  • You have a lot of inconsistencies.

    • 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)

Suggested code

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\$
2
  • \$\begingroup\$ Is srandom.choices different from random.choices? \$\endgroup\$ Commented Jul 7, 2020 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\$ Commented Jul 7, 2020 at 9:10

You must log in to answer this question.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.