How random is this password-generating algorithm?

This code's sole purpose is to create a very random password, using letters and numbers.

I think that it fulfills its purpose, but I wonder, could it be done better? I'm not talking about efficiency, but security.

In your eyes, how secure is this method for generating passwords, and if it is barely secure at all, what better practices are there for this sort of thing?

```
import random as r
import sys
import os
from subprocess import call
alpha_lower = "abcdefghijklmnopqrstuvwxyz"
alpha_upper = "ABCDEFGHIJKLMNOPQRSTUVWXYZ"
call('color a', shell=True)
class MutateSeed(object):
def __init__(self, seed, alpha_lower, alpha_upper):
self.seed = seed
self.mutated_seed = []
self.alphabet_lower = list(alpha_lower)
self.alphabet_upper = list(alpha_upper)
def mutate_seed(self):
for x in range(self.seed):
a = r.randint(0, 9)
b = r.randint(0, 9)
c = r.randint(0, 9)
d = r.randint(0, 25)
e = r.randint(0, 25)
f = r.randint(0, 25)

con = r.randint(1, 6)
if con == 1:
mut = a
elif con == 2:
mut = b
elif con == 3:
mut = c
elif con == 4:
lower_or_higher = r.randint(0, 1)
if lower_or_higher == 0:
mut = self.alphabet_lower[d]
elif lower_or_higher == 1:
mut = self.alphabet_upper[d]
elif con == 5:
lower_or_higher = r.randint(0, 1)
if lower_or_higher == 0:
mut = self.alphabet_lower[e]
elif lower_or_higher == 1:
mut = self.alphabet_upper[e]
elif con == 6:
lower_or_higher = r.randint(0, 1)
if lower_or_higher == 0:
mut = self.alphabet_lower[f]
elif lower_or_higher == 1:
mut = se

Copy&pasting code multiple times does not make it more secure. If we clean up your mutate_seed method, we'd end up with this:

def mutate_seed(self):
    for x in range(self.seed):
        number_or_letter = r.randint(0, 1)
        if number_or_letter:
            self.mutated_seed.append(r.randint(0, 9))
        else:
            lower_or_higher = r.randint(0, 1)
            if lower_or_higher:
                self.mutated_seed.append(r.choice(self.alphabet_lower))
            else:
                self.mutated_seed.append(r.choice(self.alphabet_upper))

Quick Notes on the changes:

-
I don't request random values which I don't end up using. Throwing away values from the random number generator does not make those numbers that you use more secure.

-
I don't use single-letter variable names or abbreviations like con or mut that obscure the meaning.

-
random.choice is so simple and nice – consider using it.

-
This solution features far less duplicate code. Did you calculate r.ranint(1, 6) as an analogy for six-sided dice?

Now the code is cleaned up so that we can do some more substantial reviewing.

There is no use in separating between numbers, lowercase letters, and uppercase letters. They all should be in the same character pool so that each character has the same probability of occuring at each position in the password. With your current methods, a digit is much more likely to occur than any given letter, simply because there are less digits than letters.

So we should have something like

lower = [chr(x) for x in range(ord('a'), ord('z') + 1)] # I can't be bothered to type
upper = [chr(x) for x in range(ord('A'), ord('Z') + 1)] #   out all letters manually
digits = [str(x) for x in range(0, 9)]
character_pool = lower + upper + digits
...
password = [random.choice(character_pool) for _ in range(0, length)]

The interface of your class is horrible. We have to create an instance, then call the mutate_seed method, then access an instance variable, then must join that array to a string until we can finally display it. Consider packing all of this functionality into a single, convenient function instead:

def make_password(length, *collections_of_characters):
    # join all character collections into a single set
    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)

Then using this is as simple as

password = make_password(10,
                         "0123456789",
                         "abcdefghijklmnopqrstuvwxyz", 
                         "ABCDEFGHIJKLMNOPQRSTUVWXYZ")
print(password)

If you need a certain set of characters more often, put those into a list:

alphanumeric_chars = ["0123456789",
                      "abcdefghijklmnopqrstuvwxyz", 
                      "ABCDEFGHIJKLMNOPQRSTUVWXYZ"]
password = make_password(10, *alphanumeric_chars)
print(password)

or create a wrapper function:

def make_alphanumeric_password(length):
    return make_password(length,
                         "0123456789",
                         "abcdefghijklmnopqrstuvwxyz", 
                         "ABCDEFGHIJKLMNOPQRSTUVWXYZ")
print(make_alphanumeric_password(10))

Not all abstractions map cleanly to classes.

Your code uses the word “seed” quite often. A seed is the initial state of a random number generator. When set to the same seed each time, the random number generator will produce the same sequence of numbers. In your class, “seed” seems to mean “length” instead. Use clear, obvious names for your variables rather than using technical lingo if you aren't sure what it means – although a quick trip to Wikipedia would have cleared that up.

You issue a few Windows-only system commands such as cls. Unless this is central to the working of your application, don't manipulate the console, such as setting the color or clearing the screen. Users can do this themselves if they want to. Removing these commands also makes your code usable on other operating systems like OS X or Linux.

You are using the random library as a random number generator. The numbers produced by this library aren't really random, they are just pseudo-random. This is usually not an issue as long as a really random seed is chosen, but because the number sequence is derived algorithmically, it should not be used for really really important stuff. Most passwords are only important and not really really important, so this should be fine ;-) but it's good to remember that real randomness is something different.

That is one aspect where the security of your program is not optimal, but good enough. Writing the password to a file is however absolutely irresponsible. When the password is only displayed on the command line, we can assume that it will only be immediately visible until th

StackExchange Code Review Q#49438, answer score: 10