6
\$\begingroup\$

Is there anything I should add or change? Is it easy to follow? How could I shorten it?
How can I improve the overall program to make it a program you would use on a regular basis?

#Programmer: DeliriousMistakes
#Date: 10-17-15
#File: password.py

#This program lets you store and create passwords

import random
import shelve

print "Welcome to the account saver!"


def random_password():
    """Creates a new random password"""
    print'\n'
    account = raw_input("Account: ")
    username = raw_input("Username: ")
    digits_in_pass = raw_input("Length of password: ")
    while not digits_in_pass.isdigit():
        print "Incorrect input!"
        digits_in_pass = raw_input("Length of password: ")
    password = gen_password(int(digits_in_pass))
    entry = finish_new_account(account, password, username)
    print entry + '\n'


def manual_input():
    """Manually input a password"""
    print'\n'
    account = raw_input("Account: ")
    username = raw_input("Username: ")
    password = raw_input("Password: ")
    entry = finish_new_account(account, password, username)
    print entry + '\n'


def find_account():
    """Find an existing account"""
    print '\n'
    search = raw_input("For which account are you searching: ")
    f = shelve.open("accounts.dat")
    if search in f:
        account = f[search]
        print account
    else:
        print "I'm sorry we could not find any account related to " + search
    print '\n'
    f.close()


def finish_new_account(account, password, username):
    """Sends entry to save() and returns a message"""
    entry = create_entry(account, password, username)
    save(account, entry)
    return "Save successful. " + "\n" + str(entry) + "\n"


def create_entry(account, password, username):
    """Creates the entry"""
    return "Account: " + account + " - Username: " + username + " - Password: " + password


def save(account, entry):
    """Saves account"""
    f = shelve.open("accounts.dat")
    saves = [entry]
    f[account] = saves
    f.sync()
    f.close()


def delete_account():
    """Deletes an account"""
    print'\n'
    account = raw_input("What account do you want to delete?: ")
    f = shelve.open("accounts.dat")
    if account in f:
        confirm_deletion = raw_input("Are you sure you want to delete " + account + "?: ")
        if confirm_deletion.lower() in ('yes', 'y'):
            del f[account]
            print "This account has been deleted."
        else:
            print "Canceled... "
    else:
        print "I'm sorry we could not find any account related to " + account
    print '\n'
    f.close


def all_accounts():
    """prints all accounts: A hidden command"""
    f = shelve.open("accounts.dat")
    klist = f.keys()
    f.close()
    print klist
    print "\n"


def gen_password(digits_in_pass):
    """Returns a randomly generated password"""
    alphanumeric_chars = ["0123456789",
                          "abcdefghijklmnopqrstuvwxyz",
                          "ABCDEFGHIJKLMNOPQRSTUVWXYZ"]
    return make_password(digits_in_pass, *alphanumeric_chars)


def make_password(length, *collections_of_characters):
    """Creates a random password"""
    characters = set()
    for collection in collections_of_characters:
        characters.update(str(c) for c in collection)
    characters = list(characters)

    password = [random.choice(characters) for _ in range(0, length)]
    return "".join(password)


def program_start():
    """MAIN"""
    choice = ''
    while choice != "5":
        choice = raw_input("""Press 1 to manually input an account
Press 2 to generate a password
Press 3 to search for an existing account
Press 4 to delete an account
Press 5 to exit
""")
        if choice == "1":
            manual_input()
        elif choice == "2":
            random_password()
        elif choice == "3":
            find_account()
        elif choice == "4":
            delete_account()
        elif choice == "all accounts":
            all_accounts()
        else:
            print " "
program_start()

