17
\$\begingroup\$

pwgen is a nice password generator utility. When you run it, it fills the terminal with a bunch of random passwords, giving you many options to choose from and pick something you like, for example:

lvk3U7cKJYkl pLBJ007977Qx b9xhj8NWPfWQ
pMgUJBUuXwpG OAAqf6Y9TXqc fJOyxoGYCRSQ
bpbwp6f2MxEH fUYTJUqg0ZMB GjVVEQxuer0k
oqTEvV1LmdJu si47MkHNRpAw 3GKV8NdGMvwf

Although there are ports of pwgen in multiple systems, it's not so easy to find in Windows. So I put together a simple Python script that's more portable, as it can run in any system with Python.

I added some extra features I often want:

  • Skip characters that may be ambiguous, such as l1ioO0Z2I
  • Avoid doubled characters (slow down typing)

Here it goes:

#!/usr/bin/env python

from __future__ import print_function

import random
import string
import re

from argparse import ArgumentParser

terminal_width = 80
terminal_height = 25

default_length = 12

alphabet_default = string.ascii_letters + string.digits
alphabet_complex = alphabet_default + '`~!@#$%^&*()_+-={}[];:<>?,./'
alphabet_easy = re.sub(r'[l1ioO0Z2I]', '', alphabet_default)

double_letter = re.compile(r'(.)\1')


def randomstring(alphabet, length=16):
    return ''.join(random.choice(alphabet) for _ in range(length))


def has_double_letter(word):
    return double_letter.search(word) is not None


def easy_to_type_randomstring(alphabet, length=16):
    while True:
        word = randomstring(alphabet, length)
        if not has_double_letter(word):
            return word


def pwgen(alphabet, easy, length=16):
    for _ in range(terminal_height - 3):
        for _ in range(terminal_width // (length + 1)):
            if easy:
                print(easy_to_type_randomstring(alphabet, length), end=' ')
            else:
                print(randomstring(alphabet, length), end=' ')
        print()


def main():
    parser = ArgumentParser(description='Generate random passwords')
    parser.add_argument('-a', '--alphabet',
                        help='override the default alphabet')
    parser.add_argument('--complex', action='store_true', default=False,
                        help='use a very complex default alphabet', dest='complex_')
    parser.add_argument('--easy', action='store_true', default=False,
                        help='use a simple default alphabet, without ambiguous or doubled characters')
    parser.add_argument('-l', '--length', type=int, default=default_length)
    args = parser.parse_args()

    alphabet = args.alphabet
    complex_ = args.complex_
    easy = args.easy
    length = args.length

    if alphabet is None:
        if complex_:
            alphabet = alphabet_complex
        elif easy:
            alphabet = alphabet_easy
        else:
            alphabet = alphabet_default
    elif len(alphabet) < length:
        length = len(alphabet)

    pwgen(alphabet, easy, length)


if __name__ == '__main__':
    main()

How would you improve this? I'm looking for comments about all aspects of this code.

I know that the terminal_width = 80 and terminal_height = 25 variables don't really reflect what their names imply. It's not terribly important, and good enough for my purposes, but if there's a way to make the script detect the real terminal width and height without importing dependencies that reduce portability, that would be pretty awesome.

\$\endgroup\$
6
\$\begingroup\$

Mostly a matter of personal preference but I'd define a variable in pwgen like :

get_string = easy_to_type_randomstring if easy else randomstring

to avoid duplicated logic.

Then, you can simplify your code by using join instead of having multiple print.

def pwgen(alphabet, easy, length=16):
    get_string = easy_to_type_randomstring if easy else randomstring
    for _ in range(terminal_height - 3):
        print(' '.join(get_string(alphabet, length)
            for _ in range(terminal_width // (length + 1))))
\$\endgroup\$
  • \$\begingroup\$ That's more than just your taste, it's an excellent point! Duplicated logic is clearly not cool, and your solution is also more efficient, since it evaluates the if-else only once before the loop. Very well spotted, and thanks a lot! \$\endgroup\$ – janos Feb 2 '15 at 10:37
  • \$\begingroup\$ Glad you like it :-) \$\endgroup\$ – SylvainD Feb 2 '15 at 10:38
7
\$\begingroup\$

Looking at the following:

terminal_width = 80
terminal_height = 25

default_length = 12

alphabet_default = string.ascii_letters + string.digits
alphabet_complex = alphabet_default + '`~!@#$%^&*()_+-={}[];:<>?,./'
alphabet_easy = re.sub(r'[l1ioO0Z2I]', '', alphabet_default)

double_letter = re.compile(r'(.)\1')

you never change them, so they are constants. Constants are written ALL CAPS in Python as a convention.


elif len(alphabet) < length:
    length = len(alphabet)

Are you sure the user really wants what you are doing here? Maybe he wants a 20 character password using only the 10 digits, with this you are silently not doing what he expects.

\$\endgroup\$
  • 1
    \$\begingroup\$ You're right about the naming, and well spotted the bug in that length check. Thanks! \$\endgroup\$ – janos Feb 2 '15 at 7:42

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.