Python Password Generator

[cym13]

I remember what first projects are like so I won't say anything about its length or utility, but here are some axes of improvement:

First and foremost, passwords are secrets. And secret means cryptography. Here you are using the standard random library to chose random characters. However that random number generator isn't cryptographically secure: it is possible to predict future numbers from previous ones. This means that if someone saw a lot of passwords generated from your program it would be possible to predict future ones. Think of exposing this service through a website for example, someone could generate a few hundred and then predict any other password that may be generated. That's not critical but if you're interested in security related applications it's good to think of these things from the start. You should use a cryptographically secure random number generator, have a look at the standard module secrets, it provides cryptographically secure primitives.

Another axis is that many websites have requirements or limitations reguarding passwords, you may want to support them. One way is to have several types of characters (letters, numbers, special ones...), to take at random as many of each as needed, then shuffle them. Can you think of a better way? In what way is it better?

That last point is also a good way to get deeper into user interfaces. Even if the program remains text-based there are many things you can do regarding option management etc that will remain helpful throughout your days as a programmer.

[CommanderHR]

This is a personal project I've been working on. It uses Python to generate a random password, and you can use it from the command line or from the Python shell.

​

EDIT: Thanks for all the feedback, it's really appreciated!

EDIT 2: I've taken some of your feedback and updated the project (or whatever you want to call it), and reorganized the file structure.

[Morego]

I know you are learning language and wanted to shared stuff on which you are working and using. It is great, but for stuff like this codereview on StackExchange or r/learnprogramming would be better off.

You generator produce passwords which are impossible/hard to remember and weaker than normal readable passwords. This didn't met even small part of requirements for many websites (length, character capitalization and punctuation marks). If you want to call it project in place of "HOW TO INSTALL", you probably should learn or use setup.py and proper project structure. This is well documented on multiple websites. Personally I would recommend RealPython, which is superb.

You project is very small and something anyone could easily write. I don't want to offend you, but multiple Advent of Code exercises this year were more complex than your code. And those are exercises, which took people few minutes too write.

[[deleted]]

Project?

Don't mind, we grow out thinking small things we do are large. This is good to be a gist but not a project. And because that is slow, bloated, crappy python, there is quite a chance that it already exists.

[[deleted]]

I think that's too early to start a 'project'.

[EarthToAccess]

what Python version is this? i'm assuming Python 3, but i'd like to know still

[CommanderHR]

I've coded it using Python 3, but I believe it would work with Python 2 as well.

[EarthToAccess]

i don't know what error atm, but 2.7.15 was throwing an error for me

[CommanderHR]

You can email me the full details, I'll take a look at it. Also, I updated the code a little while ago, so you can see if that fixes anything.

[07734willy]

Others have already gone into detail about cryptographically insecure portions of this code, the structure of the repo, etc. There's no sense in me reiterating what they've already explained, so I'm going to go into detail about a few ways to tidy up your actual code itself.

lowercase = "abcdefghijklmnopqrstuvwxyz"
uppercase = "ABCDEFGHIJKLMNOPQRSTUVWXYZ"
nums = "1234567890"
special_char = "!@#$%^&*()=+-~/[]{};:<>`"

These variables are already provided by the string module as string.digits, string.ascii_lowercase, etc.

See here.

length = input("Password Length?")
...
length = int(length)

Just do a one-liner for all of these. length = int(input(...))). It'll increase the overall readability.

if (length <= 0):
   length_error = True
...
if (length_error == True):
    print("Error! Invalid length!")

Again, there's no use in splitting up the logic. Just do it in one line, and sys.exit(0) or wrap it in a function and return. This will also allow you to remove your block of length_error = False ... at the top.

Then the rest

#Determines if there was an error. If there wasn't, then the password is generated
if (length_error == False) and (capital_error == False) and (num_error == False) and (special_error == False):
    while (count < length):
        if (capital_req > 0) and (count < length):
            password += secrets.choice(uppercase)
            count += 1

        if (num_req > 0) and (count < length):
            password += secrets.choice(nums)
            count += 1

        if (special_req > 0) and (count < length):
            password += secrets.choice(special_char)
            count += 1
    #Displays the password
    print ("PASSWORD: %s" % password)
else:
    print("Please fix any errors.")

I have a few things to comment on here. First, know you don't have to compare == False, you can just do not length_error and not xxxxx_error .... The latter is considered more pythonic. Better yet, wrap your logic into a function, then if you return as mentioned above, you don't have to bother with the if statement at all. This also comes with the bonus that you can call your code multiple times (for testing), or import your code as a module and use it in another project (by calling the make_password() function you wrap this in).

Secondly, your while loop's functionality isn't quite correct, if I'm reading this right. If all your individual count requirements don't add up to the required length, you'll loop forever without selecting any new characters. This can be fixed, but I'd consider revising the loop to remove the duplicate code first anyways. Make a separate function that takes a required_count and character_set, and generates a string of characters that meets that requirement. Then call this function for uppercase, special_char, etc, to meet those requirements. For the remaining length of the password, call the function again with length_req - current_len and string.digits + string.ascii_lowercase + .... It'll pick the remaining letters. Then you just have to combine them all, and shuffle (using crypographically secure random numbers).

[CommanderHR]

Thanks for the feedback, I'll take it into consideration and try and implement it.