#      ___           ___           ___                   ___                       ___           ___           ___     
#     /\  \         /\  \         /\__\      ___        /\  \          ___        /\  \         /\__\         /\  \    
#    /::\  \       /::\  \       /:/  /     /\  \      /::\  \        /\  \      /::\  \       /:/  /        /::\  \   
#   /:/\:\  \     /:/\:\  \     /:/  /      \:\  \    /:/\:\  \       \:\  \    /:/\:\  \     /:/  /        /:/\ \  \  
#  /:/  \:\__\   /::\~\:\  \   /:/  /       /::\__\  /::\~\:\  \      /::\__\  /:/  \:\  \   /:/  /  ___   _\:\~\ \  \ 
# /:/__/ \:|__| /:/\:\ \:\__\ /:/__/     __/:/\/__/ /:/\:\ \:\__\  __/:/\/__/ /:/__/ \:\__\ /:/__/  /\__\ /\ \:\ \ \__\
# \:\  \ /:/  / \:\~\:\ \/__/ \:\  \    /\/:/  /    \/_|::\/:/  / /\/:/  /    \:\  \ /:/  / \:\  \ /:/  / \:\ \:\ \/__/
#  \:\  /:/  /   \:\ \:\__\    \:\  \   \::/__/        |:|::/  /  \::/__/      \:\  /:/  /   \:\  /:/  /   \:\ \:\__\  
#   \:\/:/  /     \:\ \/__/     \:\  \   \:\__\        |:|\/__/    \:\__\       \:\/:/  /     \:\/:/  /     \:\/:/  /  
#    \::/__/       \:\__\        \:\__\   \/__/        |:|  |       \/__/        \::/  /       \::/  /       \::/  /   
#     ~~            \/__/         \/__/                 \|__|                     \/__/         \/__/         \/__/ 
#      ___                       ___           ___           ___           ___           ___           ___     
#     /\__\          ___        /\  \         /\  \         /\  \         /\__\         /\  \         /\  \    
#    /::|  |        /\  \      /::\  \        \:\  \       /::\  \       /:/  /        /::\  \       /::\  \   
#   /:|:|  |        \:\  \    /:/\ \  \        \:\  \     /:/\:\  \     /:/__/        /:/\:\  \     /:/\ \  \  
#  /:/|:|__|__      /::\__\  _\:\~\ \  \       /::\  \   /::\~\:\  \   /::\__\____   /::\~\:\  \   _\:\~\ \  \ 
# /:/ |::::\__\  __/:/\/__/ /\ \:\ \ \__\     /:/\:\__\ /:/\:\ \:\__\ /:/\:::::\__\ /:/\:\ \:\__\ /\ \:\ \ \__\
# \/__/~~/:/  / /\/:/  /    \:\ \:\ \/__/    /:/  \/__/ \/__\:\/:/  / \/_|:|~~|~    \:\~\:\ \/__/ \:\ \:\ \/__/
#       /:/  /  \::/__/      \:\ \:\__\     /:/  /           \::/  /     |:|  |      \:\ \:\__\    \:\ \:\__\  
#      /:/  /    \:\__\       \:\/:/  /     \/__/            /:/  /      |:|  |       \:\ \/__/     \:\/:/  /  
#     /:/  /      \/__/        \::/  /                      /:/  /       |:|  |        \:\__\        \::/  /   
#     \/__/                     \/__/                       \/__/         \|__|         \/__/         \/__/
\$\endgroup\$
0

2 Answers 2

Reset to default
2
\$\begingroup\$

For random_password, instead of attempting to get input once then looping while the input's invalid, it's actually more Pythonic to just run an infinite loop that breaks when the condition is satisfied. So you'd do this:

while True:
    digits_in_pass = raw_input("Length of password: ")
    if digits_in_pass.isdigit()
        break
    print "Incorrect input!"

That said, it's better to just turn your string into an int rather than testing with isdigit. So now you could change it to trying to get the int value and break if it's successful, but otherwise print your error message.

while True:
    digits_in_pass = raw_input("Length of password: ")
    try:
        digits_in_pass = int(digits_in_pass)
    except ValueError:
        print "Invalid input! Input must be an integer"

I also made a more useful error message, as "Incorrect input" could mean anything and doesn't pinpoint what makes it 'incorrect'.

Instead of finishing by printing entry, it would be better to return entry. You could then print it from outside the function if you wish, or store it if necessary. If you really want it printed, then why not directly print it in your finish_new_account function? You could add a flag for disabling printing with an optional value at the end like this:

def finish_new_account(account, password, username, log=True):
    """Sends entry to save() and returns a message"""
    entry = create_entry(account, password, username)
    save(account, entry)
    if log:
        print ("Save successful. " + "\n" + str(entry) + "\n")

log will be True by default, so if the user doesn't pass any value then the message will be printed. But they can manually pass False or log=False to prevent the print from happening. This way you can avoid the printing when you don't need it.

While we're here. entry returns a string, so there's no need to call str at all. You don't need to concatenate to include those \n characters, and you should use str.format to insert values into strings, like this:

        print ("Save successful.\n{}\n".format(entry))

manual_input and random_password are so similar, maybe you could just add an optional parameter there and make a new function. Like new_password(automatic=True). This way you only need to differentiate between whether the user enters an actual password or the length of an automatic password. Otherwise they function the same.

For find_account you should make "accounts.dat" a constant at the top of your file. Something like ACCOUNTS = "accounts.dat". There's also no need to set account and then print it, just print f[search] directly. Also use with open(file) as f. It's safer because it will always close the file even if errors occur. And you should note in the docstring that you're printing the account, otherwise it sounds like you're returning it.

def find_account():
    """Find and print an account if it's found."""

    print '\n'
    search = raw_input("For which account are you searching: ")
    f = shelve.open("accounts.dat")
    if search in f:
        print f[search]
    else:
        print "I'm sorry we could not find any account related to " + search
    print '\n'

In delete_account you're not actually closing f at the end. You put f.close, but without your brackets this wont run anything. You need f.close().

There's a much easier way to get all the numbers and letters. The string module contains constants that will provide you with sets like ascii_letters for all the letters and digits for all the numbers. You can even use letters if you'd like to include non ascii letters like áéíóú. But to get the same as what you wanted, you could just do this

from string import ascii_letters, digits

CHARACTERS = ascii_letters + digits

and then in make_password we can just use the constant CHARACTERS. Note that you don't need to make them a list to use random.choice. Also you can pass just length to range, no need for the 0 as the start defaults to that. And lastly, you could just pass the list comprehension expression directly to join.

def make_password(length, *collections_of_characters):
    """Creates a random password"""

    return "".join(random.choice(CHARACTERS) for _ in range(length))
\$\endgroup\$
0
7
\$\begingroup\$

Security holes

  1. The program generates passwords by calling random.choice. But this is a pseudorandom function, which means that the passwords are predictable. You must use random.SystemRandom instead.

  2. The passwords are stored using shelve. A shelve database is not encrypted. This means that anyone who can get hold of the accounts.dat file can read your passwords. The password file must be encrypted with a master password.

\$\endgroup\$
5
  • \$\begingroup\$ Thank you for the input. What could you use to encrypt it with a master password and save it? \$\endgroup\$ Commented Oct 22, 2015 at 22:04
  • 1
    \$\begingroup\$ I would use gpg. \$\endgroup\$ Commented Oct 22, 2015 at 22:38
  • \$\begingroup\$ Thank you I'll start my research on how to use gpg \$\endgroup\$ Commented Oct 22, 2015 at 22:41
  • \$\begingroup\$ Pseudorandom isn't an issue. The issue is that it's not a cryptographic PRNG. random.SystemRandom is also pseudorandom, but it's a cryptographic PRNG. \$\endgroup\$ Commented Oct 23, 2015 at 3:25
  • \$\begingroup\$ @user2357112: I disagree. Even a CSPRNG isn't suitable for password generation, unless it has a source of entropy, which is what SystemRandom provides. \$\endgroup\$ Commented Oct 23, 2015 at 8:04

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.